Skip to content

Commit

Permalink
Merge pull request #4577 from koba1t/fix/replacement_wildcard_and_cre…
Browse files Browse the repository at this point in the history
…ate_option_error_message

fix error message using replacement wildcard and create option
  • Loading branch information
k8s-ci-robot committed Apr 13, 2022
2 parents e5041ba + 2f2e14e commit ec21271
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 5 deletions.
12 changes: 9 additions & 3 deletions api/filters/replacement/replacement.go
Expand Up @@ -4,6 +4,7 @@
package replacement

import (
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -117,6 +118,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") //nolint:goerr113
}
}
t, err = node.Pipe(yaml.LookupCreate(value.YNode().Kind, fieldPath...))
} else {
t, err = node.Pipe(&yaml.PathMatcher{Path: fieldPath})
Expand All @@ -142,9 +151,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
Expand Down
39 changes: 39 additions & 0 deletions api/filters/replacement/replacement_test.go
Expand Up @@ -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",
},
"multiple field paths in target": {
input: `apiVersion: v1
kind: ConfigMap
Expand Down
3 changes: 3 additions & 0 deletions kyaml/yaml/fns.go
Expand Up @@ -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 == "*":
// 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)
Expand Down
11 changes: 9 additions & 2 deletions kyaml/yaml/fns_test.go
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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, "wildcard is not supported in PathGetter")
}

func TestLookup(t *testing.T) {
s := `n: o
a:
Expand Down

0 comments on commit ec21271

Please sign in to comment.