Merge pull request #2245 from pwittrock/merge3

Use OpenAPI when 3-way merging resources
This commit is contained in:
Kubernetes Prow Robot
2020-03-03 12:51:46 -08:00
committed by GitHub
24 changed files with 991 additions and 507 deletions

View File

@@ -5,6 +5,7 @@ package fieldmeta
import (
"encoding/json"
"reflect"
"strconv"
"strings"
@@ -36,16 +37,29 @@ type PartialFieldSetter struct {
Value string `yaml:"value" json:"value"`
}
// IsEmpty returns true if the FieldMeta has any empty Schema
func (fm *FieldMeta) IsEmpty() bool {
if fm == nil {
return true
}
return reflect.DeepEqual(fm.Schema, spec.Schema{})
}
// Read reads the FieldMeta from a node
func (fm *FieldMeta) Read(n *yaml.RNode) error {
if n.YNode().LineComment != "" {
v := strings.TrimLeft(n.YNode().LineComment, "#")
// check for metadata on head and line comments
comments := []string{n.YNode().LineComment, n.YNode().HeadComment}
for _, c := range comments {
if c == "" {
continue
}
c := strings.TrimLeft(c, "#")
// if it doesn't Unmarshal that is fine, it means there is no metadata
// other comments are valid, they just don't parse
// TODO: consider most sophisticated parsing techniques similar to what is used
// TODO: consider more sophisticated parsing techniques similar to what is used
// for go struct tags.
if err := fm.Schema.UnmarshalJSON([]byte(v)); err != nil {
if err := fm.Schema.UnmarshalJSON([]byte(c)); err != nil {
// note: don't return an error if the comment isn't a fieldmeta struct
return nil
}

View File

@@ -7,6 +7,7 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"reflect"
"sync"
"github.com/go-openapi/spec"
@@ -32,6 +33,14 @@ type ResourceSchema struct {
Schema *spec.Schema
}
// IsEmpty returns true if the ResourceSchema is empty
func (rs *ResourceSchema) IsEmpty() bool {
if rs == nil || rs.Schema == nil {
return true
}
return reflect.DeepEqual(*rs.Schema, spec.Schema{})
}
// SchemaForResourceType returns the Schema for the given Resource
// TODO(pwittrock): create a version of this function that will return a schema
// which can be used for duck-typed Resources -- e.g. contains common fields such
@@ -193,15 +202,15 @@ func SuppressBuiltInSchemaUse() {
}
// Elements returns the Schema for the elements of an array.
func (r *ResourceSchema) Elements() *ResourceSchema {
func (rs *ResourceSchema) Elements() *ResourceSchema {
// load the schema from swagger.json
initSchema()
if len(r.Schema.Type) != 1 || r.Schema.Type[0] != "array" {
if len(rs.Schema.Type) != 1 || rs.Schema.Type[0] != "array" {
// either not an array, or array has multiple types
return nil
}
s := *r.Schema.Items.Schema
s := *rs.Schema.Items.Schema
for s.Ref.String() != "" {
sc, e := Resolve(&s.Ref)
if e != nil {
@@ -219,8 +228,8 @@ const Elements = "[]"
// Field is called.
// If any Field or Elements call returns nil, then Lookup returns
// nil immediately.
func (r *ResourceSchema) Lookup(path ...string) *ResourceSchema {
s := r
func (rs *ResourceSchema) Lookup(path ...string) *ResourceSchema {
s := rs
for _, p := range path {
if s == nil {
break
@@ -235,19 +244,19 @@ func (r *ResourceSchema) Lookup(path ...string) *ResourceSchema {
}
// Field returns the Schema for a field.
func (r *ResourceSchema) Field(field string) *ResourceSchema {
func (rs *ResourceSchema) Field(field string) *ResourceSchema {
// load the schema from swagger.json
initSchema()
// locate the Schema
s, found := r.Schema.Properties[field]
s, found := rs.Schema.Properties[field]
switch {
case found:
// no-op, continue with s as the schema
case r.Schema.AdditionalProperties != nil && r.Schema.AdditionalProperties.Schema != nil:
case rs.Schema.AdditionalProperties != nil && rs.Schema.AdditionalProperties.Schema != nil:
// map field type -- use Schema of the value
// (the key doesn't matter, they all have the same value type)
s = *r.Schema.AdditionalProperties.Schema
s = *rs.Schema.AdditionalProperties.Schema
default:
// no Schema found from either swagger.json or line comments
return nil
@@ -267,14 +276,14 @@ func (r *ResourceSchema) Field(field string) *ResourceSchema {
}
// PatchStrategyAndKey returns the patch strategy and merge key extensions
func (r *ResourceSchema) PatchStrategyAndKey() (string, string) {
ps, found := r.Schema.Extensions[kubernetesPatchStrategyExtensionKey]
func (rs *ResourceSchema) PatchStrategyAndKey() (string, string) {
ps, found := rs.Schema.Extensions[kubernetesPatchStrategyExtensionKey]
if !found {
// merge key and patch strategy must appear together
return "", ""
}
mk, found := r.Schema.Extensions[kubernetesMergeKeyExtensionKey]
mk, found := rs.Schema.Extensions[kubernetesMergeKeyExtensionKey]
if !found {
// merge key and patch strategy must appear together
return "", ""

View File

@@ -4,21 +4,21 @@
package merge2_test
var elementTestCases = []testCase{
{`merge Element -- keep field in dest`,
`
{description: `merge Element -- keep field in dest`,
source: `
kind: Deployment
items:
- name: foo
image: foo:v1
`,
`
dest: `
kind: Deployment
items:
- name: foo
image: foo:v0
command: ['run.sh']
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -27,21 +27,21 @@ items:
`,
},
{`merge Element -- add field to dest`,
`
{description: `merge Element -- add field to dest`,
source: `
kind: Deployment
items:
- name: foo
image: foo:v1
command: ['run.sh']
`,
`
dest: `
kind: Deployment
items:
- name: foo
image: foo:v0
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -50,19 +50,19 @@ items:
`,
},
{`merge Element -- add list, empty in dest`,
`
{description: `merge Element -- add list, empty in dest`,
source: `
kind: Deployment
items:
- name: foo
image: foo:v1
command: ['run.sh']
`,
`
dest: `
kind: Deployment
items: []
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -71,18 +71,18 @@ items:
`,
},
{`merge Element -- add list, missing from dest`,
`
{description: `merge Element -- add list, missing from dest`,
source: `
kind: Deployment
items:
- name: foo
image: foo:v1
command: ['run.sh']
`,
`
dest: `
kind: Deployment
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -91,8 +91,8 @@ items:
`,
},
{`merge Element -- add Element first`,
`
{description: `merge Element -- add Element first`,
source: `
kind: Deployment
items:
- name: bar
@@ -101,13 +101,13 @@ items:
- name: foo
image: foo:v1
`,
`
dest: `
kind: Deployment
items:
- name: foo
image: foo:v0
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -118,8 +118,8 @@ items:
`,
},
{`merge Element -- add Element second`,
`
{description: `merge Element -- add Element second`,
source: `
kind: Deployment
items:
- name: foo
@@ -128,13 +128,13 @@ items:
image: bar:v1
command: ['run2.sh']
`,
`
dest: `
kind: Deployment
items:
- name: foo
image: foo:v0
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -148,11 +148,11 @@ items:
//
// Test Case
//
{`keep list -- list missing from src`,
`
{description: `keep list -- list missing from src`,
source: `
kind: Deployment
`,
`
dest: `
kind: Deployment
items:
- name: foo
@@ -161,7 +161,7 @@ items:
image: bar:v1
command: ['run2.sh']
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -175,14 +175,14 @@ items:
//
// Test Case
//
{`keep Element -- element missing in src`,
`
{description: `keep Element -- element missing in src`,
source: `
kind: Deployment
items:
- name: foo
image: foo:v1
`,
`
dest: `
kind: Deployment
items:
- name: foo
@@ -191,7 +191,7 @@ items:
image: bar:v1
command: ['run2.sh']
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -205,12 +205,12 @@ items:
//
// Test Case
//
{`keep element -- empty list in src`,
`
{description: `keep element -- empty list in src`,
source: `
kind: Deployment
items: {}
`,
`
dest: `
kind: Deployment
items:
- name: foo
@@ -219,7 +219,7 @@ items:
image: bar:v1
command: ['run2.sh']
`,
`
expected: `
kind: Deployment
items:
- name: foo
@@ -233,12 +233,12 @@ items:
//
// Test Case
//
{`remove Element -- null in src`,
`
{description: `remove Element -- null in src`,
source: `
kind: Deployment
items: null
`,
`
dest: `
kind: Deployment
items:
- name: foo
@@ -247,8 +247,97 @@ items:
image: bar:v1
command: ['run2.sh']
`,
`
expected: `
kind: Deployment
`,
},
//
// Test Case
//
{description: `no infer merge keys no merge'`,
source: `
kind: Deployment
containers:
- name: foo
command: ['run2.sh']
`,
dest: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
expected: `
kind: Deployment
containers:
- name: foo
command: ['run2.sh']
`,
noInfer: true,
},
//
// Test Case
//
{description: `no infer merge keys merge using schema`,
source: `
kind: Deployment
apiVersion: apps/v1
spec:
template:
spec:
containers:
- name: foo
command: ['run2.sh']
`,
dest: `
kind: Deployment
apiVersion: apps/v1
spec:
template:
spec:
containers:
- name: foo
image: foo:bar
`,
expected: `
kind: Deployment
apiVersion: apps/v1
spec:
template:
spec:
containers:
- name: foo
image: foo:bar
command: ['run2.sh']
`,
noInfer: true,
},
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema as line comment'`,
source: `
kind: Deployment
containers:
- name: foo
command: ['run2.sh']
`,
dest: `
kind: Deployment
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-patch-merge-key":"name","x-kubernetes-patch-strategy": "merge"}
- name: foo # hell ow
image: foo:bar
`,
expected: `
kind: Deployment
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-patch-merge-key":"name","x-kubernetes-patch-strategy": "merge"}
- name: foo
image: foo:bar
command: ['run2.sh']
`,
noInfer: true,
},
}

View File

@@ -4,21 +4,21 @@
package merge2_test
var listTestCases = []testCase{
{`replace List -- different value in dest`,
`
{description: `replace List -- different value in dest`,
source: `
kind: Deployment
items:
- 1
- 2
- 3
`,
`
dest: `
kind: Deployment
items:
- 0
- 1
`,
`
expected: `
kind: Deployment
items:
- 1
@@ -27,18 +27,18 @@ items:
`,
},
{`replace List -- missing from dest`,
`
{description: `replace List -- missing from dest`,
source: `
kind: Deployment
items:
- 1
- 2
- 3
`,
`
dest: `
kind: Deployment
`,
`
expected: `
kind: Deployment
items:
- 1
@@ -50,22 +50,22 @@ items:
//
// Test Case
//
{`keep List -- same value in src and dest`,
`
{description: `keep List -- same value in src and dest`,
source: `
kind: Deployment
items:
- 1
- 2
- 3
`,
`
dest: `
kind: Deployment
items:
- 1
- 2
- 3
`,
`
expected: `
kind: Deployment
items:
- 1
@@ -77,18 +77,18 @@ items:
//
// Test Case
//
{`keep List -- unspecified in src`,
`
{description: `keep List -- unspecified in src`,
source: `
kind: Deployment
`,
`
dest: `
kind: Deployment
items:
- 1
- 2
- 3
`,
`
expected: `
kind: Deployment
items:
- 1
@@ -100,19 +100,19 @@ items:
//
// Test Case
//
{`remove List -- null in src`,
`
{description: `remove List -- null in src`,
source: `
kind: Deployment
items: null
`,
`
dest: `
kind: Deployment
items:
- 1
- 2
- 3
`,
`
expected: `
kind: Deployment
`,
},
@@ -120,19 +120,19 @@ kind: Deployment
//
// Test Case
//
{`remove list -- empty in src`,
`
{description: `remove list -- empty in src`,
source: `
kind: Deployment
items: {}
`,
`
dest: `
kind: Deployment
items:
- 1
- 2
- 3
`,
`
expected: `
kind: Deployment
items: {}
`,

View File

@@ -4,19 +4,19 @@
package merge2_test
var mapTestCases = []testCase{
{`merge Map -- update field in dest`,
`
{description: `merge Map -- update field in dest`,
source: `
kind: Deployment
spec:
foo: bar1
`,
`
dest: `
kind: Deployment
spec:
foo: bar0
baz: buz
`,
`
expected: `
kind: Deployment
spec:
foo: bar1
@@ -24,19 +24,19 @@ spec:
`,
},
{`merge Map -- add field to dest`,
`
{description: `merge Map -- add field to dest`,
source: `
kind: Deployment
spec:
foo: bar1
baz: buz
`,
`
dest: `
kind: Deployment
spec:
foo: bar0
`,
`
expected: `
kind: Deployment
spec:
foo: bar1
@@ -44,18 +44,18 @@ spec:
`,
},
{`merge Map -- add list, empty in dest`,
`
{description: `merge Map -- add list, empty in dest`,
source: `
kind: Deployment
spec:
foo: bar1
baz: buz
`,
`
dest: `
kind: Deployment
spec: {}
`,
`
expected: `
kind: Deployment
spec:
foo: bar1
@@ -63,17 +63,17 @@ spec:
`,
},
{`merge Map -- add list, missing from dest`,
`
{description: `merge Map -- add list, missing from dest`,
source: `
kind: Deployment
spec:
foo: bar1
baz: buz
`,
`
dest: `
kind: Deployment
`,
`
expected: `
kind: Deployment
spec:
foo: bar1
@@ -81,19 +81,19 @@ spec:
`,
},
{`merge Map -- add Map first`,
`
{description: `merge Map -- add Map first`,
source: `
kind: Deployment
spec:
foo: bar1
baz: buz
`,
`
dest: `
kind: Deployment
spec:
foo: bar1
`,
`
expected: `
kind: Deployment
spec:
foo: bar1
@@ -101,19 +101,19 @@ spec:
`,
},
{`merge Map -- add Map second`,
`
{description: `merge Map -- add Map second`,
source: `
kind: Deployment
spec:
baz: buz
foo: bar1
`,
`
dest: `
kind: Deployment
spec:
foo: bar1
`,
`
expected: `
kind: Deployment
spec:
foo: bar1
@@ -124,17 +124,17 @@ spec:
//
// Test Case
//
{`keep map -- map missing from src`,
`
{description: `keep map -- map missing from src`,
source: `
kind: Deployment
`,
`
dest: `
kind: Deployment
spec:
foo: bar1
baz: buz
`,
`
expected: `
kind: Deployment
spec:
foo: bar1
@@ -145,18 +145,18 @@ spec:
//
// Test Case
//
{`keep map -- empty list in src`,
`
{description: `keep map -- empty list in src`,
source: `
kind: Deployment
items: {}
`,
`
dest: `
kind: Deployment
spec:
foo: bar1
baz: buz
`,
`
expected: `
kind: Deployment
spec:
foo: bar1
@@ -168,18 +168,18 @@ items: {}
//
// Test Case
//
{`remove Map -- null in src`,
`
{description: `remove Map -- null in src`,
source: `
kind: Deployment
spec: null
`,
`
dest: `
kind: Deployment
spec:
foo: bar1
baz: buz
`,
`
expected: `
kind: Deployment
`,
},

View File

@@ -6,6 +6,7 @@
package merge2
import (
"sigs.k8s.io/kustomize/kyaml/openapi"
"sigs.k8s.io/kustomize/kyaml/yaml"
"sigs.k8s.io/kustomize/kyaml/yaml/walk"
)
@@ -16,7 +17,7 @@ func Merge(src, dest *yaml.RNode) (*yaml.RNode, error) {
}
// Merge parses the arguments, and merges fields from srcStr into destStr.
func MergeStrings(srcStr, destStr string) (string, error) {
func MergeStrings(srcStr, destStr string, infer bool) (string, error) {
src, err := yaml.Parse(srcStr)
if err != nil {
return "", err
@@ -26,10 +27,15 @@ func MergeStrings(srcStr, destStr string) (string, error) {
return "", err
}
result, err := Merge(src, dest)
result, err := walk.Walker{
Sources: []*yaml.RNode{dest, src},
Visitor: Merger{},
InferAssociativeLists: infer,
}.Walk()
if err != nil {
return "", err
}
return result.String()
}
@@ -39,7 +45,7 @@ type Merger struct {
var _ walk.Visitor = Merger{}
func (m Merger) VisitMap(nodes walk.Sources) (*yaml.RNode, error) {
func (m Merger) VisitMap(nodes walk.Sources, s *openapi.ResourceSchema) (*yaml.RNode, error) {
if err := m.SetComments(nodes); err != nil {
return nil, err
}
@@ -58,7 +64,7 @@ func (m Merger) VisitMap(nodes walk.Sources) (*yaml.RNode, error) {
return nodes.Dest(), nil
}
func (m Merger) VisitScalar(nodes walk.Sources) (*yaml.RNode, error) {
func (m Merger) VisitScalar(nodes walk.Sources, s *openapi.ResourceSchema) (*yaml.RNode, error) {
if err := m.SetComments(nodes); err != nil {
return nil, err
}
@@ -73,7 +79,7 @@ func (m Merger) VisitScalar(nodes walk.Sources) (*yaml.RNode, error) {
return nodes.Dest(), nil
}
func (m Merger) VisitList(nodes walk.Sources, kind walk.ListKind) (*yaml.RNode, error) {
func (m Merger) VisitList(nodes walk.Sources, s *openapi.ResourceSchema, kind walk.ListKind) (*yaml.RNode, error) {
if err := m.SetComments(nodes); err != nil {
return nil, err
}

View File

@@ -365,7 +365,7 @@ a:
b:
# header comment
c: d
`)
`, true)
if !assert.NoError(t, err) {
return
}
@@ -385,7 +385,7 @@ a:
b:
c: d
# footer comment
`)
`, true)
if !assert.NoError(t, err) {
return
}
@@ -404,7 +404,7 @@ a:
a:
b:
c: d # line comment
`)
`, true)
if !assert.NoError(t, err) {
return
}
@@ -426,7 +426,7 @@ a:
b:
# replace comment
c: d
`)
`, true)
if !assert.NoError(t, err) {
return
}
@@ -447,7 +447,7 @@ a:
b:
c: d
# replace comment
`)
`, true)
if !assert.NoError(t, err) {
return
}
@@ -466,7 +466,7 @@ a:
a:
b:
c: d # replace comment
`)
`, true)
if !assert.NoError(t, err) {
return
}
@@ -484,7 +484,7 @@ a:
a:
b:
c: d # replace comment
`)
`, true)
if !assert.NoError(t, err) {
return
}

View File

@@ -17,24 +17,27 @@ var testCases = [][]testCase{scalarTestCases, listTestCases, elementTestCases, m
func TestMerge(t *testing.T) {
for i := range testCases {
for _, tc := range testCases[i] {
actual, err := MergeStrings(tc.source, tc.dest)
if !assert.NoError(t, err, tc.description) {
t.FailNow()
}
e, err := filters.FormatInput(bytes.NewBufferString(tc.expected))
if !assert.NoError(t, err) {
t.FailNow()
}
estr := strings.TrimSpace(e.String())
a, err := filters.FormatInput(bytes.NewBufferString(actual))
if !assert.NoError(t, err) {
t.FailNow()
}
astr := strings.TrimSpace(a.String())
if !assert.Equal(t, estr, astr, tc.description) {
t.FailNow()
}
for j := range testCases[i] {
tc := testCases[i][j]
t.Run(tc.description, func(t *testing.T) {
actual, err := MergeStrings(tc.source, tc.dest, !tc.noInfer)
if !assert.NoError(t, err, tc.description) {
t.FailNow()
}
e, err := filters.FormatInput(bytes.NewBufferString(tc.expected))
if !assert.NoError(t, err) {
t.FailNow()
}
estr := strings.TrimSpace(e.String())
a, err := filters.FormatInput(bytes.NewBufferString(actual))
if !assert.NoError(t, err) {
t.FailNow()
}
astr := strings.TrimSpace(a.String())
if !assert.Equal(t, estr, astr, tc.description) {
t.FailNow()
}
})
}
}
}
@@ -44,4 +47,5 @@ type testCase struct {
source string
dest string
expected string
noInfer bool
}

View File

@@ -4,30 +4,30 @@
package merge2_test
var scalarTestCases = []testCase{
{`replace scalar -- different value in dest`,
`
{description: `replace scalar -- different value in dest`,
source: `
kind: Deployment
field: value1
`,
`
dest: `
kind: Deployment
field: value0
`,
`
expected: `
kind: Deployment
field: value1
`,
},
{`replace scalar -- missing from dest`,
`
{description: `replace scalar -- missing from dest`,
source: `
kind: Deployment
field: value1
`,
`
dest: `
kind: Deployment
`,
`
expected: `
kind: Deployment
field: value1
`,
@@ -36,16 +36,16 @@ field: value1
//
// Test Case
//
{`keep scalar -- same value in src and dest`,
`
{description: `keep scalar -- same value in src and dest`,
source: `
kind: Deployment
field: value1
`,
`
dest: `
kind: Deployment
field: value1
`,
`
expected: `
kind: Deployment
field: value1
`,
@@ -54,15 +54,15 @@ field: value1
//
// Test Case
//
{`keep scalar -- unspecified in src`,
`
{description: `keep scalar -- unspecified in src`,
source: `
kind: Deployment
`,
`
dest: `
kind: Deployment
field: value1
`,
`
expected: `
kind: Deployment
field: value1
`,
@@ -71,16 +71,16 @@ field: value1
//
// Test Case
//
{`remove scalar -- null in src`,
`
{description: `remove scalar -- null in src`,
source: `
kind: Deployment
field: null
`,
`
dest: `
kind: Deployment
field: value1
`,
`
expected: `
kind: Deployment
`,
},
@@ -88,16 +88,16 @@ kind: Deployment
//
// Test Case
//
{`remove scalar -- empty in src`,
`
{description: `remove scalar -- empty in src`,
source: `
kind: Deployment
field: {}
`,
`
dest: `
kind: Deployment
field: value1
`,
`
expected: `
kind: Deployment
field: {}
`,
@@ -106,15 +106,15 @@ field: {}
//
// Test Case
//
{`remove scalar -- null in src, missing in dest`,
`
{description: `remove scalar -- null in src, missing in dest`,
source: `
kind: Deployment
field: null
`,
`
dest: `
kind: Deployment
`,
`
expected: `
kind: Deployment
`,
},
@@ -122,15 +122,15 @@ kind: Deployment
//
// Test Case
//
{`merge an empty value`,
`
{description: `merge an empty value`,
source: `
kind: Deployment
field: {}
`,
`
dest: `
kind: Deployment
`,
`
expected: `
kind: Deployment
field: {}
`,

View File

@@ -7,14 +7,14 @@ var elementTestCases = []testCase{
//
// Test Case
//
{`Add an element to an existing list`,
`
{description: `Add an element to an existing list`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:1
`,
`
update: `
kind: Deployment
containers:
- name: foo
@@ -22,13 +22,13 @@ containers:
- name: baz
image: baz:2
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:1
`,
`
expected: `
kind: Deployment
containers:
- name: foo
@@ -36,65 +36,65 @@ containers:
- image: baz:2
name: baz
`, nil},
`},
//
// Test Case
//
{`Add an element to a non-existing list`,
`
{description: `Add an element to a non-existing list`,
origin: `
kind: Deployment`,
`
update: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
local: `
kind: Deployment
`,
`
expected: `
kind: Deployment
containers:
- image: foo:bar
name: foo
`, nil},
`},
{`Add an element to a non-existing list, existing in dest`,
`
{description: `Add an element to a non-existing list, existing in dest`,
origin: `
kind: Deployment`,
`
update: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
local: `
kind: Deployment
containers:
- name: baz
image: baz:bar
`,
`
expected: `
kind: Deployment
containers:
- name: baz
image: baz:bar
- image: foo:bar
name: foo
`, nil},
`},
//
// Test Case
// TODO(pwittrock): Figure out if there is something better we can do here
// This element is missing from the destination -- only the new fields are added
{`Add a field to the element, element missing from dest`,
`
{description: `Add a field to the element, element missing from dest`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar`,
`
update: `
kind: Deployment
containers:
- name: foo
@@ -102,22 +102,22 @@ containers:
command:
- run.sh
`,
`
local: `
kind: Deployment
`,
`
expected: `
kind: Deployment
containers:
- command:
- run.sh
name: foo
`, nil},
`},
//
// Test Case
//
{`Update a field on the elem, element missing from the dest`,
`
{description: `Update a field on the elem, element missing from the dest`,
origin: `
kind: Deployment
containers:
- name: foo
@@ -125,7 +125,7 @@ containers:
command:
- run.sh
`,
`
update: `
kind: Deployment
containers:
- name: foo
@@ -133,210 +133,210 @@ containers:
command:
- run2.sh
`,
`
local: `
kind: Deployment
`,
`
expected: `
kind: Deployment
containers:
- command:
- run2.sh
name: foo
`, nil},
`},
//
// Test Case
//
{`Update a field on the elem, element present in the dest`,
`
{description: `Update a field on the elem, element present in the dest`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run.sh']
`,
`
update: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run2.sh']
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run.sh']
`,
`
expected: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run2.sh']
`, nil},
`},
//
// Test Case
//
{`Add a field on the elem, element present in the dest`,
`
{description: `Add a field on the elem, element present in the dest`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
update: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run2.sh']
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
expected: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run2.sh']
`, nil},
`},
//
// Test Case
//
{`Add a field on the elem, element and field present in the dest`,
`
{description: `Add a field on the elem, element and field present in the dest`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
update: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run2.sh']
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run.sh']
`,
`
expected: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run2.sh']
`, nil},
`},
//
// Test Case
//
{`Ignore an element`,
`
{description: `Ignore an element`,
origin: `
kind: Deployment
containers: {}
`,
`
update: `
kind: Deployment
containers: {}
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
expected: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`, nil},
`},
//
// Test Case
//
{`Leave deleted`,
`
{description: `Leave deleted`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
update: `
kind: Deployment
`,
`
local: `
kind: Deployment
`,
`
expected: `
kind: Deployment
`, nil},
`},
//
// Test Case
//
{`Remove an element -- matching`,
`
{description: `Remove an element -- matching`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
update: `
kind: Deployment
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
expected: `
kind: Deployment
`, nil},
`},
//
// Test Case
//
{`Remove an element -- field missing from update`,
`
{description: `Remove an element -- field missing from update`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
update: `
kind: Deployment
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run.sh']
`,
`
expected: `
kind: Deployment
`, nil},
`},
//
// Test Case
//
{`Remove an element -- element missing`,
`
{description: `Remove an element -- element missing`,
origin: `
kind: Deployment
containers:
- name: foo
@@ -344,13 +344,13 @@ containers:
- name: baz
image: baz:bar
`,
`
update: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
local: `
kind: Deployment
containers:
- name: foo
@@ -359,60 +359,273 @@ containers:
- name: baz
image: baz:bar
`,
`
expected: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run.sh']
`, nil},
`},
//
// Test Case
//
{`Remove an element -- empty containers`,
`
{description: `Remove an element -- empty containers`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
update: `
kind: Deployment
containers: {}
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run.sh']
`,
`
expected: `
kind: Deployment
`, nil},
`},
//
// Test Case
//
{`Remove an element -- missing list field`,
`
{description: `Remove an element -- missing list field`,
origin: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
`
update: `
kind: Deployment
`,
`
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
command: ['run.sh']
`,
`
expected: `
kind: Deployment
`, nil},
`},
//
// Test Case
//
{description: `no infer merge keys no merge'`,
origin: `
kind: Deployment
containers:
- name: foo
`,
update: `
kind: Deployment
containers:
- name: foo
command: ['run2.sh']
`,
local: `
kind: Deployment
containers:
- name: foo
image: foo:bar
`,
expected: `
kind: Deployment
containers:
- name: foo
command: ['run2.sh']
`,
noInfer: true,
},
//
// Test Case
//
{description: `no infer merge keys merge using schema`,
origin: `
kind: Deployment
apiVersion: apps/v1
spec:
template:
spec:
containers:
- name: foo
`,
update: `
kind: Deployment
apiVersion: apps/v1
spec:
template:
spec:
containers:
- name: foo
command: ['run2.sh']
`,
local: `
kind: Deployment
apiVersion: apps/v1
spec:
template:
spec:
containers:
- name: foo
image: foo:bar
`,
expected: `
kind: Deployment
apiVersion: apps/v1
spec:
template:
spec:
containers:
- name: foo
image: foo:bar
command: ['run2.sh']
`,
noInfer: true,
},
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema as line comment'`,
origin: `
kind: Deployment
containers:
- name: foo
`,
update: `
kind: Deployment
containers:
- name: foo
command: ['run2.sh']
`,
local: `
kind: Deployment
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-patch-merge-key":"name","x-kubernetes-patch-strategy": "merge"}
- name: foo # hell ow
image: foo:bar
`,
expected: `
kind: Deployment
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-patch-merge-key":"name","x-kubernetes-patch-strategy": "merge"}
- name: foo # hell ow
image: foo:bar
command: ['run2.sh']
`,
noInfer: true,
},
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema as head comment'`,
origin: `
kind: Deployment
containers:
- name: foo
`,
update: `
kind: Deployment
containers:
- name: foo
command: ['run2.sh']
`,
local: `
kind: Deployment
# {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-patch-merge-key":"name","x-kubernetes-patch-strategy": "merge"}
containers:
- name: foo # hell ow
image: foo:bar
`,
expected: `
kind: Deployment
# {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-patch-merge-key":"name","x-kubernetes-patch-strategy": "merge"}
containers:
- name: foo # hell ow
image: foo:bar
command: ['run2.sh']
`,
noInfer: true,
},
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema to parent field'`,
origin: `
kind: Deployment
spec:
containers:
- name: foo
`,
update: `
kind: Deployment
spec:
containers:
- name: foo
command: ['run2.sh']
`,
local: `
kind: Deployment
spec: # {"$ref":"#/definitions/io.k8s.api.core.v1.PodSpec"}
containers:
- name: foo # hell ow
image: foo:bar
`,
expected: `
kind: Deployment
spec: # {"$ref":"#/definitions/io.k8s.api.core.v1.PodSpec"}
containers:
- name: foo # hell ow
image: foo:bar
command: ['run2.sh']
`,
noInfer: true,
},
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema to parent field header'`,
origin: `
kind: Deployment
spec:
containers:
- name: foo
`,
update: `
kind: Deployment
spec:
containers:
- name: foo
command: ['run2.sh']
`,
local: `
kind: Deployment
# {"$ref":"#/definitions/io.k8s.api.core.v1.PodSpec"}
spec:
containers:
- name: foo # hell ow
image: foo:bar
`,
expected: `
kind: Deployment
# {"$ref":"#/definitions/io.k8s.api.core.v1.PodSpec"}
spec:
containers:
- name: foo # hell ow
image: foo:bar
command: ['run2.sh']
`,
noInfer: true,
},
}

View File

@@ -9,224 +9,226 @@ var listTestCases = []testCase{
//
// Test Case
//
{`Replace list`,
`
{description: `Replace list`,
origin: `
list:
- 1
- 2
- 3`,
`
update: `
list:
- 2
- 3
- 4`,
`
local: `
list:
- 1
- 2
- 3`,
`
expected: `
list:
- 2
- 3
- 4`, nil},
- 4`},
//
// Test Case
//
{`Add an updated list`,
`
{description: `Add an updated list`,
origin: `
apiVersion: apps/v1
list: # old value
- 1
- 2
- 3
`,
`
update: `
apiVersion: apps/v1
list: # new value
- 2
- 3
- 4
`,
`
local: `
apiVersion: apps/v1`,
`
expected: `
apiVersion: apps/v1
list:
- 2
- 3
- 4
`, nil},
`},
//
// Test Case
//
{`Add keep an omitted field`,
`
{description: `Add keep an omitted field`,
origin: `
apiVersion: apps/v1
kind: Deployment`,
`
update: `
apiVersion: apps/v1
kind: StatefulSet`,
`
local: `
apiVersion: apps/v1
list: # not present in sources
- 2
- 3
- 4
`,
`
expected: `
apiVersion: apps/v1
list: # not present in sources
- 2
- 3
- 4
kind: StatefulSet
`, nil},
`},
//
// Test Case
//
// TODO(#36): consider making this an error
{`Change an updated field`,
`
{description: `Change an updated field`,
origin: `
apiVersion: apps/v1
list: # old value
- 1
- 2
- 3`,
`
update: `
apiVersion: apps/v1
list: # new value
- 2
- 3
- 4`,
`
local: `
apiVersion: apps/v1
list: # conflicting value
- a
- b
- c`,
`
expected: `
apiVersion: apps/v1
list: # conflicting value
- 2
- 3
- 4
`, nil},
`},
//
// Test Case
//
{`Ignore a field -- set`,
`
{description: `Ignore a field -- set`,
origin: `
apiVersion: apps/v1
list: # ignore value
- 1
- 2
- 3
`,
`
update: `
apiVersion: apps/v1
list: # ignore value
- 1
- 2
- 3`, `
- 3`,
local: `
apiVersion: apps/v1
list:
- 2
- 3
- 4
`, `
`,
expected: `
apiVersion: apps/v1
list:
- 2
- 3
- 4
`, nil},
`},
//
// Test Case
//
{`Ignore a field -- empty`,
`
{description: `Ignore a field -- empty`,
origin: `
apiVersion: apps/v1
list: # ignore value
- 1
- 2
- 3`,
`
update: `
apiVersion: apps/v1
list: # ignore value
- 1
- 2
- 3`,
`
local: `
apiVersion: apps/v1
`,
`
expected: `
apiVersion: apps/v1
`, nil},
`},
//
// Test Case
//
{`Explicitly clear a field`,
`
{description: `Explicitly clear a field`,
origin: `
apiVersion: apps/v1`,
`
update: `
apiVersion: apps/v1
list: null # clear`,
`
local: `
apiVersion: apps/v1
list: # value to clear
- 1
- 2
- 3`,
`
apiVersion: apps/v1`, nil},
expected: `
apiVersion: apps/v1`},
//
// Test Case
//
{`Implicitly clear a field`,
`
{description: `Implicitly clear a field`,
origin: `
apiVersion: apps/v1
list: # clear value
- 1
- 2
- 3`,
`
update: `
apiVersion: apps/v1`,
`
local: `
apiVersion: apps/v1
list: # old value
- 1
- 2
- 3`,
`
apiVersion: apps/v1`, nil},
expected: `
apiVersion: apps/v1`},
//
// Test Case
//
// TODO(#36): consider making this an error
{`Implicitly clear a changed field`,
`
{description: `Implicitly clear a changed field`,
origin: `
apiVersion: apps/v1
list: # old value
- 1
- 2
- 3`,
`
update: `
apiVersion: apps/v1`,
`
local: `
apiVersion: apps/v1
list: # old value
- a
- b
- c`,
`
apiVersion: apps/v1`, nil},
expected: `
apiVersion: apps/v1`},
}

View File

@@ -7,267 +7,267 @@ var mapTestCases = []testCase{
//
// Test Case
//
{`Add the annotations map field`,
`
{description: `Add the annotations map field`,
origin: `
kind: Deployment`,
`
update: `
kind: Deployment
metadata:
annotations:
d: e # add these annotations
`,
`
local: `
kind: Deployment`,
`
expected: `
kind: Deployment
metadata:
annotations:
d: e # add these annotations`, nil},
d: e # add these annotations`},
//
// Test Case
//
{`Add an annotation to the field`,
`
{description: `Add an annotation to the field`,
origin: `
kind: Deployment
metadata:
annotations:
a: b`,
`
update: `
kind: Deployment
metadata:
annotations:
a: b
d: e # add these annotations`,
`
local: `
kind: Deployment
metadata:
annotations:
g: h # keep these annotations`,
`
expected: `
kind: Deployment
metadata:
annotations:
g: h # keep these annotations
d: e # add these annotations`, nil},
d: e # add these annotations`},
//
// Test Case
//
{`Add an annotation to the field, field missing from dest`,
`
{description: `Add an annotation to the field, field missing from dest`,
origin: `
kind: Deployment
metadata:
annotations:
a: b # ignored because unchanged`,
`
update: `
kind: Deployment
metadata:
annotations:
a: b # ignore because unchanged
d: e`,
`
local: `
kind: Deployment`,
`
expected: `
kind: Deployment
metadata:
annotations:
d: e`, nil},
d: e`},
//
// Test Case
//
{`Update an annotation on the field, field messing rom the dest`,
`
{description: `Update an annotation on the field, field messing rom the dest`,
origin: `
kind: Deployment
metadata:
annotations:
a: b
d: c`,
`
update: `
kind: Deployment
metadata:
annotations:
a: b
d: e # set these annotations`,
`
local: `
kind: Deployment
metadata:
annotations:
g: h # keep these annotations`,
`
expected: `
kind: Deployment
metadata:
annotations:
g: h # keep these annotations
d: e # set these annotations`, nil},
d: e # set these annotations`},
//
// Test Case
//
{`Add an annotation to the field, field missing from dest`,
`
{description: `Add an annotation to the field, field missing from dest`,
origin: `
kind: Deployment
metadata:
annotations:
a: b # ignored because unchanged`,
`
update: `
kind: Deployment
metadata:
annotations:
a: b # ignore because unchanged
d: e`,
`
local: `
kind: Deployment`,
`
expected: `
kind: Deployment
metadata:
annotations:
d: e`, nil},
d: e`},
//
// Test Case
//
{`Remove an annotation`,
`
{description: `Remove an annotation`,
origin: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
a: b`,
`
update: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations: {}`,
`
local: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
c: d
a: b`,
`
expected: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
c: d`, nil},
c: d`},
//
// Test Case
//
// TODO(#36) support ~annotations~: {} deletion
{`Specify a field as empty that isn't present in the source`,
`
{description: `Specify a field as empty that isn't present in the source`,
origin: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo`,
`
update: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo
annotations: null`,
`
local: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo
annotations:
a: b`,
`
expected: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo`, nil},
name: foo`},
//
// Test Case
//
{`Remove an annotation`,
`
{description: `Remove an annotation`,
origin: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
a: b`,
`
update: `
apiVersion: apps/v1
kind: Deployment`,
`
local: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
c: d
a: b`,
`
expected: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
c: d`, nil},
c: d`},
//
// Test Case
//
{`Remove annotations field`,
`
{description: `Remove annotations field`,
origin: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
a: b`,
`
update: `
apiVersion: apps/v1
kind: Deployment`,
`
local: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo`,
`
expected: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo
`, nil},
`},
//
// Test Case
//
{`Remove annotations field, but keep in dest`,
`
{description: `Remove annotations field, but keep in dest`,
origin: `
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
a: b`,
`
update: `
apiVersion: apps/v1
kind: Deployment`,
`
local: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo
annotations:
foo: bar # keep this annotation even though the parent field was removed`,
`
expected: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo
annotations:
foo: bar # keep this annotation even though the parent field was removed`, nil},
foo: bar # keep this annotation even though the parent field was removed`},
//
// Test Case
//
{`Remove annotations, but they are already empty`,
`
{description: `Remove annotations, but they are already empty`,
origin: `
apiVersion: apps/v1
kind: Deployment
metadata:
@@ -275,24 +275,24 @@ metadata:
annotations:
a: b
`,
`
update: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo
`,
`
local: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo
annotations: {}
`,
`
expected: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo
annotations: {}
`, nil},
`},
}

View File

@@ -13,11 +13,12 @@ import (
func Merge(dest, original, update *yaml.RNode) (*yaml.RNode, error) {
// if update == nil && original != nil => declarative deletion
return walk.Walker{Visitor: Visitor{},
return walk.Walker{
Visitor: Visitor{},
Sources: []*yaml.RNode{dest, original, update}}.Walk()
}
func MergeStrings(dest, original, update string) (string, error) {
func MergeStrings(dest, original, update string, infer bool) (string, error) {
srcOriginal, err := yaml.Parse(original)
if err != nil {
return "", err
@@ -31,7 +32,10 @@ func MergeStrings(dest, original, update string) (string, error) {
return "", err
}
result, err := Merge(d, srcOriginal, srcUpdated)
result, err := walk.Walker{
InferAssociativeLists: infer,
Visitor: Visitor{},
Sources: []*yaml.RNode{d, srcOriginal, srcUpdated}}.Walk()
if err != nil {
return "", err
}

View File

@@ -15,24 +15,27 @@ var testCases = [][]testCase{scalarTestCases, listTestCases, mapTestCases, eleme
func TestMerge(t *testing.T) {
for i := range testCases {
for _, tc := range testCases[i] {
actual, err := MergeStrings(tc.local, tc.origin, tc.update)
if tc.err == nil {
if !assert.NoError(t, err, tc.description) {
t.FailNow()
for j := range testCases[i] {
tc := testCases[i][j]
t.Run(tc.description, func(t *testing.T) {
actual, err := MergeStrings(tc.local, tc.origin, tc.update, !tc.noInfer)
if tc.err == nil {
if !assert.NoError(t, err, tc.description) {
t.FailNow()
}
if !assert.Equal(t,
strings.TrimSpace(tc.expected), strings.TrimSpace(actual), tc.description) {
t.FailNow()
}
} else {
if !assert.Errorf(t, err, tc.description) {
t.FailNow()
}
if !assert.Contains(t, tc.err.Error(), err.Error()) {
t.FailNow()
}
}
if !assert.Equal(t,
strings.TrimSpace(tc.expected), strings.TrimSpace(actual), tc.description) {
t.FailNow()
}
} else {
if !assert.Errorf(t, err, tc.description) {
t.FailNow()
}
if !assert.Contains(t, tc.err.Error(), err.Error()) {
t.FailNow()
}
}
})
}
}
}
@@ -44,4 +47,5 @@ type testCase struct {
local string
expected string
err error
noInfer bool
}

View File

@@ -8,128 +8,128 @@ var scalarTestCases = []testCase{
//
// Test Case
//
{`Set and updated a field`,
`kind: Deployment`,
`kind: StatefulSet`,
`kind: Deployment`,
`kind: StatefulSet`, nil},
{description: `Set and updated a field`,
origin: `kind: Deployment`,
update: `kind: StatefulSet`,
local: `kind: Deployment`,
expected: `kind: StatefulSet`},
{`Add an updated field`,
`
{description: `Add an updated field`,
origin: `
apiVersion: apps/v1
kind: Deployment # old value`,
`
update: `
apiVersion: apps/v1
kind: StatefulSet # new value`,
`
local: `
apiVersion: apps/v1`,
`
expected: `
apiVersion: apps/v1
kind: StatefulSet # new value`, nil},
kind: StatefulSet # new value`},
{`Add keep an omitted field`,
`
{description: `Add keep an omitted field`,
origin: `
apiVersion: apps/v1
kind: Deployment`,
`
update: `
apiVersion: apps/v1
kind: StatefulSet`,
`
local: `
apiVersion: apps/v1
spec: foo # field not present in source
`,
`
expected: `
apiVersion: apps/v1
spec: foo # field not present in source
kind: StatefulSet
`, nil},
`},
//
// Test Case
//
// TODO(#36): consider making this an error
{`Change an updated field`,
`
{description: `Change an updated field`,
origin: `
apiVersion: apps/v1
kind: Deployment # old value`,
`
update: `
apiVersion: apps/v1
kind: StatefulSet # new value`,
`
local: `
apiVersion: apps/v1
kind: Service # conflicting value`,
`
expected: `
apiVersion: apps/v1
kind: StatefulSet # new value`, nil},
kind: StatefulSet # new value`},
{`Ignore a field`,
`
{description: `Ignore a field`,
origin: `
apiVersion: apps/v1
kind: Deployment # ignore this field`,
`
update: `
apiVersion: apps/v1
kind: Deployment # ignore this field`,
`
local: `
apiVersion: apps/v1`,
`
apiVersion: apps/v1`, nil},
expected: `
apiVersion: apps/v1`},
{`Explicitly clear a field`,
`
{description: `Explicitly clear a field`,
origin: `
apiVersion: apps/v1`,
`
update: `
apiVersion: apps/v1
kind: null # clear this value`,
`
local: `
apiVersion: apps/v1
kind: Deployment # value to be cleared`,
`
apiVersion: apps/v1`, nil},
expected: `
apiVersion: apps/v1`},
{`Implicitly clear a field`,
`
{description: `Implicitly clear a field`,
origin: `
apiVersion: apps/v1
kind: Deployment # clear this field`,
`
update: `
apiVersion: apps/v1`,
`
local: `
apiVersion: apps/v1
kind: Deployment # clear this field`,
`
apiVersion: apps/v1`, nil},
expected: `
apiVersion: apps/v1`},
//
// Test Case
//
// TODO(#36): consider making this an error
{`Implicitly clear a changed field`,
`
{description: `Implicitly clear a changed field`,
origin: `
apiVersion: apps/v1
kind: Deployment`,
`
update: `
apiVersion: apps/v1`,
`
local: `
apiVersion: apps/v1
kind: StatefulSet`,
`
apiVersion: apps/v1`, nil},
expected: `
apiVersion: apps/v1`},
//
// Test Case
//
{`Merge an empty scalar value`,
`
{description: `Merge an empty scalar value`,
origin: `
apiVersion: apps/v1
`,
`
update: `
apiVersion: apps/v1
kind: {}
`,
`
local: `
apiVersion: apps/v1
`,
`
expected: `
apiVersion: apps/v1
kind: {}
`, nil},
`},
}

View File

@@ -4,6 +4,7 @@
package merge3
import (
"sigs.k8s.io/kustomize/kyaml/openapi"
"sigs.k8s.io/kustomize/kyaml/yaml"
"sigs.k8s.io/kustomize/kyaml/yaml/walk"
)
@@ -17,7 +18,7 @@ const (
type Visitor struct{}
func (m Visitor) VisitMap(nodes walk.Sources) (*yaml.RNode, error) {
func (m Visitor) VisitMap(nodes walk.Sources, s *openapi.ResourceSchema) (*yaml.RNode, error) {
if yaml.IsNull(nodes.Updated()) || yaml.IsNull(nodes.Dest()) {
// explicitly cleared from either dest or update
return walk.ClearNode, nil
@@ -36,7 +37,7 @@ func (m Visitor) VisitMap(nodes walk.Sources) (*yaml.RNode, error) {
return nodes.Dest(), nil
}
func (m Visitor) visitAList(nodes walk.Sources) (*yaml.RNode, error) {
func (m Visitor) visitAList(nodes walk.Sources, _ *openapi.ResourceSchema) (*yaml.RNode, error) {
if yaml.IsEmpty(nodes.Updated()) && !yaml.IsEmpty(nodes.Origin()) {
// implicitly cleared from update -- element was deleted
return walk.ClearNode, nil
@@ -51,7 +52,7 @@ func (m Visitor) visitAList(nodes walk.Sources) (*yaml.RNode, error) {
return nodes.Dest(), nil
}
func (m Visitor) VisitScalar(nodes walk.Sources) (*yaml.RNode, error) {
func (m Visitor) VisitScalar(nodes walk.Sources, s *openapi.ResourceSchema) (*yaml.RNode, error) {
if yaml.IsNull(nodes.Updated()) || yaml.IsNull(nodes.Dest()) {
// explicitly cleared from either dest or update
return nil, nil
@@ -103,9 +104,9 @@ func (m Visitor) visitNAList(nodes walk.Sources) (*yaml.RNode, error) {
return nodes.Dest(), nil
}
func (m Visitor) VisitList(nodes walk.Sources, kind walk.ListKind) (*yaml.RNode, error) {
func (m Visitor) VisitList(nodes walk.Sources, s *openapi.ResourceSchema, kind walk.ListKind) (*yaml.RNode, error) {
if kind == walk.AssociativeList {
return m.visitAList(nodes)
return m.visitAList(nodes, s)
}
// non-associative list
return m.visitNAList(nodes)

View File

@@ -0,0 +1,34 @@
// Copyright 2019 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0
// Package schema contains libraries for working with the yaml and openapi packages.
package schema
import (
"sigs.k8s.io/kustomize/kyaml/openapi"
"sigs.k8s.io/kustomize/kyaml/yaml"
)
// IsAssociative returns true if all elements in the list contain an AssociativeSequenceKey
// as a field.
func IsAssociative(schema *openapi.ResourceSchema, nodes []*yaml.RNode, infer bool) bool {
if schema != nil {
// use the schema to identify if the list is associative
s, _ := schema.PatchStrategyAndKey()
return s == "merge"
}
if !infer {
return false
}
for i := range nodes {
node := nodes[i]
if yaml.IsEmpty(node) {
continue
}
if node.IsAssociative() {
return true
}
}
return false
}

View File

@@ -651,24 +651,8 @@ func (rn *RNode) VisitElements(fn func(node *RNode) error) error {
// The order sets the precedence of the merge keys -- if multiple keys are present
// in Resources in a list, then the FIRST key which ALL elements in the list have is used as the
// associative key for merging that list.
var AssociativeSequenceKeys = []string{
"mountPath", "devicePath", "ip", "type", "topologyKey", "name", "containerPort",
}
// IsAssociative returns true if all elements in the list contain an AssociativeSequenceKey
// as a field.
func IsAssociative(nodes []*RNode) bool {
for i := range nodes {
node := nodes[i]
if IsEmpty(node) {
continue
}
if node.IsAssociative() {
return true
}
}
return false
}
// Only infer name as a merge key.
var AssociativeSequenceKeys = []string{"name"}
// IsAssociative returns true if the RNode contains an AssociativeSequenceKey as a field.
func (rn *RNode) IsAssociative() bool {

View File

@@ -7,28 +7,44 @@ import (
"strings"
"github.com/go-errors/errors"
"sigs.k8s.io/kustomize/kyaml/openapi"
"sigs.k8s.io/kustomize/kyaml/sets"
"sigs.k8s.io/kustomize/kyaml/yaml"
)
func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) {
// may require initializing the dest node
dest, err := l.Sources.setDestNode(l.VisitList(l.Sources, AssociativeList))
dest, err := l.Sources.setDestNode(l.VisitList(l.Sources, l.Schema, AssociativeList))
if dest == nil || err != nil {
return nil, err
}
// find the list of elements we need to recursively walk
key, err := l.elementKey()
if err != nil {
return nil, err
var key string
if l.Schema != nil {
_, key = l.Schema.PatchStrategyAndKey()
}
if key == "" { // no key from the schema, try to infer one
// find the list of elements we need to recursively walk
key, err = l.elementKey()
if err != nil {
return nil, err
}
}
values := l.elementValues(key)
// recursively set the elements in the list
var s *openapi.ResourceSchema
if l.Schema != nil {
s = l.Schema.Elements()
}
for _, value := range values {
val, err := Walker{Visitor: l,
Sources: l.elementValue(key, value)}.Walk()
val, err := Walker{
InferAssociativeLists: l.InferAssociativeLists,
Visitor: l,
Schema: s,
Sources: l.elementValue(key, value),
}.Walk()
if err != nil {
return nil, err
}

View File

@@ -6,6 +6,8 @@ package walk
import (
"sort"
"sigs.k8s.io/kustomize/kyaml/fieldmeta"
"sigs.k8s.io/kustomize/kyaml/openapi"
"sigs.k8s.io/kustomize/kyaml/sets"
"sigs.k8s.io/kustomize/kyaml/yaml"
)
@@ -18,15 +20,27 @@ import (
// - set each source field value on l.Dest
func (l Walker) walkMap() (*yaml.RNode, error) {
// get the new map value
dest, err := l.Sources.setDestNode(l.VisitMap(l.Sources))
dest, err := l.Sources.setDestNode(l.VisitMap(l.Sources, l.Schema))
if dest == nil || err != nil {
return nil, err
}
// recursively set the field values on the map
for _, key := range l.fieldNames() {
val, err := Walker{Visitor: l,
Sources: l.fieldValue(key), Path: append(l.Path, key)}.Walk()
var s *openapi.ResourceSchema
if l.Schema != nil {
s = l.Schema.Field(key)
}
fv, commentSch := l.fieldValue(key)
if commentSch != nil {
s = commentSch
}
val, err := Walker{
InferAssociativeLists: l.InferAssociativeLists,
Visitor: l,
Schema: s,
Sources: fv,
Path: append(l.Path, key)}.Walk()
if err != nil {
return nil, err
}
@@ -42,11 +56,40 @@ func (l Walker) walkMap() (*yaml.RNode, error) {
}
// valueIfPresent returns node.Value if node is non-nil, otherwise returns nil
func (l Walker) valueIfPresent(node *yaml.MapNode) *yaml.RNode {
func (l Walker) valueIfPresent(node *yaml.MapNode) (*yaml.RNode, *openapi.ResourceSchema) {
if node == nil {
return nil
return nil, nil
}
return node.Value
// parse the schema for the field if present
var s *openapi.ResourceSchema
fm := fieldmeta.FieldMeta{}
var err error
// check the value for a schema
if err = fm.Read(node.Value); err == nil {
s = &openapi.ResourceSchema{Schema: &fm.Schema}
if fm.Schema.Ref.String() != "" {
r, err := openapi.Resolve(&fm.Schema.Ref)
if err == nil && r != nil {
s.Schema = r
}
}
}
// check the key for a schema -- this will be used
// when the value is a Sequence (comments are attached)
// to the key
if fm.IsEmpty() {
if err = fm.Read(node.Key); err == nil {
s = &openapi.ResourceSchema{Schema: &fm.Schema}
}
if fm.Schema.Ref.String() != "" {
r, err := openapi.Resolve(&fm.Schema.Ref)
if err == nil && r != nil {
s.Schema = r
}
}
}
return node.Value, s
}
// fieldNames returns a sorted slice containing the names of all fields that appear in any of
@@ -67,15 +110,20 @@ func (l Walker) fieldNames() []string {
}
// fieldValue returns a slice containing each source's value for fieldName
func (l Walker) fieldValue(fieldName string) []*yaml.RNode {
func (l Walker) fieldValue(fieldName string) ([]*yaml.RNode, *openapi.ResourceSchema) {
var fields []*yaml.RNode
var sch *openapi.ResourceSchema
for i := range l.Sources {
if l.Sources[i] == nil {
fields = append(fields, nil)
continue
}
field := l.Sources[i].Field(fieldName)
fields = append(fields, l.valueIfPresent(field))
f, s := l.valueIfPresent(field)
fields = append(fields, f)
if sch == nil && !s.IsEmpty() {
sch = s
}
}
return fields
return fields, sch
}

View File

@@ -9,5 +9,5 @@ import (
// walkNonAssociativeSequence returns the value of VisitList
func (l Walker) walkNonAssociativeSequence() (*yaml.RNode, error) {
return l.VisitList(l.Sources, NonAssociateList)
return l.VisitList(l.Sources, l.Schema, NonAssociateList)
}

View File

@@ -7,5 +7,5 @@ import "sigs.k8s.io/kustomize/kyaml/yaml"
// walkScalar returns the value of VisitScalar
func (l Walker) walkScalar() (*yaml.RNode, error) {
return l.VisitScalar(l.Sources)
return l.VisitScalar(l.Sources, l.Schema)
}

View File

@@ -4,6 +4,7 @@
package walk
import (
"sigs.k8s.io/kustomize/kyaml/openapi"
"sigs.k8s.io/kustomize/kyaml/yaml"
)
@@ -16,11 +17,11 @@ const (
// Visitor is invoked by walk with source and destination node pairs
type Visitor interface {
VisitMap(nodes Sources) (*yaml.RNode, error)
VisitMap(Sources, *openapi.ResourceSchema) (*yaml.RNode, error)
VisitScalar(nodes Sources) (*yaml.RNode, error)
VisitScalar(Sources, *openapi.ResourceSchema) (*yaml.RNode, error)
VisitList(nodes Sources, kind ListKind) (*yaml.RNode, error)
VisitList(Sources, *openapi.ResourceSchema, ListKind) (*yaml.RNode, error)
}
// ClearNode is returned if GrepFilter should do nothing after calling Set

View File

@@ -8,20 +8,30 @@ import (
"os"
"strings"
"sigs.k8s.io/kustomize/kyaml/fieldmeta"
"sigs.k8s.io/kustomize/kyaml/openapi"
"sigs.k8s.io/kustomize/kyaml/yaml"
"sigs.k8s.io/kustomize/kyaml/yaml/schema"
)
// Filter walks the Source RNode and modifies the RNode provided to GrepFilter.
// Walker walks the Source RNode and modifies the RNode provided to GrepFilter.
type Walker struct {
// Visitor is invoked by GrepFilter
Visitor
Schema *openapi.ResourceSchema
// Source is the RNode to walk. All Source fields and associative list elements
// will be visited.
Sources Sources
// Path is the field path to the current Source Node.
Path []string
// InferAssociativeLists if set to true will infer merge strategies for
// fields which it doesn't have the schema based on the fields in the
// list elements.
InferAssociativeLists bool
}
func (l Walker) Kind() yaml.Kind {
@@ -35,6 +45,8 @@ func (l Walker) Kind() yaml.Kind {
// GrepFilter implements yaml.GrepFilter
func (l Walker) Walk() (*yaml.RNode, error) {
l.Schema = l.GetSchema()
// invoke the handler for the corresponding node type
switch l.Kind() {
case yaml.MappingNode:
@@ -46,7 +58,7 @@ func (l Walker) Walk() (*yaml.RNode, error) {
if err := yaml.ErrorIfAnyInvalidAndNonNull(yaml.SequenceNode, l.Sources...); err != nil {
return nil, err
}
if yaml.IsAssociative(l.Sources) {
if schema.IsAssociative(l.Schema, l.Sources, l.InferAssociativeLists) {
return l.walkAssociativeSequence()
}
return l.walkNonAssociativeSequence()
@@ -64,6 +76,49 @@ func (l Walker) Walk() (*yaml.RNode, error) {
}
}
func (l Walker) GetSchema() *openapi.ResourceSchema {
for i := range l.Sources {
r := l.Sources[i]
if yaml.IsEmpty(r) {
continue
}
fm := fieldmeta.FieldMeta{}
if err := fm.Read(r); err == nil && !fm.IsEmpty() {
// per-field schema, this is fine
if fm.Schema.Ref.String() != "" {
// resolve the reference
s, err := openapi.Resolve(&fm.Schema.Ref)
if err == nil && s != nil {
fm.Schema = *s
}
}
return &openapi.ResourceSchema{Schema: &fm.Schema}
}
}
if l.Schema != nil {
return l.Schema
}
for i := range l.Sources {
r := l.Sources[i]
if yaml.IsEmpty(r) {
continue
}
m, _ := r.GetMeta()
if m.Kind == "" || m.APIVersion == "" {
continue
}
s := openapi.SchemaForResourceType(yaml.TypeMeta{Kind: m.Kind, APIVersion: m.APIVersion})
if s != nil {
return s
}
}
return nil
}
const (
DestIndex = iota
OriginIndex