Merge pull request #2855 from phanimarupaka/DeleteSubst

Delete substitution and fix delete setters

cmd/config/configcobra/cmds.go

@@ -20,6 +20,7 @@ var (
 	CreateSetter       = commands.CreateSetterCommand
 	CreateSubstitution = commands.CreateSubstitutionCommand
 	DeleteSetter       = commands.DeleteSetterCommand
+	DeleteSubstitution = commands.DeleteSubstitutionCommand
 	Fmt                = commands.FmtCommand
 	Grep               = commands.GrepCommand
 	Init               = commands.InitCommand

cmd/config/internal/commands/cmddeletesetter.go

@@ -7,6 +7,7 @@ import (
 	"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/fieldmeta"
 	"sigs.k8s.io/kustomize/kyaml/openapi"
 	"sigs.k8s.io/kustomize/kyaml/setters2/settersutil"
 )
@@ -16,7 +17,7 @@ func NewDeleteSetterRunner(parent string) *DeleteSetterRunner {
 	r := &DeleteSetterRunner{}
 	c := &cobra.Command{
 		Use:     "delete-setter DIR NAME",
-		Args:    cobra.MinimumNArgs(2),
+		Args:    cobra.ExactArgs(2),
 		Short:   commands.DeleteSetterShort,
 		Long:    commands.DeleteSetterLong,
 		Example: commands.DeleteSetterExamples,
@@ -42,6 +43,7 @@ type DeleteSetterRunner struct {
 func (r *DeleteSetterRunner) preRunE(c *cobra.Command, args []string) error {
 	var err error
 	r.DeleteSetter.Name = args[1]
+	r.DeleteSetter.DefinitionPrefix = fieldmeta.SetterDefinitionPrefix
 
 	r.OpenAPIFile, err = ext.GetOpenAPIFile(args)
 	if err != nil {

cmd/config/internal/commands/cmddeletesetter_test.go

@@ -30,25 +30,25 @@ func TestDeleteSetterCommand(t *testing.T) {
 	}{
 		{
 			name: "delete replicas",
-			args: []string{"replicas", "hello world"},
+			args: []string{"replicas-setter"},
 			input: `
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: nginx-deployment
 spec:
-  replicas: 3 # {"$openapi" : "replicas"}}
+  replicas: 3 # {"$openapi" : "replicas-setter"}
  `,
 			inputOpenAPI: `
 apiVersion: v1alpha1
 kind: Example
 openAPI:
   definitions:
-    io.k8s.cli.setters.replicas:
+    io.k8s.cli.setters.replicas-setter:
       description: hello world
       x-k8s-cli:
         setter:
-          name: replicas
+          name: replicas-setter
           value: "3"
           setBy: me
 `,
@@ -67,32 +67,33 @@ spec:
 		},
 		{
 			name: "delete only one setter",
-			args: []string{"replicas", "hello world"},
+			args: []string{"replicas-setter"},
 			input: `
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: nginx-deployment
 spec:
-  replicas: 3 # {"$openapi" : "replicas"}}
+  replicas: 3 # {"$openapi" : "replicas-setter"}
+  foo: nginx # {"$openapi" : "image"}
  `,
 			inputOpenAPI: `
 apiVersion: v1alpha1
 kind: Example
 openAPI:
   definitions:
-    io.k8s.cli.setters.replicas:
+    io.k8s.cli.setters.replicas-setter:
       description: hello world
       x-k8s-cli:
         setter:
-          name: replicas
+          name: replicas-setter
           value: "3"
           setBy: me
     io.k8s.cli.setters.image:
       x-k8s-cli:
         setter:
           name: image
-          value: 1.0
+          value: nginx
 `,
 			expectedOpenAPI: `
 apiVersion: v1alpha1
@@ -103,7 +104,7 @@ openAPI:
       x-k8s-cli:
         setter:
           name: image
-          value: 1.0
+          value: nginx
  `,
 			expectedResources: `
 apiVersion: apps/v1
@@ -112,29 +113,95 @@ metadata:
   name: nginx-deployment
 spec:
   replicas: 3
+  foo: nginx # {"$openapi" : "image"}
  `,
 		},
+
+		{
+			name: "delete array setter",
+			args: []string{"list"},
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.list:
+      items:
+        type: string
+      maxItems: 3
+      type: array
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: list
+          value: ""
+          listValues:
+          - a
+          - b
+          - c
+          setBy: me
+ `,
+			input: `
+apiVersion: example.com/v1beta1
+kind: Example1
+spec:
+  list: # {"$openapi":"list"}
+  - "a"
+  - "b"
+  - "c"
+---
+apiVersion: example.com/v1beta1
+kind: Example2
+spec:
+  list: # {"$openapi":"list2"}
+  - "a"
+  - "b"
+  - "c"
+ `,
+			expectedResources: `
+apiVersion: example.com/v1beta1
+kind: Example1
+spec:
+  list:
+  - "a"
+  - "b"
+  - "c"
+---
+apiVersion: example.com/v1beta1
+kind: Example2
+spec:
+  list: # {"$openapi":"list2"}
+  - "a"
+  - "b"
+  - "c"
+ `,
+			expectedOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+`,
+		},
+
 		{
 			name: "delete non exist setter error",
-			args: []string{"image", "hello world"},
+			args: []string{"image"},
 			input: `
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: nginx-deployment
 spec:
-  replicas: 3 # {"$openapi" : "replicas"}}
+  replicas: 3 # {"$openapi" : "replicas-setter"}
  `,
 			inputOpenAPI: `
 apiVersion: v1alpha1
 kind: Example
 openAPI:
   definitions:
-    io.k8s.cli.setters.replicas:
+    io.k8s.cli.setters.replicas-setter:
       description: hello world
       x-k8s-cli:
         setter:
-          name: replicas
+          name: replicas-setter
           value: "3"
           setBy: me
 `,
@@ -143,11 +210,11 @@ apiVersion: v1alpha1
 kind: Example
 openAPI:
   definitions:
-    io.k8s.cli.setters.replicas:
+    io.k8s.cli.setters.replicas-setter:
       description: hello world
       x-k8s-cli:
         setter:
-          name: replicas
+          name: replicas-setter
           value: "3"
           setBy: me
  `,
@@ -157,13 +224,13 @@ kind: Deployment
 metadata:
   name: nginx-deployment
 spec:
-  replicas: 3 # {"$openapi" : "replicas"}}
+  replicas: 3 # {"$openapi" : "replicas-setter"}
  `,
-			err: `setter does not exist`,
+			err: `setter with name image does not exist`,
 		},
 		{
 			name: "delete setter used in substitution error",
-			args: []string{"image-name", "hello world"},
+			args: []string{"image-name"},
 			input: `
 apiVersion: apps/v1
 kind: Deployment
@@ -220,7 +287,7 @@ openAPI:
 apiVersion: apps/v1
 kind: Deployment
  `,
-			err: `setter is used in substitution image, please delete the substitution first`,
+			err: `setter is used in substitution image, please delete the parent substitution first`,
 		},
 	}
 	for i := range tests {

cmd/config/internal/commands/cmddeletesubstitution.go

@@ -0,0 +1,62 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package commands
+
+import (
+	"github.com/spf13/cobra"
+	"sigs.k8s.io/kustomize/cmd/config/ext"
+	"sigs.k8s.io/kustomize/kyaml/fieldmeta"
+	"sigs.k8s.io/kustomize/kyaml/openapi"
+	"sigs.k8s.io/kustomize/kyaml/setters2/settersutil"
+)
+
+// NewDeleteRunner returns a command runner.
+func NewDeleteSubstitutionRunner(parent string) *DeleteSubstitutionRunner {
+	r := &DeleteSubstitutionRunner{}
+	c := &cobra.Command{
+		Use:     "delete-subst DIR NAME",
+		Args:    cobra.ExactArgs(2),
+		PreRunE: r.preRunE,
+		RunE:    r.runE,
+	}
+	fixDocs(parent, c)
+	r.Command = c
+
+	return r
+}
+
+func DeleteSubstitutionCommand(parent string) *cobra.Command {
+	return NewDeleteSubstitutionRunner(parent).Command
+}
+
+type DeleteSubstitutionRunner struct {
+	Command            *cobra.Command
+	DeleteSubstitution settersutil.DeleterCreator
+	OpenAPIFile        string
+}
+
+func (r *DeleteSubstitutionRunner) preRunE(c *cobra.Command, args []string) error {
+	var err error
+	r.DeleteSubstitution.Name = args[1]
+	r.DeleteSubstitution.DefinitionPrefix = fieldmeta.SubstitutionDefinitionPrefix
+
+	r.OpenAPIFile, err = ext.GetOpenAPIFile(args)
+	if err != nil {
+		return err
+	}
+
+	if err := openapi.AddSchemaFromFile(r.OpenAPIFile); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (r *DeleteSubstitutionRunner) runE(c *cobra.Command, args []string) error {
+	return handleError(c, r.delete(c, args))
+}
+
+func (r *DeleteSubstitutionRunner) delete(c *cobra.Command, args []string) error {
+	return r.DeleteSubstitution.Delete(r.OpenAPIFile, args[0])
+}

cmd/config/internal/commands/cmddeletesubstitution_test.go

@@ -0,0 +1,497 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package commands_test
+
+import (
+	"bytes"
+	"io/ioutil"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/kustomize/cmd/config/ext"
+	"sigs.k8s.io/kustomize/cmd/config/internal/commands"
+	"sigs.k8s.io/kustomize/kyaml/openapi"
+)
+
+func TestDeleteSubstitutionCommand(t *testing.T) {
+	var tests = []struct {
+		name              string
+		input             string
+		args              []string
+		schema            string
+		out               string
+		inputOpenAPI      string
+		expectedOpenAPI   string
+		expectedResources string
+		err               string
+	}{
+		{
+			name: "delete only subst if setter has same name - long ref",
+			args: []string{"my.image"},
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my.image:
+      x-k8s-cli:
+        setter:
+          name: my.image
+          value: "nginx"
+    io.k8s.cli.setters.my-tag:
+      x-k8s-cli:
+        setter:
+          name: my-tag
+          value: "1.7.9"
+    io.k8s.cli.substitutions.my.image:
+      x-k8s-cli:
+        substitution:
+          name: my.image
+          pattern: ${my.image}:${my-tag}
+          values:
+          - marker: ${my.image}
+            ref: '#/definitions/io.k8s.cli.setters.my.image'
+          - marker: ${my-tag}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag'
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+ `,
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3 # {"$openapi":"replicas"}
+  template:
+    spec:
+      containers:
+      - name: nginx # {"$ref":"#/definitions/io.k8s.cli.setters.my.image"}
+        image: nginx:1.7.9 # {"$ref":"#/definitions/io.k8s.cli.substitutions.my.image"}
+      - name: sidecar
+        image: nginx:1.7.9 # {"$ref":"#/definitions/io.k8s.cli.substitutions.my.image"}
+ `,
+			expectedResources: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3 # {"$openapi":"replicas"}
+  template:
+    spec:
+      containers:
+      - name: nginx # {"$ref":"#/definitions/io.k8s.cli.setters.my.image"}
+        image: nginx:1.7.9
+      - name: sidecar
+        image: nginx:1.7.9
+ `,
+			expectedOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my.image:
+      x-k8s-cli:
+        setter:
+          name: my.image
+          value: "nginx"
+    io.k8s.cli.setters.my-tag:
+      x-k8s-cli:
+        setter:
+          name: my-tag
+          value: "1.7.9"
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+ `,
+		},
+		{
+			name: "delete subst - short ref",
+			args: []string{"my-image-sub"},
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image:
+      x-k8s-cli:
+        setter:
+          name: my-image
+          value: "nginx"
+    io.k8s.cli.setters.my-tag:
+      x-k8s-cli:
+        setter:
+          name: my-tag
+          value: "1.7.9"
+    io.k8s.cli.substitutions.my-image-sub:
+      x-k8s-cli:
+        substitution:
+          name: my-image-sub
+          pattern: ${my-image}:${my-tag}
+          values:
+          - marker: ${my-image}
+            ref: '#/definitions/io.k8s.cli.setters.my-image'
+          - marker: ${my-tag}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag'
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+ `,
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3 # {"$openapi":"replicas"}
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.7.9 # {"$openapi":"my-image-sub"}
+      - name: sidecar
+        image: nginx:1.7.9 # {"$openapi":"my-image-sub"}
+ `,
+			expectedResources: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3 # {"$openapi":"replicas"}
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.7.9
+      - name: sidecar
+        image: nginx:1.7.9
+ `,
+			expectedOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image:
+      x-k8s-cli:
+        setter:
+          name: my-image
+          value: "nginx"
+    io.k8s.cli.setters.my-tag:
+      x-k8s-cli:
+        setter:
+          name: my-tag
+          value: "1.7.9"
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+ `,
+		},
+		{
+			name: "error if subst doesn't exist",
+			args: []string{"my-image-sub-not-present"},
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image:
+      x-k8s-cli:
+        setter:
+          name: my-image
+          value: "nginx"
+    io.k8s.cli.setters.my-tag:
+      x-k8s-cli:
+        setter:
+          name: my-tag
+          value: "1.7.9"
+    io.k8s.cli.substitutions.my-image-sub:
+      x-k8s-cli:
+        substitution:
+          name: my-image-sub
+          pattern: ${my-image}:${my-tag}
+          values:
+          - marker: ${my-image}
+            ref: '#/definitions/io.k8s.cli.setters.my-image'
+          - marker: ${my-tag}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag'
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+ `,
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3 # {"$openapi":"replicas"}
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.7.9 # {"$openapi":"my-image-sub"}
+      - name: sidecar
+        image: nginx:1.7.9 # {"$openapi":"my-image-sub"}
+ `,
+			expectedResources: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3 # {"$openapi":"replicas"}
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.7.9
+      - name: sidecar
+        image: nginx:1.7.9
+ `,
+			expectedOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image:
+      x-k8s-cli:
+        setter:
+          name: my-image
+          value: "nginx"
+    io.k8s.cli.setters.my-tag:
+      x-k8s-cli:
+        setter:
+          name: my-tag
+          value: "1.7.9"
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+ `,
+			err: "substitution with name my-image-sub-not-present does not exist",
+		},
+
+		{
+			name: "substitution referenced by other substitution",
+			args: []string{"my-image-subst"},
+			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-image-setter}::${my-tag-setter}
+          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'
+    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'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+ `,
+			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-image-setter}::${my-tag-setter}
+          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'
+    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'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+ `,
+			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: "substitution is used in substitution my-nested-subst, please delete the parent substitution first",
+		},
+	}
+	for i := range tests {
+		test := tests[i]
+		t.Run(test.name, func(t *testing.T) {
+			// reset the openAPI afterward
+			openapi.ResetOpenAPI()
+			defer openapi.ResetOpenAPI()
+
+			f, err := ioutil.TempFile("", "k8s-cli-")
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+			defer os.Remove(f.Name())
+
+			err = ioutil.WriteFile(f.Name(), []byte(test.inputOpenAPI), 0600)
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+
+			old := ext.GetOpenAPIFile
+			defer func() { ext.GetOpenAPIFile = old }()
+			ext.GetOpenAPIFile = func(args []string) (s string, err error) {
+				return f.Name(), nil
+			}
+
+			r, err := ioutil.TempFile("", "k8s-cli-*.yaml")
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+			defer os.Remove(r.Name())
+			err = ioutil.WriteFile(r.Name(), []byte(test.input), 0600)
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+
+			runner := commands.NewDeleteSubstitutionRunner("")
+			out := &bytes.Buffer{}
+			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()
+			}
+
+			if !assert.Equal(t, test.out, out.String()) {
+				t.FailNow()
+			}
+
+			actualResources, err := ioutil.ReadFile(r.Name())
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+			if !assert.Equal(t,
+				strings.TrimSpace(test.expectedResources),
+				strings.TrimSpace(string(actualResources))) {
+				return
+			}
+
+			actualOpenAPI, err := ioutil.ReadFile(f.Name())
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+			if !assert.Equal(t,
+				strings.TrimSpace(test.expectedOpenAPI),
+				strings.TrimSpace(string(actualOpenAPI))) {
+				t.FailNow()
+			}
+		})
+	}
+}

kyaml/setters2/delete.go

@@ -13,23 +13,17 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
-// Delete delete setter or substitution references from resource fields.
-// Requires that FieldName have been set.
+// Delete delete openAPI definition references from resource fields.
 type Delete struct {
+	// Name is the name of the openAPI definition to delete.
+	Name string
 
-	// FieldName if delete the OpenAPI reference to fields with this name or path
-	// FieldName may be the full name of the field, full path to the field, or the path suffix.
-	// e.g. all of the following would match spec.template.spec.containers.image --
-	// [image, containers.image, spec.containers.image, template.spec.containers.image,
-	//  spec.template.spec.containers.image]
-	FieldName string
+	// DefinitionPrefix is the prefix of the OpenAPI definition type
+	DefinitionPrefix string
 }
 
 // Filter implements yaml.Filter
 func (d *Delete) Filter(object *yaml.RNode) (*yaml.RNode, error) {
-	if d.FieldName == "" {
-		return nil, errors.Errorf("must specify fieldName")
-	}
 	return object, accept(d, object)
 }
 
@@ -38,31 +32,40 @@ func (d *Delete) visitSequence(_ *yaml.RNode, _ string, _ *openapi.ResourceSchem
 	return nil
 }
 
-func (d *Delete) visitMapping(_ *yaml.RNode, _ string, _ *openapi.ResourceSchema) error {
-	// no-op
+func (d *Delete) visitMapping(object *yaml.RNode, _ string, _ *openapi.ResourceSchema) error {
+	fieldRNodes, err := object.FieldRNodes()
+	if err != nil {
+		return err
+	}
+
+	// for each of the field node key visit it as scalar to delete the array setter comment
+	for _, fieldRNode := range fieldRNodes {
+		err := d.visitScalar(fieldRNode, "", nil, nil)
+		if err != nil {
+			return err
+		}
+	}
 	return nil
 }
 
 // visitScalar implements visitor
 // visitScalar will remove the reference on each scalar field whose name matches.
-func (d *Delete) visitScalar(object *yaml.RNode, p string, _, _ *openapi.ResourceSchema) error {
-	// check if the field matches
-	if d.FieldName != "" && !strings.HasSuffix(p, d.FieldName) {
-		return nil
-	}
-
+func (d *Delete) visitScalar(object *yaml.RNode, _ string, _, _ *openapi.ResourceSchema) error {
 	// read the field metadata
 	fm := fieldmeta.FieldMeta{}
 	if err := fm.Read(object); err != nil {
 		return err
 	}
 
-	// remove the ref on the metadata
-	fm.Schema.Ref = spec.Ref{}
+	// Delete the reference iff the ref string matches with DefinitionPrefix
+	if strings.HasSuffix(fm.Schema.Ref.String(), d.DefinitionPrefix+d.Name) {
+		// remove the ref on the metadata
+		fm.Schema.Ref = spec.Ref{}
 
-	// write the field metadata
-	if err := fm.Write(object); err != nil {
-		return err
+		// write the field metadata
+		if err := fm.Write(object); err != nil {
+			return err
+		}
 	}
 
 	return nil
@@ -70,16 +73,19 @@ func (d *Delete) visitScalar(object *yaml.RNode, p string, _, _ *openapi.Resourc
 
 // DeleterDefinition may be used to update a files OpenAPI definitions with a new setter.
 type DeleterDefinition struct {
-	// Name is the name of the setter to create or update.
+	// Name is the name of the openAPI definition to delete.
 	Name string `yaml:"name"`
+
+	// DefinitionPrefix is the prefix of the OpenAPI definition type
+	DefinitionPrefix string `yaml:"definitionPrefix"`
 }
 
 func (dd DeleterDefinition) DeleteFromFile(path string) error {
 	return yaml.UpdateFile(dd, path)
 }
 
-// SubstReferringSetter check if the setter used in substitution and return the substitution name if true
-func SubstReferringSetter(definitions *yaml.RNode, key string) string {
+// SubstReferringDefinition check if the definition used in substitution and return the substitution name if true
+func SubstReferringDefinition(definitions *yaml.RNode, key string) string {
 	fieldNames, err := definitions.Fields()
 	if err != nil {
 		return ""
@@ -115,7 +121,18 @@ func SubstReferringSetter(definitions *yaml.RNode, key string) string {
 }
 
 func (dd DeleterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) {
-	key := fieldmeta.SetterDefinitionPrefix + dd.Name
+	key := dd.DefinitionPrefix + dd.Name
+	var defType string
+
+	switch dd.DefinitionPrefix {
+	case fieldmeta.SubstitutionDefinitionPrefix:
+		defType = "substitution"
+	case fieldmeta.SetterDefinitionPrefix:
+		defType = "setter"
+	default:
+		return nil, errors.Errorf("the input delete definitionPrefix does't match any of openAPI definitions, "+
+			"allowed values [%s, %s]", fieldmeta.SetterDefinitionPrefix, fieldmeta.SubstitutionDefinitionPrefix)
+	}
 
 	definitions, err := object.Pipe(yaml.Lookup(openapi.SupplementaryOpenAPIFieldName, "definitions"))
 	if err != nil || definitions == nil {
@@ -123,13 +140,13 @@ func (dd DeleterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 	}
 	// return error if the setter to be deleted doesn't exist
 	if definitions.Field(key) == nil {
-		return nil, errors.Errorf("setter does not exist")
+		return nil, errors.Errorf("%s with name %s does not exist", defType, dd.Name)
 	}
 
-	subst := SubstReferringSetter(definitions, key)
+	subst := SubstReferringDefinition(definitions, key)
 
 	if subst != "" {
-		return nil, errors.Errorf("setter is used in substitution %s, please delete the substitution first", subst)
+		return nil, errors.Errorf("%s is used in substitution %s, please delete the parent substitution first", defType, subst)
 	}
 
 	_, err = definitions.Pipe(yaml.FieldClearer{Name: key})

kyaml/setters2/delete_test.go

@@ -7,116 +7,12 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"sigs.k8s.io/kustomize/kyaml/yaml"
+	"sigs.k8s.io/kustomize/kyaml/fieldmeta"
 )
 
-func TestDelete_Filter(t *testing.T) {
-	var tests = []struct {
-		name           string
-		description    string
-		setter         string
-		input          string
-		expectedOutput string
-	}{
-		{
-			name:   "delete-replicas",
-			setter: "replicas",
-			input: `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx-deployment
-  annotations:
-    replicas: 3 # {"$openapi":"replicas"}
-spec:
-  replicas: 3 # {"$openapi":"replicas"}
- `,
-			expectedOutput: `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx-deployment
-  annotations:
-    replicas: 3
-spec:
-  replicas: 3
- `,
-		},
-		{
-			name:   "delete-foo-annotation",
-			setter: "foo",
-			input: `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx-deployment
-  annotations:
-    foo: 3 # {"$openapi":"foo"}
- `,
-			expectedOutput: `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx-deployment
-  annotations:
-    foo: 3
- `,
-		},
-		{
-			name:   "delete-replicas-enum",
-			setter: "replicas",
-			input: `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx-deployment
-spec:
-  replicas: 1 # {"$openapi":"replicas"}
- `,
-			expectedOutput: `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx-deployment
-spec:
-  replicas: 1
- `,
-		},
-	}
-	for i := range tests {
-		test := tests[i]
-		t.Run(test.name, func(t *testing.T) {
-			// parse the input to be modified
-			r, err := yaml.Parse(test.input)
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
-
-			// invoke the delete
-			instance := &Delete{FieldName: test.setter}
-			result, err := instance.Filter(r)
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
-
-			// compare the actual and expected output
-			actual, err := result.String()
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
-			actual = strings.TrimSpace(actual)
-			expected := strings.TrimSpace(test.expectedOutput)
-			if !assert.Equal(t, expected, actual) {
-				t.FailNow()
-			}
-		})
-	}
-}
-
 var resourcefile2 = `apiVersion: resource.dev/v1alpha1
 kind: resourcefile
 metadata:
@@ -154,7 +50,8 @@ func TestDelete_Filter2(t *testing.T) {
 
 	//add a deleter definition
 	dd := DeleterDefinition{
-		Name: "image",
+		Name:             "image",
+		DefinitionPrefix: fieldmeta.SetterDefinitionPrefix,
 	}
 
 	err = dd.DeleteFromFile(path)

kyaml/setters2/set.go

@@ -45,7 +45,7 @@ func (s *Set) isMatch(name string) bool {
 	return s.SetAll || s.Name == name
 }
 
-func (s *Set) visitMapping(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error {
+func (s *Set) visitMapping(_ *yaml.RNode, p string, _ *openapi.ResourceSchema) error {
 	return nil
 }
 

kyaml/setters2/settersutil/deletecreator_test.go

@@ -10,6 +10,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/kustomize/kyaml/fieldmeta"
+	"sigs.k8s.io/kustomize/kyaml/openapi"
 )
 
 var openAPIFile = `
@@ -25,7 +27,6 @@ openAPI:
         setter:
           name: tag
           value: "sometag"
-
 `
 
 var resourceFile = `
@@ -40,18 +41,20 @@ spec:
 `
 
 func TestDeleterCreator_Delete(t *testing.T) {
+	openapi.ResetOpenAPI()
+	defer openapi.ResetOpenAPI()
 	openAPI, err := ioutil.TempFile("", "openAPI.yaml")
 	if !assert.NoError(t, err) {
 		t.FailNow()
 	}
 	defer os.Remove(openAPI.Name())
-	//write openapi to temp dir
+	// write openapi to temp dir
 	err = ioutil.WriteFile(openAPI.Name(), []byte(openAPIFile), 0666)
 	if !assert.NoError(t, err) {
 		t.FailNow()
 	}
 
-	//write resource file to temp dir
+	// write resource file to temp dir
 	resource, err := ioutil.TempFile("", "k8s-cli-*.yaml")
 	if !assert.NoError(t, err) {
 		t.FailNow()
@@ -62,9 +65,15 @@ func TestDeleterCreator_Delete(t *testing.T) {
 		t.FailNow()
 	}
 
-	//add a delete creator
+	// add a delete creator
 	dc := DeleterCreator{
-		Name: "image",
+		Name:             "image",
+		DefinitionPrefix: fieldmeta.SetterDefinitionPrefix,
+	}
+
+	err = openapi.AddSchemaFromFile(openAPI.Name())
+	if !assert.NoError(t, err) {
+		t.FailNow()
 	}
 
 	err = dc.Delete(openAPI.Name(), resource.Name())

kyaml/setters2/settersutil/deletercreator.go

@@ -9,16 +9,20 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/setters2"
 )
 
-// DeleterCreator delete a setter in the OpenAPI definitions, and removes references
-// to the setter from matching resource fields.
+// DeleterCreator delete a definition in the OpenAPI definitions, and removes references
+// to the definition from matching resource fields.
 type DeleterCreator struct {
-	// Name is the name of the setter to create or update.
+	// Name is the name of the setter or substitution to delete
 	Name string
+
+	// DefinitionPrefix is the prefix of the OpenAPI definition type
+	DefinitionPrefix string
 }
 
 func (d DeleterCreator) Delete(openAPIPath, resourcesPath string) error {
 	dd := setters2.DeleterDefinition{
-		Name: d.Name,
+		Name:             d.Name,
+		DefinitionPrefix: d.DefinitionPrefix,
 	}
 	if err := dd.DeleteFromFile(openAPIPath); err != nil {
 		return err
@@ -35,7 +39,8 @@ func (d DeleterCreator) Delete(openAPIPath, resourcesPath string) error {
 		Inputs: []kio.Reader{inout},
 		Filters: []kio.Filter{kio.FilterAll(
 			&setters2.Delete{
-				FieldName: d.Name,
+				Name:             d.Name,
+				DefinitionPrefix: d.DefinitionPrefix,
 			})},
 		Outputs: []kio.Writer{inout},
 	}.Execute()

kyaml/yaml/rnode.go

@@ -384,6 +384,23 @@ func (rn *RNode) Fields() ([]string, error) {
 	return fields, nil
 }
 
+// FieldRNodes returns the list of field key RNodes for a MappingNode.
+// Returns an error for non-MappingNodes.
+func (rn *RNode) FieldRNodes() ([]*RNode, error) {
+	if err := ErrorIfInvalid(rn, yaml.MappingNode); err != nil {
+		return nil, errors.Wrap(err)
+	}
+	var fields []*RNode
+	for i := 0; i < len(rn.Content()); i += 2 {
+		yNode := rn.Content()[i]
+		// for each key node in the input mapping node contents create equivalent rNode
+		rNode := &RNode{}
+		rNode.SetYNode(yNode)
+		fields = append(fields, rNode)
+	}
+	return fields, nil
+}
+
 // Field returns a fieldName, fieldValue pair for MappingNodes.
 // Returns nil for non-MappingNodes.
 func (rn *RNode) Field(field string) *MapNode {

kyaml/yaml/rnode_test.go

@@ -206,6 +206,62 @@ func TestIsMissingOrNull(t *testing.T) {
 	}
 }
 
+func TestFieldRNodes(t *testing.T) {
+	testCases := []struct {
+		testName string
+		input    string
+		output   []string
+		err      string
+	}{
+		{
+			testName: "simple document",
+			input: `apiVersion: example.com/v1beta1
+kind: Example1
+spec:
+  list:
+  - "a"
+  - "b"
+  - "c"`,
+			output: []string{"apiVersion", "kind", "spec", "list"},
+		},
+		{
+			testName: "non mapping node error",
+			input:    `apiVersion`,
+			err:      "wrong Node Kind for  expected: MappingNode was ScalarNode: value: {apiVersion}",
+		},
+	}
+
+	for i := range testCases {
+		tc := testCases[i]
+		t.Run(tc.testName, func(t *testing.T) {
+			rNode, err := Parse(tc.input)
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+
+			fieldRNodes, err := rNode.FieldRNodes()
+			if tc.err == "" {
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+			} else {
+				if !assert.Equal(t, tc.err, err.Error()) {
+					t.FailNow()
+				}
+			}
+			for i := range fieldRNodes {
+				actual, err := fieldRNodes[i].String()
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+				if !assert.Equal(t, tc.output[i], strings.TrimSpace(actual)) {
+					t.FailNow()
+				}
+			}
+		})
+	}
+}
+
 func TestIsEmptyMap(t *testing.T) {
 	node := NewMapRNode(nil)
 	// empty map