From 41abeb85be96cce6294f92dc7a41e3fee0ac21f1 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Wed, 21 Oct 2020 14:56:20 -0700 Subject: [PATCH 1/4] refactor `edit add patch` command --- api/types/patch.go | 9 ++ api/types/patch_test.go | 125 ++++++++++++++++++ .../internal/commands/edit/add/addpatch.go | 66 +++++---- .../commands/edit/add/addpatch_test.go | 82 ++++++++++-- 4 files changed, 245 insertions(+), 37 deletions(-) create mode 100644 api/types/patch_test.go diff --git a/api/types/patch.go b/api/types/patch.go index a0de0df8b..e98764bd7 100644 --- a/api/types/patch.go +++ b/api/types/patch.go @@ -17,3 +17,12 @@ type Patch struct { // Target points to the resources that the patch is applied to Target *Selector `json:"target,omitempty" yaml:"target,omitempty"` } + +// Equals return true if p equals another. +func (p *Patch) Equals(another Patch) bool { + targetEqual := (p.Target == another.Target) || + (p.Target != nil && another.Target != nil && *p.Target == *another.Target) + return p.Path == another.Path && + p.Patch == another.Patch && + targetEqual +} diff --git a/api/types/patch_test.go b/api/types/patch_test.go new file mode 100644 index 000000000..1e30de0d5 --- /dev/null +++ b/api/types/patch_test.go @@ -0,0 +1,125 @@ +package types_test + +import ( + "testing" + + "sigs.k8s.io/kustomize/api/resid" + . "sigs.k8s.io/kustomize/api/types" +) + +func TestPatchEquals(t *testing.T) { + selector := Selector{ + Gvk: resid.Gvk{ + Group: "group", + Version: "version", + Kind: "kind", + }, + Name: "name", + Namespace: "namespace", + LabelSelector: "selector", + AnnotationSelector: "selector", + } + type testcase struct { + patch1 Patch + patch2 Patch + expect bool + name string + } + testcases := []testcase{ + { + name: "empty patches", + patch1: Patch{}, + patch2: Patch{}, + expect: true, + }, + { + name: "full patches", + patch1: Patch{ + Path: "foo", + Patch: "bar", + Target: &Selector{ + Gvk: resid.Gvk{ + Group: "group", + Version: "version", + Kind: "kind", + }, + Name: "name", + Namespace: "namespace", + LabelSelector: "selector", + AnnotationSelector: "selector", + }, + }, + patch2: Patch{ + Path: "foo", + Patch: "bar", + Target: &Selector{ + Gvk: resid.Gvk{ + Group: "group", + Version: "version", + Kind: "kind", + }, + Name: "name", + Namespace: "namespace", + LabelSelector: "selector", + AnnotationSelector: "selector", + }, + }, + expect: true, + }, + { + name: "same target", + patch1: Patch{ + Path: "foo", + Patch: "bar", + Target: &selector, + }, + patch2: Patch{ + Path: "foo", + Patch: "bar", + Target: &selector, + }, + expect: true, + }, + { + name: "omit target", + patch1: Patch{ + Path: "foo", + Patch: "bar", + }, + patch2: Patch{ + Path: "foo", + Patch: "bar", + }, + expect: true, + }, + { + name: "one nil target", + patch1: Patch{ + Path: "foo", + Patch: "bar", + Target: &selector, + }, + patch2: Patch{ + Path: "foo", + Patch: "bar", + }, + expect: false, + }, + { + name: "different path", + patch1: Patch{ + Path: "foo", + }, + patch2: Patch{ + Path: "bar", + }, + expect: false, + }, + } + + for _, tc := range testcases { + if tc.expect != tc.patch1.Equals(tc.patch2) { + t.Fatalf("%s: unexpected result %v", tc.name, !tc.expect) + } + } +} diff --git a/kustomize/internal/commands/edit/add/addpatch.go b/kustomize/internal/commands/edit/add/addpatch.go index 5a53df6e9..7d82a3c1c 100644 --- a/kustomize/internal/commands/edit/add/addpatch.go +++ b/kustomize/internal/commands/edit/add/addpatch.go @@ -9,54 +9,67 @@ import ( "github.com/spf13/cobra" "sigs.k8s.io/kustomize/api/filesys" - "sigs.k8s.io/kustomize/kustomize/v3/internal/commands/edit/patch" + "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kustomize/v3/internal/commands/kustfile" - "sigs.k8s.io/kustomize/kustomize/v3/internal/commands/util" ) type addPatchOptions struct { - patchFilePaths []string + Patch types.Patch } // newCmdAddPatch adds the name of a file containing a patch to the kustomization file. func newCmdAddPatch(fSys filesys.FileSystem) *cobra.Command { var o addPatchOptions + o.Patch.Target = &types.Selector{} cmd := &cobra.Command{ Use: "patch", - Short: "Add the name of a file containing a patch to the kustomization file.", + Short: "Add an item to patches field.", + Long: `This command will add an item to patches field in the kustomization file. +Each item may: + + - be either a strategic merge patch, or a JSON patch + - be either a file, or an inline string + - target a single resource or multiple resources + +For more information please see https://kubernetes-sigs.github.io/kustomize/api-reference/kustomization/patches/ +`, Example: ` - add patch {filepath}`, + add patch --path {filepath} --group {target group name} --version {target version}`, RunE: func(cmd *cobra.Command, args []string) error { - err := o.Validate(args) + err := o.Validate() if err != nil { return err } return o.RunAddPatch(fSys) }, } + cmd.Flags().StringVar(&o.Patch.Path, "path", "", "Path to the patch file. Cannot be used with --patch at the same time.") + cmd.Flags().StringVar(&o.Patch.Patch, "patch", "", "Literal string of patch content. Cannot be used with --path at the same time.") + cmd.Flags().StringVar(&o.Patch.Target.Group, "group", "", "API group in patch target") + cmd.Flags().StringVar(&o.Patch.Target.Version, "version", "", "API version in patch target") + cmd.Flags().StringVar(&o.Patch.Target.Kind, "kind", "", "Resource kind in patch target") + cmd.Flags().StringVar(&o.Patch.Target.Name, "name", "", "Resource name in patch target") + cmd.Flags().StringVar(&o.Patch.Target.Namespace, "namespace", "", "Resource namespace in patch target") + cmd.Flags().StringVar(&o.Patch.Target.AnnotationSelector, "annotation-selector", "", "annotationSelector in patch target") + cmd.Flags().StringVar(&o.Patch.Target.LabelSelector, "label-selector", "", "labelSelector in patch target") + return cmd } // Validate validates addPatch command. -func (o *addPatchOptions) Validate(args []string) error { - if len(args) == 0 { - return errors.New("must specify a patch file") +func (o *addPatchOptions) Validate() error { + if o.Patch.Patch != "" && o.Patch.Path != "" { + return errors.New("patch and path can't be set at the same time") + } + if o.Patch.Patch == "" && o.Patch.Path == "" { + return errors.New("must provide either patch or path") } - o.patchFilePaths = args return nil } // RunAddPatch runs addPatch command (do real work). func (o *addPatchOptions) RunAddPatch(fSys filesys.FileSystem) error { - patches, err := util.GlobPatterns(fSys, o.patchFilePaths) - if err != nil { - return err - } - if len(patches) == 0 { - return nil - } - mf, err := kustfile.NewKustomizationFile(fSys) if err != nil { return err @@ -67,13 +80,18 @@ func (o *addPatchOptions) RunAddPatch(fSys filesys.FileSystem) error { return err } - for _, p := range patches { - if patch.Exist(m.PatchesStrategicMerge, p) { - log.Printf("patch %s already in kustomization file", p) - continue - } - m.PatchesStrategicMerge = patch.Append(m.PatchesStrategicMerge, p) + // Omit target if it's empty + emptyTarget := types.Selector{} + if o.Patch.Target != nil && *o.Patch.Target == emptyTarget { + o.Patch.Target = nil } + for _, p := range m.Patches { + if p.Equals(o.Patch) { + log.Printf("patch %#v already in kustomization file", p) + return nil + } + } + m.Patches = append(m.Patches, o.Patch) return mf.Write(m) } diff --git a/kustomize/internal/commands/edit/add/addpatch_test.go b/kustomize/internal/commands/edit/add/addpatch_test.go index 1a5994c1d..3e05301d8 100644 --- a/kustomize/internal/commands/edit/add/addpatch_test.go +++ b/kustomize/internal/commands/edit/add/addpatch_test.go @@ -15,19 +15,34 @@ const ( patchFileName = "myWonderfulPatch.yaml" patchFileContent = ` Lorem ipsum dolor sit amet, consectetur adipiscing elit, -sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ` + kind = "myKind" + group = "myGroup" + version = "myVersion" + name = "myName" + namespace = "myNamespace" + annotationSelector = "myAnnotationSelector" + labelSelector = "myLabelSelector" ) -func TestAddPatchHappyPath(t *testing.T) { +func TestAddPatchWithFilePath(t *testing.T) { fSys := filesys.MakeEmptyDirInMemory() fSys.WriteFile(patchFileName, []byte(patchFileContent)) - fSys.WriteFile(patchFileName+"another", []byte(patchFileContent)) testutils_test.WriteTestKustomization(fSys) cmd := newCmdAddPatch(fSys) - args := []string{patchFileName + "*"} - err := cmd.RunE(cmd, args) + args := []string{ + "--path", patchFileName, + "--kind", kind, + "--group", group, + "--version", version, + "--name", name, + "--namespace", namespace, + "--annotation-selector", annotationSelector, + "--label-selector", labelSelector, + } + cmd.SetArgs(args) + err := cmd.Execute() if err != nil { t.Errorf("unexpected cmd error: %v", err) } @@ -35,11 +50,42 @@ func TestAddPatchHappyPath(t *testing.T) { if err != nil { t.Errorf("unexpected read error: %v", err) } - if !strings.Contains(string(content), patchFileName) { - t.Errorf("expected patch name in kustomization") + for i := 1; i < len(args); i += 2 { + if !strings.Contains(string(content), args[i]) { + t.Errorf("expected flag value of %s in kustomization but got\n%s", args[i-1], content) + } } - if !strings.Contains(string(content), patchFileName+"another") { - t.Errorf("expected patch name in kustomization") +} + +func TestAddPatchWithPatchContent(t *testing.T) { + fSys := filesys.MakeEmptyDirInMemory() + fSys.WriteFile(patchFileName, []byte(patchFileContent)) + testutils_test.WriteTestKustomization(fSys) + + cmd := newCmdAddPatch(fSys) + args := []string{ + "--patch", patchFileContent, + "--kind", kind, + "--group", group, + "--version", version, + "--name", name, + "--namespace", namespace, + "--annotation-selector", annotationSelector, + "--label-selector", labelSelector, + } + cmd.SetArgs(args) + err := cmd.Execute() + if err != nil { + t.Errorf("unexpected cmd error: %v", err) + } + content, err := testutils_test.ReadTestKustomization(fSys) + if err != nil { + t.Errorf("unexpected read error: %v", err) + } + for i := 1; i < len(args); i += 2 { + if !strings.Contains(string(content), strings.Trim(args[i], " \n")) { + t.Errorf("expected flag value of %s in kustomization but got\n%s", args[i-1], content) + } } } @@ -49,14 +95,24 @@ func TestAddPatchAlreadyThere(t *testing.T) { testutils_test.WriteTestKustomization(fSys) cmd := newCmdAddPatch(fSys) - args := []string{patchFileName} - err := cmd.RunE(cmd, args) + args := []string{ + "--path", patchFileName, + "--kind", kind, + "--group", group, + "--version", version, + "--name", name, + "--namespace", namespace, + "--annotation-selector", annotationSelector, + "--label-selector", labelSelector, + } + cmd.SetArgs(args) + err := cmd.Execute() if err != nil { t.Fatalf("unexpected cmd error: %v", err) } // adding an existing patch shouldn't return an error - err = cmd.RunE(cmd, args) + err = cmd.Execute() if err != nil { t.Errorf("unexpected cmd error: %v", err) } @@ -70,7 +126,7 @@ func TestAddPatchNoArgs(t *testing.T) { if err == nil { t.Errorf("expected error: %v", err) } - if err.Error() != "must specify a patch file" { + if err.Error() != "must provide either patch or path" { t.Errorf("incorrect error: %v", err.Error()) } } From bb77a7c86df258d9e70b3521db33152c89dc188d Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Wed, 21 Oct 2020 18:48:53 -0700 Subject: [PATCH 2/4] refactor `edit remove patch` --- .../internal/generateddocs/commands/docs.go | 26 +-- examples/springboot/README.md | 29 ++- examples/zh/springboot.md | 33 +++- kustomize/internal/commands/edit/add/all.go | 2 +- .../commands/edit/patch/strategicmerge.go | 41 ----- .../edit/patch/strategicmerge_test.go | 83 --------- .../internal/commands/edit/remove/all.go | 2 +- .../commands/edit/remove/removepatch.go | 63 ++++--- .../commands/edit/remove/removepatch_test.go | 166 ++++++++++++------ 9 files changed, 211 insertions(+), 234 deletions(-) delete mode 100644 kustomize/internal/commands/edit/patch/strategicmerge.go delete mode 100644 kustomize/internal/commands/edit/patch/strategicmerge_test.go diff --git a/cmd/config/internal/generateddocs/commands/docs.go b/cmd/config/internal/generateddocs/commands/docs.go index 9daca802c..56aee047e 100644 --- a/cmd/config/internal/generateddocs/commands/docs.go +++ b/cmd/config/internal/generateddocs/commands/docs.go @@ -35,22 +35,26 @@ var CatExamples = ` # unwrap Resource config from a directory in an ResourceList ... | kustomize cfg cat` -var CompletionShort = `Install shell completion.` +var CompletionShort = `Generate shell completion.` var CompletionLong = ` -Install shell completion for kustomize commands and flags -- supports bash, fish and zsh. +Generate shell completion for ` + "`" + `kustomize` + "`" + ` -- supports bash, zsh, fish and powershell. +` +var CompletionExamples = ` + # load completion for Bash + source <(kustomize completion bash) - kustomize install-completion + # install for Bash in Linux + kustomize completion bash > /etc/bash_completion.d/kustomize -Registers the completion command with known shells (e.g. .bashrc, .bash_profile, etc): + # install for Bash in MacOS + kustomize completion bash > /usr/local/etc/bash_completion.d/kustomize - complete -C /Users/USER/go/bin/kustomize kustomize + # package for Bash + kustomize completion bash > /usr/share/bash-completion/completions/kustomize -Because the completion command is embedded in kustomize directly, there is no need to update -it separately from the kustomize binary. - -To uninstall shell completion run: - - COMP_UNINSTALL=1 kustomize install-completion` + # package for zsh + kustomize completion zsh > /usr/share/zsh/site-functions/_kustomize +` var CountShort = `[Alpha] Count Resources Config from a local directory.` var CountLong = ` diff --git a/examples/springboot/README.md b/examples/springboot/README.md index e5545621e..be35b4e8e 100644 --- a/examples/springboot/README.md +++ b/examples/springboot/README.md @@ -119,7 +119,7 @@ spec: value: prod EOF -kustomize edit add patch patch.yaml +kustomize edit add patch --path patch.yaml --name sbdemo --kind Deployment --group apps --version v1 cat <$DEMO_HOME/application-prod.properties spring.jpa.hibernate.ddl-auto=update @@ -284,17 +284,32 @@ Add these patches to the kustomization: ``` cd $DEMO_HOME -kustomize edit add patch memorylimit_patch.yaml -kustomize edit add patch healthcheck_patch.yaml +kustomize edit add patch --path memorylimit_patch.yaml --name sbdemo --kind Deployment --group apps --version v1 +kustomize edit add patch --path healthcheck_patch.yaml --name sbdemo --kind Deployment --group apps --version v1 ``` `kustomization.yaml` should have patches field: > ``` -> patchesStrategicMerge: -> - patch.yaml -> - memorylimit_patch.yaml -> - healthcheck_patch.yaml +> patches: +> - path: patch.yaml +> target: +> group: apps +> version: v1 +> kind: Deployment +> name: sbdemo +> - path: memorylimit_patch.yaml +> target: +> group: apps +> version: v1 +> kind: Deployment +> name: sbdemo +> - path: healthcheck_patch.yaml +> target: +> group: apps +> version: v1 +> kind: Deployment +> name: sbdemo > ``` The output of the following command can now be applied diff --git a/examples/zh/springboot.md b/examples/zh/springboot.md index 63ba8555f..5b4fefd6b 100644 --- a/examples/zh/springboot.md +++ b/examples/zh/springboot.md @@ -106,7 +106,7 @@ spec: value: prod EOF -kustomize edit add patch patch.yaml +kustomize edit add patch --path patch.yaml --name sbdemo --kind Deployment --group apps --version v1 cat <$DEMO_HOME/application-prod.properties spring.jpa.hibernate.ddl-auto=update @@ -260,20 +260,35 @@ cat $DEMO_HOME/healthcheck_patch.yaml ``` cd $DEMO_HOME -kustomize edit add patch memorylimit_patch.yaml -kustomize edit add patch healthcheck_patch.yaml +kustomize edit add patch --path memorylimit_patch.yaml --name sbdemo --kind Deployment --group apps --version v1 +kustomize edit add patch --path healthcheck_patch.yaml --name sbdemo --kind Deployment --group apps --version v1 ``` -执行上面的命令后，`kustomization.yaml` 的 patchesStrategicMerge 字段如下： +执行上面的命令后，`kustomization.yaml` 的 patches 字段如下： > ``` -> patchesStrategicMerge: -> - patch.yaml -> - memorylimit_patch.yaml -> - healthcheck_patch.yaml +> patches: +> - path: patch.yaml +> target: +> group: apps +> version: v1 +> kind: Deployment +> name: sbdemo +> - path: memorylimit_patch.yaml +> target: +> group: apps +> version: v1 +> kind: Deployment +> name: sbdemo +> - path: healthcheck_patch.yaml +> target: +> group: apps +> version: v1 +> kind: Deployment +> name: sbdemo > ``` -现在就可以将完整的配置输出并在集群中部署（将结果通过管道输出给 `kubectl apply`），在生产环境创建Spring Boot 应用。 +现在就可以将完整的配置输出并在集群中部署（将结果通过管道输出给 `kubectl apply`），在生产环境创建 Spring Boot 应用。 ``` diff --git a/kustomize/internal/commands/edit/add/all.go b/kustomize/internal/commands/edit/add/all.go index e6ecbc05d..1e34f7160 100644 --- a/kustomize/internal/commands/edit/add/all.go +++ b/kustomize/internal/commands/edit/add/all.go @@ -29,7 +29,7 @@ func NewCmdAdd( kustomize edit add resource # Adds a patch to the kustomization - kustomize edit add patch + kustomize edit add patch --path {filepath} --group {target group name} --version {target version} # Adds a component to the kustomization kustomize edit add component diff --git a/kustomize/internal/commands/edit/patch/strategicmerge.go b/kustomize/internal/commands/edit/patch/strategicmerge.go deleted file mode 100644 index d1a20733b..000000000 --- a/kustomize/internal/commands/edit/patch/strategicmerge.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2019 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 - -package patch - -import "sigs.k8s.io/kustomize/api/types" - -// Append appends a slice of patch paths to a PatchStrategicMerge slice -func Append(patches []types.PatchStrategicMerge, paths ...string) []types.PatchStrategicMerge { - for _, p := range paths { - patches = append(patches, types.PatchStrategicMerge(p)) - } - return patches -} - -// Exist determines if a patch path exists in a slice of PatchStrategicMerge -func Exist(patches []types.PatchStrategicMerge, path string) bool { - for _, p := range patches { - if p == types.PatchStrategicMerge(path) { - return true - } - } - return false -} - -// Delete deletes patches from a PatchStrategicMerge slice -func Delete(patches []types.PatchStrategicMerge, paths ...string) []types.PatchStrategicMerge { - // Convert paths into PatchStrategicMerge slice - convertedPath := make([]types.PatchStrategicMerge, len(paths)) - for i, p := range paths { - convertedPath[i] = types.PatchStrategicMerge(p) - } - - filteredPatches := make([]types.PatchStrategicMerge, 0, len(patches)) - for _, containedPatch := range patches { - if !Exist(convertedPath, string(containedPatch)) { - filteredPatches = append(filteredPatches, containedPatch) - } - } - return filteredPatches -} diff --git a/kustomize/internal/commands/edit/patch/strategicmerge_test.go b/kustomize/internal/commands/edit/patch/strategicmerge_test.go deleted file mode 100644 index 66615caab..000000000 --- a/kustomize/internal/commands/edit/patch/strategicmerge_test.go +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2019 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2. - -package patch - -import ( - "testing" - - "sigs.k8s.io/kustomize/api/types" -) - -func buildPatchStrategicMergeSlice(patchStrings []string) []types.PatchStrategicMerge { - var patches []types.PatchStrategicMerge - for _, patchString := range patchStrings { - patches = append(patches, types.PatchStrategicMerge(patchString)) - } - return patches -} - -func TestAppend(t *testing.T) { - patchStrings := []string{"patch1.yaml", "patch2.yaml"} - patches := buildPatchStrategicMergeSlice(patchStrings) - - patches = Append(patches, "patch3.yaml") - - for i, k := range []string{"patch1.yaml", "patch2.yaml", "patch3.yaml"} { - if patches[i] != types.PatchStrategicMerge(k) { - t.Fatalf("patches[%d] must be %s, got %s", i, k, patches[i]) - } - } -} - -func TestExistTrue(t *testing.T) { - patchStrings := []string{"patch1.yaml", "patch2.yaml"} - patches := buildPatchStrategicMergeSlice(patchStrings) - - for _, patchString := range patchStrings { - if !Exist(patches, patchString) { - t.Fatalf("%s must exist", patchString) - } - } -} - -func TestExistFalse(t *testing.T) { - patchStrings := []string{"patch1.yaml", "patch2.yaml"} - patches := buildPatchStrategicMergeSlice(patchStrings) - - for _, patchString := range []string{"invalid1.yaml", "invalid2.yaml"} { - if Exist(patches, patchString) { - t.Fatalf("%s must not exist", patchString) - } - } -} - -func TestDelete(t *testing.T) { - patchStrings := []string{"patch1.yaml", "patch2.yaml"} - patches := buildPatchStrategicMergeSlice(patchStrings) - - patches = Delete(patches, "patch1.yaml") - - if Exist(patches, "patch1.yaml") { - t.Fatalf("patch1.yaml should be deleted") - } - if !Exist(patches, "patch2.yaml") { - t.Fatalf("patch2.yaml should exist") - } - if len(patches) != 1 { - t.Fatalf("Length of slice must be 1: actual %d", len(patches)) - } -} - -func TestDeleteMultiple(t *testing.T) { - patchStrings := []string{"patch1.yaml", "patch2.yaml"} - patches := buildPatchStrategicMergeSlice(patchStrings) - - patches = Delete(patches, "patch2.yaml", "patch4.yaml", "patch1.yaml", "patch3.yaml") - - for _, k := range patchStrings { - if Exist(patches, k) { - t.Fatalf("%s should be deleted", k) - } - } -} diff --git a/kustomize/internal/commands/edit/remove/all.go b/kustomize/internal/commands/edit/remove/all.go index 78be733f0..56701c6e0 100644 --- a/kustomize/internal/commands/edit/remove/all.go +++ b/kustomize/internal/commands/edit/remove/all.go @@ -23,7 +23,7 @@ func NewCmdRemove( kustomize edit remove resource {pattern} # Removes one or more patches from the kustomization file - kustomize edit remove patch + kustomize edit remove patch --path {filepath} --group {target group name} --version {target version} # Removes one or more commonLabels from the kustomization file kustomize edit remove label {labelKey1},{labelKey2} diff --git a/kustomize/internal/commands/edit/remove/removepatch.go b/kustomize/internal/commands/edit/remove/removepatch.go index e1e0762fe..f3645fc1b 100644 --- a/kustomize/internal/commands/edit/remove/removepatch.go +++ b/kustomize/internal/commands/edit/remove/removepatch.go @@ -10,55 +10,58 @@ import ( "github.com/spf13/cobra" "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/konfig" - "sigs.k8s.io/kustomize/kustomize/v3/internal/commands/edit/patch" + "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kustomize/v3/internal/commands/kustfile" - "sigs.k8s.io/kustomize/kustomize/v3/internal/commands/util" ) type removePatchOptions struct { - patchFilePaths []string + Patch types.Patch } // newCmdRemovePatch removes the name of a file containing a patch from the kustomization file. func newCmdRemovePatch(fSys filesys.FileSystem) *cobra.Command { var o removePatchOptions + o.Patch.Target = &types.Selector{} cmd := &cobra.Command{ Use: "patch", - Short: "Removes one or more patches from " + + Short: "Removes a patch from " + konfig.DefaultKustomizationFileName(), + Long: `Removes a patch from patches field. The fields specified by flags must +exactly match the patch item to successfully remote the item.`, Example: ` - remove patch {filepath}`, + remove patch --path {filepath} --group {target group name} --version {target version}`, RunE: func(cmd *cobra.Command, args []string) error { - err := o.Validate(args) + err := o.Validate() if err != nil { return err } return o.RunRemovePatch(fSys) }, } + cmd.Flags().StringVar(&o.Patch.Path, "path", "", "Path to the patch file. Cannot be used with --patch at the same time.") + cmd.Flags().StringVar(&o.Patch.Patch, "patch", "", "Literal string of patch content. Cannot be used with --path at the same time.") + cmd.Flags().StringVar(&o.Patch.Target.Group, "group", "", "API group in patch target") + cmd.Flags().StringVar(&o.Patch.Target.Version, "version", "", "API version in patch target") + cmd.Flags().StringVar(&o.Patch.Target.Kind, "kind", "", "Resource kind in patch target") + cmd.Flags().StringVar(&o.Patch.Target.Name, "name", "", "Resource name in patch target") + cmd.Flags().StringVar(&o.Patch.Target.Namespace, "namespace", "", "Resource namespace in patch target") + cmd.Flags().StringVar(&o.Patch.Target.AnnotationSelector, "annotation-selector", "", "annotationSelector in patch target") + cmd.Flags().StringVar(&o.Patch.Target.LabelSelector, "label-selector", "", "labelSelector in patch target") + return cmd } // Validate validates removePatch command. -func (o *removePatchOptions) Validate(args []string) error { - if len(args) == 0 { - return errors.New("must specify a patch file") +func (o *removePatchOptions) Validate() error { + if o.Patch.Patch != "" && o.Patch.Path != "" { + return errors.New("patch and path can't be set at the same time") } - o.patchFilePaths = args return nil } // RunRemovePatch runs removePatch command (do real work). func (o *removePatchOptions) RunRemovePatch(fSys filesys.FileSystem) error { - patches, err := util.GlobPatterns(fSys, o.patchFilePaths) - if err != nil { - return err - } - if len(patches) == 0 { - return nil - } - mf, err := kustfile.NewKustomizationFile(fSys) if err != nil { return err @@ -69,15 +72,23 @@ func (o *removePatchOptions) RunRemovePatch(fSys filesys.FileSystem) error { return err } - var removePatches []string - for _, p := range patches { - if !patch.Exist(m.PatchesStrategicMerge, p) { - log.Printf("patch %s doesn't exist in kustomization file", p) - continue - } - removePatches = append(removePatches, p) + // Omit target if it's empty + emptyTarget := types.Selector{} + if o.Patch.Target != nil && *o.Patch.Target == emptyTarget { + o.Patch.Target = nil } - m.PatchesStrategicMerge = patch.Delete(m.PatchesStrategicMerge, removePatches...) + + var patches []types.Patch + for _, p := range m.Patches { + if !p.Equals(o.Patch) { + patches = append(patches, p) + } + } + if len(patches) == len(m.Patches) { + log.Printf("patch %s doesn't exist in kustomization file", o.Patch) + return nil + } + m.Patches = patches return mf.Write(m) } diff --git a/kustomize/internal/commands/edit/remove/removepatch_test.go b/kustomize/internal/commands/edit/remove/removepatch_test.go index 2df4ae2c3..af2222336 100644 --- a/kustomize/internal/commands/edit/remove/removepatch_test.go +++ b/kustomize/internal/commands/edit/remove/removepatch_test.go @@ -4,29 +4,53 @@ package remove import ( - "fmt" - "strings" "testing" "sigs.k8s.io/kustomize/api/filesys" - "sigs.k8s.io/kustomize/kustomize/v3/internal/commands/edit/patch" testutils_test "sigs.k8s.io/kustomize/kustomize/v3/internal/commands/testutils" ) const ( - patchFileContent = ` -Lorem ipsum dolor sit amet, consectetur adipiscing elit, -sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. -` + patchFileContent = `- op: replace + path: /some/existing/path + value: new value` + kind = "myKind" + group = "myGroup" + version = "myVersion" + name = "myName" + namespace = "myNamespace" + annotationSelector = "myAnnotationSelector" + labelSelector = "myLabelSelector" ) func makeKustomizationPatchFS() filesys.FileSystem { fSys := filesys.MakeEmptyDirInMemory() patches := []string{"patch1.yaml", "patch2.yaml"} - testutils_test.WriteTestKustomizationWith(fSys, []byte( - fmt.Sprintf("patchesStrategicMerge:\n - %s", - strings.Join(patches, "\n - ")))) + testutils_test.WriteTestKustomizationWith(fSys, []byte(` +patches: +- path: patch1.yaml + target: + group: myGroup + version: myVersion + kind: myKind + name: myName + namespace: myNamespace + labelSelector: myLabelSelector + annotationSelector: myAnnotationSelector +- path: patch2.yaml + target: + group: myGroup + version: myVersion + kind: myKind +- patch: |- + - op: replace + path: /some/existing/path + value: new value + target: + kind: myKind + labelSelector: myLabelSelector +`)) for _, p := range patches { fSys.WriteFile(p, []byte(patchFileContent)) @@ -38,69 +62,86 @@ func makeKustomizationPatchFS() filesys.FileSystem { func TestRemovePatch(t *testing.T) { fSys := makeKustomizationPatchFS() cmd := newCmdRemovePatch(fSys) - args := []string{"patch1.yaml"} - err := cmd.RunE(cmd, args) + patchPath := "patch1.yaml" + args := []string{ + "--path", patchPath, + "--kind", kind, + "--group", group, + "--version", version, + "--name", name, + "--namespace", namespace, + "--annotation-selector", annotationSelector, + "--label-selector", labelSelector, + } + cmd.SetArgs(args) + err := cmd.Execute() if err != nil { - t.Errorf("unexpected error %v", err) + t.Fatalf("unexpected error %v", err) } m := readKustomizationFS(t, fSys) - for _, k := range args { - if patch.Exist(m.PatchesStrategicMerge, k) { - t.Errorf("%s must be deleted", k) + for _, p := range m.Patches { + if p.Path == patchPath { + t.Fatalf("%s must be deleted", patchPath) } } } -func TestRemovePatchMultipleArgs(t *testing.T) { +func TestRemovePatch2(t *testing.T) { fSys := makeKustomizationPatchFS() cmd := newCmdRemovePatch(fSys) - args := []string{"patch1.yaml", "patch2.yaml"} - err := cmd.RunE(cmd, args) + args := []string{ + "--patch", patchFileContent, + "--kind", kind, + "--label-selector", labelSelector, + } + cmd.SetArgs(args) + err := cmd.Execute() if err != nil { - t.Errorf("unexpected error %v", err) + t.Fatalf("unexpected error %v", err) } m := readKustomizationFS(t, fSys) - for _, k := range args { - if patch.Exist(m.PatchesStrategicMerge, k) { - t.Errorf("%s must be deleted", k) + for _, p := range m.Patches { + if p.Patch == patchFileContent { + t.Fatalf("%s must be deleted", patchFileContent) } } } -func TestRemovePatchGlob(t *testing.T) { - fSys := makeKustomizationPatchFS() - cmd := newCmdRemovePatch(fSys) - args := []string{"patch*.yaml"} - err := cmd.RunE(cmd, args) - - if err != nil { - t.Errorf("unexpected error %v", err) - } - - m := readKustomizationFS(t, fSys) - if len(m.PatchesStrategicMerge) != 0 { - t.Errorf("all patch must be deleted") - } -} - func TestRemovePatchNotDefinedInKustomization(t *testing.T) { fSys := makeKustomizationPatchFS() cmd := newCmdRemovePatch(fSys) - args := []string{"patch3.yaml"} - err := cmd.RunE(cmd, args) + args := []string{ + "--path", "patch3.yaml", + "--kind", kind, + "--group", group, + "--version", version, + "--name", name, + "--namespace", namespace, + "--annotation-selector", annotationSelector, + "--label-selector", labelSelector, + } + cmd.SetArgs(args) + err := cmd.Execute() if err != nil { - t.Errorf("unexpected error %v", err) + t.Fatalf("unexpected error %v", err) } m := readKustomizationFS(t, fSys) for _, k := range []string{"patch1.yaml", "patch2.yaml"} { - if !patch.Exist(m.PatchesStrategicMerge, k) { - t.Errorf("%s must exist", k) + found := false + for _, p := range m.Patches { + if p.Path == k { + found = true + break + } + } + if !found { + t.Fatalf("%s must exist", k) } } } @@ -108,30 +149,45 @@ func TestRemovePatchNotDefinedInKustomization(t *testing.T) { func TestRemovePatchNotExist(t *testing.T) { fSys := makeKustomizationPatchFS() cmd := newCmdRemovePatch(fSys) - args := []string{"patch4.yaml"} - err := cmd.RunE(cmd, args) + args := []string{ + "--path", "patch4.yaml", + "--kind", kind, + "--group", group, + "--version", version, + "--name", name, + "--namespace", namespace, + "--annotation-selector", annotationSelector, + "--label-selector", labelSelector, + } + cmd.SetArgs(args) + err := cmd.Execute() if err != nil { - t.Errorf("unexpected error %v", err) + t.Fatalf("unexpected error %v", err) } m := readKustomizationFS(t, fSys) for _, k := range []string{"patch1.yaml", "patch2.yaml"} { - if !patch.Exist(m.PatchesStrategicMerge, k) { - t.Errorf("%s must exist", k) + found := false + for _, p := range m.Patches { + if p.Path == k { + found = true + break + } + } + if !found { + t.Fatalf("%s must exist", k) } } } func TestRemovePatchNoArgs(t *testing.T) { + // if no flags specified, we should do nothing fSys := makeKustomizationPatchFS() cmd := newCmdRemovePatch(fSys) - err := cmd.RunE(cmd, nil) + err := cmd.Execute() - if err == nil { - t.Errorf("expected an error") - } - if err.Error() != "must specify a patch file" { - t.Errorf("incorrect error: %v", err.Error()) + if err != nil { + t.Fatalf("unexpected error %v", err) } } From cd2ebd30465753dbb59483756278f23a00d4f9af Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 27 Oct 2020 15:10:29 -0700 Subject: [PATCH 3/4] code review --- api/types/patch.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/types/patch.go b/api/types/patch.go index e98764bd7..48b521d7e 100644 --- a/api/types/patch.go +++ b/api/types/patch.go @@ -18,11 +18,11 @@ type Patch struct { Target *Selector `json:"target,omitempty" yaml:"target,omitempty"` } -// Equals return true if p equals another. -func (p *Patch) Equals(another Patch) bool { - targetEqual := (p.Target == another.Target) || - (p.Target != nil && another.Target != nil && *p.Target == *another.Target) - return p.Path == another.Path && - p.Patch == another.Patch && +// Equals return true if p equals o. +func (p *Patch) Equals(o Patch) bool { + targetEqual := (p.Target == o.Target) || + (p.Target != nil && o.Target != nil && *p.Target == *o.Target) + return p.Path == o.Path && + p.Patch == o.Patch && targetEqual } From 9aafc61c5b7a291b039090d12ce21a2965ada4a4 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Wed, 28 Oct 2020 12:24:51 -0700 Subject: [PATCH 4/4] disable edit add patch command tests temporarily --- examples/configGeneration.md | 41 +++++++++++++++++++++------------ examples/springboot/README.md | 4 ++-- examples/zh/configGeneration.md | 33 ++++++++++++++++++-------- examples/zh/springboot.md | 4 ++-- 4 files changed, 53 insertions(+), 29 deletions(-) diff --git a/examples/configGeneration.md b/examples/configGeneration.md index ab73afbfb..b719c19b2 100644 --- a/examples/configGeneration.md +++ b/examples/configGeneration.md @@ -4,13 +4,13 @@ ## ConfigMap generation and rolling updates -Kustomize provides two ways of adding ConfigMap in one `kustomization`, either by declaring ConfigMap as a [resource] or declaring ConfigMap from a ConfigMapGenerator. The formats inside `kustomization.yaml` are +Kustomize provides two ways of adding ConfigMap in one `kustomization`, either by declaring ConfigMap as a [resource] or declaring ConfigMap from a ConfigMapGenerator. The formats inside `kustomization.yaml` are > ``` > # declare ConfigMap as a resource > resources: > - configmap.yaml -> +> > # declare ConfigMap from a ConfigMapGenerator > configMapGenerator: > - name: a-configmap @@ -28,7 +28,9 @@ In this demo, the same [hello_world](helloWorld/README.md) is used while the Con ### Establish base and staging Establish the base with a configMapGenerator + + ``` DEMO_HOME=$(mktemp -d) @@ -46,16 +48,18 @@ commonLabels: resources: - deployment.yaml - service.yaml -configMapGenerator: -- name: the-map - literals: - - altGreeting=Good Morning! +configMapGenerator: +- name: the-map + literals: + - altGreeting=Good Morning! - enableRisky="false" EOF ``` Establish the staging with a patch applied to the ConfigMap + + ``` OVERLAYS=$DEMO_HOME/overlays mkdir -p $OVERLAYS/staging @@ -92,8 +96,8 @@ configured with data from a configMap. The deployment refers to this map by name: - + ``` grep -C 2 configMapKeyRef $BASE/deployment.yaml ``` @@ -106,12 +110,12 @@ changed, so such updates have no effect. The recommended way to change a deployment's configuration is to - 1. create a new configMap with a new name, - 1. patch the _deployment_, modifying the name value of +1. create a new configMap with a new name, +1. patch the _deployment_, modifying the name value of the appropriate `configMapKeyRef` field. This latter change initiates rolling update to the pods -in the deployment. The older configMap, when no longer +in the deployment. The older configMap, when no longer referenced by any other resource, is eventually [garbage collected](/../../issues/242). @@ -120,6 +124,7 @@ collected](/../../issues/242). The _staging_ [variant] here has a configMap [patch]: + ``` cat $OVERLAYS/staging/map.yaml ``` @@ -131,6 +136,7 @@ resource spec. The ConfigMap it modifies is declared from a configMapGenerator. + ``` grep -C 4 configMapGenerator $BASE/kustomization.yaml ``` @@ -139,11 +145,12 @@ For a patch to work, the names in the `metadata/name` fields must match. However, the name values specified in the file are -_not_ what gets used in the cluster. By design, -kustomize modifies names of ConfigMaps declared from ConfigMapGenerator. To see the names +_not_ what gets used in the cluster. By design, +kustomize modifies names of ConfigMaps declared from ConfigMapGenerator. To see the names ultimately used in the cluster, just run kustomize: + ``` kustomize build $OVERLAYS/staging |\ grep -B 8 -A 1 staging-the-map @@ -161,7 +168,8 @@ The suffix to the configMap name is generated from a hash of the maps content - in this case the name suffix is _5276h4th55_: - + + ``` kustomize build $OVERLAYS/staging | grep 5276h4th55 ``` @@ -170,6 +178,7 @@ Now modify the map patch, to change the greeting the server will use: + ``` sed -i.bak 's/pineapple/kiwi/' $OVERLAYS/staging/map.yaml ``` @@ -184,6 +193,7 @@ kustomize build $OVERLAYS/staging |\ Run kustomize again to see the new configMap names: + ``` kustomize build $OVERLAYS/staging |\ grep -B 8 -A 1 staging-the-map @@ -194,7 +204,8 @@ in three new names ending in _c2g8fcbf88_ - one in the configMap name itself, and two in the deployment that uses the map: - + + ``` test 3 == \ $(kustomize build $OVERLAYS/staging | grep c2g8fcbf88 | wc -l); \ @@ -204,7 +215,7 @@ test 3 == \ Applying these resources to the cluster will result in a rolling update of the deployments pods, retargetting them from the _5276h4th55_ maps to the _c2g8fcbf88_ -maps. The system will later garbage collect the +maps. The system will later garbage collect the unused maps. ## Rollback diff --git a/examples/springboot/README.md b/examples/springboot/README.md index be35b4e8e..93064f59d 100644 --- a/examples/springboot/README.md +++ b/examples/springboot/README.md @@ -102,7 +102,7 @@ For Spring Boot application, we can set an active profile through the environmen the application will pick up an extra `application-.properties` file. With this, we can customize the configMap in two steps. Add an environment variable through the patch and add a file to the configMap. - + ``` cat <$DEMO_HOME/patch.yaml apiVersion: apps/v1 @@ -281,7 +281,7 @@ The output contains Add these patches to the kustomization: - + ``` cd $DEMO_HOME kustomize edit add patch --path memorylimit_patch.yaml --name sbdemo --kind Deployment --group apps --version v1 diff --git a/examples/zh/configGeneration.md b/examples/zh/configGeneration.md index ce2169c64..661be162c 100644 --- a/examples/zh/configGeneration.md +++ b/examples/zh/configGeneration.md @@ -5,6 +5,7 @@ ## ConfigMap 的生成和滚动更新 kustomize 提供了两种添加 ConfigMap 的方法： + - 将 ConfigMap 声明为 [resource] - 通过 ConfigMapGenerator 声明 ConfigMap @@ -14,7 +15,7 @@ kustomize 提供了两种添加 ConfigMap 的方法： > # 将 ConfigMap 声明为 resource > resources: > - configmap.yaml -> +> > # 在 ConfigMapGenerator 中声明 ConfigMap > configMapGenerator: > - name: a-configmap @@ -30,7 +31,9 @@ kustomize 提供了两种添加 ConfigMap 的方法： ### 建立 base 和 staging 使用 configMapGenerator 建立 base + + ``` DEMO_HOME=$(mktemp -d) @@ -48,16 +51,18 @@ commonLabels: resources: - deployment.yaml - service.yaml -configMapGenerator: -- name: the-map - literals: - - altGreeting=Good Morning! +configMapGenerator: +- name: the-map + literals: + - altGreeting=Good Morning! - enableRisky="false" EOF ``` 通过应用 ConfigMap patch 的方式建立 staging + + ``` OVERLAYS=$DEMO_HOME/overlays mkdir -p $OVERLAYS/staging @@ -94,6 +99,7 @@ EOF deployment 按照名称引用此 ConfigMap ： + ``` grep -C 2 configMapKeyRef $BASE/deployment.yaml ``` @@ -102,16 +108,17 @@ grep -C 2 configMapKeyRef $BASE/deployment.yaml 更改 Deployment 配置的推荐方法是： - 1. 使用新名称创建一个新的 configMap - 2. 为_deployment_ 添加 patch，修改相应 `configMapKeyRef` 字段的名称值。 +1. 使用新名称创建一个新的 configMap +2. 为*deployment* 添加 patch，修改相应 `configMapKeyRef` 字段的名称值。 后一种更改会启动对 deployment 中的 pod 的滚动更新。旧的 configMap 在不再被任何其他资源引用时最终会被[垃圾回收](/../../issues/242)。 -### 如何使用 kustomize +### 如何使用 kustomize _staging_ 的 [variant] 包含一个 configMap 的 [patch]： + ``` cat $OVERLAYS/staging/map.yaml ``` @@ -121,6 +128,7 @@ cat $OVERLAYS/staging/map.yaml 在 ConfigMapGenerator 中声明 ConfigMap 的修改。 + ``` grep -C 4 configMapGenerator $BASE/kustomization.yaml ``` @@ -130,6 +138,7 @@ grep -C 4 configMapGenerator $BASE/kustomization.yaml 但是，文件中指定的名称值不是群集中使用的名称值。根据设计，kustomize 修改从 ConfigMapGenerator 声明的 ConfigMaps 的名称。要查看最终在群集中使用的名称，只需运行 kustomize： + ``` kustomize build $OVERLAYS/staging |\ grep -B 8 -A 1 staging-the-map @@ -141,7 +150,8 @@ kustomize build $OVERLAYS/staging |\ configMap 名称的后缀是由 map 内容的哈希生成的 - 在这种情况下，名称后缀是 _5276h4th55_ ： - + + ``` kustomize build $OVERLAYS/staging | grep 5276h4th55 ``` @@ -149,6 +159,7 @@ kustomize build $OVERLAYS/staging | grep 5276h4th55 现在修改 map patch ，更改该服务将使用的问候消息： + ``` sed -i.bak 's/pineapple/kiwi/' $OVERLAYS/staging/map.yaml ``` @@ -163,6 +174,7 @@ kustomize build $OVERLAYS/staging |\ 再次运行 kustomize 查看新的 configMap 名称： + ``` kustomize build $OVERLAYS/staging |\ grep -B 8 -A 1 staging-the-map @@ -170,7 +182,8 @@ kustomize build $OVERLAYS/staging |\ 确认 configMap 内容的更改将会生成以 _c2g8fcbf88_ 结尾的三个新名称 - 一个在 configMap 的名称中，另两个在使用 ConfigMap 的 deployment 中： - + + ``` test 3 == \ $(kustomize build $OVERLAYS/staging | grep c2g8fcbf88 | wc -l); \ diff --git a/examples/zh/springboot.md b/examples/zh/springboot.md index 5b4fefd6b..99552648a 100644 --- a/examples/zh/springboot.md +++ b/examples/zh/springboot.md @@ -89,7 +89,7 @@ cat kustomization.yaml 1. 通过 patch 添加一个环境变量 2. 将文件添加到 ConfigMap 中 - + ``` cat <$DEMO_HOME/patch.yaml apiVersion: apps/v1 @@ -257,7 +257,7 @@ cat $DEMO_HOME/healthcheck_patch.yaml 将这些 patch 添加到 `kustomization.yaml` 中： - + ``` cd $DEMO_HOME kustomize edit add patch --path memorylimit_patch.yaml --name sbdemo --kind Deployment --group apps --version v1