Merge pull request #2748 from monopole/tweakSequenceTreatment

Tweak sequence treatment in fieldspecs

api/filters/annotations/annotations.go

@@ -32,7 +32,7 @@ func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
 					FsSlice:    f.FsSlice,
 					SetValue:   filtersutil.SetEntry(k, f.Annotations[k], yaml.StringTag),
 					CreateKind: yaml.MappingNode, // Annotations are MappingNodes.
-					CreateTag:  "!!map",
+					CreateTag:  "!!map",          // TODO: change to yaml.NodeTagMap
 				}); err != nil {
 					return nil, err
 				}

api/filters/fieldspec/fieldspec.go

@@ -41,7 +41,7 @@ func (fltr Filter) Filter(obj *yaml.RNode) (*yaml.RNode, error) {
 	if err := fltr.filter(obj); err != nil {
 		s, _ := obj.String()
 		return nil, errors.WrapPrefixf(err,
-			"obj %v at path %v", s, fltr.FieldSpec.Path)
+			"obj '%s' at path '%v'", s, fltr.FieldSpec.Path)
 	}
 	return obj, nil
 }
@@ -56,24 +56,27 @@ func (fltr Filter) filter(obj *yaml.RNode) error {
 		return fltr.seq(obj)
 	case yaml.MappingNode:
 		return fltr.field(obj)
+	default:
+		return errors.Errorf("expected sequence or mapping node")
 	}
-	// not found -- this might be an error since the type doesn't match
-
-	return errors.Errorf("unsupported yaml node")
 }
 
 // field calls filter on the field matching the next path element
 func (fltr Filter) field(obj *yaml.RNode) error {
 	fieldName, isSeq := isSequenceField(fltr.path[0])
-
 	// lookup the field matching the next path element
 	var lookupField yaml.Filter
 	var kind yaml.Kind
-	var tag string
+	tag := "" // TODO: change to 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)
+		if isSeq {
+			// The query path thinks this field should be a sequence;
+			// accept this hint for use later if the tag is NodeTagNull.
+			kind = yaml.SequenceNode
+		}
 	case len(fltr.path) <= 1:
 		// create the field if it is missing: use the provided node kind
 		lookupField = yaml.LookupCreate(fltr.CreateKind, fieldName)
@@ -83,7 +86,7 @@ func (fltr Filter) field(obj *yaml.RNode) error {
 		// create the field if it is missing: must be a mapping node
 		lookupField = yaml.LookupCreate(yaml.MappingNode, fieldName)
 		kind = yaml.MappingNode
-		tag = "!!map"
+		tag = "!!map" // TODO: change to yaml.NodeTagMap
 	}
 
 	// locate (or maybe create) the field
@@ -94,7 +97,7 @@ func (fltr Filter) field(obj *yaml.RNode) error {
 
 	// if the value exists, but is null, then change it to the creation type
 	// TODO: update yaml.LookupCreate to support this
-	if field.YNode().Tag == "!!null" {
+	if field.YNode().Tag == "!!null" { // TODO: change to yaml.NodeTagNull
 		field.YNode().Kind = kind
 		field.YNode().Tag = tag
 	}

api/filters/fieldspec/fieldspec_test.go

@@ -176,7 +176,8 @@ kind: Bar
 a:
   b: a
 `,
-		error: "obj kind: Bar\na:\n  b: a\n at path a/b/c: unsupported yaml node",
+		error: "obj 'kind: Bar\na:\n  b: a\n' at path 'a/b/c': " +
+			"expected sequence or mapping node",
 		filter: fieldspec.Filter{
 			SetValue: filtersutil.SetScalar("e"),
 		},
@@ -333,6 +334,107 @@ a:
 			CreateKind: yaml.ScalarNode,
 		},
 	},
+	{
+		name: "successfully set field on array entry no sequence hint",
+		fieldSpec: `
+path: spec/containers/image
+version: v1
+kind: Bar
+`,
+		input: `
+apiVersion: v1
+kind: Bar
+spec:
+  containers:
+  - image: foo
+`,
+		expected: `
+apiVersion: v1
+kind: Bar
+spec:
+  containers:
+  - image: bar
+`,
+		filter: fieldspec.Filter{
+			SetValue:   filtersutil.SetScalar("bar"),
+			CreateKind: yaml.ScalarNode,
+		},
+	},
+	{
+		name: "successfully set field on array entry with sequence hint",
+		fieldSpec: `
+path: spec/containers[]/image
+version: v1
+kind: Bar
+`,
+		input: `
+apiVersion: v1
+kind: Bar
+spec:
+  containers:
+  - image: foo
+`,
+		expected: `
+apiVersion: v1
+kind: Bar
+spec:
+  containers:
+  - image: bar
+`,
+		filter: fieldspec.Filter{
+			SetValue:   filtersutil.SetScalar("bar"),
+			CreateKind: yaml.ScalarNode,
+		},
+	},
+	{
+		name: "failure to set field on array entry with sequence hint in path",
+		fieldSpec: `
+path: spec/containers[]/image
+version: v1
+kind: Bar
+`,
+		input: `
+apiVersion: v1
+kind: Bar
+spec:
+  containers:
+`,
+		expected: `
+apiVersion: v1
+kind: Bar
+spec:
+  containers: []
+`,
+		filter: fieldspec.Filter{
+			SetValue:   filtersutil.SetScalar("bar"),
+			CreateKind: yaml.ScalarNode,
+		},
+	},
+	{
+		name: "failure to set field on array entry, no sequence hint in path",
+		fieldSpec: `
+path: spec/containers/image
+version: v1
+kind: Bar
+`,
+		input: `
+apiVersion: v1
+kind: Bar
+spec:
+  containers:
+`,
+		expected: `
+apiVersion: v1
+kind: Bar
+spec:
+  containers:
+`,
+		filter: fieldspec.Filter{
+			SetValue:   filtersutil.SetScalar("bar"),
+			CreateKind: yaml.ScalarNode,
+		},
+		error: "obj '' at path 'spec/containers/image': expected sequence or mapping node",
+	},
 }
 
 func TestFilter_Filter(t *testing.T) {

api/filters/labels/labels.go

@@ -33,7 +33,7 @@ func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
 					FsSlice:    f.FsSlice,
 					SetValue:   filtersutil.SetEntry(k, f.Labels[k], yaml.StringTag),
 					CreateKind: yaml.MappingNode, // Labels are MappingNodes.
-					CreateTag:  "!!map",
+					CreateTag:  "!!map",          // TODO: change to yaml.NodeTagMap
 				}); err != nil {
 					return nil, err
 				}

api/filters/replicacount/replicacount.go

@@ -27,7 +27,7 @@ func (rc Filter) run(node *yaml.RNode) (*yaml.RNode, error) {
 		FieldSpec:  rc.FieldSpec,
 		SetValue:   rc.set,
 		CreateKind: yaml.ScalarNode, // replicas is a ScalarNode
-		CreateTag:  yaml.IntTag,
+		CreateTag:  yaml.IntTag,     //  yaml.NodeTagInt
 	})
 	return node, err
 }

kyaml/fieldmeta/fieldmeta.go

@@ -210,11 +210,11 @@ func (it FieldValueType) Validate(value string) error {
 func (it FieldValueType) Tag() string {
 	switch it {
 	case String:
-		return yaml.StringTag
+		return yaml.NodeTagString
 	case Bool:
-		return yaml.BoolTag
+		return yaml.NodeTagBool
 	case Int:
-		return yaml.IntTag
+		return yaml.NodeTagInt
 	}
 	return ""
 }
@@ -222,17 +222,17 @@ func (it FieldValueType) Tag() string {
 func (it FieldValueType) TagForValue(value string) string {
 	switch it {
 	case String:
-		return yaml.StringTag
+		return yaml.NodeTagString
 	case Bool:
 		if _, err := strconv.ParseBool(string(it)); err != nil {
 			return ""
 		}
-		return yaml.BoolTag
+		return yaml.NodeTagBool
 	case Int:
 		if _, err := strconv.ParseInt(string(it), 0, 32); err != nil {
 			return ""
 		}
-		return yaml.IntTag
+		return yaml.NodeTagInt
 	}
 	return ""
 }

kyaml/setters2/set.go

@@ -142,7 +142,7 @@ func (s *Set) substitute(field *yaml.RNode, ext *CliExtension) (bool, error) {
 	field.YNode().Value = res
 
 	// substitutions are always strings
-	field.YNode().Tag = yaml.StringTag
+	field.YNode().Tag = yaml.NodeTagString
 
 	return true, nil
 }
@@ -379,7 +379,7 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 	// values are always represented as strings the OpenAPI
 	// since the are unmarshalled into strings.  Use double quote style to
 	// ensure this consistently.
-	v.YNode().Tag = yaml.StringTag
+	v.YNode().Tag = yaml.NodeTagString
 	v.YNode().Style = yaml.DoubleQuotedStyle
 
 	if t != "array" {
@@ -395,7 +395,7 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 		// create the list values
 		var elements []*yaml.Node
 		n := yaml.NewScalarRNode(s.Value).YNode()
-		n.Tag = yaml.StringTag
+		n.Tag = yaml.NodeTagString
 		n.Style = yaml.DoubleQuotedStyle
 		elements = append(elements, n)
 		for i := range s.ListValues {

kyaml/yaml/compatibility.go

@@ -14,10 +14,10 @@ import (
 
 // typeToTag maps OpenAPI schema types to yaml 1.2 tags
 var typeToTag = map[string]string{
-	"string":  StringTag,
-	"integer": IntTag,
-	"boolean": BoolTag,
-	"number":  "!!float",
+	"string":  NodeTagString,
+	"integer": NodeTagInt,
+	"boolean": NodeTagBool,
+	"number":  NodeTagFloat,
 }
 
 // FormatNonStringStyle makes sure that values which parse as non-string values in yaml 1.1

kyaml/yaml/compatibility_test.go

@@ -114,7 +114,7 @@ var valueToTagMap = func() map[string]string {
 	// https://yaml.org/type/null.html
 	values := []string{"~", "null", "Null", "NULL"}
 	for i := range values {
-		val[values[i]] = "!!null"
+		val[values[i]] = yaml.NodeTagNull
 	}
 
 	// https://yaml.org/type/bool.html
@@ -122,7 +122,7 @@ var valueToTagMap = func() map[string]string {
 		"y", "Y", "yes", "Yes", "YES", "true", "True", "TRUE", "on", "On", "ON", "n", "N", "no",
 		"No", "NO", "false", "False", "FALSE", "off", "Off", "OFF"}
 	for i := range values {
-		val[values[i]] = "!!bool"
+		val[values[i]] = yaml.NodeTagBool
 	}
 
 	// https://yaml.org/type/float.html
@@ -130,7 +130,7 @@ var valueToTagMap = func() map[string]string {
 		".nan", ".NaN", ".NAN", ".inf", ".Inf", ".INF",
 		"+.inf", "+.Inf", "+.INF", "-.inf", "-.Inf", "-.INF"}
 	for i := range values {
-		val[values[i]] = "!!float"
+		val[values[i]] = yaml.NodeTagFloat
 	}
 
 	return val

kyaml/yaml/kfns.go

@@ -51,7 +51,7 @@ type AnnotationSetter struct {
 func (s AnnotationSetter) Filter(rn *RNode) (*RNode, error) {
 	// some tools get confused about the type if annotations are not quoted
 	v := NewScalarRNode(s.Value)
-	v.YNode().Tag = StringTag
+	v.YNode().Tag = NodeTagString
 	v.YNode().Style = yaml.SingleQuotedStyle
 
 	if err := ClearEmptyAnnotations(rn); err != nil {
@@ -103,7 +103,7 @@ type LabelSetter struct {
 func (s LabelSetter) Filter(rn *RNode) (*RNode, error) {
 	// some tools get confused about the type if labels are not quoted
 	v := NewScalarRNode(s.Value)
-	v.YNode().Tag = StringTag
+	v.YNode().Tag = NodeTagString
 	v.YNode().Style = yaml.SingleQuotedStyle
 	return rn.Pipe(
 		PathGetter{Path: []string{"metadata", "labels"}, Create: yaml.MappingNode},

kyaml/yaml/types.go

@@ -16,9 +16,21 @@ import (
 )
 
 const (
-	// NullNodeTag is the tag set for a yaml.Document that contains no data -- e.g. it isn't a
-	// Map, Slice, Document, etc
-	NullNodeTag = "!!null"
+	// NodeTagNull is the tag set for a yaml.Document that contains no data;
+	// e.g. it isn't a Map, Slice, Document, etc
+	NodeTagNull   = "!!null"
+	NodeTagFloat  = "!!float"
+	NodeTagString = "!!str"
+	NodeTagBool   = "!!bool"
+	NodeTagInt    = "!!int"
+	NodeTagMap    = "!!map"
+	NodeTagEmpty  = ""
+
+	// TODO: deprecate these
+	NullNodeTag = NodeTagNull
+	StringTag   = NodeTagString
+	BoolTag     = NodeTagBool
+	IntTag      = NodeTagInt
 )
 
 // NullNode returns a RNode point represents a null; value
@@ -604,12 +616,6 @@ func (rn *RNode) VisitFields(fn func(node *MapNode) error) error {
 	return nil
 }
 
-const (
-	StringTag = "!!str"
-	BoolTag   = "!!bool"
-	IntTag    = "!!int"
-)
-
 // Elements returns the list of elements in the RNode.
 // Returns an error for non-SequenceNodes.
 func (rn *RNode) Elements() ([]*RNode, error) {