Skip to content

Commit 4ee4c1d

Browse files
committed
Parse x-kubernetes-list-type in CRD as the key of strategic merge patch (#1)
* Add testcases to reproduce #5878, testing CRD OpenApi extensions * Add x-kubernetes-list-type CRD extension support
1 parent 7558804 commit 4ee4c1d

File tree

12 files changed

+337
-51
lines changed

12 files changed

+337
-51
lines changed

examples/customOpenAPIschema.md

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,24 @@ spec:
3737
EOF
3838
```
3939

40-
This resource has an image field. Let's change its value from server
41-
to nginx with a patch.
40+
This resource has an image field. Let's change its value from server
41+
to nginx with a patch.
4242

4343
Kustomize gets its merge key information from the OpenAPI data
4444
provided by the kubernetes API server. It doesn't have information
45-
about custom resources, so we will have to provide our own
46-
schema file.
45+
about custom resources, so we will have to provide our own
46+
schema file.
4747

4848
Note: CRDs support declarative validation using an OpenAPI v3 schema.
4949
See https://book.kubebuilder.io/reference/generating-crd.html#validation.
5050

5151
You can get an OpenAPI document like this by fetching the OpenAPI
5252
document from your locally favored cluster with the command
5353
`kustomize openapi fetch`. Kustomize will use the OpenAPI extensions
54-
`x-kubernetes-patch-merge-key` and `x-kubernetes-patch-strategy` to
55-
perform a strategic merge. `x-kubernetes-patch-strategy` should be set
56-
to "merge", and you can set your merge key to whatever you like. Below,
57-
our custom resource inherits merge keys from `PodTemplateSpec`.
54+
`x-kubernetes-list-map-keys` and `x-kubernetes-list-type` to
55+
perform a strategic merge. `x-kubernetes-list-type` should be set
56+
to "map", and you can set your merge keys to whatever you like. Below,
57+
our custom resource inherits merge keys from `PodTemplateSpec`.
5858

5959
<!-- @addCustomSchema @testAgainstLatestRelease -->
6060
```
@@ -124,6 +124,10 @@ cat <<EOF >>$DEMO_HOME/mycr_schema.json
124124
"\$ref": "#/definitions/io.k8s.api.core.v1.Container"
125125
},
126126
"type": "array",
127+
"x-kubernetes-list-map-keys": [
128+
"name"
129+
],
130+
"x-kubernetes-list-type": "map",
127131
"x-kubernetes-patch-merge-key": "name",
128132
"x-kubernetes-patch-strategy": "merge"
129133
}

go.work.sum

Lines changed: 179 additions & 4 deletions
Large diffs are not rendered by default.

kyaml/fn/framework/parser/testdata/schema1.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
"properties": {
2424
"targets": {
2525
"type": "array",
26-
"x-kubernetes-patch-merge-key": "app",
27-
"x-kubernetes-patch-strategy": "merge",
26+
"x-kubernetes-list-map-keys": [
27+
"app"
28+
],
29+
"x-kubernetes-list-type": "map",
2830
"items": {
2931
"type": "object",
3032
"required": [

kyaml/fn/framework/parser/testdata/schema2.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
"properties": {
2424
"targets": {
2525
"type": "array",
26-
"x-kubernetes-patch-merge-key": "app",
27-
"x-kubernetes-patch-strategy": "merge",
26+
"x-kubernetes-list-map-keys": [
27+
"app"
28+
],
29+
"x-kubernetes-list-type": "map",
2830
"items": {
2931
"type": "object",
3032
"required": [

kyaml/fn/framework/testdata/template-processor/schemas/foo.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
"properties": {
2424
"targets": {
2525
"type": "array",
26-
"x-kubernetes-patch-merge-key": "app",
27-
"x-kubernetes-patch-strategy": "merge",
26+
"x-kubernetes-list-map-keys": [
27+
"app"
28+
],
29+
"x-kubernetes-list-type": "map",
2830
"items": {
2931
"type": "object",
3032
"required": [

kyaml/openapi/example_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ func Example_arrayMerge() {
2828
f := s.Lookup("spec", "template", "spec", "containers")
2929
fmt.Println(f.Schema.Description[:70] + "...")
3030
fmt.Println(f.Schema.Type)
31-
fmt.Println(f.PatchStrategyAndKey()) // merge patch strategy on name
31+
fmt.Println(f.PatchStrategyAndKeyList()) // merge patch strategy on name
3232

3333
// Output:
3434
// List of containers belonging to the pod. Containers cannot currently b...
3535
// [array]
36-
// merge name
36+
// merge [name]
3737
}
3838

3939
func Example_arrayReplace() {
@@ -42,11 +42,12 @@ func Example_arrayReplace() {
4242
f := s.Lookup("spec", "template", "spec", "containers", openapi.Elements, "args")
4343
fmt.Println(f.Schema.Description[:70] + "...")
4444
fmt.Println(f.Schema.Type)
45-
fmt.Println(f.PatchStrategyAndKey()) // no patch strategy or merge key
45+
fmt.Println(f.PatchStrategyAndKeyList()) // no patch strategy or merge keys; empty string + empty list
4646

4747
// Output:
4848
// Arguments to the entrypoint. The docker image's CMD is used if this is...
4949
// [array]
50+
// []
5051
}
5152

5253
func Example_arrayElement() {

kyaml/openapi/kustomizationapi/swagger.json

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@
7070
"$ref": "#/definitions/io.k8s.api.apps.v1.ConfigMapArgs"
7171
},
7272
"type": "array",
73+
"x-kubernetes-list-map-keys": [
74+
"name"
75+
],
76+
"x-kubernetes-list-type": "map",
7377
"x-kubernetes-patch-merge-key": "name",
7478
"x-kubernetes-patch-strategy": "merge"
7579
},
@@ -78,6 +82,10 @@
7882
"$ref": "#/definitions/io.k8s.api.apps.v1.SecretArgs"
7983
},
8084
"type": "array",
85+
"x-kubernetes-list-map-keys": [
86+
"name"
87+
],
88+
"x-kubernetes-list-type": "map",
8189
"x-kubernetes-patch-merge-key": "name",
8290
"x-kubernetes-patch-strategy": "merge"
8391
}
@@ -98,7 +106,8 @@
98106
"items": {
99107
"type": "string"
100108
},
101-
"type": "array"
109+
"type": "array",
110+
"x-kubernetes-list-type": "atomic"
102111
},
103112
"files": {
104113
"items": {
@@ -110,7 +119,8 @@
110119
"items": {
111120
"type": "string"
112121
},
113-
"type": "array"
122+
"type": "array",
123+
"x-kubernetes-list-type": "atomic"
114124
},
115125
"env": {
116126
"type": "string"
@@ -127,4 +137,4 @@
127137
]
128138
}
129139
}
130-
}
140+
}

kyaml/openapi/openapi.go

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -536,42 +536,35 @@ func (rs *ResourceSchema) Field(field string) *ResourceSchema {
536536

537537
// PatchStrategyAndKeyList returns the patch strategy and complete merge key list
538538
func (rs *ResourceSchema) PatchStrategyAndKeyList() (string, []string) {
539-
ps, found := rs.Schema.Extensions[kubernetesPatchStrategyExtensionKey]
540-
if !found {
541-
// empty patch strategy
542-
return "", []string{}
543-
}
544-
mkList, found := rs.Schema.Extensions[kubernetesMergeKeyMapList]
539+
// Try CRD extension first
540+
lt, found := rs.Schema.Extensions[kubernetesListTypeExtensionKey]
545541
if found {
542+
mkList, foundKeys := rs.Schema.Extensions[kubernetesMergeKeyMapList]
543+
if !foundKeys {
544+
// no mergeKey -- may be "atomic" or "set" list
545+
return lt.(string), []string{}
546+
}
547+
546548
// mkList is []interface, convert to []string
547549
mkListStr := make([]string, len(mkList.([]interface{})))
548550
for i, v := range mkList.([]interface{}) {
549551
mkListStr[i] = v.(string)
550552
}
551-
return ps.(string), mkListStr
553+
return lt.(string), mkListStr
552554
}
553-
mk, found := rs.Schema.Extensions[kubernetesMergeKeyExtensionKey]
554-
if !found {
555-
// no mergeKey -- may be a primitive associative list (e.g. finalizers)
556-
return ps.(string), []string{}
557-
}
558-
return ps.(string), []string{mk.(string)}
559-
}
560555

561-
// PatchStrategyAndKey returns the patch strategy and merge key extensions
562-
func (rs *ResourceSchema) PatchStrategyAndKey() (string, string) {
556+
// Fall back to k8s builtin extension
563557
ps, found := rs.Schema.Extensions[kubernetesPatchStrategyExtensionKey]
564558
if !found {
565559
// empty patch strategy
566-
return "", ""
560+
return "", []string{}
567561
}
568-
569562
mk, found := rs.Schema.Extensions[kubernetesMergeKeyExtensionKey]
570563
if !found {
571564
// no mergeKey -- may be a primitive associative list (e.g. finalizers)
572-
mk = ""
565+
return ps.(string), []string{}
573566
}
574-
return ps.(string), mk.(string)
567+
return ps.(string), []string{mk.(string)}
575568
}
576569

577570
const (
@@ -599,6 +592,10 @@ const (
599592
// -- the extension is an array of strings
600593
kubernetesMergeKeyMapList = "x-kubernetes-list-map-keys"
601594

595+
// kubernetesListTypeExtensionKey is the key to lookup the kubernetes list
596+
// type in CRD -- the extension is a strings
597+
kubernetesListTypeExtensionKey = "x-kubernetes-list-type"
598+
602599
// groupKey is the key to lookup the group from the GVK extension
603600
groupKey = "group"
604601
// versionKey is the key to lookup the version from the GVK extension

kyaml/yaml/merge2/element_test.go

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ spec:
560560
//
561561
// Test Case
562562
//
563-
{description: `no infer merge keys merge using explicit schema as line comment'`,
563+
{description: `no infer merge keys merge using explicit schema with builtin extension as line comment'`,
564564
source: `
565565
apiVersion: custom
566566
kind: Deployment
@@ -579,6 +579,37 @@ containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"t
579579
apiVersion: custom
580580
kind: Deployment
581581
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-patch-merge-key":"name","x-kubernetes-patch-strategy": "merge"}
582+
- name: foo
583+
image: foo:bar
584+
command: ['run2.sh']
585+
`,
586+
infer: false,
587+
mergeOptions: yaml.MergeOptions{
588+
ListIncreaseDirection: yaml.MergeOptionsListAppend,
589+
},
590+
},
591+
//
592+
// Test Case
593+
//
594+
{description: `no infer merge keys merge using explicit schema with crd extension as line comment'`,
595+
source: `
596+
apiVersion: custom
597+
kind: Deployment
598+
containers:
599+
- name: foo
600+
command: ['run2.sh']
601+
`,
602+
dest: `
603+
apiVersion: custom
604+
kind: Deployment
605+
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-list-map-keys":["name"],"x-kubernetes-list-type": "map"}
606+
- name: foo # hell ow
607+
image: foo:bar
608+
`,
609+
expected: `
610+
apiVersion: custom
611+
kind: Deployment
612+
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-list-map-keys":["name"],"x-kubernetes-list-type": "map"}
582613
- name: foo
583614
image: foo:bar
584615
command: ['run2.sh']
@@ -596,7 +627,7 @@ kind: Deployment
596627
metadata:
597628
finalizers:
598629
- a
599-
- b
630+
- b
600631
`,
601632
dest: `
602633
apiVersion: apps/v1

kyaml/yaml/merge3/element_test.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ spec:
742742
// Test Case
743743
//
744744
{
745-
description: `no infer merge keys merge using explicit schema as line comment'`,
745+
description: `no infer merge keys merge using explicit schema with builtin extension as line comment'`,
746746
origin: `
747747
apiVersion: custom
748748
kind: Deployment
@@ -786,6 +786,54 @@ spec:
786786
infer: false,
787787
},
788788

789+
//
790+
// Test Case
791+
//
792+
{
793+
description: `no infer merge keys merge using explicit schema with crd extension as line comment'`,
794+
origin: `
795+
apiVersion: custom
796+
kind: Deployment
797+
spec:
798+
template:
799+
spec:
800+
containers:
801+
- name: foo
802+
`,
803+
update: `
804+
apiVersion: custom
805+
kind: Deployment
806+
spec:
807+
template:
808+
spec:
809+
containers:
810+
- name: foo
811+
command: ['run2.sh']
812+
`,
813+
local: `
814+
apiVersion: custom
815+
kind: Deployment
816+
spec:
817+
template:
818+
spec:
819+
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-list-map-keys":["name"],"x-kubernetes-list-type": "map"}
820+
- name: foo # hell ow
821+
image: foo:bar
822+
`,
823+
expected: `
824+
apiVersion: custom
825+
kind: Deployment
826+
spec:
827+
template:
828+
spec:
829+
containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"type":"array","x-kubernetes-list-map-keys":["name"],"x-kubernetes-list-type": "map"}
830+
- name: foo # hell ow
831+
image: foo:bar
832+
command: ['run2.sh']
833+
`,
834+
infer: false,
835+
},
836+
789837
//
790838
// Test Case
791839
//

0 commit comments

Comments
 (0)