Use OpenAPI when merging (3way) resources

- When merging (3way) resources use the patch strategy from the openAPI if the definition exists for the field
- Allow disabling of guessing patch strategy merge keys when no definition exists
- Support defining strategy and key directly on configuration fields through line and header coments
- Support attaching schema to parent fields of lists, and propagating -- e.g. that a field is a PodTemplate
This commit is contained in:
Phillip Wittrock
2020-02-27 10:08:40 -08:00
parent 8991b193c6
commit 5d1a0346b5
24 changed files with 991 additions and 507 deletions

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: {}
`,