From 5e84de2a8966c69a15407c361247878772c5ccbd Mon Sep 17 00:00:00 2001 From: koba1t Date: Tue, 12 Apr 2022 05:14:24 +0900 Subject: [PATCH 1/6] fix errror message using replacement wildcard and create option --- api/filters/replacement/replacement.go | 3 -- api/filters/replacement/replacement_test.go | 39 +++++++++++++++++++++ kyaml/yaml/fns.go | 3 ++ kyaml/yaml/fns_test.go | 11 ++++-- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index a96dfff719..76cb6089ff 100644 --- a/api/filters/replacement/replacement.go +++ b/api/filters/replacement/replacement.go @@ -142,9 +142,6 @@ func applyToOneNode(options *types.FieldOptions, t *yaml.RNode, value *yaml.RNod } for _, scalarNode := range t.YNode().Content { - if options != nil && options.Create { - return fmt.Errorf("cannot use create option in a multi-value target") - } rn := yaml.NewRNode(scalarNode) if err := setTargetValue(options, rn, value); err != nil { return err diff --git a/api/filters/replacement/replacement_test.go b/api/filters/replacement/replacement_test.go index 71069dfad2..6e4f46b0dc 100644 --- a/api/filters/replacement/replacement_test.go +++ b/api/filters/replacement/replacement_test.go @@ -1634,6 +1634,45 @@ spec: - name: deployment-name value: sample-deploy`, }, + "index contains '*' character and create options": { + input: `apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: sample-deploy + name: sample-deploy +spec: + replicas: 1 + selector: + matchLabels: + app: sample-deploy + template: + metadata: + labels: + app: sample-deploy + spec: + containers: + - image: nginx + name: main + env: + - name: other-env + value: YYYYY +`, + replacements: `replacements: +- source: + kind: Deployment + name: sample-deploy + fieldPath: metadata.name + targets: + - select: + kind: Deployment + fieldPaths: + - spec.template.spec.containers.*.env.[name=deployment-name].value + options: + create: true +`, + expectedErr: "cannot support create option in a multi-value target now", + }, "multiple field paths in target": { input: `apiVersion: v1 kind: ConfigMap diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index c5ac0acfac..fe72d13cea 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -541,6 +541,9 @@ func (l PathGetter) getFilter(part, nextPart string, fieldPath *[]string) (Filte case part == "-": // part is a hyphen return GetElementByIndex(-1), nil + case part == "*": + // part is a hyphen + return nil, fmt.Errorf("cannot support create option in a multi-value target now") case IsListIndex(part): // part is surrounded by brackets return l.elemFilter(part) diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 8ad049e8e2..8346b7b881 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -142,7 +142,7 @@ func TestElementSetter(t *testing.T) { node = MustParse(` - a: b -- c: d +- c: d `) // If given a key and no values, ElementSetter will // change node to be an empty list @@ -154,7 +154,7 @@ func TestElementSetter(t *testing.T) { node = MustParse(` - a: b -- c: d +- c: d `) // Return error because ElementSetter will assume all elements are scalar when // there is only value provided. @@ -580,6 +580,13 @@ a: {} assert.Equal(t, "h\n", assertNoErrorString(t)(rn.String())) } +func TestLookup_Fn_create_with_wildcard_error(t *testing.T) { + node, err := Parse(s) + assert.NoError(t, err) + _, err = node.Pipe(LookupCreate(yaml.MappingNode, "a", "b", "*", "t")) + assert.Error(t, err, "cannot support create option in a multi-value target now") +} + func TestLookup(t *testing.T) { s := `n: o a: From cdc4a5083b2d597f157e8bc7cc11f858710ddf3b Mon Sep 17 00:00:00 2001 From: koba1t Date: Tue, 12 Apr 2022 12:53:58 +0900 Subject: [PATCH 2/6] fix lint error --- kyaml/yaml/fns.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index fe72d13cea..7ef1a54edd 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -542,8 +542,8 @@ func (l PathGetter) getFilter(part, nextPart string, fieldPath *[]string) (Filte // part is a hyphen return GetElementByIndex(-1), nil case part == "*": - // part is a hyphen - return nil, fmt.Errorf("cannot support create option in a multi-value target now") + // part is a asterisk + return nil, errors.Errorf("cannot support create option in a multi-value target now") case IsListIndex(part): // part is surrounded by brackets return l.elemFilter(part) From 01ab069bd2a74f29a5e02fca3a9d8ff79465a145 Mon Sep 17 00:00:00 2001 From: koba1t Date: Wed, 13 Apr 2022 04:55:21 +0900 Subject: [PATCH 3/6] fix error message --- api/filters/replacement/replacement.go | 10 ++++++++++ api/filters/replacement/replacement_test.go | 2 +- kyaml/yaml/fns.go | 4 ++-- kyaml/yaml/fns_test.go | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index 76cb6089ff..e8d177661f 100644 --- a/api/filters/replacement/replacement.go +++ b/api/filters/replacement/replacement.go @@ -7,6 +7,8 @@ import ( "fmt" "strings" + "errors" + "sigs.k8s.io/kustomize/api/internal/utils" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" @@ -117,6 +119,14 @@ func applyToNode(node *yaml.RNode, value *yaml.RNode, target *types.TargetSelect var t *yaml.RNode var err error if target.Options != nil && target.Options.Create { + // create option is not supported in a wildcard matching. + // Because, PathMatcher is not supported create option. + // So, if create option is set, we fallback to PathGetter. + for _, f := range fieldPath { + if f == "*" { + return errors.New("cannot support create option in a multi-value target") + } + } t, err = node.Pipe(yaml.LookupCreate(value.YNode().Kind, fieldPath...)) } else { t, err = node.Pipe(&yaml.PathMatcher{Path: fieldPath}) diff --git a/api/filters/replacement/replacement_test.go b/api/filters/replacement/replacement_test.go index 6e4f46b0dc..14e3adabea 100644 --- a/api/filters/replacement/replacement_test.go +++ b/api/filters/replacement/replacement_test.go @@ -1671,7 +1671,7 @@ spec: options: create: true `, - expectedErr: "cannot support create option in a multi-value target now", + expectedErr: "cannot support create option in a multi-value target", }, "multiple field paths in target": { input: `apiVersion: v1 diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 7ef1a54edd..e693f88a1b 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -542,8 +542,8 @@ func (l PathGetter) getFilter(part, nextPart string, fieldPath *[]string) (Filte // part is a hyphen return GetElementByIndex(-1), nil case part == "*": - // part is a asterisk - return nil, errors.Errorf("cannot support create option in a multi-value target now") + // PathGetter is not support for wildcard matching + return nil, errors.Errorf("wildcard is not supported in PathGetter") case IsListIndex(part): // part is surrounded by brackets return l.elemFilter(part) diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 8346b7b881..e217a07b1b 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -584,7 +584,7 @@ func TestLookup_Fn_create_with_wildcard_error(t *testing.T) { node, err := Parse(s) assert.NoError(t, err) _, err = node.Pipe(LookupCreate(yaml.MappingNode, "a", "b", "*", "t")) - assert.Error(t, err, "cannot support create option in a multi-value target now") + assert.Error(t, err, "wildcard is not supported in PathGetter") } func TestLookup(t *testing.T) { From 02e0b38bb0e9783f4c6f5988ccf9bedd18cd1a95 Mon Sep 17 00:00:00 2001 From: koba1t Date: Wed, 13 Apr 2022 05:09:52 +0900 Subject: [PATCH 4/6] fix lint error --- api/filters/replacement/replacement.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index e8d177661f..92c01ae629 100644 --- a/api/filters/replacement/replacement.go +++ b/api/filters/replacement/replacement.go @@ -7,8 +7,6 @@ import ( "fmt" "strings" - "errors" - "sigs.k8s.io/kustomize/api/internal/utils" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" @@ -124,7 +122,7 @@ func applyToNode(node *yaml.RNode, value *yaml.RNode, target *types.TargetSelect // So, if create option is set, we fallback to PathGetter. for _, f := range fieldPath { if f == "*" { - return errors.New("cannot support create option in a multi-value target") + return fmt.Errorf("cannot support create option in a multi-value target") } } t, err = node.Pipe(yaml.LookupCreate(value.YNode().Kind, fieldPath...)) From ed72bb02d4cd2a6a665a6018fd8741f80877a710 Mon Sep 17 00:00:00 2001 From: koba1t Date: Wed, 13 Apr 2022 05:20:53 +0900 Subject: [PATCH 5/6] fix lint error --- api/filters/replacement/replacement.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index 92c01ae629..39faaca6a0 100644 --- a/api/filters/replacement/replacement.go +++ b/api/filters/replacement/replacement.go @@ -4,6 +4,7 @@ package replacement import ( + "errors" "fmt" "strings" @@ -122,7 +123,7 @@ func applyToNode(node *yaml.RNode, value *yaml.RNode, target *types.TargetSelect // So, if create option is set, we fallback to PathGetter. for _, f := range fieldPath { if f == "*" { - return fmt.Errorf("cannot support create option in a multi-value target") + return errors.New("cannot support create option in a multi-value target") } } t, err = node.Pipe(yaml.LookupCreate(value.YNode().Kind, fieldPath...)) From 2f2e14e953c8291f4a4aade09e355fb5eab10612 Mon Sep 17 00:00:00 2001 From: koba1t Date: Wed, 13 Apr 2022 12:43:35 +0900 Subject: [PATCH 6/6] fix lint error to nolint:goerr113 --- api/filters/replacement/replacement.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index 39faaca6a0..0201905bef 100644 --- a/api/filters/replacement/replacement.go +++ b/api/filters/replacement/replacement.go @@ -123,7 +123,7 @@ func applyToNode(node *yaml.RNode, value *yaml.RNode, target *types.TargetSelect // So, if create option is set, we fallback to PathGetter. for _, f := range fieldPath { if f == "*" { - return errors.New("cannot support create option in a multi-value target") + return errors.New("cannot support create option in a multi-value target") //nolint:goerr113 } } t, err = node.Pipe(yaml.LookupCreate(value.YNode().Kind, fieldPath...))