diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index acf0cf73af9..d81c76cdd85 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -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 diff --git a/api/resource/factory_test.go b/api/resource/factory_test.go index ed186a8b2b1..45c02ebc2c0 100644 --- a/api/resource/factory_test.go +++ b/api/resource/factory_test.go @@ -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}, diff --git a/kyaml/kio/byteio_reader_test.go b/kyaml/kio/byteio_reader_test.go index 79734ce765b..9bd2b58b07c 100644 --- a/kyaml/kio/byteio_reader_test.go +++ b/kyaml/kio/byteio_reader_test.go @@ -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{` diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index 1981718755f..48a02544450 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -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" +) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 22a9c14d9d0..1c8ca386fbf 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -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"` diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index b6e0c0cf897..ae6f00747cd 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -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(` diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index 156e7eead7e..84b16c71dfb 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -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() diff --git a/kyaml/yaml/rnode_test.go b/kyaml/yaml/rnode_test.go index 4ba1e87a4e9..0c154df15f2 100644 --- a/kyaml/yaml/rnode_test.go +++ b/kyaml/yaml/rnode_test.go @@ -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: + ! "<<" : *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