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 11 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
1 change: 1 addition & 0 deletions api/go.mod
Expand Up @@ -3,6 +3,7 @@ module sigs.k8s.io/kustomize/api
go 1.16

require (
github.com/davecgh/go-spew v1.1.1 // indirect
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
github.com/evanphx/json-patch v4.11.0+incompatible
github.com/go-errors/errors v1.0.1
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
Expand Down
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
9 changes: 6 additions & 3 deletions kyaml/internal/forked/github.com/go-yaml/yaml/yaml.go
Expand Up @@ -340,6 +340,10 @@ const (
AliasNode
)

const (
MergeTag = "!!merge"
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
)

type Style uint32

const (
Expand Down Expand Up @@ -373,7 +377,7 @@ const (
// Address yaml.Node
// }
// err := yaml.Unmarshal(data, &person)
//
//
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
// Or by itself:
//
// var person Node
Expand All @@ -383,7 +387,7 @@ type Node struct {
// Kind defines whether the node is a document, a mapping, a sequence,
// a scalar value, or an alias to another node. The specific data type of
// scalar nodes may be obtained via the ShortTag and LongTag methods.
Kind Kind
Kind Kind

// Style allows customizing the apperance of the node in the tree.
Style Style
Expand Down Expand Up @@ -431,7 +435,6 @@ func (n *Node) IsZero() bool {
n.HeadComment == "" && n.LineComment == "" && n.FootComment == "" && n.Line == 0 && n.Column == 0
}


// LongTag returns the long form of the tag that indicates the data type for
// the node. If the Tag field isn't explicitly defined, one will be computed
// based on the node properties.
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
33 changes: 33 additions & 0 deletions kyaml/yaml/fns.go
Expand Up @@ -612,6 +612,39 @@ func Set(value *RNode) FieldSetter {
return FieldSetter{Value: value}
}

// MapEntrySetter sets a field or map entry to a value.
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
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 *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 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
54 changes: 53 additions & 1 deletion kyaml/yaml/rnode.go
Expand Up @@ -565,6 +565,10 @@ func (rn *RNode) SetMapField(value *RNode, path ...string) error {
)
}

func (rn *RNode) AssocMapEntry(key, value *RNode) error {
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
return rn.PipeE(MapEntrySetter{Key: key, Value: value})
}

func (rn *RNode) GetDataMap() map[string]string {
n, err := rn.Pipe(Lookup(DataField))
if err != nil {
Expand Down Expand Up @@ -932,7 +936,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:
KnVerey marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand All @@ -945,6 +959,44 @@ func deAnchor(yn *yaml.Node) (res *yaml.Node, err error) {
}
}

// 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
}
newContent := make([]*yaml.Node, 0)
toMerge := make([]*yaml.Node, 0)
for i := 0; i < len(yn.Content); i += 2 {
if yn.Content[i].Tag == "!!merge" {
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
toMerge = append(toMerge, yn.Content[i+1].Alias)
RafaeLeal marked this conversation as resolved.
Show resolved Hide resolved
} else {
newContent = append(newContent, yn.Content[i], yn.Content[i+1])
}
}
yn.Content = newContent
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()
rnToMerge.VisitFields(func(node *MapNode) error {
rn.AssocMapEntry(node.Key, node.Value)
return nil
})
}
*yn = *rn.value
return nil
}

// GetValidatedMetadata returns metadata after subjecting it to some tests.
func (rn *RNode) GetValidatedMetadata() (ResourceMeta, error) {
m, err := rn.GetMeta()
Expand Down