Wildcard support for creation in ReplacementTransformer (#4886)

* Ahead-of-time wildcard path expansion solution

* Wrapped PathGetter solution

This approach doesn't work when multiple existing sequence elements
should match, i.e. because the sequence contains maps and we're
searching on a key they all contain (target all containers with a certain
image would be one use case for this). PathGetter just takes the first
match in that case, which is not what we want.

* Add creation support to PathMatcher

* Regression test for existing bug when creation is enabled and sequence query should match multiple elements

* PathMatcher Create tests and support for sequence appending

* revert hyphen append support

PathGetter treats it as meaning 'last' not 'append' and does not have test coverage for its handling of this when create is set. Semantics are dubious given that multiple Replacement fieldPaths may be specified, which would cause successive appends.

* This also provides a solution to issue 1493

* Review feedback
This commit is contained in:
Katrina Verey
2022-12-06 15:40:37 -05:00
committed by GitHub
parent 2ed910abb0
commit 903fbb6ed2
5 changed files with 661 additions and 73 deletions

View File

@@ -550,7 +550,7 @@ func (l PathGetter) getFilter(part, nextPart string, fieldPath *[]string) (Filte
default:
// mapping node
*fieldPath = append(*fieldPath, part)
return l.fieldFilter(part, l.getKind(nextPart))
return l.fieldFilter(part, getPathPartKind(nextPart, l.Create))
}
}
@@ -590,15 +590,18 @@ func (l PathGetter) fieldFilter(
return FieldMatcher{Name: name, Create: &RNode{value: &yaml.Node{Kind: kind, Style: l.Style}}}, nil
}
func (l PathGetter) getKind(nextPart string) yaml.Kind {
func getPathPartKind(nextPart string, defaultKind yaml.Kind) yaml.Kind {
if IsListIndex(nextPart) {
// if nextPart is of the form [a=b], then it is an index into a Sequence
// so the current part must be a SequenceNode
return yaml.SequenceNode
}
if IsIdxNumber(nextPart) {
return yaml.SequenceNode
}
if nextPart == "" {
// final name in the path, use the l.Create defined Kind
return l.Create
// final name in the path, use the default kind provided
return defaultKind
}
// non-sequence intermediate Node

View File

@@ -4,9 +4,13 @@
package yaml
import (
"fmt"
"regexp"
"strconv"
"strings"
"sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/internal/forked/github.com/go-yaml/yaml"
)
// PathMatcher returns all RNodes matching the path wrapped in a SequenceNode.
@@ -21,7 +25,7 @@ type PathMatcher struct {
// Each path part may be one of:
// * FieldMatcher -- e.g. "spec"
// * Map Key -- e.g. "app.k8s.io/version"
// * List Entry -- e.g. "[name=nginx]" or "[=-jar]"
// * List Entry -- e.g. "[name=nginx]" or "[=-jar]" or "0"
//
// Map Keys and Fields are equivalent.
// See FieldMatcher for more on Fields and Map Keys.
@@ -43,10 +47,18 @@ type PathMatcher struct {
// This is useful for if the nodes are to be printed in FlowStyle.
StripComments bool
val *RNode
field string
matchRegex string
indexNumber int
// Create will cause missing path parts to be created as they are walked.
//
// * The leaf Node (final path) will be created with a Kind matching Create
// * Intermediary Nodes will be created as either a MappingNodes or
// SequenceNodes as appropriate for each's Path location.
// * Nodes identified by an index will only be created if the index indicates
// an append operation (i.e. index=len(list))
Create yaml.Kind `yaml:"create,omitempty"`
val *RNode
field string
matchRegex string
}
func (p *PathMatcher) stripComments(n *Node) {
@@ -109,7 +121,7 @@ func (p *PathMatcher) doMatchEvery(rn *RNode) (*RNode, error) {
func (p *PathMatcher) visitEveryElem(elem *RNode) error {
fieldName := p.Path[0]
// recurse on the matching element
pm := &PathMatcher{Path: p.Path[1:]}
pm := &PathMatcher{Path: p.Path[1:], Create: p.Create}
add, err := pm.filter(elem)
for k, v := range pm.Matches {
p.Matches[k] = v
@@ -125,13 +137,25 @@ func (p *PathMatcher) visitEveryElem(elem *RNode) error {
func (p *PathMatcher) doField(rn *RNode) (*RNode, error) {
// lookup the field
field, err := rn.Pipe(Get(p.Path[0]))
if err != nil || field == nil {
// if the field doesn't exist, return nil
if err != nil || (!IsCreate(p.Create) && field == nil) {
return nil, err
}
if IsCreate(p.Create) && field == nil {
var nextPart string
if len(p.Path) > 1 {
nextPart = p.Path[1]
}
nextPartKind := getPathPartKind(nextPart, p.Create)
field = &RNode{value: &yaml.Node{Kind: nextPartKind}}
err := rn.PipeE(SetField(p.Path[0], field))
if err != nil {
return nil, err
}
}
// recurse on the field, removing the first element of the path
pm := &PathMatcher{Path: p.Path[1:]}
pm := &PathMatcher{Path: p.Path[1:], Create: p.Create}
p.val, err = pm.filter(field)
p.Matches = pm.Matches
return p.val, err
@@ -144,18 +168,33 @@ func (p *PathMatcher) doIndexSeq(rn *RNode) (*RNode, error) {
if err != nil {
return nil, err
}
p.indexNumber = idx
elements, err := rn.Elements()
if err != nil {
return nil, err
}
if len(elements) == idx && IsCreate(p.Create) {
var nextPart string
if len(p.Path) > 1 {
nextPart = p.Path[1]
}
elem := &yaml.Node{Kind: getPathPartKind(nextPart, p.Create)}
err = rn.PipeE(Append(elem))
if err != nil {
return nil, errors.WrapPrefixf(err, "failed to append element for %q", p.Path[0])
}
elements = append(elements, NewRNode(elem))
}
if len(elements) < idx+1 {
return nil, fmt.Errorf("index %d specified but only %d elements found", idx, len(elements))
}
// get target element
element := elements[idx]
// recurse on the matching element
pm := &PathMatcher{Path: p.Path[1:]}
pm := &PathMatcher{Path: p.Path[1:], Create: p.Create}
add, err := pm.filter(element)
for k, v := range pm.Matches {
p.Matches[k] = v
@@ -176,16 +215,39 @@ func (p *PathMatcher) doSeq(rn *RNode) (*RNode, error) {
return nil, err
}
if p.field == "" {
primitiveElement := len(p.field) == 0
if primitiveElement {
err = rn.VisitElements(p.visitPrimitiveElem)
} else {
err = rn.VisitElements(p.visitElem)
}
if err != nil || p.val == nil || len(p.val.YNode().Content) == 0 {
if err != nil {
return nil, err
}
if !p.val.IsNil() && len(p.val.YNode().Content) == 0 {
p.val = nil
}
return p.val, nil
if !IsCreate(p.Create) || p.val != nil {
return p.val, nil
}
var elem *yaml.Node
valueNode := NewScalarRNode(p.matchRegex).YNode()
if primitiveElement {
elem = valueNode
} else {
elem = &yaml.Node{
Kind: yaml.MappingNode,
Content: []*yaml.Node{{Kind: yaml.ScalarNode, Value: p.field}, valueNode},
}
}
err = rn.PipeE(Append(elem))
if err != nil {
return nil, errors.WrapPrefixf(err, "failed to create element for %q", p.Path[0])
}
// re-do the sequence search; this time we'll find the element we just created
return p.doSeq(rn)
}
func (p *PathMatcher) visitPrimitiveElem(elem *RNode) error {
@@ -228,7 +290,7 @@ func (p *PathMatcher) visitElem(elem *RNode) error {
}
// recurse on the matching element
pm := &PathMatcher{Path: p.Path[1:]}
pm := &PathMatcher{Path: p.Path[1:], Create: p.Create}
add, err := pm.filter(elem)
for k, v := range pm.Matches {
p.Matches[k] = v

View File

@@ -8,6 +8,8 @@ import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/kyaml/internal/forked/github.com/go-yaml/yaml"
)
func TestPathMatcher_Filter(t *testing.T) {
@@ -30,6 +32,7 @@ spec:
containers:
- name: nginx
image: nginx:1.7.9
args: [-c, conf.yaml]
ports:
- containerPort: 80
- name: sidecar
@@ -45,7 +48,7 @@ spec:
}{
{[]string{
"spec", "template", "spec", "containers", "[name=.*]"},
"- name: nginx\n image: nginx:1.7.9\n ports:\n - containerPort: 80\n" +
"- name: nginx\n image: nginx:1.7.9\n args: [-c, conf.yaml]\n ports:\n - containerPort: 80\n" +
"- name: sidecar\n image: sidecar:1.0.0\n ports:\n - containerPort: 8081\n - containerPort: 9090\n"},
{[]string{
"spec", "template", "spec", "containers", "[name=.*]", "image"},
@@ -83,6 +86,9 @@ spec:
{[]string{
"spec", "template", "spec", "containers", "*", "ports", "*"},
"- containerPort: 80\n- containerPort: 8081\n- containerPort: 9090\n"},
{[]string{
"spec", "template", "spec", "containers", "[name=.*]", "args", "1"},
"- conf.yaml\n"},
}
for i, u := range updates {
result, err := node.Pipe(&PathMatcher{Path: u.path})
@@ -92,3 +98,192 @@ spec:
assert.Equal(t, u.value, result.MustString(), fmt.Sprintf("%d", i))
}
}
func TestPathMatcher_Filter_Create(t *testing.T) {
testCases := map[string]struct {
path []string
matches []string
modifiedNodeMustContain string
create yaml.Kind
expectErr string
}{
"create non-primitive sequence item that does not exist": {
path: []string{"spec", "template", "spec", "containers", "[name=please-create-me]"},
matches: []string{
"name: please-create-me\n",
},
modifiedNodeMustContain: "- name: please-create-me",
create: yaml.MappingNode,
},
"create non-primitive item in empty sequence by index": {
path: []string{"spec", "template", "spec", "containers", "[name=nginx]", "envFrom", "0"},
matches: []string{"{}\n"},
modifiedNodeMustContain: "envFrom:\n - {}\n",
create: yaml.MappingNode,
},
"create primitive item in empty sequence by index": {
path: []string{"spec", "template", "spec", "containers", "[name=sidecar]", "args", "0"},
matches: []string{"\n"},
modifiedNodeMustContain: "args:\n -\n",
create: yaml.ScalarNode,
},
"append primitive item to sequence by index": {
path: []string{"spec", "template", "spec", "containers", "[name=nginx]", "args", "2"},
matches: []string{"\n"},
create: yaml.ScalarNode,
modifiedNodeMustContain: "args: [-c, conf.yaml, '']",
},
"append non-primitive item to sequence by index": {
path: []string{"spec", "template", "spec", "containers", "[name=nginx]", "ports", "1"},
matches: []string{"{}\n"},
modifiedNodeMustContain: "ports: [{containerPort: 80}, {}]",
create: yaml.MappingNode,
},
"appending non-primitive element in middle of sequence": {
path: []string{"spec", "template", "spec", "containers", "2", "imagePullPolicy"},
matches: []string{"\n"},
create: yaml.ScalarNode,
modifiedNodeMustContain: "\n - imagePullPolicy:\n",
},
"fail to create non-primitive item by non-zero index in created sequence": {
path: []string{"spec", "template", "spec", "containers", "[name=nginx]", "envFrom", "1"},
matches: []string{},
create: yaml.MappingNode,
expectErr: "index 1 specified but only 0 elements found",
},
"fail to create primitive item by non-zero index in created sequence": {
path: []string{"spec", "template", "spec", "containers", "[name=sidecar]", "args", "1"},
matches: []string{},
create: yaml.ScalarNode,
expectErr: "index 1 specified but only 0 elements found",
},
"fail to create non-primitive item by distant index in existing sequence": {
path: []string{"spec", "template", "spec", "containers", "3"},
matches: []string{},
create: yaml.MappingNode,
expectErr: "index 3 specified but only 2 elements found",
},
"fail to create primitive item by distant index in existing sequence": {
path: []string{"spec", "template", "spec", "containers", "[name=nginx]", "args", "3"},
matches: []string{},
create: yaml.ScalarNode,
expectErr: "index 3 specified but only 2 elements found",
},
"create primitive sequence item that does not exist": {
path: []string{"metadata", "finalizers", "[=create-me]"},
matches: []string{
"create-me\n",
},
modifiedNodeMustContain: "finalizers:\n - create-me\n",
create: yaml.ScalarNode,
},
"create series of maps that do not exist": {
path: []string{"spec", "selector", "matchLabels", "does-not-exist"},
matches: []string{
"{}\n",
},
modifiedNodeMustContain: "selector:\n matchLabels:\n app: nginx\n does-not-exist: {}\n",
create: yaml.MappingNode,
},
"create scalar below series of maps and sequences that do not exist": {
path: []string{"spec", "template", "spec", "containers", "[name=please-create-me]", "env", "[key=please-create-me]", "value"},
matches: []string{
"\n",
},
modifiedNodeMustContain: "- name: please-create-me\n env:\n - key: please-create-me\n value:\n",
create: yaml.ScalarNode,
},
"find sequence items that already exist": {
path: []string{"spec", "template", "spec", "containers", "[name=.*]"},
matches: []string{
"name: nginx\nimage: nginx:1.7.9\nargs: [-c, conf.yaml]\nports: [{containerPort: 80}]\nenv:\n- key: CONTAINER_NAME\n value: nginx\n",
"name: sidecar\nimage: sidecar:1.0.0\nports:\n- containerPort: 8081\n- containerPort: 9090\n",
},
create: yaml.MappingNode,
},
"find and create sequence below wildcard that exists on some sequence items": {
path: []string{"spec", "template", "spec", "containers", "[name=.*]", "env"},
matches: []string{
"- key: CONTAINER_NAME\n value: nginx\n",
"[]\n",
},
create: yaml.SequenceNode,
},
"find field below wildcard that exists on all sequence items": {
path: []string{"spec", "template", "spec", "containers", "[name=.*]", "ports"},
matches: []string{
"[{containerPort: 80}]\n",
"- containerPort: 8081\n- containerPort: 9090\n",
},
create: yaml.SequenceNode,
},
"find field below query that targets a specific item": {
path: []string{"spec", "template", "spec", "containers", "[name=nginx]", "env"},
matches: []string{
"- key: CONTAINER_NAME\n value: nginx\n",
},
create: yaml.SequenceNode,
},
"create field below query that targets any value of a field that does not exist": {
path: []string{"spec", "template", "spec", "containers", "[foo=.*]", "env"},
matches: []string{
"[]\n",
},
// This is kinda weird. The query doesn't match anything, and we can't tell that it is a
// wildcard rather than a literal, so we use the value to create the field.
modifiedNodeMustContain: "- foo: .*\n env: []\n",
create: yaml.SequenceNode,
},
}
nodeStr := `apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
labels:
app: nginx
spec:
replicas: 3
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: nginx:1.7.9
args: [-c, conf.yaml]
ports: [{containerPort: 80}]
env:
- key: CONTAINER_NAME
value: nginx
- name: sidecar
image: sidecar:1.0.0
ports:
- containerPort: 8081
- containerPort: 9090
`
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
node := MustParse(nodeStr)
result, err := node.Pipe(&PathMatcher{Path: tc.path, Create: tc.create})
if tc.expectErr != "" {
require.EqualError(t, err, tc.expectErr)
return
}
require.NoError(t, err)
matches, err := result.Elements()
require.NoError(t, err)
require.Equalf(t, len(tc.matches), len(matches), "Full sequence wrapper of result:\n%s", result.MustString())
modifiedNode := node.MustString()
for i, expected := range tc.matches {
assert.Equal(t, tc.create, matches[i].YNode().Kind)
assert.Equal(t, expected, matches[i].MustString())
assert.Contains(t, modifiedNode, tc.modifiedNodeMustContain)
}
})
}
}