diff --git a/cmd/config/configcobra/cmds.go b/cmd/config/configcobra/cmds.go index da4d4f9a5..19a203c1b 100644 --- a/cmd/config/configcobra/cmds.go +++ b/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 diff --git a/cmd/config/internal/commands/cmddeletesetter.go b/cmd/config/internal/commands/cmddeletesetter.go index 5607046f2..e81c3dfd1 100644 --- a/cmd/config/internal/commands/cmddeletesetter.go +++ b/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 { diff --git a/cmd/config/internal/commands/cmddeletesetter_test.go b/cmd/config/internal/commands/cmddeletesetter_test.go index fa6f11d7b..88f55088f 100644 --- a/cmd/config/internal/commands/cmddeletesetter_test.go +++ b/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 { diff --git a/cmd/config/internal/commands/cmddeletesubstitution.go b/cmd/config/internal/commands/cmddeletesubstitution.go new file mode 100644 index 000000000..cf1997473 --- /dev/null +++ b/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]) +} diff --git a/cmd/config/internal/commands/cmddeletesubstitution_test.go b/cmd/config/internal/commands/cmddeletesubstitution_test.go new file mode 100644 index 000000000..9dd6c2d69 --- /dev/null +++ b/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() + } + }) + } +} diff --git a/kyaml/setters2/delete.go b/kyaml/setters2/delete.go index d6943138a..f594e2f88 100644 --- a/kyaml/setters2/delete.go +++ b/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}) diff --git a/kyaml/setters2/delete_test.go b/kyaml/setters2/delete_test.go index ca9dc2a68..695f07c39 100644 --- a/kyaml/setters2/delete_test.go +++ b/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) diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index e03be56e5..826973590 100644 --- a/kyaml/setters2/set.go +++ b/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 } diff --git a/kyaml/setters2/settersutil/deletecreator_test.go b/kyaml/setters2/settersutil/deletecreator_test.go index 5ab259ab9..6d8e6015a 100644 --- a/kyaml/setters2/settersutil/deletecreator_test.go +++ b/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()) diff --git a/kyaml/setters2/settersutil/deletercreator.go b/kyaml/setters2/settersutil/deletercreator.go index dfdf94331..b84eabfa8 100644 --- a/kyaml/setters2/settersutil/deletercreator.go +++ b/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() diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index 4a99ebda1..6cefce016 100644 --- a/kyaml/yaml/rnode.go +++ b/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 { diff --git a/kyaml/yaml/rnode_test.go b/kyaml/yaml/rnode_test.go index 06eca1a00..ab4037ee6 100644 --- a/kyaml/yaml/rnode_test.go +++ b/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