Merge pull request #3519 from monopole/fieldspectest

More fieldspec filter tests and comments.
This commit is contained in:
Jeff Regan
2021-01-31 20:01:47 -08:00
committed by GitHub
3 changed files with 256 additions and 206 deletions

View File

@@ -4,6 +4,7 @@
package fieldspec
import (
"fmt"
"strings"
"sigs.k8s.io/kustomize/api/filters/filtersutil"
@@ -15,7 +16,16 @@ import (
var _ yaml.Filter = Filter{}
// Filter applies a single fieldSpec to a single object
// Filter possibly mutates its object argument using a FieldSpec.
// If the object matches the FieldSpec, and the node found
// by following the fieldSpec's path is non-null, this filter calls
// the setValue function on the node at the end of the path.
// If any part of the path doesn't exist, the filter returns
// without doing anything and without error, unless it was set
// to create the path. If set to create, it creates a tree of maps
// along the path, and the leaf node gets the setValue called on it.
// Error on GVK mismatch, empty or poorly formed path.
// Filter expect kustomize style paths, not JSON paths.
// Filter stores internal state and should not be reused
type Filter struct {
// FieldSpec contains the path to the value to set.
@@ -39,7 +49,8 @@ func (fltr Filter) Filter(obj *yaml.RNode) (*yaml.RNode, error) {
return obj, errors.Wrap(err)
}
fltr.path = utils.PathSplitter(fltr.FieldSpec.Path)
if err := fltr.filter(obj); err != nil {
err := fltr.filter(obj)
if err != nil {
s, _ := obj.String()
return nil, errors.WrapPrefixf(err,
"considering field '%s' of object\n%v", fltr.FieldSpec.Path, s)
@@ -47,6 +58,7 @@ func (fltr Filter) Filter(obj *yaml.RNode) (*yaml.RNode, error) {
return obj, nil
}
// Recursively called.
func (fltr Filter) filter(obj *yaml.RNode) error {
if len(fltr.path) == 0 {
// found the field -- set its value
@@ -57,9 +69,9 @@ func (fltr Filter) filter(obj *yaml.RNode) error {
}
switch obj.YNode().Kind {
case yaml.SequenceNode:
return fltr.seq(obj)
return fltr.handleSequence(obj)
case yaml.MappingNode:
return fltr.field(obj)
return fltr.handleMap(obj)
case yaml.AliasNode:
return fltr.filter(yaml.NewRNode(obj.YNode().Alias))
default:
@@ -67,17 +79,20 @@ func (fltr Filter) filter(obj *yaml.RNode) error {
}
}
// field calls filter on the field matching the next path element
func (fltr Filter) field(obj *yaml.RNode) error {
// handleMap calls filter on the map field matching the next path element
func (fltr Filter) handleMap(obj *yaml.RNode) error {
fieldName, isSeq := isSequenceField(fltr.path[0])
if fieldName == "" {
return fmt.Errorf("cannot set or create an empty field name")
}
// lookup the field matching the next path element
var lookupField yaml.Filter
var operation yaml.Filter
var kind yaml.Kind
tag := yaml.NodeTagEmpty
switch {
case !fltr.FieldSpec.CreateIfNotPresent || fltr.CreateKind == 0 || isSeq:
// dont' create the field if we don't find it
lookupField = yaml.Lookup(fieldName)
// don't create the field if we don't find it
operation = yaml.Lookup(fieldName)
if isSeq {
// The query path thinks this field should be a sequence;
// accept this hint for use later if the tag is NodeTagNull.
@@ -85,21 +100,25 @@ func (fltr Filter) field(obj *yaml.RNode) error {
}
case len(fltr.path) <= 1:
// create the field if it is missing: use the provided node kind
lookupField = yaml.LookupCreate(fltr.CreateKind, fieldName)
operation = yaml.LookupCreate(fltr.CreateKind, fieldName)
kind = fltr.CreateKind
tag = fltr.CreateTag
default:
// create the field if it is missing: must be a mapping node
lookupField = yaml.LookupCreate(yaml.MappingNode, fieldName)
operation = yaml.LookupCreate(yaml.MappingNode, fieldName)
kind = yaml.MappingNode
tag = yaml.NodeTagMap
}
// locate (or maybe create) the field
field, err := obj.Pipe(lookupField)
if err != nil || field == nil {
field, err := obj.Pipe(operation)
if err != nil {
return errors.WrapPrefixf(err, "fieldName: %s", fieldName)
}
if field == nil {
// No error if field not found.
return nil
}
// if the value exists, but is null and kind is set,
// then change it to the creation type
@@ -117,7 +136,7 @@ func (fltr Filter) field(obj *yaml.RNode) error {
}
// seq calls filter on all sequence elements
func (fltr Filter) seq(obj *yaml.RNode) error {
func (fltr Filter) handleSequence(obj *yaml.RNode) error {
if err := obj.VisitElements(func(node *yaml.RNode) error {
// recurse on each element -- re-allocating a Filter is
// not strictly required, but is more consistent with field
@@ -128,16 +147,14 @@ func (fltr Filter) seq(obj *yaml.RNode) error {
return errors.WrapPrefixf(err,
"visit traversal on path: %v", fltr.path)
}
return nil
}
// isSequenceField returns true if the path element is for a sequence field.
// isSequence also returns the path element with the '[]' suffix trimmed
func isSequenceField(name string) (string, bool) {
isSeq := strings.HasSuffix(name, "[]")
name = strings.TrimSuffix(name, "[]")
return name, isSeq
shorter := strings.TrimSuffix(name, "[]")
return shorter, shorter != name
}
// isMatchGVK returns true if the fs.GVK matches the obj GVK.

View File

@@ -15,209 +15,243 @@ import (
"sigs.k8s.io/kustomize/kyaml/yaml"
)
type TestCase struct {
name string
input string
expected string
filter fieldspec.Filter
fieldSpec string
error string
}
var tests = []TestCase{
{
name: "update",
fieldSpec: `
func TestFilter_Filter(t *testing.T) {
testCases := map[string]struct {
input string
expected string
filter fieldspec.Filter
fieldSpec string
error string
}{
"path not found": {
fieldSpec: `
path: a/b
group: foo
kind: Bar
`,
input: `
input: `
apiVersion: foo
kind: Bar
xxx:
`,
expected: `
apiVersion: foo
kind: Bar
xxx:
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
"empty path": {
fieldSpec: `
group: foo
kind: Bar
`,
input: `
apiVersion: foo
kind: Bar
xxx:
`,
expected: `
apiVersion: foo
kind: Bar
xxx:
`,
error: `considering field '' of object
apiVersion: foo
kind: Bar
xxx:
: cannot set or create an empty field name`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
"update": {
fieldSpec: `
path: a/b
group: foo
kind: Bar
`,
input: `
apiVersion: foo/v1beta1
kind: Bar
a:
b: c
`,
expected: `
expected: `
apiVersion: foo/v1beta1
kind: Bar
a:
b: e
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
},
{
name: "update-kind-not-match",
fieldSpec: `
"update-kind-not-match": {
fieldSpec: `
path: a/b
group: foo
kind: Bar1
`,
input: `
input: `
apiVersion: foo/v1beta1
kind: Bar2
a:
b: c
`,
expected: `
expected: `
apiVersion: foo/v1beta1
kind: Bar2
a:
b: c
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
},
{
name: "update-group-not-match",
fieldSpec: `
"update-group-not-match": {
fieldSpec: `
path: a/b
group: foo1
kind: Bar
`,
input: `
input: `
apiVersion: foo2/v1beta1
kind: Bar
a:
b: c
`,
expected: `
expected: `
apiVersion: foo2/v1beta1
kind: Bar
a:
b: c
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
},
{
name: "update-version-not-match",
fieldSpec: `
"update-version-not-match": {
fieldSpec: `
path: a/b
group: foo
version: v1beta1
kind: Bar
`,
input: `
input: `
apiVersion: foo/v1beta2
kind: Bar
a:
b: c
`,
expected: `
expected: `
apiVersion: foo/v1beta2
kind: Bar
a:
b: c
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
},
{
name: "bad-version",
fieldSpec: `
"bad-version": {
fieldSpec: `
path: a/b
group: foo
version: v1beta1
kind: Bar
`,
input: `
input: `
apiVersion: foo/v1beta2/something
kind: Bar
a:
b: c
`,
expected: `
expected: `
apiVersion: foo/v1beta2/something
kind: Bar
a:
b: c
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
},
{
name: "bad-meta",
fieldSpec: `
"bad-meta": {
fieldSpec: `
path: a/b
group: foo
version: v1beta1
kind: Bar
`,
input: `
input: `
a:
b: c
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
error: "missing Resource metadata",
},
error: "missing Resource metadata",
},
{
name: "miss-match-type",
fieldSpec: `
"miss-match-type": {
fieldSpec: `
path: a/b/c
kind: Bar
`,
input: `
input: `
kind: Bar
a:
b: a
`,
error: `considering field 'a/b/c' of object
error: `considering field 'a/b/c' of object
kind: Bar
a:
b: a
: expected sequence or mapping node`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
},
{
name: "add",
fieldSpec: `
"add": {
fieldSpec: `
path: a/b/c/d
group: foo
create: true
kind: Bar
`,
input: `
input: `
apiVersion: foo/v1beta1
kind: Bar
a: {}
`,
expected: `
expected: `
apiVersion: foo/v1beta1
kind: Bar
a: {b: {c: {d: e}}}
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "update-in-sequence",
fieldSpec: `
"update-in-sequence": {
fieldSpec: `
path: a/b[]/c/d
group: foo
kind: Bar
`,
input: `
input: `
apiVersion: foo/v1beta1
kind: Bar
a:
@@ -225,7 +259,7 @@ a:
- c:
d: a
`,
expected: `
expected: `
apiVersion: foo/v1beta1
kind: Bar
a:
@@ -233,244 +267,237 @@ a:
- c:
d: e
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
},
},
},
// Don't create a sequence
{
name: "empty-sequence-no-create",
fieldSpec: `
// Don't create a sequence
"empty-sequence-no-create": {
fieldSpec: `
path: a/b[]/c/d
group: foo
create: true
kind: Bar
`,
input: `
input: `
apiVersion: foo/v1beta1
kind: Bar
a: {}
`,
expected: `
expected: `
apiVersion: foo/v1beta1
kind: Bar
a: {}
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
},
},
},
// Create a new field for an element in a sequence
{
name: "empty-sequence-create",
fieldSpec: `
// Create a new field for an element in a sequence
"empty-sequence-create": {
fieldSpec: `
path: a/b[]/c/d
group: foo
create: true
kind: Bar
`,
input: `
input: `
apiVersion: foo/v1beta1
kind: Bar
a:
b:
- c: {}
`,
expected: `
expected: `
apiVersion: foo/v1beta1
kind: Bar
a:
b:
- c: {d: e}
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "group v1",
fieldSpec: `
"group v1": {
fieldSpec: `
path: a/b
group: v1
create: true
kind: Bar
`,
input: `
input: `
apiVersion: v1
kind: Bar
`,
expected: `
expected: `
apiVersion: v1
kind: Bar
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "version v1",
fieldSpec: `
"version v1": {
fieldSpec: `
path: a/b
version: v1
create: true
kind: Bar
`,
input: `
input: `
apiVersion: v1
kind: Bar
`,
expected: `
expected: `
apiVersion: v1
kind: Bar
a:
b: e
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("e"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "successfully set field on array entry no sequence hint",
fieldSpec: `
"successfully set field on array entry no sequence hint": {
fieldSpec: `
path: spec/containers/image
version: v1
kind: Bar
`,
input: `
input: `
apiVersion: v1
kind: Bar
spec:
containers:
- image: foo
`,
expected: `
expected: `
apiVersion: v1
kind: Bar
spec:
containers:
- image: bar
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "successfully set field on array entry with sequence hint",
fieldSpec: `
"successfully set field on array entry with sequence hint": {
fieldSpec: `
path: spec/containers[]/image
version: v1
kind: Bar
`,
input: `
input: `
apiVersion: v1
kind: Bar
spec:
containers:
- image: foo
`,
expected: `
expected: `
apiVersion: v1
kind: Bar
spec:
containers:
- image: bar
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "failure to set field on array entry with sequence hint in path",
fieldSpec: `
"failure to set field on array entry with sequence hint in path": {
fieldSpec: `
path: spec/containers[]/image
version: v1
kind: Bar
`,
input: `
input: `
apiVersion: v1
kind: Bar
spec:
containers:
`,
expected: `
expected: `
apiVersion: v1
kind: Bar
spec:
containers: []
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "failure to set field on array entry, no sequence hint in path",
fieldSpec: `
"failure to set field on array entry, no sequence hint in path": {
fieldSpec: `
path: spec/containers/image
version: v1
kind: Bar
`,
input: `
input: `
apiVersion: v1
kind: Bar
spec:
containers:
`,
expected: `
expected: `
apiVersion: v1
kind: Bar
spec:
containers:
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "filedname with slash '/'",
fieldSpec: `
"fieldname with slash '/'": {
fieldSpec: `
path: a/b\/c/d
version: v1
kind: Bar
`,
input: `
input: `
apiVersion: v1
kind: Bar
a:
b/c:
d: foo
`,
expected: `
expected: `
apiVersion: v1
kind: Bar
a:
b/c:
d: bar
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
},
},
},
{
name: "filedname with multiple '/'",
fieldSpec: `
"fieldname with multiple '/'": {
fieldSpec: `
path: a/b\/c/d\/e/f
version: v1
kind: Bar
`,
input: `
input: `
apiVersion: v1
kind: Bar
a:
@@ -478,7 +505,7 @@ a:
d/e:
f: foo
`,
expected: `
expected: `
apiVersion: v1
kind: Bar
a:
@@ -486,25 +513,24 @@ a:
d/e:
f: bar
`,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
filter: fieldspec.Filter{
SetValue: filtersutil.SetScalar("bar"),
CreateKind: yaml.ScalarNode,
},
},
},
}
}
func TestFilter_Filter(t *testing.T) {
for i := range tests {
test := tests[i]
t.Run(test.name, func(t *testing.T) {
err := yaml.Unmarshal([]byte(test.fieldSpec), &test.filter.FieldSpec)
for n := range testCases {
tc := testCases[n]
t.Run(n, func(t *testing.T) {
err := yaml.Unmarshal([]byte(tc.fieldSpec), &tc.filter.FieldSpec)
if !assert.NoError(t, err) {
t.FailNow()
}
out := &bytes.Buffer{}
rw := &kio.ByteReadWriter{
Reader: bytes.NewBufferString(test.input),
Reader: bytes.NewBufferString(tc.input),
Writer: out,
OmitReaderAnnotations: true,
}
@@ -512,11 +538,11 @@ func TestFilter_Filter(t *testing.T) {
// run the filter
err = kio.Pipeline{
Inputs: []kio.Reader{rw},
Filters: []kio.Filter{kio.FilterAll(test.filter)},
Filters: []kio.Filter{kio.FilterAll(tc.filter)},
Outputs: []kio.Writer{rw},
}.Execute()
if test.error != "" {
if !assert.EqualError(t, err, test.error) {
if tc.error != "" {
if !assert.EqualError(t, err, tc.error) {
t.FailNow()
}
// stop rest of test
@@ -529,7 +555,7 @@ func TestFilter_Filter(t *testing.T) {
// check results
if !assert.Equal(t,
strings.TrimSpace(test.expected),
strings.TrimSpace(tc.expected),
strings.TrimSpace(out.String())) {
t.FailNow()
}

View File

@@ -17,6 +17,8 @@ type nameReferenceTransformer struct {
backRefs []builtinconfig.NameBackReferences
}
const doDebug = false
var _ resmap.Transformer = &nameReferenceTransformer{}
type filterMap map[*resource.Resource][]nameref.Filter
@@ -47,7 +49,7 @@ func newNameReferenceTransformer(
//
func (t *nameReferenceTransformer) Transform(m resmap.ResMap) error {
fMap := t.determineFilters(m.Resources())
// debug(fMap)
debug(fMap)
for r, fList := range fMap {
c := m.SubsetThatCouldBeReferencedByResource(r)
for _, f := range fList {
@@ -61,8 +63,10 @@ func (t *nameReferenceTransformer) Transform(m resmap.ResMap) error {
return nil
}
//nolint: deadcode, unused
func debug(fMap filterMap) {
if !doDebug {
return
}
fmt.Printf("filterMap has %d entries:\n", len(fMap))
rCount := 0
for r, fList := range fMap {
@@ -117,6 +121,9 @@ func (t *nameReferenceTransformer) determineFilters(
// that might need updating.
fMap[res] = append(fMap[res], nameref.Filter{
// Name field to write in the Referrer.
// If the path specified here isn't found in
// the Referrer, nothing happens (no error,
// no field creation).
NameFieldToUpdate: referrerSpec,
// Specification of object class to read from.
// Always read from metadata/name field.