Merge pull request #3129 from Shell32-Natsu/patches-command

Update edit patch command

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

kustomize/internal/commands/edit/add/all.go

@@ -29,7 +29,7 @@ func NewCmdAdd(
 	kustomize edit add resource <filepath>
 
 	# Adds a patch to the kustomization
-	kustomize edit add patch <filepath>
+	kustomize edit add patch --path {filepath} --group {target group name} --version {target version}
 
 	# Adds a component to the kustomization
 	kustomize edit add component <filepath>

kustomize/internal/commands/edit/patch/strategicmerge.go

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

kustomize/internal/commands/edit/patch/strategicmerge_test.go

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

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

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

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