kubernetes-sigs/kustomize
1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/kustomize.git synced 2026-06-13 01:50:55 +00:00
Code Issues Packages Projects Releases 1 Wiki Activity

Fix error during expansion of !!merge <<: anchor tags (#4383)

Browse Source 
* WIP

* Fix merge corner cases

* Add test for explicit !!merge tag

* Fix tests

* Cleanup

* Cleanup

* Fix deanchoring lists

* Add test case for keeping comments

* Add MapEntrySetter and fix json marshalling after deanchoring

* Keep duplicated keys

* Move MergeTag definition to yaml alias

* Remove go-spew from api

* Add support for sequence nodes on merge tags

* Add docstring to MapEntrySetter.Key

* Add docstring to MapEntrySetter struct

* Add tests to MapEntrySetter

* Fix duplicate merge key

* Revert whitespace changes on forked go-yaml

* Remove AssocMapEntry function

* Refactoring merge order

* Return errors on VisitFields and PipeE

* Add tests for each non-conforming map merges
This commit is contained in:
Rafael Leal
2022-03-23 13:36:17 -03:00
committed by GitHub
parent 3490fb8716
commit 97de780feb
8 changed files with 787 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
api/resmap/reswrangler_test.go
View File

@@ -958,6 +958,53 @@ metadata:
`), strings.TrimSpace(string(yaml)))
}
func TestDeAnchorIntoDoc(t *testing.T) {
input := `apiVersion: apps/v1
kind: Deployment
metadata:
name: probes-test
spec:
template:
spec:
readinessProbe: &probe
periodSeconds: 5
timeoutSeconds: 3
failureThreshold: 3
httpGet:
port: http
path: /health
livenessProbe:
<<: *probe
`
rm, err := rmF.NewResMapFromBytes([]byte(input))
assert.NoError(t, err)
assert.NoError(t, rm.DeAnchor())
yaml, err := rm.AsYaml()
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(`apiVersion: apps/v1
kind: Deployment
metadata:
name: probes-test
spec:
template:
spec:
livenessProbe:
failureThreshold: 3
httpGet:
path: /health
port: http
periodSeconds: 5
timeoutSeconds: 3
readinessProbe:
failureThreshold: 3
httpGet:
path: /health
port: http
periodSeconds: 5
timeoutSeconds: 3
`), strings.TrimSpace(string(yaml)))
}
// Anchor references don't cross YAML document boundaries.
func TestDeAnchorMultiDoc(t *testing.T) {
input := `apiVersion: v1

8
api/resource/factory_test.go
View File

@@ -335,13 +335,7 @@ kind: List
"listWithAnchorReference": {
input: []types.PatchStrategicMerge{patchList2},
expectedOut: []*Resource{testDeploymentA, testDeploymentB},
// The error using kyaml is:
// json: unsupported type: map[interface {}]interface {}
// maybe arising from too many conversions between
// yaml, json, Resource, RNode, etc.
// These conversions go away after closing #3506
// TODO(#3271) This shouldn't have an error, but does when kyaml is used.
expectedErr: true,
expectedErr: false,
},
"listWithNoEntries": {
input: []types.PatchStrategicMerge{patchList3},

56
kyaml/kio/byteio_reader_test.go
View File

@@ -743,7 +743,7 @@ items:
`},
},
},
"listWithAnchors": {
"mergeAnchors": {
input: []byte(`
apiVersion: v1
kind: DeploymentList
@@ -764,7 +764,59 @@ items:
metadata:
name: deployment-b
spec:
*hostAliases
<<: *hostAliases
`),
exp: expected{
sOut: []string{`
apiVersion: v1
kind: DeploymentList
items:
- apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment-a
spec:
template:
spec:
hostAliases:
- hostnames:
- a.example.com
ip: 8.8.8.8
- apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment-b
spec:
template:
spec:
hostAliases:
- hostnames:
- a.example.com
ip: 8.8.8.8
`},
},
},
"listWithAnchors": {
input: []byte(`
apiVersion: v1
kind: DeploymentList
items:
- apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment-a
spec: &hostAliases
template:
spec:
hostAliases:
- hostnames:
- a.example.com
ip: 8.8.8.8
- apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment-b
spec: *hostAliases
`),
exp: expected{
sOut: []string{`

4
kyaml/yaml/alias.go
View File

@@ -93,3 +93,7 @@ var FoldedStyle yaml.Style = yaml.FoldedStyle
var LiteralStyle yaml.Style = yaml.LiteralStyle
var SingleQuotedStyle yaml.Style = yaml.SingleQuotedStyle
var TaggedStyle yaml.Style = yaml.TaggedStyle
const (
MergeTag = "!!merge"
)

44
kyaml/yaml/fns.go
View File

@@ -612,6 +612,50 @@ func Set(value *RNode) FieldSetter {
return FieldSetter{Value: value}
}
// MapEntrySetter sets a map entry to a value. If it finds a key with the same
// value, it will override both Key and Value RNodes, including style and any
// other metadata. If it doesn't find the key, it will insert a new map entry.
// It will set the field, even if it's empty or nil, unlike the FieldSetter.
// This is useful for rebuilding some pre-existing RNode structure.
type MapEntrySetter struct {
// Name is the name of the field or key to lookup in a MappingNode.
// If Name is unspecified, it will use the Key's Value
Name string `yaml:"name,omitempty"`
// Value is the value to set.
Value *RNode `yaml:"value,omitempty"`
// Key is the map key to set.
Key *RNode `yaml:"key,omitempty"`
}
func (s MapEntrySetter) Filter(rn *RNode) (*RNode, error) {
if rn == nil {
return nil, errors.Errorf("Can't set map entry on a nil RNode")
}
if err := ErrorIfInvalid(rn, yaml.MappingNode); err != nil {
return nil, err
}
if s.Name == "" {
s.Name = GetValue(s.Key)
}
for i := 0; i < len(rn.Content()); i = IncrementFieldIndex(i) {
isMatchingField := rn.Content()[i].Value == s.Name
if isMatchingField {
rn.Content()[i] = s.Key.YNode()
rn.Content()[i+1] = s.Value.YNode()
return rn, nil
}
}
// create the field
rn.YNode().Content = append(
rn.YNode().Content,
s.Key.YNode(),
s.Value.YNode())
return rn, nil
}
// FieldSetter sets a field or map entry to a value.
type FieldSetter struct {
Kind string `yaml:"kind,omitempty"`

78
kyaml/yaml/fns_test.go
View File

@@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/internal/forked/github.com/go-yaml/yaml"
. "sigs.k8s.io/kustomize/kyaml/yaml"
)
@@ -791,6 +792,83 @@ spec:
}
}
func TestMapEntrySetter(t *testing.T) {
withStyle := func(rn *RNode, style yaml.Style) *RNode {
rn.YNode().Style = style
return rn
}
testCases := []struct {
desc string
input string
setter MapEntrySetter
expected string
expectedErr error
}{
{
desc: "it should override Key's value map entry",
input: "foo: baz\n",
setter: MapEntrySetter{
Key: NewScalarRNode("foo"),
Value: NewScalarRNode("bar"),
},
expected: "foo: bar\n",
},
{
desc: "it should override Name's map entry",
input: "foo: baz\n",
setter: MapEntrySetter{
Name: "foo",
Key: NewScalarRNode("bar"),
Value: NewScalarRNode("baz"),
},
expected: "bar: baz\n",
},
{
desc: "it should insert new map entry",
input: "foo: baz\n",
setter: MapEntrySetter{
Key: NewScalarRNode("bar"),
Value: NewScalarRNode("42"),
},
expected: "foo: baz\nbar: 42\n",
},
{
desc: "it should override the style",
input: "foo: baz\n",
setter: MapEntrySetter{
Key: withStyle(NewScalarRNode("foo"), yaml.DoubleQuotedStyle),
Value: withStyle(NewScalarRNode("bar"), yaml.SingleQuotedStyle),
},
expected: `"foo": 'bar'` + "\n",
},
{
desc: "it should return error on sequence nodes",
input: "- foo: baz\n",
setter: MapEntrySetter{
Key: NewScalarRNode("foo"),
Value: NewScalarRNode("bar"),
},
expectedErr: errors.Errorf("wrong Node Kind for expected: MappingNode was SequenceNode: value: {- foo: baz}"),
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
node, err := Parse(tc.input)
assert.NoError(t, err)
k, err := tc.setter.Filter(node)
if tc.expectedErr == nil {
assert.NoError(t, err)
assert.Equal(t, tc.expected, assertNoErrorString(t)(node.String()))
assert.Equal(t, tc.expected, assertNoErrorString(t)(k.String()))
} else {
assert.NotNil(t, err)
assert.Equal(t, tc.expectedErr.Error(), err.Error())
}
})
}
}
func TestFieldSetter(t *testing.T) {
// Change field
node, err := Parse(`

109
kyaml/yaml/rnode.go
View File

@@ -932,7 +932,17 @@ func deAnchor(yn *yaml.Node) (res *yaml.Node, err error) {
return yn, nil
case yaml.AliasNode:
return deAnchor(yn.Alias)
case yaml.DocumentNode, yaml.MappingNode, yaml.SequenceNode:
case yaml.MappingNode:
toMerge, err := removeMergeTags(yn)
if err != nil {
return nil, err
}
err = mergeAll(yn, toMerge)
if err != nil {
return nil, err
}
fallthrough
case yaml.DocumentNode, yaml.SequenceNode:
for i := range yn.Content {
yn.Content[i], err = deAnchor(yn.Content[i])
if err != nil {
@@ -945,6 +955,103 @@ func deAnchor(yn *yaml.Node) (res *yaml.Node, err error) {
}
}
// isMerge returns if the node is tagged with !!merge
func isMerge(yn *yaml.Node) bool {
return yn.Tag == MergeTag
}
// findMergeValues receives either a MappingNode, a AliasNode or a potentially
// mixed list of MappingNodes and AliasNodes. It returns a list of MappingNodes.
func findMergeValues(yn *yaml.Node) ([]*yaml.Node, error) {
if yn == nil {
return []*yaml.Node{}, nil
}
switch yn.Kind {
case MappingNode:
return []*yaml.Node{yn}, nil
case AliasNode:
if yn.Alias != nil && yn.Alias.Kind != MappingNode {
return nil, errors.Errorf("invalid map merge: received alias for a non-map value")
}
return []*yaml.Node{yn.Alias}, nil
case SequenceNode:
mergeValues := []*yaml.Node{}
for i := 0; i < len(yn.Content); i++ {
if yn.Content[i].Kind == SequenceNode {
return nil, errors.Errorf("invalid map merge: received a nested sequence")
}
newMergeValues, err := findMergeValues(yn.Content[i])
if err != nil {
return nil, err
}
mergeValues = append(newMergeValues, mergeValues...)
}
return mergeValues, nil
default:
return nil, errors.Errorf("map merge requires map or sequence of maps as the value")
}
}
// getMergeTagValue receives a MappingNode yaml node, and it searches for
// merge tagged keys and return its value yaml node. If the key is duplicated,
// it fails.
func getMergeTagValue(yn *yaml.Node) (*yaml.Node, error) {
var result *yaml.Node
for i := 0; i < len(yn.Content); i += 2 {
key := yn.Content[i]
value := yn.Content[i+1]
if isMerge(key) {
if result != nil {
return nil, fmt.Errorf("duplicate merge key")
}
result = value
}
}
return result, nil
}
// removeMergeTags removes all merge tags and returns a ordered list of yaml
// nodes to merge and a error
func removeMergeTags(yn *yaml.Node) ([]*yaml.Node, error) {
if yn == nil || yn.Content == nil {
return nil, nil
}
if yn.Kind != yaml.MappingNode {
return nil, nil
}
value, err := getMergeTagValue(yn)
if err != nil {
return nil, err
}
toMerge, err := findMergeValues(value)
if err != nil {
return nil, err
}
err = NewRNode(yn).PipeE(Clear("<<"))
if err != nil {
return nil, err
}
return toMerge, nil
}
func mergeAll(yn *yaml.Node, toMerge []*yaml.Node) error {
// We only need to start with a copy of the existing node because we need to
// maintain duplicated keys and style
rn := NewRNode(yn).Copy()
toMerge = append(toMerge, yn)
for i := range toMerge {
rnToMerge := NewRNode(toMerge[i]).Copy()
err := rnToMerge.VisitFields(func(node *MapNode) error {
return rn.PipeE(MapEntrySetter{Key: node.Key, Value: node.Value})
})
if err != nil {
return err
}
}
*yn = *rn.value
return nil
}
// GetValidatedMetadata returns metadata after subjecting it to some tests.
func (rn *RNode) GetValidatedMetadata() (ResourceMeta, error) {
m, err := rn.GetMeta()

451
kyaml/yaml/rnode_test.go
View File

@@ -721,6 +721,457 @@ data:
`), strings.TrimSpace(actual))
}
func TestDeAnchorMerge(t *testing.T) {
testCases := []struct {
description string
input string
expected string
expectedErr error
}{
// *********
// Test Case
// *********
{
description: "simplest merge tag",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color-used
foo: bar
primaryColor:
<<: *color-used
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
foo: bar
primaryColor:
foo: bar
`,
},
// *********
// Test Case
// *********
{
description: "keep duplicated keys",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: "#FF0000"
color: "#FF00FF"
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: "#FF0000"
color: "#FF00FF"
`,
},
// *********
// Test Case
// *********
{
description: "keep json",
input: `{"apiVersion": "v1", "kind": "MergeTagTest", "spec": {"color": {"rgb": "#FF0000"}}}`,
expected: `{"apiVersion": "v1", "kind": "MergeTagTest", "spec": {"color": {"rgb": "#FF0000"}}}`,
},
// *********
// Test Case
// *********
{
description: "keep comments",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color-used
foo: bar
primaryColor:
# use same color because is pretty
rgb: "#FF0000"
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
foo: bar
primaryColor:
# use same color because is pretty
rgb: "#FF0000"
`,
},
// *********
// Test Case
// *********
{
description: "works with explicit merge tag",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color-used
foo: bar
primaryColor:
!!merge <<: *color-used
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
foo: bar
primaryColor:
foo: bar
`,
},
// *********
// Test Case
// *********
{
description: "works with explicit long merge tag",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color-used
foo: bar
primaryColor:
!<tag:yaml.org,2002:merge> "<<" : *color-used
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
foo: bar
primaryColor:
foo: bar
`,
},
// *********
// Test Case
// *********
{
description: "merging properties",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color-used
rgb: "#FF0000"
primaryColor:
<<: *color-used
pretty: true
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
rgb: "#FF0000"
primaryColor:
pretty: true
rgb: "#FF0000"
`,
},
// *********
// Test Case
// *********
{
description: "overriding value",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color-used
rgb: "#FF0000"
pretty: false
primaryColor:
<<: *color-used
pretty: true
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
rgb: "#FF0000"
pretty: false
primaryColor:
pretty: true
rgb: "#FF0000"
`,
},
// *********
// Test Case
// *********
{
description: "returns error when defining multiple merge keys",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color
rgb: "#FF0000"
pretty: false
primaryColor: &primary
rgb: "#0000FF"
alpha: 50
secondaryColor:
<<: *color
<<: *primary
secondary: true
`,
expectedErr: fmt.Errorf("duplicate merge key"),
},
// *********
// Test Case
// *********
{
description: "merging multiple anchors with sequence node",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color
rgb: "#FF0000"
pretty: false
primaryColor: &primary
rgb: "#0000FF"
alpha: 50
secondaryColor:
<<: [ *color, *primary ]
secondary: true
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
rgb: "#FF0000"
pretty: false
primaryColor:
rgb: "#0000FF"
alpha: 50
secondaryColor:
secondary: true
rgb: "#FF0000"
alpha: 50
pretty: false
`,
},
// *********
// Test Case
// *********
{
description: "merging inline map",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color
rgb: "#FF0000"
pretty: false
primaryColor:
<<: {"pretty": true}
rgb: "#0000FF"
alpha: 50
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
rgb: "#FF0000"
pretty: false
primaryColor:
rgb: "#0000FF"
alpha: 50
"pretty": true
`,
},
// *********
// Test Case
// *********
{
description: "merging inline sequence map",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color
rgb: "#FF0000"
pretty: false
primaryColor:
<<: [ *color, {"name": "ugly blue"}]
rgb: "#0000FF"
alpha: 50
`,
expected: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color:
rgb: "#FF0000"
pretty: false
primaryColor:
rgb: "#0000FF"
alpha: 50
"name": "ugly blue"
pretty: false
`,
},
// *********
// Test Case
// *********
{
description: "error on nested lists on merges",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color
rgb: "#FF0000"
pretty: false
primaryColor:
<<: [ *color, [{"name": "ugly blue"}]]
rgb: "#0000FF"
alpha: 50
`,
expectedErr: fmt.Errorf("invalid map merge: received a nested sequence"),
},
// *********
// Test Case
// *********
{
description: "error on non-map references on merges",
input: `
apiVersion: v1
kind: MergeTagTest
metadata:
name: test
data:
color: &color
- rgb: "#FF0000"
pretty: false
primaryColor:
<<: [ *color, [{"name": "ugly blue"}]]
rgb: "#0000FF"
alpha: 50
`,
expectedErr: fmt.Errorf("invalid map merge: received alias for a non-map value"),
},
// *********
// Test Case
// *********
{
description: "merging on a list",
input: `
apiVersion: v1
kind: MergeTagTestList
items:
- apiVersion: v1
kind: MergeTagTest
metadata:
name: test
spec: &merge-spec
something: true
- apiVersion: v1
kind: MergeTagTest
metadata:
name: test
spec:
<<: *merge-spec
`,
expected: `
apiVersion: v1
kind: MergeTagTestList
items:
- apiVersion: v1
kind: MergeTagTest
metadata:
name: test
spec:
something: true
- apiVersion: v1
kind: MergeTagTest
metadata:
name: test
spec:
something: true
`,
},
}
for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
rn, err := Parse(tc.input)
assert.NoError(t, err)
err = rn.DeAnchor()
if tc.expectedErr == nil {
assert.NoError(t, err)
actual, err := rn.String()
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(tc.expected), strings.TrimSpace(actual))
} else {
assert.NotNil(t, err)
assert.Equal(t, tc.expectedErr.Error(), err.Error())
}
})
}
}
func TestRNode_UnmarshalJSON(t *testing.T) {
testCases := []struct {
testName string