Avoid Setter and Substitution with same name

cmd/config/go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/coreos/go-etcd v2.0.0+incompatible // indirect
 	github.com/cpuguy83/go-md2man v1.0.10 // indirect
 	github.com/go-errors/errors v1.0.1
+	github.com/go-openapi/spec v0.19.5
 	github.com/olekukonko/tablewriter v0.0.4
 	github.com/posener/complete/v2 v2.0.1-alpha.12
 	github.com/spf13/cobra v1.0.0

cmd/config/go.sum

@@ -173,6 +173,7 @@ github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B
 github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
 github.com/spf13/cobra v0.0.5 h1:f0B+LkLX6DtmRH1isoNA9VTtNUK9K8xYd28JNNfOv/s=
 github.com/spf13/cobra v0.0.5/go.mod h1:3K3wKZymM7VvHMDS9+Akkh4K60UwM26emMESw8tLCHU=
+github.com/spf13/cobra v1.0.0 h1:6m/oheQuQ13N9ks4hubMG6BnvwOeaJrqSPLahSnczz8=
 github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE=
 github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo=
 github.com/spf13/pflag v0.0.0-20170130214245-9ff6c6923cff/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
@@ -225,6 +226,7 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL
 golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20191004110552-13f9640d40b9 h1:rjwSpXsdiK0dV8/Naq3kAw9ymfAeJIyd0upUIElB+lI=
 golang.org/x/net v0.0.0-20191004110552-13f9640d40b9/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
+golang.org/x/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8=
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=

cmd/config/internal/commands/cmdcreatesetter.go

@@ -4,11 +4,17 @@
 package commands
 
 import (
+	"fmt"
+
+	"github.com/go-openapi/spec"
 	"github.com/spf13/cobra"
 	"sigs.k8s.io/kustomize/cmd/config/ext"
 	"sigs.k8s.io/kustomize/cmd/config/internal/generateddocs/commands"
+	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/kio"
+	"sigs.k8s.io/kustomize/kyaml/openapi"
 	"sigs.k8s.io/kustomize/kyaml/setters"
+	"sigs.k8s.io/kustomize/kyaml/setters2"
 	"sigs.k8s.io/kustomize/kyaml/setters2/settersutil"
 )
 
@@ -86,12 +92,29 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error {
 	if setterVersion == "v2" {
 		var err error
 		r.OpenAPIFile, err = ext.GetOpenAPIFile(args)
-		r.CreateSetter.Description = r.Set.SetPartialField.Description
-		r.CreateSetter.SetBy = r.Set.SetPartialField.SetBy
-		r.CreateSetter.Type = r.Set.SetPartialField.Type
 		if err != nil {
 			return err
 		}
+
+		if err := openapi.AddSchemaFromFile(r.OpenAPIFile); err != nil {
+			return err
+		}
+
+		// check if substitution with same name exists and throw error
+		ref, err := spec.NewRef(setters2.DefinitionsPrefix + setters2.SubstitutionDefinitionPrefix + r.CreateSetter.Name)
+		if err != nil {
+			return err
+		}
+
+		_, err = openapi.Resolve(&ref)
+		if err == nil {
+			return errors.Errorf(fmt.Sprintf("substitution with name %s already exists, "+
+				"substitution and setter can't have same name", r.CreateSetter.Name))
+		}
+
+		r.CreateSetter.Description = r.Set.SetPartialField.Description
+		r.CreateSetter.SetBy = r.Set.SetPartialField.SetBy
+		r.CreateSetter.Type = r.Set.SetPartialField.Type
 	}
 	return nil
 }

cmd/config/internal/commands/cmdcreatesetter_test.go

@@ -22,8 +22,10 @@ func TestCreateSetterCommand(t *testing.T) {
 		input             string
 		args              []string
 		out               string
+		inputOpenAPI      string
 		expectedOpenAPI   string
 		expectedResources string
+		err               string
 	}{
 		{
 			name: "add replicas",
@@ -36,6 +38,10 @@ metadata:
 spec:
   replicas: 3
  `,
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+`,
 			expectedOpenAPI: `
 apiVersion: v1alpha1
 kind: Example
@@ -58,6 +64,27 @@ spec:
   replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"}
  `,
 		},
+		{
+			name: "error if substitution with same name exists",
+			args: []string{"my-image", "3", "--description", "hello world", "--set-by", "me"},
+			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, substitution and setter can't have same name",
+		},
 	}
 	for i := range tests {
 		test := tests[i]
@@ -71,10 +98,7 @@ spec:
 				t.FailNow()
 			}
 			defer os.Remove(f.Name())
-			err = ioutil.WriteFile(f.Name(), []byte(`
-apiVersion: v1alpha1
-kind: Example
-`), 0600)
+			err = ioutil.WriteFile(f.Name(), []byte(test.inputOpenAPI), 0600)
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}
@@ -99,6 +123,14 @@ kind: Example
 			runner.Command.SetOut(out)
 			runner.Command.SetArgs(append([]string{r.Name()}, test.args...))
 			err = runner.Command.Execute()
+			if test.err != "" {
+				if !assert.NotNil(t, err) {
+					t.FailNow()
+				} else {
+					assert.Equal(t, err.Error(), test.err)
+					return
+				}
+			}
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}

cmd/config/internal/commands/cmdcreatesubstitution.go

@@ -4,12 +4,15 @@
 package commands
 
 import (
+	"fmt"
 	"regexp"
 	"strings"
 
+	"github.com/go-openapi/spec"
 	"github.com/spf13/cobra"
 	"sigs.k8s.io/kustomize/cmd/config/ext"
 	"sigs.k8s.io/kustomize/kyaml/errors"
+	"sigs.k8s.io/kustomize/kyaml/openapi"
 	"sigs.k8s.io/kustomize/kyaml/setters2"
 	"sigs.k8s.io/kustomize/kyaml/setters2/settersutil"
 )
@@ -63,6 +66,22 @@ func (r *CreateSubstitutionRunner) preRunE(c *cobra.Command, args []string) erro
 		return err
 	}
 
+	if err := openapi.AddSchemaFromFile(r.OpenAPIFile); err != nil {
+		return err
+	}
+
+	// check if setter with same name exists and throw error
+	ref, err := spec.NewRef(setters2.DefinitionsPrefix + setters2.SetterDefinitionPrefix + r.CreateSubstitution.Name)
+	if err != nil {
+		return err
+	}
+
+	_, err = openapi.Resolve(&ref) // resolve the setter to its openAPI def
+	if err == nil {
+		return errors.Errorf(fmt.Sprintf("setter with name %s already exists, "+
+			"substitution and setter can't have same name", r.CreateSubstitution.Name))
+	}
+
 	// extract setter name tokens from pattern enclosed in ${}
 	re := regexp.MustCompile(`\$\{([^}]*)\}`)
 	markers := re.FindAll([]byte(r.CreateSubstitution.Pattern), -1)

cmd/config/internal/commands/cmdcreatesubstitution_test.go

@@ -25,6 +25,7 @@ func TestCreateSubstitutionCommand(t *testing.T) {
 		out               string
 		expectedOpenAPI   string
 		expectedResources string
+		err               string
 	}{
 		{
 			name: "substitution replicas",
@@ -103,6 +104,23 @@ spec:
         image: sidecar:1.7.9
  `,
 		},
+		{
+			name: "error if setter with same name exists",
+			args: []string{
+				"my-image", "--field-value", "nginx:1.7.9", "--pattern", "${my-image-setter}:${my-tag-setter}"},
+			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, substitution and setter can't have same name",
+		},
 		{
 			name: "substitution and create setters 1",
 			args: []string{
@@ -206,6 +224,14 @@ spec:
 			runner.Command.SetOut(out)
 			runner.Command.SetArgs(append([]string{r.Name()}, test.args...))
 			err = runner.Command.Execute()
+
+			if test.err != "" {
+				if !assert.NotNil(t, err) {
+					t.FailNow()
+				}
+				assert.Equal(t, err.Error(), test.err)
+				return
+			}
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}