Localize patch, ReplacementTransformer (#4943)

* Localize PatchStrategicMergeTransformer, ReplacementTransformer

* Improve readability

api/internal/localizer/builtinplugins.go

@@ -4,6 +4,7 @@
 package localizer
 
 import (
+	"sigs.k8s.io/kustomize/api/filters/fieldspec"
 	"sigs.k8s.io/kustomize/api/filters/filtersutil"
 	"sigs.k8s.io/kustomize/api/filters/fsslice"
 	"sigs.k8s.io/kustomize/api/internal/plugins/builtinhelpers"
@@ -19,36 +20,62 @@ import (
 // Note that this excludes helm, which needs a repo.
 type localizeBuiltinPlugins struct {
 	lc *localizer
+
+	// locPathFn is used by localizeNode to set the localized path on the plugin.
+	locPathFn func(string) (string, error)
 }
 
 var _ kio.Filter = &localizeBuiltinPlugins{}
 
 // Filter localizes the built-in plugins with file paths.
 func (lbp *localizeBuiltinPlugins) Filter(plugins []*yaml.RNode) ([]*yaml.RNode, error) {
-	localizedPlugins, err := kio.FilterAll(fsslice.Filter{
+	for _, singlePlugin := range plugins {
-		FsSlice: types.FsSlice{
+		err := singlePlugin.PipeE(fsslice.Filter{
-			types.FieldSpec{
+			FsSlice: types.FsSlice{
-				Gvk:  resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.PatchTransformer.String()},
+				types.FieldSpec{
-				Path: "path",
+					Gvk:  resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.PatchTransformer.String()},
+					Path: "path",
+				},
+				types.FieldSpec{
+					Gvk:  resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.PatchJson6902Transformer.String()},
+					Path: "path",
+				},
+				types.FieldSpec{
+					Gvk:  resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.ReplacementTransformer.String()},
+					Path: "replacements/path",
+				},
 			},
-			types.FieldSpec{
+			SetValue: func(node *yaml.RNode) error {
-				Gvk:  resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.PatchJson6902Transformer.String()},
+				lbp.locPathFn = lbp.lc.localizeFile
-				Path: "path",
+				return lbp.localizeNode(node)
 			},
-		},
+		}, fieldspec.Filter{
-		SetValue: lbp.localizeNode,
+			FieldSpec: types.FieldSpec{
-	}).Filter(plugins)
+				Gvk:  resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.PatchStrategicMergeTransformer.String()},
-
+				Path: "paths",
-	// TODO(annasong): localize ReplacementTransformer, PatchStrategicMergeTransformer, ConfigMapGenerator, SecretGenerator
+			},
-
+			SetValue: func(node *yaml.RNode) error {
-	return localizedPlugins, errors.Wrap(err)
+				lbp.locPathFn = lbp.lc.localizeK8sResource
-}
+				return errors.Wrap(node.VisitElements(lbp.localizeNode))
-
+			},
-// localizeNode sets the scalar node to its value localized as a file path.
+		})
-func (lbp *localizeBuiltinPlugins) localizeNode(node *yaml.RNode) error {
+		// TODO(annasong): localize ConfigMapGenerator, SecretGenerator,
-	localizedPath, err := lbp.lc.localizeFile(node.YNode().Value)
+		// HelmChartInflationGenerator
-	if err != nil {
+		if err != nil {
-		return errors.WrapPrefixf(err, "unable to localize built-in plugin path")
+			return nil, errors.Wrap(err)
+		}
 	}
-	return filtersutil.SetScalar(localizedPath)(node)
+	return plugins, nil
+}
+
+// localizeNode sets the scalar node to its value localized by locPathFn.
+func (lbp *localizeBuiltinPlugins) localizeNode(node *yaml.RNode) error {
+	localizedPath, err := lbp.locPathFn(node.YNode().Value)
+	if err != nil {
+		return err
+	}
+	if localizedPath != "" {
+		err = filtersutil.SetScalar(localizedPath)(node)
+	}
+	return err
 }

api/internal/localizer/localizer.go

@@ -181,16 +181,12 @@ func (lc *localizer) localizeNativeFields(kust *types.Kustomization) error {
 	}
 	//nolint:staticcheck
 	for i, patch := range kust.PatchesStrategicMerge {
-		_, isFile, err := lc.loadResource(string(patch))
+		localizedPath, err := lc.localizeK8sResource(string(patch))
 		if err != nil {
-			return errors.WrapPrefixf(err, "invalid patchesStrategicMerge entry")
+			return errors.WrapPrefixf(err, "unable to localize patchesStrategicMerge entry")
 		}
-		if isFile {
+		if localizedPath != "" {
-			newPath, err := lc.localizeFile(string(patch))
+			kust.PatchesStrategicMerge[i] = types.PatchStrategicMerge(localizedPath)
-			if err != nil {
-				return errors.WrapPrefixf(err, "unable to localize patchesStrategicMerge entry")
-			}
-			kust.PatchesStrategicMerge[i] = types.PatchStrategicMerge(newPath)
 		}
 	}
 	for i, replacement := range kust.Replacements {
@@ -383,11 +379,11 @@ func (lc *localizer) localizeBuiltinPlugins(kust *types.Kustomization) error {
 		"validators":   kust.Validators,
 	} {
 		for i, entry := range entries {
-			rm, isPath, err := lc.loadResource(entry)
+			rm, isPath, err := lc.loadK8sResource(entry)
 			if err != nil {
 				return errors.WrapPrefixf(err, "unable to load %s entry", fieldName)
 			}
-			err = rm.ApplyFilter(&localizeBuiltinPlugins{lc})
+			err = rm.ApplyFilter(&localizeBuiltinPlugins{lc: lc})
 			if err != nil {
 				return errors.Wrap(err)
 			}
@@ -410,9 +406,26 @@ func (lc *localizer) localizeBuiltinPlugins(kust *types.Kustomization) error {
 	return nil
 }
 
-// loadResource tries to load resourceEntry as a file path or inline.
+// localizeK8sResource returns the localized file path if resourceEntry is a
-// On success, loadResource returns the loaded resource map and whether resourceEntry is a file path.
+// file containing a kubernetes resource.
-func (lc *localizer) loadResource(resourceEntry string) (resmap.ResMap, bool, error) {
+// localizeK8sResource returns the empty string if resourceEntry is an inline
+// resource.
+func (lc *localizer) localizeK8sResource(resourceEntry string) (string, error) {
+	_, isFile, err := lc.loadK8sResource(resourceEntry)
+	if err != nil {
+		return "", err
+	}
+	if isFile {
+		return lc.localizeFile(resourceEntry)
+	}
+	return "", nil
+}
+
+// loadK8sResource tries to load resourceEntry as a file path or inline
+// kubernetes resource.
+// On success, loadK8sResource returns the loaded resource map and whether
+// resourceEntry is a file path.
+func (lc *localizer) loadK8sResource(resourceEntry string) (resmap.ResMap, bool, error) {
 	rm, inlineErr := lc.rFactory.NewResMapFromBytes([]byte(resourceEntry))
 	if inlineErr != nil {
 		var fileErr error

api/internal/localizer/localizer_test.go

@@ -15,7 +15,8 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
-const podConfiguration = `apiVersion: v1
+const (
+	podConfiguration = `apiVersion: v1
 kind: Pod
 metadata:
   name: pod
@@ -26,6 +27,53 @@ spec:
     ports:
     - containerPort: 80`
 
+	replacementTransformerWithPath = `apiVersion: builtin
+kind: ReplacementTransformer
+metadata:
+  name: replacement
+replacements:
+- path: replacement.yaml
+- source:
+    fieldPath: metadata.[name=my-pod]
+    group: apps
+    namespace: test
+    version: v1
+  targets:
+  - fieldPaths:
+    - spec.containers.0.name
+    select:
+      name: another-pod
+`
+
+	replacements = `
+- source:
+    name: src
+    fieldPath: path
+    options:
+      delimiter: '='
+      index: 1
+  targets:
+  - select:
+      kind: Pod
+    reject:
+      version: v1
+    fieldPaths:
+    - metadata.annotations.config\.kubernetes\.io/local-config
+    - sequence.*
+- source:
+    kind: Deployment
+    fieldPath: sequence.-
+  targets:
+  - select:
+      namespace: my
+    fieldPaths:
+    - path
+    options:
+      delimiter: '='
+      index: 0
+`
+)
+
 func makeMemoryFs(t *testing.T) filesys.FileSystem {
 	t.Helper()
 	req := require.New(t)
@@ -457,7 +505,7 @@ kind: Kustomization
 replacements:
 - path: replacement.yaml
 - source:
-    fieldPath: path
+    fieldPath: path.0
     name: map
   targets:
   - fieldPaths:
@@ -465,21 +513,7 @@ replacements:
     select:
       name: my-map
 `,
-		"replacement.yaml": `source:
+		"replacement.yaml": replacements,
-  fieldPath: path.to.some.field
-  kind: Pod
-  options:
-    delimiter: /
-targets:
-- fieldPaths:
-  - config\.kubernetes\.io.annotations
-  - second.path
-  - path.*.to.[some=field]
-  reject:
-  - group: apps
-    version: v2
-  select:
-    namespace: my`,
 	}
 	checkLocalizeInTargetSuccess(t, kustAndReplacement)
 }
@@ -730,6 +764,39 @@ target:
 	checkLocalizeInTargetSuccess(t, kustAndPatches)
 }
 
+func TestLocalizeTransformersPatchSM(t *testing.T) {
+	kustAndPatches := map[string]string{
+		"kustomization.yaml": `transformers:
+- patch.yaml
+`,
+		"patch.yaml": `apiVersion: builtin
+kind: PatchStrategicMergeTransformer
+metadata:
+  name: path
+paths:
+- nested-patch.yaml
+- |-
+  apiVersion: v1
+  kind: Pod
+  metadata:
+    name: my-pod
+`,
+		"nested-patch.yaml": podConfiguration,
+	}
+	checkLocalizeInTargetSuccess(t, kustAndPatches)
+}
+
+func TestLocalizeTransformersReplacement(t *testing.T) {
+	kustAndReplacements := map[string]string{
+		"kustomization.yaml": `transformers:
+- replacement-transformer.yaml
+`,
+		"replacement-transformer.yaml": replacementTransformerWithPath,
+		"replacement.yaml":             replacements,
+	}
+	checkLocalizeInTargetSuccess(t, kustAndReplacements)
+}
+
 func TestLocalizePluginsNoPaths(t *testing.T) {
 	kustAndPlugins := map[string]string{
 		"kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
@@ -758,37 +825,10 @@ func TestLocalizeValidators(t *testing.T) {
 		"kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 validators:
-- |
+- replacement-no-change.yaml
-  apiVersion: builtin
-  kind: ReplacementTransformer
-  metadata:
-    name: replacement
-  replacements:
-  - source:
-      fieldPath: metadata.name
-      kind: ConfigMap
-    targets:
-    - fieldPaths:
-      - metadata.name
-      select:
-        kind: ConfigMap
-- replacement.yaml
-`,
-		"replacement.yaml": `apiVersion: builtin
-kind: ReplacementTransformer
-metadata:
-  name: replacement-2
-replacements:
-- source:
-    fieldPath: spec.containers.1.image
-    kind: Custom
-    namespace: test
-  targets:
-  - fieldPaths:
-    - path.*.to.[some=field]
-    select:
-      namespace: test
 `,
+		"replacement-no-change.yaml": replacementTransformerWithPath,
+		"replacement.yaml":           replacements,
 	}
 	checkLocalizeInTargetSuccess(t, kustAndPlugin)
 }
@@ -855,6 +895,26 @@ when parsing as filepath received error: %s`, test.errPrefix, test.inlineErrMsg,
 	}
 }
 
+func TestLocalizeBuiltinPluginsFileError(t *testing.T) {
+	kustAndPatches := map[string]string{
+		"kustomization.yaml": `transformers:
+- patch.yaml
+`,
+		"patch.yaml": `apiVersion: builtin
+kind: PatchTransformer
+metadata:
+  name: my-patch
+path: patchSM.yaml
+`,
+	}
+	_, actual := makeFileSystems(t, "/a", kustAndPatches)
+
+	err := Run("/a", "", "/dst", actual)
+	require.EqualError(t, err, "unable to localize target \"/a\": "+
+		"considering field 'path' of object PatchTransformer.builtin.[noGrp]/my-patch.[noNs]: "+
+		"invalid file reference: '/a/patchSM.yaml' doesn't exist")
+}
+
 func TestLocalizeDirInTarget(t *testing.T) {
 	type testCase struct {
 		name  string