From 22a60178707bb10c8a1b774f01a8e5b2634ce680 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Tue, 26 May 2020 17:22:04 -0700 Subject: [PATCH 1/6] update the pr to handle the case when the setter to be deleted is used in substitution --- cmd/config/configcobra/cmds.go | 1 + .../internal/commands/cmddeletesetter.go | 64 ++++ .../internal/commands/cmddeletesetter_test.go | 304 ++++++++++++++++++ kyaml/fieldmeta/fieldmeta.go | 14 +- kyaml/setters2/delete.go | 145 +++++++++ kyaml/setters2/delete_test.go | 191 +++++++++++ .../settersutil/deletecreator_test.go | 107 ++++++ kyaml/setters2/settersutil/deletercreator.go | 42 +++ 8 files changed, 864 insertions(+), 4 deletions(-) create mode 100644 cmd/config/internal/commands/cmddeletesetter.go create mode 100644 cmd/config/internal/commands/cmddeletesetter_test.go create mode 100644 kyaml/setters2/delete.go create mode 100644 kyaml/setters2/delete_test.go create mode 100644 kyaml/setters2/settersutil/deletecreator_test.go create mode 100644 kyaml/setters2/settersutil/deletercreator.go diff --git a/cmd/config/configcobra/cmds.go b/cmd/config/configcobra/cmds.go index 062b52952..da4d4f9a5 100644 --- a/cmd/config/configcobra/cmds.go +++ b/cmd/config/configcobra/cmds.go @@ -19,6 +19,7 @@ var ( Count = commands.CountCommand CreateSetter = commands.CreateSetterCommand CreateSubstitution = commands.CreateSubstitutionCommand + DeleteSetter = commands.DeleteSetterCommand 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 new file mode 100644 index 000000000..d8e63370a --- /dev/null +++ b/cmd/config/internal/commands/cmddeletesetter.go @@ -0,0 +1,64 @@ +// 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/cmd/config/internal/generateddocs/commands" + "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" +) + +// NewDeleteRunner returns a command runner. +func NewDeleteSetterRunner(parent string) *DeleteSetterRunner { + r := &DeleteSetterRunner{} + c := &cobra.Command{ + Use: "delete-setter DIR NAME", + Args: cobra.MinimumNArgs(2), + Short: commands.SetShort, + Long: commands.SetLong, + Example: commands.SetExamples, + PreRunE: r.preRunE, + RunE: r.runE, + } + fixDocs(parent, c) + r.Command = c + + return r +} + +func DeleteSetterCommand(parent string) *cobra.Command { + return NewDeleteSetterRunner(parent).Command +} + +type DeleteSetterRunner struct { + Command *cobra.Command + DeleteSetter settersutil.DeleterCreator + OpenAPIFile string +} + +func (r *DeleteSetterRunner) preRunE(c *cobra.Command, args []string) error { + var err error + r.DeleteSetter.Name = args[1] + + 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 *DeleteSetterRunner) runE(c *cobra.Command, args []string) error { + return handleError(c, r.delete(c, args)) +} + +func (r *DeleteSetterRunner) delete(c *cobra.Command, args []string) error { + return r.DeleteSetter.Delete(r.OpenAPIFile, args[0]) +} diff --git a/cmd/config/internal/commands/cmddeletesetter_test.go b/cmd/config/internal/commands/cmddeletesetter_test.go new file mode 100644 index 000000000..14905527c --- /dev/null +++ b/cmd/config/internal/commands/cmddeletesetter_test.go @@ -0,0 +1,304 @@ +// 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 TestDeleteSetterCommand(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 replicas", + args: []string{"replicas", "hello world"}, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$openapi" : "replicas"}} + `, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me +`, + expectedOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: {} + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + `, + }, + { + name: "delete only one setter", + args: []string{"replicas", "hello world"}, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$openapi" : "replicas"}} + `, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me + io.k8s.cli.setters.image: + x-k8s-cli: + setter: + name: image + value: 1.0 +`, + expectedOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.image: + x-k8s-cli: + setter: + name: image + value: 1.0 + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + `, + }, + { + name: "delete non exist setter error", + args: []string{"image", "hello world"}, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$openapi" : "replicas"}} + `, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me +`, + expectedOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$openapi" : "replicas"}} + `, + err: `setter does not exist`, + }, + { + name: "delete setter used in substitution error", + args: []string{"image-name", "hello world"}, + input: ` +apiVersion: apps/v1 +kind: Deployment + `, + inputOpenAPI: ` +openAPI: + definitions: + io.k8s.cli.setters.image-name: + x-k8s-cli: + setter: + name: image-name + value: "nginx" + io.k8s.cli.setters.image-tag: + x-k8s-cli: + setter: + name: image-tag + value: "1.8.1" + io.k8s.cli.substitutions.image: + x-k8s-cli: + substitution: + name: image + pattern: IMAGE_NAME:IMAGE_TAG + values: + - marker: "IMAGE_NAME" + ref: "#/definitions/io.k8s.cli.setters.image-name" + - marker: "IMAGE_TAG" + ref: "#/definitions/io.k8s.cli.setters.image-tag" +`, + expectedOpenAPI: ` +openAPI: + definitions: + io.k8s.cli.setters.image-name: + x-k8s-cli: + setter: + name: image-name + value: "nginx" + io.k8s.cli.setters.image-tag: + x-k8s-cli: + setter: + name: image-tag + value: "1.8.1" + io.k8s.cli.substitutions.image: + x-k8s-cli: + substitution: + name: image + pattern: IMAGE_NAME:IMAGE_TAG + values: + - marker: "IMAGE_NAME" + ref: "#/definitions/io.k8s.cli.setters.image-name" + - marker: "IMAGE_TAG" + ref: "#/definitions/io.k8s.cli.setters.image-tag" + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment + `, + err: `setter is used in substitution, please delete the 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.NewDeleteSetterRunner("") + 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))) { + t.FailNow() + } + + 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/fieldmeta/fieldmeta.go b/kyaml/fieldmeta/fieldmeta.go index b24a792a9..30d48cc58 100644 --- a/kyaml/fieldmeta/fieldmeta.go +++ b/kyaml/fieldmeta/fieldmeta.go @@ -151,10 +151,16 @@ func (fm *FieldMeta) Write(n *yaml.RNode) error { delete(fm.Schema.VendorExtensible.Extensions, "x-kustomize") } - // Ex: {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} should be converted to - // {"openAPI":"replicas"} and added to the line comment - arr := strings.Split(fm.Schema.Ref.String(), ".") - n.YNode().LineComment = fmt.Sprintf(`{"%s":"%s"}`, shortHandRef, arr[len(arr)-1]) + // Ref is removed when a setter is deleted, so the Ref string could be empty. + if fm.Schema.Ref.String() != "" { + // Ex: {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} should be converted to + // {"openAPI":"replicas"} and added to the line comment + arr := strings.Split(fm.Schema.Ref.String(), ".") + n.YNode().LineComment = fmt.Sprintf(`{"%s":"%s"}`, shortHandRef, arr[len(arr)-1]) + } else { + n.YNode().LineComment = "" + } + return nil } diff --git a/kyaml/setters2/delete.go b/kyaml/setters2/delete.go new file mode 100644 index 000000000..9025749c8 --- /dev/null +++ b/kyaml/setters2/delete.go @@ -0,0 +1,145 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package setters2 + +import ( + "strings" + + "github.com/go-openapi/spec" + "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/fieldmeta" + "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +// Delete delete setter or substitution references from resource fields. +// Requires that FieldName have been set. +type Delete struct { + + // 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 +} + +// 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) +} + +func (d *Delete) visitSequence(_ *yaml.RNode, _ string, _ *openapi.ResourceSchema) error { + // no-op + 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 + } + + // 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{} + + // write the field metadata + if err := fm.Write(object); err != nil { + return err + } + + return nil +} + +// 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 string `yaml:"name"` +} + +func (dd DeleterDefinition) DeleteFromFile(path string) error { + return yaml.UpdateFile(dd, path) +} + +// ContainedInSubstituion check if the setter used in substituition +func ContainedInSubstituion(definitions *yaml.RNode, key string) bool { + fieldNames, err := definitions.Fields() + if err != nil { + return false + } + for _, fieldname := range fieldNames { + // the definition key -- contains the substitution name + subkey := definitions.Field(fieldname).Key.YNode().Value + if strings.HasPrefix(subkey, fieldmeta.SubstitutionDefinitionPrefix) { + substNode, err := definitions.Field(fieldname).Value.Pipe(yaml.Lookup(K8sCliExtensionKey, "substitution")) + if err != nil { + continue + } + + b, err := substNode.String() + if err != nil { + continue + } + + subst := SubstitutionDefinition{} + if err := yaml.Unmarshal([]byte(b), &subst); err != nil { + continue + } + // Check the ref in value to see if it contains the setter key + for _, v := range subst.Values { + if strings.HasSuffix(v.Ref, key) { + return true + } + } + } + } + + return false +} + +func (dd DeleterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) { + key := fieldmeta.SetterDefinitionPrefix + dd.Name + + definitions, err := object.Pipe(yaml.Lookup(openapi.SupplementaryOpenAPIFieldName, "definitions")) + if err != nil || definitions == nil { + return nil, err + } + // return error if the setter to be deleted doesn't exist + if definitions.Field(key) == nil { + return nil, errors.Errorf("setter does not exist") + } + + if ContainedInSubstituion(definitions, key) { + return nil, errors.Errorf("setter is used in substitution, please delete the substitution first") + } + + for i := 0; i < len(definitions.Content()); i += 2 { + if definitions.Content()[i].Value == key { + if len(definitions.YNode().Content) > i+2 { + l := len(definitions.YNode().Content) + // remove from the middle of the list + definitions.YNode().Content = definitions.Content()[:i] + definitions.YNode().Content = append( + definitions.YNode().Content, + definitions.Content()[i+2:l]...) + } else { + // remove from the end of the list + definitions.YNode().Content = definitions.Content()[:i] + } + } + } + + return object, nil +} diff --git a/kyaml/setters2/delete_test.go b/kyaml/setters2/delete_test.go new file mode 100644 index 000000000..ca9dc2a68 --- /dev/null +++ b/kyaml/setters2/delete_test.go @@ -0,0 +1,191 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package setters2 + +import ( + "io/ioutil" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +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: + name: hello-world-set +upstream: + type: git + git: + commit: 5c1c019b59299a4f6c7edd1ff5ff54d720621bbe + directory: /package-examples/helloworld-set + ref: v0.1.0 +packageMetadata: + shortDescription: example package using setters +openAPI: + definitions: + io.k8s.cli.setters.image: + x-k8s-cli: + setter: + name: image + value: "2" + io.k8s.cli.setters.tag: + x-k8s-cli: + setter: + name: tag + value: "sometag" +` + +func TestDelete_Filter2(t *testing.T) { + path := filepath.Join(os.TempDir(), "resourcefile2") + + //write initial resourcefile to temp path + err := ioutil.WriteFile(path, []byte(resourcefile2), 0666) + if !assert.NoError(t, err) { + t.FailNow() + } + + //add a deleter definition + dd := DeleterDefinition{ + Name: "image", + } + + err = dd.DeleteFromFile(path) + if !assert.NoError(t, err) { + t.FailNow() + } + + b, err := ioutil.ReadFile(path) + if err != nil { + t.FailNow() + } + + expected := `apiVersion: resource.dev/v1alpha1 +kind: resourcefile +metadata: + name: hello-world-set +upstream: + type: git + git: + commit: 5c1c019b59299a4f6c7edd1ff5ff54d720621bbe + directory: /package-examples/helloworld-set + ref: v0.1.0 +packageMetadata: + shortDescription: example package using setters +openAPI: + definitions: + io.k8s.cli.setters.tag: + x-k8s-cli: + setter: + name: tag + value: "sometag" +` + assert.Equal(t, expected, string(b)) +} diff --git a/kyaml/setters2/settersutil/deletecreator_test.go b/kyaml/setters2/settersutil/deletecreator_test.go new file mode 100644 index 000000000..5ab259ab9 --- /dev/null +++ b/kyaml/setters2/settersutil/deletecreator_test.go @@ -0,0 +1,107 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package settersutil + +import ( + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +var openAPIFile = ` +openAPI: + definitions: + io.k8s.cli.setters.image: + x-k8s-cli: + setter: + name: image + value: "2" + io.k8s.cli.setters.tag: + x-k8s-cli: + setter: + name: tag + value: "sometag" + +` + +var resourceFile = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + image: 3 # {"$openapi":"image"} +spec: + image: 3 # {"$openapi":"image"} +` + +func TestDeleterCreator_Delete(t *testing.T) { + openAPI, err := ioutil.TempFile("", "openAPI.yaml") + if !assert.NoError(t, err) { + t.FailNow() + } + defer os.Remove(openAPI.Name()) + //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 + resource, err := ioutil.TempFile("", "k8s-cli-*.yaml") + if !assert.NoError(t, err) { + t.FailNow() + } + defer os.Remove(resource.Name()) + err = ioutil.WriteFile(resource.Name(), []byte(resourceFile), 0666) + if !assert.NoError(t, err) { + t.FailNow() + } + + //add a delete creator + dc := DeleterCreator{ + Name: "image", + } + + err = dc.Delete(openAPI.Name(), resource.Name()) + if !assert.NoError(t, err) { + t.FailNow() + } + + actualOpenAPI, err := ioutil.ReadFile(openAPI.Name()) + if err != nil { + t.FailNow() + } + + actualResource, err := ioutil.ReadFile(resource.Name()) + if err != nil { + t.FailNow() + } + + expectedOpenAPI := ` +openAPI: + definitions: + io.k8s.cli.setters.tag: + x-k8s-cli: + setter: + name: tag + value: "sometag" +` + expectedResoure := ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + image: 3 +spec: + image: 3 +` + + assert.Equal(t, strings.TrimSpace(expectedOpenAPI), strings.TrimSpace(string(actualOpenAPI))) + assert.Equal(t, strings.TrimSpace(expectedResoure), strings.TrimSpace(string(actualResource))) +} diff --git a/kyaml/setters2/settersutil/deletercreator.go b/kyaml/setters2/settersutil/deletercreator.go new file mode 100644 index 000000000..dfdf94331 --- /dev/null +++ b/kyaml/setters2/settersutil/deletercreator.go @@ -0,0 +1,42 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package settersutil + +import ( + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/kustomize/kyaml/setters2" +) + +// DeleterCreator delete a setter in the OpenAPI definitions, and removes references +// to the setter from matching resource fields. +type DeleterCreator struct { + // Name is the name of the setter to create or update. + Name string +} + +func (d DeleterCreator) Delete(openAPIPath, resourcesPath string) error { + dd := setters2.DeleterDefinition{ + Name: d.Name, + } + if err := dd.DeleteFromFile(openAPIPath); err != nil { + return err + } + + // Load the updated definitions + if err := openapi.AddSchemaFromFile(openAPIPath); err != nil { + return err + } + + // Update the resources with the deleter reference + inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath} + return kio.Pipeline{ + Inputs: []kio.Reader{inout}, + Filters: []kio.Filter{kio.FilterAll( + &setters2.Delete{ + FieldName: d.Name, + })}, + Outputs: []kio.Writer{inout}, + }.Execute() +} From e4ba898e20959b70fd480e674ad4b172374a2527 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Thu, 18 Jun 2020 12:47:20 -0700 Subject: [PATCH 2/6] return subst name inerror message --- .../internal/commands/cmddeletesetter_test.go | 2 +- kyaml/setters2/delete.go | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cmd/config/internal/commands/cmddeletesetter_test.go b/cmd/config/internal/commands/cmddeletesetter_test.go index 14905527c..dd1f8ce54 100644 --- a/cmd/config/internal/commands/cmddeletesetter_test.go +++ b/cmd/config/internal/commands/cmddeletesetter_test.go @@ -222,7 +222,7 @@ openAPI: apiVersion: apps/v1 kind: Deployment `, - err: `setter is used in substitution, please delete the substitution first`, + err: `setter is used in substitution image, please delete the substitution first`, }, } for i := range tests { diff --git a/kyaml/setters2/delete.go b/kyaml/setters2/delete.go index 9025749c8..f4feaed44 100644 --- a/kyaml/setters2/delete.go +++ b/kyaml/setters2/delete.go @@ -73,11 +73,11 @@ func (dd DeleterDefinition) DeleteFromFile(path string) error { return yaml.UpdateFile(dd, path) } -// ContainedInSubstituion check if the setter used in substituition -func ContainedInSubstituion(definitions *yaml.RNode, key string) bool { +// ContainedInSubstituion check if the setter used in substituition and return the substitution name if true +func ContainedInSubstituion(definitions *yaml.RNode, key string) string { fieldNames, err := definitions.Fields() if err != nil { - return false + return "" } for _, fieldname := range fieldNames { // the definition key -- contains the substitution name @@ -100,13 +100,13 @@ func ContainedInSubstituion(definitions *yaml.RNode, key string) bool { // Check the ref in value to see if it contains the setter key for _, v := range subst.Values { if strings.HasSuffix(v.Ref, key) { - return true + return subst.Name } } } } - return false + return "" } func (dd DeleterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) { @@ -121,8 +121,10 @@ func (dd DeleterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) { return nil, errors.Errorf("setter does not exist") } - if ContainedInSubstituion(definitions, key) { - return nil, errors.Errorf("setter is used in substitution, please delete the substitution first") + subst := ContainedInSubstituion(definitions, key) + + if subst != "" { + return nil, errors.Errorf("setter is used in substitution %s, please delete the substitution first", subst) } for i := 0; i < len(definitions.Content()); i += 2 { From 25186e94af1ecdc792592c8d51cdb9c4f17a7098 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Thu, 18 Jun 2020 13:20:33 -0700 Subject: [PATCH 3/6] implement new visitor method --- kyaml/setters2/delete.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kyaml/setters2/delete.go b/kyaml/setters2/delete.go index f4feaed44..814d39624 100644 --- a/kyaml/setters2/delete.go +++ b/kyaml/setters2/delete.go @@ -38,6 +38,11 @@ func (d *Delete) visitSequence(_ *yaml.RNode, _ string, _ *openapi.ResourceSchem return nil } +func (d *Delete) visitMapping(_ *yaml.RNode, _ string, _ *openapi.ResourceSchema) error { + // no-op + 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 { From 29cadfe8b0f1258f2ed6fb0831be163e90cf7432 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Fri, 19 Jun 2020 16:35:57 -0700 Subject: [PATCH 4/6] modify the doc to simple string, will update the doc later --- cmd/config/internal/commands/cmddeletesetter.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/config/internal/commands/cmddeletesetter.go b/cmd/config/internal/commands/cmddeletesetter.go index d8e63370a..19fc9253a 100644 --- a/cmd/config/internal/commands/cmddeletesetter.go +++ b/cmd/config/internal/commands/cmddeletesetter.go @@ -6,7 +6,6 @@ package commands 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/openapi" "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" ) @@ -17,9 +16,9 @@ func NewDeleteSetterRunner(parent string) *DeleteSetterRunner { c := &cobra.Command{ Use: "delete-setter DIR NAME", Args: cobra.MinimumNArgs(2), - Short: commands.SetShort, - Long: commands.SetLong, - Example: commands.SetExamples, + Short: "delete values on Resources fields.", + Long: "", + Example: "", PreRunE: r.preRunE, RunE: r.runE, } From 99d2994b986a526f4cd028444c53c58989284da6 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Mon, 22 Jun 2020 09:54:10 -0700 Subject: [PATCH 5/6] remove empty definition --- .../internal/commands/cmddeletesetter_test.go | 2 - kyaml/setters2/delete.go | 42 +++++++++---------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/cmd/config/internal/commands/cmddeletesetter_test.go b/cmd/config/internal/commands/cmddeletesetter_test.go index dd1f8ce54..fa6f11d7b 100644 --- a/cmd/config/internal/commands/cmddeletesetter_test.go +++ b/cmd/config/internal/commands/cmddeletesetter_test.go @@ -55,8 +55,6 @@ openAPI: expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example -openAPI: - definitions: {} `, expectedResources: ` apiVersion: apps/v1 diff --git a/kyaml/setters2/delete.go b/kyaml/setters2/delete.go index 814d39624..30b7d223a 100644 --- a/kyaml/setters2/delete.go +++ b/kyaml/setters2/delete.go @@ -78,22 +78,22 @@ func (dd DeleterDefinition) DeleteFromFile(path string) error { return yaml.UpdateFile(dd, path) } -// ContainedInSubstituion check if the setter used in substituition and return the substitution name if true -func ContainedInSubstituion(definitions *yaml.RNode, key string) string { +// SubstReferringSetter check if the setter used in substitution and return the substitution name if true +func SubstReferringSetter(definitions *yaml.RNode, key string) string { fieldNames, err := definitions.Fields() if err != nil { return "" } - for _, fieldname := range fieldNames { + for _, fieldName := range fieldNames { // the definition key -- contains the substitution name - subkey := definitions.Field(fieldname).Key.YNode().Value + subkey := definitions.Field(fieldName).Key.YNode().Value if strings.HasPrefix(subkey, fieldmeta.SubstitutionDefinitionPrefix) { - substNode, err := definitions.Field(fieldname).Value.Pipe(yaml.Lookup(K8sCliExtensionKey, "substitution")) + substNode, err := definitions.Field(fieldName).Value.Pipe(yaml.Lookup(K8sCliExtensionKey, "substitution")) if err != nil { continue } - b, err := substNode.String() + b, err := substNode.MarshalJSON() if err != nil { continue } @@ -126,26 +126,26 @@ func (dd DeleterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) { return nil, errors.Errorf("setter does not exist") } - subst := ContainedInSubstituion(definitions, key) + subst := SubstReferringSetter(definitions, key) if subst != "" { return nil, errors.Errorf("setter is used in substitution %s, please delete the substitution first", subst) } - for i := 0; i < len(definitions.Content()); i += 2 { - if definitions.Content()[i].Value == key { - if len(definitions.YNode().Content) > i+2 { - l := len(definitions.YNode().Content) - // remove from the middle of the list - definitions.YNode().Content = definitions.Content()[:i] - definitions.YNode().Content = append( - definitions.YNode().Content, - definitions.Content()[i+2:l]...) - } else { - // remove from the end of the list - definitions.YNode().Content = definitions.Content()[:i] - } - } + _, err = definitions.Pipe(yaml.FieldClearer{Name:key}) + if err != nil { + return nil, err + } + // remove definitions if it's empty + _, err = object.Pipe(yaml.Lookup(openapi.SupplementaryOpenAPIFieldName), yaml.FieldClearer{Name:"definitions", IfEmpty: true}) + if err != nil { + return nil, err + } + + // remove openApi if it's empty + _, err = object.Pipe(yaml.FieldClearer{Name: openapi.SupplementaryOpenAPIFieldName, IfEmpty: true}) + if err != nil { + return nil, err } return object, nil From be0f1a7fcb108eabd21e444daa6f540548f7fd30 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Mon, 22 Jun 2020 10:00:22 -0700 Subject: [PATCH 6/6] remove the uncessary conversion --- kyaml/setters2/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kyaml/setters2/delete.go b/kyaml/setters2/delete.go index 30b7d223a..a01852e09 100644 --- a/kyaml/setters2/delete.go +++ b/kyaml/setters2/delete.go @@ -99,7 +99,7 @@ func SubstReferringSetter(definitions *yaml.RNode, key string) string { } subst := SubstitutionDefinition{} - if err := yaml.Unmarshal([]byte(b), &subst); err != nil { + if err := yaml.Unmarshal(b, &subst); err != nil { continue } // Check the ref in value to see if it contains the setter key