Skip to content

Commit

Permalink
Tweak sequence treatment in fieldspecs
Browse files Browse the repository at this point in the history
  • Loading branch information
monopole committed Jul 22, 2020
2 parents 03d6229 + 27b2c7f commit 57f9f60
Show file tree
Hide file tree
Showing 12 changed files with 162 additions and 51 deletions.
2 changes: 1 addition & 1 deletion api/filters/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
19 changes: 11 additions & 8 deletions api/filters/fieldspec/fieldspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
}
Expand Down
104 changes: 103 additions & 1 deletion api/filters/fieldspec/fieldspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
},
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion api/filters/labels/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion api/filters/replicacount/replicacount.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (rc Filter) run(node *yaml.RNode) (*yaml.RNode, error) {
FsSlice: rc.FsSlice,
SetValue: rc.set,
CreateKind: yaml.ScalarNode, // replicas is a ScalarNode
CreateTag: yaml.IntTag,
CreateTag: yaml.NodeTagInt,
})
return node, err
}
Expand Down
24 changes: 12 additions & 12 deletions api/krusty/component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ resources:
configMapGenerator:
- name: my-configmap
literals:
- testValue=1
- otherValue=10
- testValue=1
- otherValue=10
`)
th.WriteF("/app/base/deploy.yaml", `
apiVersion: v1
Expand All @@ -59,13 +59,13 @@ replicas:
- name: storefront
count: 3
resources:
- stub.yaml
- stub.yaml
configMapGenerator:
- name: my-configmap
behavior: merge
literals:
- testValue=2
- compValue=5
- testValue=2
- compValue=5
`)
th.WriteF("/app/comp/stub.yaml", `
apiVersion: v1
Expand Down Expand Up @@ -156,7 +156,7 @@ configMapGenerator:
- name: my-configmap
behavior: merge
literals:
- otherValue=9
- otherValue=9
`),
writeK("/app/prod", `
resources:
Expand Down Expand Up @@ -211,8 +211,8 @@ components:
configMapGenerator:
- name: my-configmap
behavior: merge
literals:
- otherValue=9
literals:
- otherValue=9
`),
writeK("/app/prod", `
resources:
Expand Down Expand Up @@ -327,8 +327,8 @@ configMapGenerator:
- name: my-configmap
behavior: merge
literals:
- compValue=5
- testValue=2
- compValue=5
- testValue=2
`),
},
runPath: "/app/direct-component",
Expand Down Expand Up @@ -360,7 +360,7 @@ configMapGenerator:
- name: my-configmap
behavior: merge
literals:
- otherValue=9
- otherValue=9
`),
},
runPath: "/app/prod",
Expand Down Expand Up @@ -574,7 +574,7 @@ configMapGenerator:
- name: my-configmap
behavior: merge
literals:
- otherValue=9
- otherValue=9
`),
},
runPath: "/app/prod",
Expand Down
12 changes: 6 additions & 6 deletions kyaml/fieldmeta/fieldmeta.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,29 +210,29 @@ 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 ""
}

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 ""
}
Expand Down
6 changes: 3 additions & 3 deletions kyaml/setters2/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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" {
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions kyaml/yaml/compatibility.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions kyaml/yaml/compatibility_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,23 +114,23 @@ 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
values = []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
values = []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
Expand Down
Loading

0 comments on commit 57f9f60

Please sign in to comment.