Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

replacements: fix issue with create: true option when there is an existing field #4667

Merged
merged 5 commits into from Jul 15, 2022
Merged

replacements: fix issue with create: true option when there is an existing field #4667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8699187
replacements: demonstrate broken behavior when using 'create: true' w…
natasha41575 Jun 7, 2022
b0662b5
replacements: fix issue with 'create: true' option when there is an e…
natasha41575 Jun 7, 2022
d5bafe9
Suggestion for PR 4667
KnVerey Jul 5, 2022
fe85527
code review
natasha41575 Jul 15, 2022
27028da
lint error
natasha41575 Jul 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
224 changes: 113 additions & 111 deletions api/filters/replacement/replacement.go
View file
Open in desktop
Expand Up @@ -38,22 +38,86 @@ func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
return nodes, nil
}

func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targets []*types.TargetSelector) ([]*yaml.RNode, error) {
for _, t := range targets {
if t.Select == nil {
return nil, fmt.Errorf("target must specify resources to select")
func getReplacement(nodes []*yaml.RNode, r *types.Replacement) (*yaml.RNode, error) {
source, err := selectSourceNode(nodes, r.Source)
if err != nil {
return nil, err
}

if r.Source.FieldPath == "" {
r.Source.FieldPath = types.DefaultReplacementFieldPath
}
fieldPath := kyaml_utils.SmarterPathSplitter(r.Source.FieldPath, ".")

rn, err := source.Pipe(yaml.Lookup(fieldPath...))
if err != nil {
return nil, fmt.Errorf("error looking up replacement source: %w", err)
}
if rn.IsNilOrEmpty() {
return nil, fmt.Errorf("fieldPath `%s` is missing for replacement source %s", r.Source.FieldPath, r.Source.ResId)
}

return getRefinedValue(r.Source.Options, rn)
}

// selectSourceNode finds the node that matches the selector, returning
// an error if multiple or none are found
func selectSourceNode(nodes []*yaml.RNode, selector *types.SourceSelector) (*yaml.RNode, error) {
var matches []*yaml.RNode
for _, n := range nodes {
ids, err := utils.MakeResIds(n)
if err != nil {
return nil, fmt.Errorf("error getting node IDs: %w", err)
}
if len(t.FieldPaths) == 0 {
t.FieldPaths = []string{types.DefaultReplacementFieldPath}
for _, id := range ids {
if id.IsSelectedBy(selector.ResId) {
if len(matches) > 0 {
return nil, fmt.Errorf(
"multiple matches for selector %s", selector)
}
matches = append(matches, n)
break
}
}
for _, n := range nodes {
ids, err := utils.MakeResIds(n)
}
if len(matches) == 0 {
return nil, fmt.Errorf("nothing selected by %s", selector)
}
return matches[0], nil
}

func getRefinedValue(options *types.FieldOptions, rn *yaml.RNode) (*yaml.RNode, error) {
if options == nil || options.Delimiter == "" {
return rn, nil
}
if rn.YNode().Kind != yaml.ScalarNode {
return nil, fmt.Errorf("delimiter option can only be used with scalar nodes")
}
value := strings.Split(yaml.GetValue(rn), options.Delimiter)
if options.Index >= len(value) || options.Index < 0 {
return nil, fmt.Errorf("options.index %d is out of bounds for value %s", options.Index, yaml.GetValue(rn))
}
n := rn.Copy()
n.YNode().Value = value[options.Index]
return n, nil
}

func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targetSelectors []*types.TargetSelector) ([]*yaml.RNode, error) {
for _, selector := range targetSelectors {
if selector.Select == nil {
return nil, errors.New("target must specify resources to select")
}
if len(selector.FieldPaths) == 0 {
selector.FieldPaths = []string{types.DefaultReplacementFieldPath}
}
for _, possibleTarget := range nodes {
ids, err := utils.MakeResIds(possibleTarget)
if err != nil {
return nil, err
}

// filter targets by label and annotation selectors
selectByAnnoAndLabel, err := selectByAnnoAndLabel(n, t)
selectByAnnoAndLabel, err := selectByAnnoAndLabel(possibleTarget, selector)
if err != nil {
return nil, err
}
Expand All @@ -63,8 +127,8 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targets []*types.T

// filter targets by matching resource IDs
for i, id := range ids {
if id.IsSelectedBy(t.Select.ResId) && !rejectId(t.Reject, &ids[i]) {
err := applyToNode(n, value, t)
if id.IsSelectedBy(selector.Select.ResId) && !rejectId(selector.Reject, &ids[i]) {
err := copyValueToTarget(possibleTarget, value, selector)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -113,61 +177,50 @@ func rejectId(rejects []*types.Selector, id *resid.ResId) bool {
return false
}

func applyToNode(node *yaml.RNode, value *yaml.RNode, target *types.TargetSelector) error {
for _, fp := range target.FieldPaths {
func copyValueToTarget(target *yaml.RNode, value *yaml.RNode, selector *types.TargetSelector) error {
for _, fp := range selector.FieldPaths {
fieldPath := kyaml_utils.SmarterPathSplitter(fp, ".")
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})
}
create, err := shouldCreateField(selector.Options, fieldPath)
if err != nil {
return err
}
if t != nil {
if err = applyToOneNode(target.Options, t, value); err != nil {
return err

var targetFields []*yaml.RNode
if create {
createdField, createErr := target.Pipe(yaml.LookupCreate(value.YNode().Kind, fieldPath...))
if createErr != nil {
return fmt.Errorf("error creating replacement node: %w", createErr)
}
targetFields = append(targetFields, createdField)
} else {
// may return multiple fields, always wrapped in a sequence node
foundFieldSequence, lookupErr := target.Pipe(&yaml.PathMatcher{Path: fieldPath})
if lookupErr != nil {
return fmt.Errorf("error finding field in replacement target: %w", lookupErr)
}
targetFields, err = foundFieldSequence.Elements()
if err != nil {
return fmt.Errorf("error fetching elements in replacement target: %w", err)
}
}
}
return nil
}

func applyToOneNode(options *types.FieldOptions, t *yaml.RNode, value *yaml.RNode) error {
if len(t.YNode().Content) == 0 {
if err := setTargetValue(options, t, value); err != nil {
return err
for _, t := range targetFields {
if err := setFieldValue(selector.Options, t, value); err != nil {
return err
}
}
return nil
}

for _, scalarNode := range t.YNode().Content {
rn := yaml.NewRNode(scalarNode)
if err := setTargetValue(options, rn, value); err != nil {
return err
}
}

return nil
}

func setTargetValue(options *types.FieldOptions, t *yaml.RNode, value *yaml.RNode) error {
func setFieldValue(options *types.FieldOptions, targetField *yaml.RNode, value *yaml.RNode) error {
value = value.Copy()
if options != nil && options.Delimiter != "" {
if t.YNode().Kind != yaml.ScalarNode {
if targetField.YNode().Kind != yaml.ScalarNode {
return fmt.Errorf("delimiter option can only be used with scalar nodes")
}
tv := strings.Split(t.YNode().Value, options.Delimiter)
tv := strings.Split(targetField.YNode().Value, options.Delimiter)
v := yaml.GetValue(value)
// TODO: Add a way to remove an element
switch {
Expand All @@ -181,76 +234,25 @@ func setTargetValue(options *types.FieldOptions, t *yaml.RNode, value *yaml.RNod
value.YNode().Value = strings.Join(tv, options.Delimiter)
}

if t.YNode().Kind == yaml.ScalarNode {
if targetField.YNode().Kind == yaml.ScalarNode {
// For scalar, only copy the value (leave any type intact to auto-convert int->string or string->int)
t.YNode().Value = value.YNode().Value
targetField.YNode().Value = value.YNode().Value
} else {
t.SetYNode(value.YNode())
targetField.SetYNode(value.YNode())
}

return nil
}

func getReplacement(nodes []*yaml.RNode, r *types.Replacement) (*yaml.RNode, error) {
source, err := selectSourceNode(nodes, r.Source)
if err != nil {
return nil, err
}

if r.Source.FieldPath == "" {
r.Source.FieldPath = types.DefaultReplacementFieldPath
}
fieldPath := kyaml_utils.SmarterPathSplitter(r.Source.FieldPath, ".")

rn, err := source.Pipe(yaml.Lookup(fieldPath...))
if err != nil {
return nil, err
}
if rn.IsNilOrEmpty() {
return nil, fmt.Errorf("fieldPath `%s` is missing for replacement source %s", r.Source.FieldPath, r.Source.ResId)
}

return getRefinedValue(r.Source.Options, rn)
}

func getRefinedValue(options *types.FieldOptions, rn *yaml.RNode) (*yaml.RNode, error) {
if options == nil || options.Delimiter == "" {
return rn, nil
func shouldCreateField(options *types.FieldOptions, fieldPath []string) (bool, error) {
if options == nil || !options.Create {
return false, nil
}
if rn.YNode().Kind != yaml.ScalarNode {
return nil, fmt.Errorf("delimiter option can only be used with scalar nodes")
}
value := strings.Split(yaml.GetValue(rn), options.Delimiter)
if options.Index >= len(value) || options.Index < 0 {
return nil, fmt.Errorf("options.index %d is out of bounds for value %s", options.Index, yaml.GetValue(rn))
}
n := rn.Copy()
n.YNode().Value = value[options.Index]
return n, nil
}

// selectSourceNode finds the node that matches the selector, returning
// an error if multiple or none are found
func selectSourceNode(nodes []*yaml.RNode, selector *types.SourceSelector) (*yaml.RNode, error) {
var matches []*yaml.RNode
for _, n := range nodes {
ids, err := utils.MakeResIds(n)
if err != nil {
return nil, err
// create option is not supported in a wildcard matching
for _, f := range fieldPath {
if f == "*" {
return false, fmt.Errorf("cannot support create option in a multi-value target")
}
for _, id := range ids {
if id.IsSelectedBy(selector.ResId) {
if len(matches) > 0 {
return nil, fmt.Errorf(
"multiple matches for selector %s", selector)
}
matches = append(matches, n)
break
}
}
}
if len(matches) == 0 {
return nil, fmt.Errorf("nothing selected by %s", selector)
}
return matches[0], nil
return true, nil
}
60 changes: 60 additions & 0 deletions api/filters/replacement/replacement_test.go
View file
Open in desktop
Expand Up @@ -2354,6 +2354,66 @@ spec:
name: myapp-container
ports:
- containerPort: 80
`,
},
"replace an existing mapping value": {
input: `apiVersion: batch/v1
kind: Job
metadata:
name: hello
spec:
template:
spec:
containers:
- image: busybox
name: myapp-container
restartPolicy: old
---
apiVersion: v1
kind: Pod
metadata:
name: my-pod
spec:
containers:
- image: busybox
name: myapp-container
restartPolicy: new
`,
replacements: `replacements:
- source:
kind: Pod
name: my-pod
fieldPath: spec
targets:
- select:
name: hello
kind: Job
fieldPaths:
- spec.template.spec
options:
create: true
`,
expected: `apiVersion: batch/v1
kind: Job
metadata:
name: hello
spec:
template:
spec:
containers:
- image: busybox
name: myapp-container
restartPolicy: new
---
apiVersion: v1
kind: Pod
metadata:
name: my-pod
spec:
containers:
- image: busybox
name: myapp-container
restartPolicy: new
`,
},
}
Expand Down