From fcafbef8c5910602ef810f2ee4b1fcbc1cde4fd7 Mon Sep 17 00:00:00 2001 From: qiaozp <47812250+chivalryq@users.noreply.github.com> Date: Fri, 5 Aug 2022 14:05:23 +0800 Subject: [PATCH] Fix: vela CLI provider compatibility (#4561) * Fix: vela CLI provider compatibility Signed-off-by: qiaozp * fix Signed-off-by: qiaozp * List interface won't return NotFoundError Signed-off-by: qiaozp * format imports Signed-off-by: qiaozp --- pkg/apiserver/domain/service/config.go | 6 +- pkg/apiserver/domain/service/config_test.go | 12 ++-- pkg/apiserver/domain/service/tags.go | 18 ++---- pkg/apiserver/domain/service/tags_test.go | 16 +++--- pkg/definition/definition.go | 6 ++ references/cli/provider.go | 21 +++++-- references/cli/provider_test.go | 62 +++++++++++++++++++++ 7 files changed, 109 insertions(+), 32 deletions(-) diff --git a/pkg/apiserver/domain/service/config.go b/pkg/apiserver/domain/service/config.go index e4518f74a..a7c6854be 100644 --- a/pkg/apiserver/domain/service/config.go +++ b/pkg/apiserver/domain/service/config.go @@ -72,7 +72,7 @@ func (u *configServiceImpl) ListConfigTypes(ctx context.Context, query string) ( defs := &v1beta1.ComponentDefinitionList{} if err := u.KubeClient.List(ctx, defs, client.InNamespace(types.DefaultKubeVelaNS), client.MatchingLabels{ - configCatalog: types.VelaCoreConfig, + definition.ConfigCatalog: types.VelaCoreConfig, }); err != nil { return nil, err } @@ -85,13 +85,13 @@ func (u *configServiceImpl) ListConfigTypes(ctx context.Context, query string) ( if err := u.KubeClient.List(ctx, defsLegacy, client.InNamespace(types.DefaultKubeVelaNS), client.MatchingLabels{ // leave here as the legacy format to test the compatibility - definition.UserPrefix + configCatalog: types.VelaCoreConfig, + definition.UserPrefix + definition.ConfigCatalog: types.VelaCoreConfig, }); err != nil { return nil, err } // filter repeated config,due to new labels that exist at the same time for _, legacy := range defsLegacy.Items { - if legacy.Labels[configCatalog] == types.VelaCoreConfig { + if legacy.Labels[definition.ConfigCatalog] == types.VelaCoreConfig { continue } items = append(items, legacy) diff --git a/pkg/apiserver/domain/service/config_test.go b/pkg/apiserver/domain/service/config_test.go index e37e476ec..7e3017593 100644 --- a/pkg/apiserver/domain/service/config_test.go +++ b/pkg/apiserver/domain/service/config_test.go @@ -57,8 +57,8 @@ func TestListConfigTypes(t *testing.T) { Name: "def1", Namespace: types.DefaultKubeVelaNS, Labels: map[string]string{ - configCatalog: types.VelaCoreConfig, - definitionType: types.TerraformProvider, + definition.ConfigCatalog: types.VelaCoreConfig, + definition.DefinitionType: types.TerraformProvider, }, }, } @@ -71,10 +71,10 @@ func TestListConfigTypes(t *testing.T) { Name: "def2", Namespace: types.DefaultKubeVelaNS, Annotations: map[string]string{ - definitionAlias: "Def2", + definition.DefinitionAlias: "Def2", }, Labels: map[string]string{ - configCatalog: types.VelaCoreConfig, + definition.ConfigCatalog: types.VelaCoreConfig, }, }, } @@ -153,10 +153,10 @@ func TestGetConfigType(t *testing.T) { Name: "def2", Namespace: types.DefaultKubeVelaNS, Annotations: map[string]string{ - definitionAlias: "Def2", + definition.DefinitionAlias: "Def2", }, Labels: map[string]string{ - definition.UserPrefix + configCatalog: types.VelaCoreConfig, + definition.UserPrefix + definition.ConfigCatalog: types.VelaCoreConfig, }, }, } diff --git a/pkg/apiserver/domain/service/tags.go b/pkg/apiserver/domain/service/tags.go index 70e28d2b4..8a029ae68 100644 --- a/pkg/apiserver/domain/service/tags.go +++ b/pkg/apiserver/domain/service/tags.go @@ -18,22 +18,16 @@ package service import "github.com/oam-dev/kubevela/pkg/definition" -const ( - definitionAlias = "alias.config.oam.dev" - definitionType = "type.config.oam.dev" - configCatalog = "catalog.config.oam.dev" -) - // DefinitionAlias will get definitionAlias value from tags func DefinitionAlias(tags map[string]string) string { if tags == nil { return "" } - val := tags[definitionAlias] + val := tags[definition.DefinitionAlias] if val != "" { return val } - return tags[definition.UserPrefix+definitionAlias] + return tags[definition.UserPrefix+definition.DefinitionAlias] } // DefinitionType will get definitionType value from tags @@ -41,11 +35,11 @@ func DefinitionType(tags map[string]string) string { if tags == nil { return "" } - val := tags[definitionType] + val := tags[definition.DefinitionType] if val != "" { return val } - return tags[definition.UserPrefix+definitionType] + return tags[definition.UserPrefix+definition.DefinitionType] } // ConfigCatalog will get configCatalog value from tags @@ -53,9 +47,9 @@ func ConfigCatalog(tags map[string]string) string { if tags == nil { return "" } - val := tags[configCatalog] + val := tags[definition.ConfigCatalog] if val != "" { return val } - return tags[definition.UserPrefix+configCatalog] + return tags[definition.UserPrefix+definition.ConfigCatalog] } diff --git a/pkg/apiserver/domain/service/tags_test.go b/pkg/apiserver/domain/service/tags_test.go index e345dfe81..e705722e8 100644 --- a/pkg/apiserver/domain/service/tags_test.go +++ b/pkg/apiserver/domain/service/tags_test.go @@ -19,20 +19,22 @@ package service import ( "testing" + "github.com/oam-dev/kubevela/pkg/definition" + "github.com/stretchr/testify/assert" ) -func TestCompatiblleTag(t *testing.T) { +func TestCompatibleTag(t *testing.T) { tg := map[string]string{ - "alias.config.oam.dev": "abc", - "type.config.oam.dev": "image-registry", - "catalog.config.oam.dev": "cata", + definition.DefinitionAlias: "abc", + definition.DefinitionType: "image-registry", + definition.ConfigCatalog: "cata", } tgOld := map[string]string{ - "custom.definition.oam.dev/alias.config.oam.dev": "abc-2", - "custom.definition.oam.dev/type.config.oam.dev": "image-registry-2", - "custom.definition.oam.dev/catalog.config.oam.dev": "cata-2", + definition.UserPrefix + definition.DefinitionAlias: "abc-2", + definition.UserPrefix + definition.DefinitionType: "image-registry-2", + definition.UserPrefix + definition.ConfigCatalog: "cata-2", } assert.Equal(t, DefinitionAlias(nil), "") diff --git a/pkg/definition/definition.go b/pkg/definition/definition.go index b7bdffd7d..8a003dc8d 100644 --- a/pkg/definition/definition.go +++ b/pkg/definition/definition.go @@ -54,6 +54,12 @@ const ( AliasKey = "definition.oam.dev/alias" // UserPrefix defines the prefix of user customized label or annotation UserPrefix = "custom.definition.oam.dev/" + // DefinitionAlias is alias of definition + DefinitionAlias = "alias.config.oam.dev" + // DefinitionType marks definition's usage type, like image-registry + DefinitionType = "type.config.oam.dev" + // ConfigCatalog marks definition is a catalog + ConfigCatalog = "catalog.config.oam.dev" ) var ( diff --git a/references/cli/provider.go b/references/cli/provider.go index cde6c5f88..ed7c40079 100644 --- a/references/cli/provider.go +++ b/references/cli/provider.go @@ -297,12 +297,25 @@ func listProviders(ctx context.Context, k8sClient client.Client, ioStreams cmdut // getTerraformProviderTypes retrieves all ComponentDefinition for Terraform Cloud Providers which are delivered by // Terraform Cloud provider addons func getTerraformProviderTypes(ctx context.Context, k8sClient client.Client) ([]v1beta1.ComponentDefinition, error) { - defs := &v1beta1.ComponentDefinitionList{} - if err := k8sClient.List(ctx, defs, client.InNamespace(types.DefaultKubeVelaNS), - client.MatchingLabels{definition.UserPrefix + "type.config.oam.dev": types.TerraformProvider}); err != nil { + // keep compatibility with old version of terraform-xxx provider definition + legacyDefs := &v1beta1.ComponentDefinitionList{} + if err := k8sClient.List(ctx, legacyDefs, client.InNamespace(types.DefaultKubeVelaNS), + client.MatchingLabels{definition.UserPrefix + definition.DefinitionType: types.TerraformProvider}); err != nil { return nil, err } - return defs.Items, nil + defs := &v1beta1.ComponentDefinitionList{} + if err := k8sClient.List(ctx, defs, client.InNamespace(types.DefaultKubeVelaNS), + client.MatchingLabels{definition.DefinitionType: types.TerraformProvider}); err != nil { + return nil, err + } + + var result []v1beta1.ComponentDefinition + result = append(result, legacyDefs.Items...) + result = append(result, defs.Items...) + if len(result) == 0 { + return nil, errors.New("no Terraform Cloud Provider ComponentDefinition found") + } + return result, nil } // getTerraformProviderType retrieves the ComponentDefinition for a Terraform Cloud Provider which is delivered by diff --git a/references/cli/provider_test.go b/references/cli/provider_test.go index 49a4a18d4..8e41e989d 100644 --- a/references/cli/provider_test.go +++ b/references/cli/provider_test.go @@ -22,6 +22,7 @@ import ( "testing" terraformapi "github.com/oam-dev/terraform-controller/api/v1beta1" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -31,6 +32,7 @@ import ( "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" "github.com/oam-dev/kubevela/apis/types" + "github.com/oam-dev/kubevela/pkg/definition" "github.com/oam-dev/kubevela/pkg/utils/util" ) @@ -93,3 +95,63 @@ func TestListProviders(t *testing.T) { }) } } + +func TestGetProviderTypes(t *testing.T) { + s := runtime.NewScheme() + v1beta1.AddToScheme(s) + corev1.AddToScheme(s) + terraformapi.AddToScheme(s) + + p1 := &v1beta1.ComponentDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Namespace: types.DefaultKubeVelaNS, + Labels: map[string]string{definition.DefinitionType: types.TerraformProvider}, + }, + } + p2 := &v1beta1.ComponentDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p2", + Namespace: types.DefaultKubeVelaNS, + Labels: map[string]string{definition.UserPrefix + definition.DefinitionType: types.TerraformProvider}, + }, + } + testCases := map[string]struct { + objects []client.Object + gotTypes map[string]bool + wantErr error + }{ + "success": { + objects: []client.Object{p1}, + gotTypes: map[string]bool{ + "p1": true, + }, + }, + "success with legacy provider": { + objects: []client.Object{p2}, + gotTypes: map[string]bool{ + "p2": true, + }, + }, + "not found": { + objects: []client.Object{}, + wantErr: errors.New("no Terraform Cloud Provider ComponentDefinition found"), + }, + } + + ctx := context.Background() + for _, tc := range testCases { + k8sClient = fake.NewClientBuilder().WithScheme(s).WithObjects(tc.objects...).Build() + providerTypes, err := getTerraformProviderTypes(ctx, k8sClient) + if tc.wantErr != nil { + assert.Contains(t, err.Error(), tc.wantErr.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, len(tc.gotTypes), len(providerTypes)) + for _, gotType := range providerTypes { + _, found := tc.gotTypes[gotType.Name] + assert.True(t, found) + } + } + } +}