add option to choose prepend/append in merge

This commit is contained in:
Donny Xia
2020-09-28 16:17:50 -07:00
parent fafe24c9df
commit 74d0d7960e
35 changed files with 592 additions and 176 deletions

View File

@@ -18,7 +18,12 @@ var _ kio.Filter = Filter{}
func (pf Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
var result []*yaml.RNode
for i := range nodes {
r, err := merge2.Merge(pf.Patch, nodes[i])
r, err := merge2.Merge(
pf.Patch, nodes[i],
yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListPrepend,
},
)
if err != nil {
return nil, err
}

View File

@@ -67,10 +67,10 @@ spec:
template:
spec:
containers:
- name: foo0
- name: foo1
- name: foo2
- name: foo3
- name: foo0
`,
},
"volumes patch": {
@@ -107,10 +107,10 @@ spec:
template:
spec:
volumes:
- name: foo0
- name: foo1
- name: foo2
- name: foo3
- name: foo0
`,
},
"nested patch": {

View File

@@ -225,13 +225,13 @@ spec:
spec:
containers:
- env:
- name: foo
value: bar
- name: FOO
valueFrom:
configMapKeyRef:
key: somekey
name: test-infra-app-env-8h5mh7f7ch
- name: foo
value: bar
image: nginx:1.8.0
name: nginx
ports:

View File

@@ -385,12 +385,12 @@ spec:
- gcePersistentDisk:
pdName: nginx-persistent-storage
name: nginx-persistent-storage
- configMap:
name: staging-team-foo-configmap-in-base-hc6g9dk6g9
name: configmap-in-base
- configMap:
name: staging-configmap-in-overlay-dc6fm46dhm
name: configmap-in-overlay
- configMap:
name: staging-team-foo-configmap-in-base-hc6g9dk6g9
name: configmap-in-base
---
apiVersion: v1
kind: Service

View File

@@ -146,12 +146,12 @@ spec:
- gcePersistentDisk:
pdName: nginx-persistent-storage
name: nginx-persistent-storage
- configMap:
name: a-b-configmap-in-base-798k5k7g9f
name: configmap-in-base
- configMap:
name: a-configmap-in-overlay-dc6fm46dhm
name: configmap-in-overlay
- configMap:
name: a-b-configmap-in-base-798k5k7g9f
name: configmap-in-base
---
apiVersion: v1
kind: Service
@@ -351,12 +351,12 @@ spec:
- gcePersistentDisk:
pdName: nginx-persistent-storage
name: nginx-persistent-storage
- configMap:
name: staging-team-foo-configmap-in-base-798k5k7g9f
name: configmap-in-base
- configMap:
name: staging-configmap-in-overlay-dc6fm46dhm
name: configmap-in-overlay
- configMap:
name: staging-team-foo-configmap-in-base-798k5k7g9f
name: configmap-in-base
---
apiVersion: v1
kind: Service

View File

@@ -9,23 +9,23 @@ operation/target/value tuples in a particular
syntax).
A kustomize file lets one specify many
patches. Each patch must be associated with
patches. Each patch must be associated with
a _target selector_:
[strategic merge patch]: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-api-machinery/strategic-merge-patch.md
[JSON patch]: jsonpatch.md
[json patch]: jsonpatch.md
> ```yaml
> patches:
> - path: <relative path to file containing patch>
> target:
> group: <optional group>
> version: <optional version>
> kind: <optional kind>
> name: <optional name>
> namespace: <optional namespace>
> labelSelector: <optional label selector>
> annotationSelector: <optional annotation selector>
> - path: <relative path to file containing patch>
> target:
> group: <optional group>
> version: <optional version>
> kind: <optional kind>
> name: <optional name>
> namespace: <optional namespace>
> labelSelector: <optional label selector>
> annotationSelector: <optional annotation selector>
> ```
E.g. select resources with _name_ matching `foo*`:
@@ -61,10 +61,10 @@ The example below shows how to inject a
sidecar container for multiple Deployment
resources.
Make a place to work:
<!-- @demoHome @testAgainstLatestRelease -->
```
DEMO_HOME=$(mktemp -d)
```
@@ -72,6 +72,7 @@ DEMO_HOME=$(mktemp -d)
Make a file describing two Deployments:
<!-- @createDeployments @testAgainstLatestRelease -->
```
cat <<EOF >$DEMO_HOME/deployments.yaml
apiVersion: apps/v1
@@ -111,6 +112,7 @@ Declare a [strategic merge patch] file
to inject a sidecar container:
<!-- @definePatch @testAgainstLatestRelease -->
```
cat <<EOF >$DEMO_HOME/patch.yaml
apiVersion: apps/v1
@@ -134,6 +136,7 @@ that specifies both a `patches` and `resources`
entry:
<!-- @createKustomization @testAgainstLatestRelease -->
```
cat <<EOF >$DEMO_HOME/kustomization.yaml
resources:
@@ -149,6 +152,7 @@ EOF
The expected result is:
<!-- @definedExpectedOutput @testAgainstLatestRelease -->
```
cat <<EOF >$DEMO_HOME/out_expected.yaml
apiVersion: apps/v1
@@ -162,16 +166,16 @@ spec:
old-label: old-value
spec:
containers:
- args:
- one
- two
image: nginx
name: nginx
- args:
- proxy
- sidecar
image: docker.io/istio/proxyv2
name: istio-proxy
- args:
- one
- two
image: nginx
name: nginx
---
apiVersion: apps/v1
kind: Deployment
@@ -184,18 +188,20 @@ spec:
key: value
spec:
containers:
- image: busybox
name: busybox
- args:
- proxy
- sidecar
image: docker.io/istio/proxyv2
name: istio-proxy
- image: busybox
name: busybox
EOF
```
Run the build:
<!-- @runIt @testAgainstLatestRelease -->
```
kustomize build $DEMO_HOME >$DEMO_HOME/out_actual.yaml
```
@@ -203,6 +209,7 @@ kustomize build $DEMO_HOME >$DEMO_HOME/out_actual.yaml
Confirm expectations:
<!-- @diffShouldExitZero @testAgainstLatestRelease -->
```
diff $DEMO_HOME/out_actual.yaml $DEMO_HOME/out_expected.yaml
```

View File

@@ -67,7 +67,9 @@ func (c MergeFilter) Filter(input []*yaml.RNode) ([]*yaml.RNode, error) {
// first resources, don't merge it
merged = resources[i]
} else {
merged, err = merge2.Merge(patch, merged)
merged, err = merge2.Merge(patch, merged, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListPrepend,
})
if err != nil {
return nil, err
}

View File

@@ -3,6 +3,10 @@
package merge2_test
import (
"sigs.k8s.io/kustomize/kyaml/yaml"
)
var elementTestCases = []testCase{
{description: `merge Element -- keep field in dest`,
source: `
@@ -37,6 +41,9 @@ spec:
image: foo:v1
command: ['run.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Element -- add field to dest`,
@@ -72,6 +79,9 @@ spec:
image: foo:v1
command: ['run.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Element -- add list, empty in dest`,
@@ -105,6 +115,9 @@ spec:
image: foo:v1
command: ['run.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Element -- add list, missing from dest`,
@@ -134,6 +147,9 @@ spec:
image: foo:v1
command: ['run.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Element -- add Element first`,
@@ -173,6 +189,9 @@ spec:
image: bar:v1
command: ['run2.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Element -- add Element second`,
@@ -212,7 +231,94 @@ spec:
image: bar:v1
command: ['run2.sh']
`,
},
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Element -- add Element third`,
source: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: bar
image: bar:v1
command: ['run2.sh']
- name: foo
image: foo:v1
`,
dest: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: foo
image: foo:v0
`,
expected: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: bar
image: bar:v1
command: ['run2.sh']
- name: foo
image: foo:v1
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListPrepend,
},
},
{description: `merge Element -- add Element fourth`,
source: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: foo
image: foo:v1
- name: bar
image: bar:v1
command: ['run2.sh']
`,
dest: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: foo
image: foo:v0
`,
expected: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: foo
image: foo:v1
- name: bar
image: bar:v1
command: ['run2.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListPrepend,
},
},
//
// Test Case
@@ -248,6 +354,9 @@ spec:
image: bar:v1
command: ['run2.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -290,6 +399,9 @@ spec:
image: bar:v1
command: ['run2.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -330,6 +442,9 @@ spec:
image: bar:v1
command: ['run2.sh']
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -364,6 +479,9 @@ spec:
template:
spec: {}
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -393,6 +511,9 @@ containers:
command: ['run2.sh']
`,
infer: true,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -431,6 +552,9 @@ spec:
command: ['run2.sh']
`,
infer: false,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -460,6 +584,9 @@ containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"t
command: ['run2.sh']
`,
infer: false,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge_primitive_finalizers`,
@@ -488,6 +615,9 @@ metadata:
- c
- a
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge_primitive_items`,
@@ -513,5 +643,8 @@ items:
- c
- a
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
}

View File

@@ -3,6 +3,10 @@
package merge2_test
import (
"sigs.k8s.io/kustomize/kyaml/yaml"
)
var listTestCases = []testCase{
{description: `strategic merge patch delete 1`,
source: `
@@ -38,6 +42,9 @@ spec:
- name: foo2
- name: foo3
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `strategic merge patch delete 2`,
source: `
@@ -73,8 +80,48 @@ spec:
- name: foo1
- name: foo2
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge k8s deployment containers`,
{description: `merge k8s deployment containers - prepend`,
source: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: foo1
- name: foo2
- name: foo3
`,
dest: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: foo0
`,
expected: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
- name: foo1
- name: foo2
- name: foo3
- name: foo0
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListPrepend,
},
},
{description: `merge k8s deployment containers - append`,
source: `
apiVersion: apps/v1
kind: Deployment
@@ -107,8 +154,11 @@ spec:
- name: foo2
- name: foo3
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge k8s deployment volumes`,
{description: `merge k8s deployment volumes - append`,
source: `
apiVersion: apps/v1
kind: Deployment
@@ -141,6 +191,46 @@ spec:
- name: foo2
- name: foo3
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge k8s deployment volumes - prepend`,
source: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
volumes:
- name: foo1
- name: foo2
- name: foo3
`,
dest: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
volumes:
- name: foo0
`,
expected: `
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
volumes:
- name: foo1
- name: foo2
- name: foo3
- name: foo0
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListPrepend,
},
},
{description: `merge k8s deployment containers -- $patch directive`,
source: `
@@ -267,6 +357,9 @@ items:
- 2
- 3
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `replace List -- missing from dest`,
@@ -287,6 +380,9 @@ items:
- 2
- 3
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -314,6 +410,9 @@ items:
- 2
- 3
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -337,6 +436,9 @@ items:
- 2
- 3
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -357,6 +459,9 @@ items:
expected: `
kind: Deployment
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -378,5 +483,8 @@ items:
kind: Deployment
items: []
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
}

View File

@@ -3,6 +3,10 @@
package merge2_test
import (
"sigs.k8s.io/kustomize/kyaml/yaml"
)
var mapTestCases = []testCase{
{description: `strategic merge patch delete 1`,
@@ -16,6 +20,9 @@ spec:
foo: bar1
`,
expected: ``,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `strategic merge patch delete 2`,
@@ -33,6 +40,9 @@ spec:
expected: `
kind: Deployment
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `strategic merge patch delete 3`,
@@ -61,6 +71,9 @@ spec:
metadata:
name: wut
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `strategic merge patch delete 4`,
@@ -89,6 +102,9 @@ spec:
`,
expected: `
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `strategic merge patch replace 1`,
@@ -111,6 +127,9 @@ spec:
metal: heavy
veggie: carrot
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Map -- update field in dest`,
@@ -131,6 +150,9 @@ spec:
foo: bar1
baz: buz
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Map -- add field to dest`,
@@ -151,6 +173,9 @@ spec:
foo: bar1
baz: buz
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Map -- add list, empty in dest`,
@@ -170,6 +195,9 @@ spec:
baz: buz
foo: bar1
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Map -- add list, missing from dest`,
@@ -188,6 +216,9 @@ spec:
foo: bar1
baz: buz
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Map -- add Map first`,
@@ -208,6 +239,9 @@ spec:
foo: bar1
baz: buz
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `merge Map -- add Map second`,
@@ -228,6 +262,9 @@ spec:
foo: bar1
baz: buz
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -249,6 +286,9 @@ spec:
foo: bar1
baz: buz
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -272,6 +312,9 @@ spec:
baz: buz
items: {}
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -291,5 +334,8 @@ spec:
expected: `
kind: Deployment
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
}

View File

@@ -12,12 +12,16 @@ import (
)
// Merge merges fields from src into dest.
func Merge(src, dest *yaml.RNode) (*yaml.RNode, error) {
return walk.Walker{Sources: []*yaml.RNode{dest, src}, Visitor: Merger{}}.Walk()
func Merge(src, dest *yaml.RNode, mergeOptions yaml.MergeOptions) (*yaml.RNode, error) {
return walk.Walker{
Sources: []*yaml.RNode{dest, src},
Visitor: Merger{},
MergeOptions: mergeOptions,
}.Walk()
}
// Merge parses the arguments, and merges fields from srcStr into destStr.
func MergeStrings(srcStr, destStr string, infer bool) (string, error) {
func MergeStrings(srcStr, destStr string, infer bool, mergeOptions yaml.MergeOptions) (string, error) {
src, err := yaml.Parse(srcStr)
if err != nil {
return "", err
@@ -31,6 +35,7 @@ func MergeStrings(srcStr, destStr string, infer bool) (string, error) {
Sources: []*yaml.RNode{dest, src},
Visitor: Merger{},
InferAssociativeLists: infer,
MergeOptions: mergeOptions,
}.Walk()
if err != nil {
return "", err

View File

@@ -55,7 +55,9 @@ metadata:
m: n1
`)
result, err := Merge(src, dest)
result, err := Merge(src, dest, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -114,7 +116,9 @@ metadata:
annotations: null
`)
result, err := Merge(src, dest)
result, err := Merge(src, dest, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -174,7 +178,9 @@ metadata:
m: n1
`)
result, err := Merge(dest, src)
result, err := Merge(dest, src, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -241,7 +247,9 @@ spec:
value: "Another Env Not In The Dest"
`)
result, err := Merge(src, dest)
result, err := Merge(src, dest, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -305,7 +313,9 @@ spec:
args: ['e', 'd', 'f']
`)
result, err := Merge(src, dest)
result, err := Merge(src, dest, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -365,7 +375,9 @@ a:
b:
# header comment
c: d
`, true)
`, true, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -385,7 +397,9 @@ a:
b:
c: d
# footer comment
`, true)
`, true, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -404,7 +418,9 @@ a:
a:
b:
c: d # line comment
`, true)
`, true, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -426,7 +442,9 @@ a:
b:
# replace comment
c: d
`, true)
`, true, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -447,7 +465,9 @@ a:
b:
c: d
# replace comment
`, true)
`, true, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -466,7 +486,9 @@ a:
a:
b:
c: d # replace comment
`, true)
`, true, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}
@@ -484,7 +506,9 @@ a:
a:
b:
c: d # replace comment
`, true)
`, true, yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
})
if !assert.NoError(t, err) {
return
}

View File

@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/assert"
"sigs.k8s.io/kustomize/kyaml/kio/filters"
"sigs.k8s.io/kustomize/kyaml/yaml"
. "sigs.k8s.io/kustomize/kyaml/yaml/merge2"
)
@@ -20,7 +21,7 @@ func TestMerge(t *testing.T) {
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.infer)
actual, err := MergeStrings(tc.source, tc.dest, tc.infer, tc.mergeOptions)
if !assert.NoError(t, err, tc.description) {
t.FailNow()
}
@@ -43,9 +44,10 @@ func TestMerge(t *testing.T) {
}
type testCase struct {
description string
source string
dest string
expected string
infer bool
description string
source string
dest string
expected string
infer bool
mergeOptions yaml.MergeOptions
}

View File

@@ -3,6 +3,10 @@
package merge2_test
import (
"sigs.k8s.io/kustomize/kyaml/yaml"
)
var scalarTestCases = []testCase{
{description: `replace scalar -- different value in dest`,
source: `
@@ -17,6 +21,9 @@ field: value0
kind: Deployment
field: value1
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
{description: `replace scalar -- missing from dest`,
@@ -31,6 +38,9 @@ kind: Deployment
kind: Deployment
field: value1
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -49,6 +59,9 @@ field: value1
kind: Deployment
field: value1
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -66,6 +79,9 @@ field: value1
kind: Deployment
field: value1
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -83,6 +99,9 @@ field: value1
expected: `
kind: Deployment
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -100,6 +119,9 @@ field: value1
expected: `
kind: Deployment
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -116,6 +138,9 @@ kind: Deployment
expected: `
kind: Deployment
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
//
@@ -133,5 +158,8 @@ kind: Deployment
kind: Deployment
field: {}
`,
mergeOptions: yaml.MergeOptions{
ListIncreaseDirection: yaml.MergeOptionsListAppend,
},
},
}

View File

@@ -247,3 +247,18 @@ func String(node *yaml.Node, opts ...string) (string, error) {
}
return val, errors.Wrap(err)
}
// MergeOptionsListIncreaseDirection is the type of list growth in merge
type MergeOptionsListIncreaseDirection int
const (
MergeOptionsListAppend MergeOptionsListIncreaseDirection = iota
MergeOptionsListPrepend
)
// MergeOptions is a struct which contains the options for merge
type MergeOptions struct {
// ListIncreaseDirection indicates should merge function prepend the items from
// source list to destination or append.
ListIncreaseDirection MergeOptionsListIncreaseDirection
}

View File

@@ -12,92 +12,70 @@ import (
"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, l.Schema, AssociativeList))
if dest == nil || err != nil {
return nil, err
}
var key, strategy string
if l.Schema != nil {
strategy, key = l.Schema.PatchStrategyAndKey()
}
if strategy == "" && key == "" { // neither strategy nor not present in the schema -- infer the key
// find the list of elements we need to recursively walk
key, err = l.elementKey()
// appendListNode will append the nodes from src to dst and return dst.
// src and dst should be both sequence node. key is used to call ElementSetter.
// ElementSetter will use key-value pair to find and set the element in sequence
// node.
func appendListNode(dst, src *yaml.RNode, key string) (*yaml.RNode, error) {
for _, elem := range src.Content() {
// If key is empty, we know this is a scalar value and we can directly set the
// node
if key == "" {
_, err := dst.Pipe(yaml.ElementSetter{Element: elem, Key: key, Value: elem.Value})
if err != nil {
return nil, err
}
continue
}
// we need to get the value for key so that we can find the element to set
// in sequence.
tmpNode := yaml.NewRNode(elem)
valueNode, err := tmpNode.Pipe(yaml.Get(key))
if err != nil {
return nil, err
}
if valueNode.IsNil() {
// no key found, directly append to dst
err = dst.PipeE(yaml.Append(elem))
if err != nil {
return nil, err
}
continue
}
v := valueNode.YNode().Value
// We use the key and value from elem to find the corresponding element in dst.
// Then we will use ElementSetter to replace the element with elem. If we cannot
// find the item, the element will be appended.
_, err = dst.Pipe(yaml.ElementSetter{Element: elem, Key: key, Value: v})
if err != nil {
return nil, err
}
}
return dst, nil
}
// non-primitive associative list -- merge the elements
if key != "" {
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{
VisitKeysAsScalars: l.VisitKeysAsScalars,
InferAssociativeLists: l.InferAssociativeLists,
Visitor: l,
Schema: s,
Sources: l.elementValue(key, value),
}.Walk()
if err != nil {
return nil, err
}
if yaml.IsMissingOrNull(val) || yaml.IsEmptyMap(val) {
_, err = dest.Pipe(yaml.ElementSetter{Key: key, Value: value})
if err != nil {
return nil, err
}
continue
}
if val.Field(key) == nil {
// make sure the key is set on the field
_, err = val.Pipe(yaml.SetField(key, yaml.NewScalarRNode(value)))
if err != nil {
return nil, err
}
}
// this handles empty and non-empty values
_, err = dest.Pipe(yaml.ElementSetter{Element: val.YNode(), Key: key, Value: value})
if err != nil {
return nil, err
}
}
// field is empty
if yaml.IsMissingOrNull(dest) {
return nil, nil
}
return dest, nil
}
// primitive associative list -- merge the values
values := l.elementPrimitiveValues()
var s *openapi.ResourceSchema
// setAssociativeSequenceElements recursively set the elements in the list
func (l *Walker) setAssociativeSequenceElements(values []string, key string, dest *yaml.RNode) (*yaml.RNode, error) {
// itemsToBeAdded contains the items that will be added to dest
itemsToBeAdded := yaml.NewListRNode()
var schema *openapi.ResourceSchema
if l.Schema != nil {
s = l.Schema.Elements()
schema = l.Schema.Elements()
}
for _, value := range values {
val, err := Walker{
VisitKeysAsScalars: l.VisitKeysAsScalars,
InferAssociativeLists: l.InferAssociativeLists,
Visitor: l,
Schema: s,
Sources: l.elementValue(key /*empty key implies primitive*/, value),
Schema: schema,
Sources: l.elementValue(key, value),
MergeOptions: l.MergeOptions,
}.Walk()
if err != nil {
return nil, err
}
if yaml.IsMissingOrNull(val) {
// delete the node from **dest** if it's null or empty
if yaml.IsMissingOrNull(val) || yaml.IsEmptyMap(val) {
_, err = dest.Pipe(yaml.ElementSetter{Key: key, Value: value})
if err != nil {
return nil, err
@@ -113,18 +91,60 @@ func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) {
}
}
// this handles empty and non-empty values
_, err = dest.Pipe(yaml.ElementSetter{Element: val.YNode(), Key: key, Value: value})
// Add the val to the sequence. val will replace the item in the sequence if
// there is an item that matches the key-value pair. Otherwise val will be appended
// the the sequence.
_, err = itemsToBeAdded.Pipe(yaml.ElementSetter{Element: val.YNode(), Key: key, Value: value})
if err != nil {
return nil, err
}
}
var err error
if l.MergeOptions.ListIncreaseDirection == yaml.MergeOptionsListPrepend {
// items from patches are needed to be prepended. so we append the
// dest to itemsToBeAdded
dest, err = appendListNode(itemsToBeAdded, dest, key)
} else {
// append the items
dest, err = appendListNode(dest, itemsToBeAdded, key)
}
if err != nil {
return nil, err
}
// sequence is empty
if yaml.IsMissingOrNull(dest) {
return nil, nil
}
return dest, nil
}
func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) {
// may require initializing the dest node
dest, err := l.Sources.setDestNode(l.VisitList(l.Sources, l.Schema, AssociativeList))
if dest == nil || err != nil {
return nil, err
}
// get the merge key from schema
var key, strategy string
if l.Schema != nil {
strategy, key = l.Schema.PatchStrategyAndKey()
}
if strategy == "" && key == "" { // neither strategy nor not present in the schema -- infer the key
// find the list of elements we need to recursively walk
key, err = l.elementKey()
if err != nil {
return nil, err
}
}
// field is empty
if yaml.IsMissingOrNull(dest) {
return nil, nil
if key != "" {
// non-primitive associative list -- merge the elements
return l.setAssociativeSequenceElements(l.elementValues(key), key, dest)
}
return dest, nil
// primitive associative list -- merge the values
return l.setAssociativeSequenceElements(l.elementPrimitiveValues(), key, dest)
}
// elementKey returns the merge key to use for the associative list
@@ -154,17 +174,23 @@ func (l Walker) elementKey() (string, error) {
// elements missing from earlier sources appear later.
func (l Walker) elementValues(key string) []string {
// use slice to to keep elements in the original order
// dest node must be first
var returnValues []string
seen := sets.String{}
// if we are doing append, dest node should be the first.
// otherwise dest node should be the last.
beginIdx := 0
if l.MergeOptions.ListIncreaseDirection == yaml.MergeOptionsListPrepend {
beginIdx = 1
}
for i := range l.Sources {
if l.Sources[i] == nil {
src := l.Sources[(i+beginIdx)%len(l.Sources)]
if src == nil {
continue
}
// add the value of the field for each element
// don't check error, we know this is a list node
values, _ := l.Sources[i].ElementValues(key)
values, _ := src.ElementValues(key)
for _, s := range values {
if seen.Has(s) {
continue
@@ -177,21 +203,25 @@ func (l Walker) elementValues(key string) []string {
}
// elementPrimitiveValues returns the primitive values in an associative list -- eg. finalizers
// TODO: figure out the right order -- currently the order is deterministic but may be improved
// upon.
func (l Walker) elementPrimitiveValues() []string {
// use slice to to keep elements in the original order
// dest node must be first
var returnValues []string
seen := sets.String{}
// if we are doing append, dest node should be the first.
// otherwise dest node should be the last.
beginIdx := 0
if l.MergeOptions.ListIncreaseDirection == yaml.MergeOptionsListPrepend {
beginIdx = 1
}
for i := range l.Sources {
if l.Sources[i] == nil {
src := l.Sources[(i+beginIdx)%len(l.Sources)]
if src == nil {
continue
}
// add the value of the field for each element
// don't check error, we know this is a list node
for _, item := range l.Sources[i].YNode().Content {
for _, item := range src.YNode().Content {
if seen.Has(item.Value) {
continue
}

View File

@@ -68,6 +68,7 @@ func (l Walker) walkMap() (*yaml.RNode, error) {
Visitor: l,
Schema: s,
Sources: fv,
MergeOptions: l.MergeOptions,
Path: append(l.Path, key)}.Walk()
if err != nil {
return nil, err

View File

@@ -36,8 +36,12 @@ type Walker struct {
// VisitKeysAsScalars if true will call VisitScalar on map entry keys,
// providing nil as the OpenAPI schema.
VisitKeysAsScalars bool
// MergeOptions is a struct to store options for merge
MergeOptions yaml.MergeOptions
}
// Kind returns the kind of the first non-null node in Sources.
func (l Walker) Kind() yaml.Kind {
for _, s := range l.Sources {
if !yaml.IsMissingOrNull(s) {
@@ -47,7 +51,8 @@ func (l Walker) Kind() yaml.Kind {
return 0
}
// GrepFilter implements yaml.GrepFilter
// Walk will recursively traverse every item in the Sources and perform corresponding
// actions on them
func (l Walker) Walk() (*yaml.RNode, error) {
l.Schema = l.GetSchema()
@@ -62,6 +67,8 @@ func (l Walker) Walk() (*yaml.RNode, error) {
if err := yaml.ErrorIfAnyInvalidAndNonNull(yaml.SequenceNode, l.Sources...); err != nil {
return nil, err
}
// AssociativeSequence means the items in the sequence are associative. They can be merged
// according to merge key.
if schema.IsAssociative(l.Schema, l.Sources, l.InferAssociativeLists) {
return l.walkAssociativeSequence()
}
@@ -129,6 +136,8 @@ const (
UpdatedIndex
)
// Sources are a list of RNodes. First item is the dest node, followed by
// multiple source nodes.
type Sources []*yaml.RNode
// Dest returns the destination node
@@ -175,29 +184,3 @@ func (s Sources) setDestNode(node *yaml.RNode, err error) (*yaml.RNode, error) {
s[0] = node
return node, nil
}
type FieldSources []*yaml.MapNode
// Dest returns the destination node
func (s FieldSources) Dest() *yaml.MapNode {
if len(s) <= DestIndex {
return nil
}
return s[DestIndex]
}
// Origin returns the origin node
func (s FieldSources) Origin() *yaml.MapNode {
if len(s) <= OriginIndex {
return nil
}
return s[OriginIndex]
}
// Updated returns the updated node
func (s FieldSources) Updated() *yaml.MapNode {
if len(s) <= UpdatedIndex {
return nil
}
return s[UpdatedIndex]
}

View File

@@ -7,4 +7,6 @@ require (
sigs.k8s.io/yaml v1.2.0
)
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../kyaml
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../api

View File

@@ -5,3 +5,5 @@ go 1.14
require sigs.k8s.io/kustomize/api v0.6.2
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../kyaml

View File

@@ -8,3 +8,5 @@ require (
)
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../kyaml

View File

@@ -321,13 +321,13 @@ spec:
old-label: old-value
spec:
containers:
- image: nginx
name: nginx
- args:
- proxy
- sidecar
image: docker.io/istio/proxyv2
name: istio-proxy
- image: nginx
name: nginx
---
apiVersion: apps/v1
kind: Deployment
@@ -747,10 +747,7 @@ spec:
name: test-deployment
ports:
- containerPort: 8080
name: take-over-the-world
protocol: TCP
- containerPort: 8080
name: take-over-the-world
name: disappearing-act
protocol: TCP
`)
}

View File

@@ -4,9 +4,9 @@ go 1.14
require (
github.com/evanphx/json-patch v4.5.0+incompatible
github.com/pkg/errors v0.8.1
sigs.k8s.io/kustomize/api v0.6.2
sigs.k8s.io/kustomize/kyaml v0.8.1
github.com/pkg/errors v0.8.1
sigs.k8s.io/yaml v1.2.0
)

View File

@@ -8,3 +8,5 @@ require (
)
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../kyaml

View File

@@ -5,3 +5,5 @@ go 1.14
require sigs.k8s.io/kustomize/api v0.6.2
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -5,3 +5,5 @@ go 1.14
require sigs.k8s.io/kustomize/api v0.6.2
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -9,3 +9,5 @@ require (
)
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -5,3 +5,5 @@ go 1.14
require sigs.k8s.io/kustomize/api v0.6.2
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -5,3 +5,5 @@ go 1.14
require sigs.k8s.io/kustomize/api v0.6.2
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -8,3 +8,5 @@ require (
)
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -8,3 +8,5 @@ require (
)
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -5,3 +5,5 @@ go 1.14
require sigs.k8s.io/kustomize/api v0.6.2
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -8,3 +8,5 @@ require (
)
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -9,3 +9,5 @@ require (
)
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml

View File

@@ -5,3 +5,5 @@ go 1.14
require sigs.k8s.io/kustomize/api v0.6.2
replace sigs.k8s.io/kustomize/api v0.6.2 => ../../../../api
replace sigs.k8s.io/kustomize/kyaml v0.8.1 => ../../../../kyaml