From 500dc52b3465bddf3c151b48fa8d8fcec35fa54e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 11 Jan 2023 14:25:13 +0800 Subject: [PATCH] [Backport release-1.7] Fix: create a config with the same name reported an incorrect error (#5307) * Fix: create a config with the same name reported an incorrect error Signed-off-by: wuzhongjian (cherry picked from commit 3c1759896b706df99e721e7c31b14a4f1056c145) * Fix: create a config with the same name reported an incorrect error Signed-off-by: wuzhongjian (cherry picked from commit 0aa456642b1149478c0140d01483cd8b4f513014) Co-authored-by: wuzhongjian --- pkg/apiserver/domain/service/config.go | 16 ++++++++++++---- pkg/apiserver/utils/bcode/016_config.go | 3 +++ pkg/config/factory.go | 23 ++++++++++++++++++++++- pkg/config/factory_test.go | 6 ++++++ test/e2e-apiserver-test/config_test.go | 13 ++++++++++++- 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/pkg/apiserver/domain/service/config.go b/pkg/apiserver/domain/service/config.go index 9bee1161b..167faeb17 100644 --- a/pkg/apiserver/domain/service/config.go +++ b/pkg/apiserver/domain/service/config.go @@ -135,6 +135,14 @@ func (u *configServiceImpl) CreateConfig(ctx context.Context, project string, re } ns = pro.GetNamespace() } + exist, err := u.Factory.IsExist(ctx, ns, req.Name) + if err != nil { + klog.Errorf("check config name is exist failure %s", err.Error()) + return nil, bcode.ErrConfigExist + } + if exist { + return nil, bcode.ErrConfigExist + } var properties = make(map[string]interface{}) if err := json.Unmarshal([]byte(req.Properties), &properties); err != nil { return nil, err @@ -154,9 +162,6 @@ func (u *configServiceImpl) CreateConfig(ctx context.Context, project string, re return nil, err } if err := u.Factory.CreateOrUpdateConfig(ctx, configItem, ns); err != nil { - if errors.Is(err, config.ErrConfigExist) { - return nil, bcode.ErrConfigExist - } return nil, err } return convertConfig(project, *configItem), nil @@ -194,9 +199,12 @@ func (u *configServiceImpl) UpdateConfig(ctx context.Context, project string, na return nil, err } if err := u.Factory.CreateOrUpdateConfig(ctx, configItem, ns); err != nil { - if errors.Is(err, config.ErrConfigExist) { + if errors.Is(err, config.ErrChangeTemplate) { return nil, bcode.ErrChangeTemplate } + if errors.Is(err, config.ErrChangeSecretType) { + return nil, bcode.ErrChangeSecretType + } return nil, err } return convertConfig(project, *configItem), nil diff --git a/pkg/apiserver/utils/bcode/016_config.go b/pkg/apiserver/utils/bcode/016_config.go index f7de5cd7a..121096f5d 100644 --- a/pkg/apiserver/utils/bcode/016_config.go +++ b/pkg/apiserver/utils/bcode/016_config.go @@ -37,4 +37,7 @@ var ( // ErrNotFoundDistribution means the distribution is not exist ErrNotFoundDistribution = NewBcode(404, 16007, "the distribution is not exist") + + // ErrChangeSecretType the secret type of the config can not be change + ErrChangeSecretType = NewBcode(400, 16008, "the secret type of the config can not be change") ) diff --git a/pkg/config/factory.go b/pkg/config/factory.go index bedb1cf67..4a00836b0 100644 --- a/pkg/config/factory.go +++ b/pkg/config/factory.go @@ -88,6 +88,12 @@ var ErrConfigNotFound = errors.New("the config is not exist") // ErrTemplateNotFound means the template is not exist var ErrTemplateNotFound = errors.New("the template is not exist") +// ErrChangeTemplate means the template of the config can not be change +var ErrChangeTemplate = errors.New("the template of the config can not be change") + +// ErrChangeSecretType means the secret type of the config can not be change +var ErrChangeSecretType = errors.New("the secret type of the config can not be change") + // NamespacedName the namespace and name model type NamespacedName struct { Name string `json:"name"` @@ -192,6 +198,7 @@ type Factory interface { ListConfigs(ctx context.Context, namespace, template, scope string, withStatus bool) ([]*Config, error) DeleteConfig(ctx context.Context, namespace, name string) error CreateOrUpdateConfig(ctx context.Context, i *Config, ns string) error + IsExist(ctx context.Context, namespace, name string) (bool, error) CreateOrUpdateDistribution(ctx context.Context, ns, name string, ads *CreateDistributionSpec) error ListDistributions(ctx context.Context, ns string) ([]*Distribution, error) @@ -549,7 +556,10 @@ func (k *kubeConfigFactory) CreateOrUpdateConfig(ctx context.Context, i *Config, var secret v1.Secret if err := k.cli.Get(ctx, pkgtypes.NamespacedName{Namespace: i.Namespace, Name: i.Name}, &secret); err == nil { if secret.Labels[types.LabelConfigType] != i.Template.Name { - return ErrConfigExist + return ErrChangeTemplate + } + if secret.Type != i.Secret.Type { + return ErrChangeSecretType } } if err := k.apiApply.Apply(ctx, i.Secret, apply.Quiet()); err != nil { @@ -571,6 +581,17 @@ func (k *kubeConfigFactory) CreateOrUpdateConfig(ctx context.Context, i *Config, return nil } +func (k *kubeConfigFactory) IsExist(ctx context.Context, namespace, name string) (bool, error) { + var secret v1.Secret + if err := k.cli.Get(ctx, pkgtypes.NamespacedName{Namespace: namespace, Name: name}, &secret); err != nil { + if apierrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return true, nil +} + func (k *kubeConfigFactory) ListConfigs(ctx context.Context, namespace, template, scope string, withStatus bool) ([]*Config, error) { var list = &v1.SecretList{} requirement := fmt.Sprintf("%s=%s", types.LabelConfigCatalog, types.VelaCoreConfig) diff --git a/pkg/config/factory_test.go b/pkg/config/factory_test.go index 412e7044c..169e92bea 100644 --- a/pkg/config/factory_test.go +++ b/pkg/config/factory_test.go @@ -144,6 +144,12 @@ var _ = Describe("test config factory", func() { Expect(len(config.Targets)).Should(Equal(1)) }) + It("check if the config exist", func() { + exist, err := fac.IsExist(context.TODO(), "default", "db-config") + Expect(err).Should(BeNil()) + Expect(exist).Should(BeTrue()) + }) + It("list the distributions", func() { distributions, err := fac.ListDistributions(context.TODO(), "default") Expect(err).Should(BeNil()) diff --git a/test/e2e-apiserver-test/config_test.go b/test/e2e-apiserver-test/config_test.go index bdbbe9aeb..c25576eaf 100644 --- a/test/e2e-apiserver-test/config_test.go +++ b/test/e2e-apiserver-test/config_test.go @@ -153,7 +153,7 @@ var _ = Describe("Test the rest api about the config", func() { Expect(config.Secret).Should(BeNil()) Expect(config.Properties["registry"]).Should(Equal("kubevela.test.com")) - By("the template is not exist") + By("the config name is exist") req = v1.CreateConfigRequest{ Name: "test-registry", Alias: "Test Registry", @@ -162,6 +162,17 @@ var _ = Describe("Test the rest api about the config", func() { Properties: `{"registry": "kubevela.test.com"}`, } res = post("/configs", req) + Expect(res.StatusCode).Should(Equal(400)) + + By("the template is not exist") + req = v1.CreateConfigRequest{ + Name: "test-registry2", + Alias: "Test Registry", + Description: "This is a demo config", + Template: v1.NamespacedName{Name: templateName + "notfound"}, + Properties: `{"registry": "kubevela.test.com"}`, + } + res = post("/configs", req) Expect(res.StatusCode).Should(Equal(404)) By("without the template")