Skip to content

Commit

Permalink
Fix error during expansion of !!merge <<: anchor tags (#4383)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
RafaeLeal committed Mar 23, 2022
1 parent 3490fb8 commit 97de780
Show file tree
Hide file tree
Showing 8 changed files with 787 additions and 10 deletions.
47 changes: 47 additions & 0 deletions api/resmap/reswrangler_test.go
Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions api/resource/factory_test.go
Expand Up @@ -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},
Expand Down
56 changes: 54 additions & 2 deletions kyaml/kio/byteio_reader_test.go
Expand Up @@ -743,7 +743,7 @@ items:
`},
},
},
"listWithAnchors": {
"mergeAnchors": {
input: []byte(`
apiVersion: v1
kind: DeploymentList
Expand All @@ -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{`
Expand Down
4 changes: 4 additions & 0 deletions kyaml/yaml/alias.go
Expand Up @@ -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 changes: 44 additions & 0 deletions kyaml/yaml/fns.go
Expand Up @@ -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"`
Expand Down
78 changes: 78 additions & 0 deletions kyaml/yaml/fns_test.go
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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(`
Expand Down

0 comments on commit 97de780

Please sign in to comment.