Merge pull request #3931 from natasha41575/ReplacementOverwritesSource

Fix issues with replacements

api/filters/fieldspec/fieldspec.go

@@ -49,7 +49,7 @@ func (fltr Filter) Filter(obj *yaml.RNode) (*yaml.RNode, error) {
 	if match := isMatchGVK(fltr.FieldSpec, obj); !match {
 		return obj, nil
 	}
-	fltr.path = utils.PathSplitter(fltr.FieldSpec.Path)
+	fltr.path = utils.PathSplitter(fltr.FieldSpec.Path, "/")
 	if err := fltr.filter(obj); err != nil {
 		s, _ := obj.String()
 		return nil, errors.WrapPrefixf(err,

api/filters/replacement/replacement.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"strings"
 
+	"sigs.k8s.io/kustomize/api/internal/utils"
 	"sigs.k8s.io/kustomize/api/types"
 	"sigs.k8s.io/kustomize/kyaml/resid"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
@@ -66,7 +67,7 @@ func rejectId(rejects []*types.Selector, id *resid.ResId) bool {
 
 func applyToNode(node *yaml.RNode, value *yaml.RNode, target *types.TargetSelector) error {
 	for _, fp := range target.FieldPaths {
-		fieldPath := strings.Split(fp, ".")
+		fieldPath := utils.SmarterPathSplitter(fp, ".")
 		var t *yaml.RNode
 		var err error
 		if target.Options != nil && target.Options.Create {
@@ -87,12 +88,11 @@ func applyToNode(node *yaml.RNode, value *yaml.RNode, target *types.TargetSelect
 }
 
 func setTargetValue(options *types.FieldOptions, t *yaml.RNode, value *yaml.RNode) error {
+	value = value.Copy()
 	if options != nil && options.Delimiter != "" {
-
 		if t.YNode().Kind != yaml.ScalarNode {
 			return fmt.Errorf("delimiter option can only be used with scalar nodes")
 		}
-
 		tv := strings.Split(t.YNode().Value, options.Delimiter)
 		v := yaml.GetValue(value)
 		// TODO: Add a way to remove an element
@@ -119,7 +119,7 @@ func getReplacement(nodes []*yaml.RNode, r *types.Replacement) (*yaml.RNode, err
 	if r.Source.FieldPath == "" {
 		r.Source.FieldPath = types.DefaultReplacementFieldPath
 	}
-	fieldPath := strings.Split(r.Source.FieldPath, ".")
+	fieldPath := utils.SmarterPathSplitter(r.Source.FieldPath, ".")
 
 	rn, err := source.Pipe(yaml.Lookup(fieldPath...))
 	if err != nil {

api/filters/replacement/replacement_test.go

@@ -1338,6 +1338,148 @@ spec:
 `,
 			expectedErr: "delimiter option can only be used with scalar nodes",
 		},
+		"list index contains '.' character": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: source
+data:
+  value: example
+---
+apiVersion: kubernetes-client.io/v1
+kind: ExternalSecret
+metadata:
+  name: some-secret
+spec:
+  backendType: secretsManager
+  data:
+    - key: some-prefix-replaceme
+      name: .first
+      version: latest
+      property: first
+    - key: some-prefix-replaceme
+      name: second
+      version: latest
+      property: second
+`,
+			replacements: `replacements:
+- source:
+    kind: ConfigMap
+    version: v1
+    name: source
+    fieldPath: data.value
+  targets:
+  - select:
+      group: kubernetes-client.io
+      version: v1
+      kind: ExternalSecret
+      name: some-secret
+    fieldPaths:
+    - spec.data.[name=.first].key
+    - spec.data.[name=second].key
+    options:
+      delimiter: "-"
+      index: 2
+`,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: source
+data:
+  value: example
+---
+apiVersion: kubernetes-client.io/v1
+kind: ExternalSecret
+metadata:
+  name: some-secret
+spec:
+  backendType: secretsManager
+  data:
+  - key: some-prefix-example
+    name: .first
+    version: latest
+    property: first
+  - key: some-prefix-example
+    name: second
+    version: latest
+    property: second`,
+		},
+		"multiple field paths in target": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: source
+data:
+  value: example
+---
+apiVersion: kubernetes-client.io/v1
+kind: ExternalSecret
+metadata:
+  name: some-secret
+spec:
+  backendType: secretsManager
+  data:
+    - key: some-prefix-replaceme
+      name: first
+      version: latest
+      property: first
+    - key: some-prefix-replaceme
+      name: second
+      version: latest
+      property: second
+    - key: some-prefix-replaceme
+      name: third
+      version: latest
+      property: third
+`,
+			replacements: `replacements:
+- source:
+    kind: ConfigMap
+    version: v1
+    name: source
+    fieldPath: data.value
+  targets:
+  - select:
+      group: kubernetes-client.io
+      version: v1
+      kind: ExternalSecret
+      name: some-secret
+    fieldPaths:
+    - spec.data.0.key
+    - spec.data.1.key
+    - spec.data.2.key
+    options:
+      delimiter: "-"
+      index: 2
+`,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: source
+data:
+  value: example
+---
+apiVersion: kubernetes-client.io/v1
+kind: ExternalSecret
+metadata:
+  name: some-secret
+spec:
+  backendType: secretsManager
+  data:
+  - key: some-prefix-example
+    name: first
+    version: latest
+    property: first
+  - key: some-prefix-example
+    name: second
+    version: latest
+    property: second
+  - key: some-prefix-example
+    name: third
+    version: latest
+    property: third
+`,
+		},
 	}
 
 	for tn, tc := range testCases {
@@ -1353,7 +1495,7 @@ spec:
 					t.Errorf("unexpected error: %s\n", err.Error())
 					t.FailNow()
 				}
-				if !assert.Equal(t, tc.expectedErr, err.Error()) {
+				if !assert.Contains(t, err.Error(), tc.expectedErr) {
 					t.FailNow()
 				}
 			}

api/internal/utils/pathsplitter.go

@@ -5,18 +5,50 @@ package utils
 
 import "strings"
 
-// PathSplitter splits a slash delimited string, permitting escaped slashes.
-func PathSplitter(path string) []string {
-	ps := strings.Split(path, "/")
+// TODO: Move these to kyaml
+
+// PathSplitter splits a delimited string, permitting escaped delimiters.
+func PathSplitter(path string, delimiter string) []string {
+	ps := strings.Split(path, delimiter)
 	var res []string
 	res = append(res, ps[0])
 	for i := 1; i < len(ps); i++ {
 		last := len(res) - 1
 		if strings.HasSuffix(res[last], `\`) {
-			res[last] = strings.TrimSuffix(res[last], `\`) + "/" + ps[i]
+			res[last] = strings.TrimSuffix(res[last], `\`) + delimiter + ps[i]
 		} else {
 			res = append(res, ps[i])
 		}
 	}
 	return res
 }
+
+// SmarterPathSplitter splits a path, retaining bracketed list entry identifiers.
+// E.g. [name=com.foo.someapp] survives as one thing after splitting
+// "spec.template.spec.containers.[name=com.foo.someapp].image"
+// See kyaml/yaml/match.go for use of list entry identifiers.
+// This function uses `PathSplitter`, so it respects list entry identifiers
+// and escaped delimiters.
+func SmarterPathSplitter(path string, delimiter string) []string {
+	var result []string
+	split := PathSplitter(path, delimiter)
+
+	for i := 0; i < len(split); i++ {
+		elem := split[i]
+		if strings.HasPrefix(elem, "[") && !strings.HasSuffix(elem, "]") {
+			// continue until we find the matching "]"
+			bracketed := []string{elem}
+			for i < len(split)-1 {
+				i++
+				bracketed = append(bracketed, split[i])
+				if strings.HasSuffix(split[i], "]") {
+					break
+				}
+			}
+			result = append(result, strings.Join(bracketed, delimiter))
+		} else {
+			result = append(result, elem)
+		}
+	}
+	return result
+}

api/internal/utils/pathsplitter_test.go

@@ -44,6 +44,47 @@ func TestPathSplitter(t *testing.T) {
 				"nginx.ingress.kubernetes.io/auth-secret"},
 		},
 	} {
-		assert.Equal(t, tc.exp, PathSplitter(tc.path))
+		assert.Equal(t, tc.exp, PathSplitter(tc.path, "/"))
+	}
+}
+
+func TestSmarterPathSplitter(t *testing.T) {
+	testCases := map[string]struct {
+		input    string
+		expected []string
+	}{
+		"simple": {
+			input:    "spec.replicas",
+			expected: []string{"spec", "replicas"},
+		},
+		"sequence": {
+			input:    "spec.data.[name=first].key",
+			expected: []string{"spec", "data", "[name=first]", "key"},
+		},
+		"key, value with . prefix": {
+			input:    "spec.data.[.name=.first].key",
+			expected: []string{"spec", "data", "[.name=.first]", "key"},
+		},
+		"key, value with . suffix": {
+			input:    "spec.data.[name.=first.].key",
+			expected: []string{"spec", "data", "[name.=first.]", "key"},
+		},
+		"multiple '.' in value": {
+			input:    "spec.data.[name=f.i.r.s.t.].key",
+			expected: []string{"spec", "data", "[name=f.i.r.s.t.]", "key"},
+		},
+		"with escaped delimiter": {
+			input:    `spec\.replicas`,
+			expected: []string{`spec.replicas`},
+		},
+		"unmatched bracket": {
+			input:    "spec.data.[name=f.i.[r.s.t..key",
+			expected: []string{"spec", "data", "[name=f.i.[r.s.t..key"},
+		},
+	}
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			assert.Equal(t, tc.expected, SmarterPathSplitter(tc.input, "."))
+		})
 	}
 }