Merge pull request #2804 from phanimarupaka/SubstWithSameName

Error on creation if setter/subst exists

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

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"},

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
 	}

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) {