refactor edit add patch command

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
+}

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)
+		}
+	}
+}

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)
 }

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())
 	}
 }