From 84724a3ebf64a4b977c7344a442c700643509acb Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Tue, 1 Jun 2021 13:39:48 -0700 Subject: [PATCH] smarter path splitter for replacements --- api/filters/fieldspec/fieldspec.go | 2 +- api/filters/replacement/replacement.go | 8 ++-- api/filters/replacement/replacement_test.go | 23 ++++++++++- api/internal/utils/pathsplitter.go | 40 +++++++++++++++++-- api/internal/utils/pathsplitter_test.go | 43 ++++++++++++++++++++- 5 files changed, 104 insertions(+), 12 deletions(-) diff --git a/api/filters/fieldspec/fieldspec.go b/api/filters/fieldspec/fieldspec.go index cbc8776fe..8739d0733 100644 --- a/api/filters/fieldspec/fieldspec.go +++ b/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, diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index 81a2e834d..35858940d 100644 --- a/api/filters/replacement/replacement.go +++ b/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 { @@ -88,13 +89,10 @@ 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 @@ -121,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 { diff --git a/api/filters/replacement/replacement_test.go b/api/filters/replacement/replacement_test.go index 5850accf8..d32b7dfdc 100644 --- a/api/filters/replacement/replacement_test.go +++ b/api/filters/replacement/replacement_test.go @@ -1381,7 +1381,28 @@ spec: delimiter: "-" index: 2 `, - expectedErr: "wrong Node Kind for spec.data expected: MappingNode was SequenceNode: value: {- key: some-prefix-replaceme", + 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 diff --git a/api/internal/utils/pathsplitter.go b/api/internal/utils/pathsplitter.go index c8d2f3342..002f43bb4 100644 --- a/api/internal/utils/pathsplitter.go +++ b/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 +} diff --git a/api/internal/utils/pathsplitter_test.go b/api/internal/utils/pathsplitter_test.go index 9b52c3806..5e133f572 100644 --- a/api/internal/utils/pathsplitter_test.go +++ b/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, ".")) + }) } }