From 83a70f783031606d0791232e3010d112517067a2 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Thu, 6 Aug 2020 06:45:26 -0700 Subject: [PATCH] Error on creation if setter/subst exists --- .../internal/commands/cmdcreatesetter.go | 13 ++ .../internal/commands/cmdcreatesetter_test.go | 18 +++ .../commands/cmdcreatesubstitution.go | 14 +- .../commands/cmdcreatesubstitution_test.go | 142 +++--------------- 4 files changed, 66 insertions(+), 121 deletions(-) diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index f2db516c5..1080c8950 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -134,6 +134,19 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { "substitution and setter can't have same name", r.CreateSetter.Name) } + // check if setter with same name exists and throw error + ref, err = spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + r.CreateSetter.Name) + if err != nil { + return err + } + + setter, _ := openapi.Resolve(&ref) + // if setter already exists with the input setter name, throw error + if setter != nil { + return errors.Errorf("setter with name %s already exists, "+ + "if you want to modify it, please delete the existing setter and recreate it", r.CreateSetter.Name) + } + r.CreateSetter.Description = r.Set.SetPartialField.Description r.CreateSetter.SetBy = r.Set.SetPartialField.SetBy r.CreateSetter.Type = r.Set.SetPartialField.Type diff --git a/cmd/config/internal/commands/cmdcreatesetter_test.go b/cmd/config/internal/commands/cmdcreatesetter_test.go index cb0365741..c6822545e 100644 --- a/cmd/config/internal/commands/cmdcreatesetter_test.go +++ b/cmd/config/internal/commands/cmdcreatesetter_test.go @@ -125,6 +125,24 @@ openAPI: err: "substitution with name my-image already exists, substitution and setter can't have same name", }, + { + name: "error if setter with same name exists", + args: []string{ + "my-image", "ubuntu"}, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.my-image: + x-k8s-cli: + setter: + name: my-image + value: "nginx" + `, + err: "setter with name my-image already exists, if you want to modify it, please delete the existing setter and recreate it", + }, + { name: "add replicas with schema", args: []string{"replicas", "3", "--description", "hello world", "--set-by", "me"}, diff --git a/cmd/config/internal/commands/cmdcreatesubstitution.go b/cmd/config/internal/commands/cmdcreatesubstitution.go index 838273bc5..a3657d76e 100644 --- a/cmd/config/internal/commands/cmdcreatesubstitution.go +++ b/cmd/config/internal/commands/cmdcreatesubstitution.go @@ -68,8 +68,20 @@ func (r *CreateSubstitutionRunner) preRunE(c *cobra.Command, args []string) erro return err } + // check if substitution with same name exists and throw error + ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + r.CreateSubstitution.Name) + if err != nil { + return err + } + + subst, _ := openapi.Resolve(&ref) + // if substitution already exists with the input substitution name, throw error + if subst != nil { + return errors.Errorf("substitution with name %s already exists", r.CreateSubstitution.Name) + } + // check if setter with same name exists and throw error - ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + r.CreateSubstitution.Name) + ref, err = spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + r.CreateSubstitution.Name) if err != nil { return err } diff --git a/cmd/config/internal/commands/cmdcreatesubstitution_test.go b/cmd/config/internal/commands/cmdcreatesubstitution_test.go index a0c97a5ed..ccc6a631a 100644 --- a/cmd/config/internal/commands/cmdcreatesubstitution_test.go +++ b/cmd/config/internal/commands/cmdcreatesubstitution_test.go @@ -104,6 +104,27 @@ spec: image: sidecar:1.7.9 `, }, + { + name: "error if substitution with same name exists", + args: []string{"my-image", "--field-value", "some:image", "--pattern", "some:${image}"}, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.substitutions.my-image: + x-k8s-cli: + substitution: + name: my-image + pattern: something/${my-image-setter}::${my-tag-setter}/nginxotherthing + values: + - marker: ${my-image-setter} + ref: '#/definitions/io.k8s.cli.setters.my-image-setter' + - marker: ${my-tag-setter} + ref: '#/definitions/io.k8s.cli.setters.my-tag-setter' + `, + err: "substitution with name my-image already exists", + }, { name: "error if setter with same name exists", args: []string{ @@ -289,125 +310,6 @@ spec: image: nginx::1.7.9 # {"$openapi":"my-image-subst"} `, }, - { - name: "nested cyclic substitution", - args: []string{"my-nested-subst", "--field-value", "something/nginx::1.7.9/nginxotherthing", - "--pattern", "something/${my-image-subst}/${my-other-setter}"}, - inputOpenAPI: ` -apiVersion: v1alpha1 -kind: Example -openAPI: - definitions: - io.k8s.cli.setters.my-image-setter: - x-k8s-cli: - setter: - name: my-image-setter - value: nginx - io.k8s.cli.setters.my-tag-setter: - x-k8s-cli: - setter: - name: my-tag-setter - value: 1.7.9 - io.k8s.cli.substitutions.my-image-subst: - x-k8s-cli: - substitution: - name: my-image-subst - pattern: ${my-nested-subst}::${my-tag-setter} - values: - - marker: ${my-nested-subst} - ref: '#/definitions/io.k8s.cli.substitutions.my-nested-subst' - - marker: ${my-tag-setter} - ref: '#/definitions/io.k8s.cli.setters.my-tag-setter' - io.k8s.cli.setters.my-other-setter: - x-k8s-cli: - setter: - name: my-other-setter - value: nginxotherthing - io.k8s.cli.substitutions.my-nested-subst: - x-k8s-cli: - substitution: - name: my-nested-subst - pattern: something/${my-image-subst}/${my-other-setter} - values: - - marker: ${my-image-subst} - ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst' - - marker: ${my-other-setter} - ref: '#/definitions/io.k8s.cli.setters.my-other-setter' - `, - input: ` -apiVersion: apps/v1 -kind: Deployment -metadata: - name: nginx-deployment -spec: - replicas: 3 - template: - spec: - containers: - - name: nginx - image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"} - - name: sidecar - image: nginx::1.7.9 # {"$openapi":"my-image-subst"} - `, - expectedOpenAPI: ` -apiVersion: v1alpha1 -kind: Example -openAPI: - definitions: - io.k8s.cli.setters.my-image-setter: - x-k8s-cli: - setter: - name: my-image-setter - value: nginx - io.k8s.cli.setters.my-tag-setter: - x-k8s-cli: - setter: - name: my-tag-setter - value: 1.7.9 - io.k8s.cli.substitutions.my-image-subst: - x-k8s-cli: - substitution: - name: my-image-subst - pattern: ${my-nested-subst}::${my-tag-setter} - values: - - marker: ${my-nested-subst} - ref: '#/definitions/io.k8s.cli.substitutions.my-nested-subst' - - marker: ${my-tag-setter} - ref: '#/definitions/io.k8s.cli.setters.my-tag-setter' - io.k8s.cli.setters.my-other-setter: - x-k8s-cli: - setter: - name: my-other-setter - value: nginxotherthing - io.k8s.cli.substitutions.my-nested-subst: - x-k8s-cli: - substitution: - name: my-nested-subst - pattern: something/${my-image-subst}/${my-other-setter} - values: - - marker: ${my-image-subst} - ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst' - - marker: ${my-other-setter} - ref: '#/definitions/io.k8s.cli.setters.my-other-setter' - - `, - expectedResources: ` -apiVersion: apps/v1 -kind: Deployment -metadata: - name: nginx-deployment -spec: - replicas: 3 - template: - spec: - containers: - - name: nginx - image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"} - - name: sidecar - image: nginx::1.7.9 # {"$openapi":"my-image-subst"} -`, - err: "cyclic substitution detected with name my-nested-subst", - }, { name: "substitution with non-existing setter with same name", args: []string{ @@ -484,7 +386,7 @@ spec: if !assert.NotNil(t, err) { t.FailNow() } - assert.Equal(t, err.Error(), test.err) + assert.Equal(t, test.err, err.Error()) return } if !assert.NoError(t, err) {