Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 24 commits into from Mar 23, 2022
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a23d598
WIP
RafaeLeal Jan 10, 2022
b2cdf1d
Fix merge corner cases
RafaeLeal Jan 11, 2022
3b5403a
Merge remote-tracking branch 'origin/master' into fix-merge-tag
RafaeLeal Jan 11, 2022
fa352cb
Add test for explicit !!merge tag
RafaeLeal Jan 11, 2022
7895a86
Fix tests
RafaeLeal Jan 11, 2022
2c80158
Cleanup
RafaeLeal Jan 11, 2022
5d6137a
Cleanup
RafaeLeal Jan 11, 2022
5c24ab3
Fix deanchoring lists
RafaeLeal Jan 11, 2022
20363a2
Add test case for keeping comments
RafaeLeal Jan 11, 2022
02cf540
Add MapEntrySetter and fix json marshalling after deanchoring
RafaeLeal Jan 20, 2022
e570dae
Keep duplicated keys
RafaeLeal Jan 20, 2022
e91608b
Merge remote-tracking branch 'origin/master' into fix-merge-tag
RafaeLeal Feb 8, 2022
a5b526f
Move MergeTag definition to yaml alias
RafaeLeal Feb 8, 2022
a5720df
Remove go-spew from api
RafaeLeal Feb 8, 2022
e7b8c35
Add support for sequence nodes on merge tags
RafaeLeal Feb 8, 2022
3d55196
Add docstring to MapEntrySetter.Key
RafaeLeal Feb 8, 2022
a5750a6
Add docstring to MapEntrySetter struct
RafaeLeal Feb 24, 2022
60a4e82
Add tests to MapEntrySetter
RafaeLeal Feb 25, 2022
a9c4541
Fix duplicate merge key
RafaeLeal Mar 3, 2022
66f89d4
Revert whitespace changes on forked go-yaml
RafaeLeal Mar 3, 2022
44cd517
Remove AssocMapEntry function
RafaeLeal Mar 3, 2022
714a233
Refactoring merge order
RafaeLeal Mar 3, 2022
f760419
Return errors on VisitFields and PipeE
RafaeLeal Mar 23, 2022
78ae5c1
Add tests for each non-conforming map merges
RafaeLeal Mar 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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"`
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved

// Value is the value to set.
Value *RNode `yaml:"value,omitempty"`

// Key is the map key to set.
Key *RNode `yaml:"key,omitempty"`
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
}

func (s MapEntrySetter) Filter(rn *RNode) (*RNode, error) {
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
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