Skip to content

Commit 169fdd7

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #5196 from ephesused/issue4928-append-honors-key-style
fix: patch additions honor source key style
2 parents 76f8d28 + 78b8139 commit 169fdd7

File tree

5 files changed

+246
-5
lines changed

5 files changed

+246
-5
lines changed

‎api/filters/patchstrategicmerge/patchstrategicmerge_test.go

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,133 @@ spec:
776776
autoscaling: true
777777
deepgram-api:
778778
some: value
779+
`,
780+
},
781+
782+
// Issue #4928
783+
"support numeric keys": {
784+
input: `
785+
apiVersion: v1
786+
kind: ConfigMap
787+
metadata:
788+
name: blabla
789+
namespace: blabla-ns
790+
data:
791+
"6443": "foobar"
792+
`,
793+
patch: yaml.MustParse(`
794+
apiVersion: v1
795+
kind: ConfigMap
796+
metadata:
797+
name: blabla
798+
namespace: blabla-ns
799+
data:
800+
"6443": "barfoo"
801+
"9110": "foo-foo"
802+
`),
803+
expected: `
804+
apiVersion: v1
805+
kind: ConfigMap
806+
metadata:
807+
name: blabla
808+
namespace: blabla-ns
809+
data:
810+
"6443": "barfoo"
811+
"9110": "foo-foo"
812+
`,
813+
},
814+
815+
"honor different key style one": {
816+
input: `
817+
apiVersion: v1
818+
kind: ConfigMap
819+
metadata:
820+
name: blabla
821+
namespace: blabla-ns
822+
data:
823+
'6443': "foobar"
824+
`,
825+
patch: yaml.MustParse(`
826+
apiVersion: v1
827+
kind: ConfigMap
828+
metadata:
829+
name: blabla
830+
namespace: blabla-ns
831+
data:
832+
"6443": "barfoo"
833+
9110: "foo-foo"
834+
`),
835+
expected: `
836+
apiVersion: v1
837+
kind: ConfigMap
838+
metadata:
839+
name: blabla
840+
namespace: blabla-ns
841+
data:
842+
'6443': "barfoo"
843+
9110: "foo-foo"
844+
`,
845+
},
846+
847+
"honor different key style two": {
848+
input: `
849+
apiVersion: v1
850+
kind: ConfigMap
851+
metadata:
852+
name: blabla
853+
namespace: blabla-ns
854+
data:
855+
"6443": "foobar"
856+
`,
857+
patch: yaml.MustParse(`
858+
apiVersion: v1
859+
kind: ConfigMap
860+
metadata:
861+
name: blabla
862+
namespace: blabla-ns
863+
data:
864+
"6443": "barfoo"
865+
'9110': "foo-foo"
866+
`),
867+
expected: `
868+
apiVersion: v1
869+
kind: ConfigMap
870+
metadata:
871+
name: blabla
872+
namespace: blabla-ns
873+
data:
874+
"6443": "barfoo"
875+
'9110': "foo-foo"
876+
`,
877+
},
878+
879+
"different key types": {
880+
input: `
881+
apiVersion: v1
882+
kind: ConfigMap
883+
metadata:
884+
name: blabla
885+
namespace: blabla-ns
886+
data:
887+
"6443": "key-string-double-quoted"
888+
`,
889+
patch: yaml.MustParse(`
890+
apiVersion: v1
891+
kind: ConfigMap
892+
metadata:
893+
name: blabla
894+
namespace: blabla-ns
895+
data:
896+
6443: "key-int"
897+
`),
898+
expected: `
899+
apiVersion: v1
900+
kind: ConfigMap
901+
metadata:
902+
name: blabla
903+
namespace: blabla-ns
904+
data:
905+
"6443": "key-int"
779906
`,
780907
},
781908
}

‎kyaml/yaml/fns.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,10 @@ type FieldSetter struct {
690690
// when setting it. Otherwise, if an existing node is found, the style is
691691
// retained.
692692
OverrideStyle bool `yaml:"overrideStyle,omitempty"`
693+
694+
// AppendKeyStyle defines the style of the key when no existing node is
695+
// found, and a new node is appended.
696+
AppendKeyStyle Style `yaml:"appendKeyStyle,omitempty"`
693697
}
694698

695699
func (s FieldSetter) Filter(rn *RNode) (*RNode, error) {
@@ -747,6 +751,7 @@ func (s FieldSetter) Filter(rn *RNode) (*RNode, error) {
747751
&yaml.Node{
748752
Kind: yaml.ScalarNode,
749753
Value: s.Name,
754+
Style: s.AppendKeyStyle,
750755
HeadComment: s.Comments.HeadComment,
751756
LineComment: s.Comments.LineComment,
752757
FootComment: s.Comments.FootComment,

‎kyaml/yaml/merge2/map_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,47 @@ spec:
417417
protocol: TCP
418418
- port: 30901
419419
targetPort: 30901
420+
`,
421+
mergeOptions: yaml.MergeOptions{
422+
ListIncreaseDirection: yaml.MergeOptionsListAppend,
423+
},
424+
},
425+
426+
//
427+
// Test Case
428+
//
429+
{description: `Verify key style behavior`,
430+
source: `
431+
kind: Deployment
432+
spec:
433+
source-and-dest: source-and-dest
434+
source-only: source-only
435+
"source-only-key-double-quoted": source-only
436+
source-and-dest-key-style-diff-1: source-and-dest
437+
'source-and-dest-key-style-diff-2': source-and-dest
438+
"source-and-dest-key-style-diff-3": source-and-dest
439+
`,
440+
dest: `
441+
kind: Deployment
442+
spec:
443+
source-and-dest: source-and-dest
444+
'source-and-dest-key-style-diff-1': source-and-dest
445+
"source-and-dest-key-style-diff-2": source-and-dest
446+
source-and-dest-key-style-diff-3: source-and-dest
447+
dest-only: dest-only
448+
'dest-only-key-single-quoted': dest-only
449+
`,
450+
expected: `
451+
kind: Deployment
452+
spec:
453+
source-and-dest: source-and-dest
454+
'source-and-dest-key-style-diff-1': source-and-dest
455+
"source-and-dest-key-style-diff-2": source-and-dest
456+
source-and-dest-key-style-diff-3: source-and-dest
457+
dest-only: dest-only
458+
'dest-only-key-single-quoted': dest-only
459+
source-only: source-only
460+
"source-only-key-double-quoted": source-only
420461
`,
421462
mergeOptions: yaml.MergeOptions{
422463
ListIncreaseDirection: yaml.MergeOptionsListAppend,

‎kyaml/yaml/merge3/map_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,4 +306,61 @@ metadata:
306306
name: foo
307307
annotations: {}
308308
`},
309+
310+
//
311+
// Test Case
312+
//
313+
{
314+
description: `Verify key style behavior`,
315+
origin: `
316+
apiVersion: v1
317+
kind: ConfigMap
318+
metadata:
319+
name: foo
320+
data:
321+
unchanged: origin
322+
unchanged-varying-key-style: origin
323+
deleted-in-update: origin
324+
deleted-in-local: origin
325+
`,
326+
update: `
327+
apiVersion: v1
328+
kind: ConfigMap
329+
metadata:
330+
name: foo
331+
data:
332+
unchanged: origin
333+
'unchanged-varying-key-style': origin
334+
deleted-in-local: origin
335+
'added-in-update': 'update'
336+
'added-in-update-and-local-same-value': 'update-and-local'
337+
'added-in-update-and-local-diff-value': 'update'
338+
`,
339+
local: `
340+
apiVersion: v1
341+
kind: ConfigMap
342+
metadata:
343+
name: foo
344+
data:
345+
unchanged: origin
346+
"unchanged-varying-key-style": origin
347+
deleted-in-update: origin
348+
"added-in-local": "local"
349+
"added-in-update-and-local-same-value": "update-and-local"
350+
"added-in-update-and-local-diff-value": "local"
351+
`,
352+
expected: `
353+
apiVersion: v1
354+
kind: ConfigMap
355+
metadata:
356+
name: foo
357+
data:
358+
unchanged: origin
359+
"unchanged-varying-key-style": origin
360+
"added-in-local": "local"
361+
"added-in-update-and-local-same-value": "update-and-local"
362+
"added-in-update-and-local-diff-value": "update"
363+
'added-in-update': 'update'
364+
`,
365+
},
309366
}

‎kyaml/yaml/walk/map.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (l Walker) walkMap() (*yaml.RNode, error) {
5858
if l.Schema != nil {
5959
s = l.Schema.Field(key)
6060
}
61-
fv, commentSch := l.fieldValue(key)
61+
fv, commentSch, keyStyles := l.fieldValue(key)
6262
if commentSch != nil {
6363
s = commentSch
6464
}
@@ -90,7 +90,13 @@ func (l Walker) walkMap() (*yaml.RNode, error) {
9090
}
9191

9292
// this handles empty and non-empty values
93-
_, err = dest.Pipe(yaml.FieldSetter{Name: key, Comments: comments, Value: val})
93+
fieldSetter := yaml.FieldSetter{
94+
Name: key,
95+
Comments: comments,
96+
AppendKeyStyle: keyStyles[val],
97+
Value: val,
98+
}
99+
_, err = dest.Pipe(fieldSetter)
94100
if err != nil {
95101
return nil, err
96102
}
@@ -153,10 +159,12 @@ func (l Walker) fieldNames() []string {
153159
return result
154160
}
155161

156-
// fieldValue returns a slice containing each source's value for fieldName
157-
func (l Walker) fieldValue(fieldName string) ([]*yaml.RNode, *openapi.ResourceSchema) {
162+
// fieldValue returns a slice containing each source's value for fieldName, the
163+
// schema, and a map of each source's value to the style for the source's key.
164+
func (l Walker) fieldValue(fieldName string) ([]*yaml.RNode, *openapi.ResourceSchema, map[*yaml.RNode]yaml.Style) {
158165
var fields []*yaml.RNode
159166
var sch *openapi.ResourceSchema
167+
keyStyles := make(map[*yaml.RNode]yaml.Style, len(l.Sources))
160168
for i := range l.Sources {
161169
if l.Sources[i] == nil {
162170
fields = append(fields, nil)
@@ -165,9 +173,12 @@ func (l Walker) fieldValue(fieldName string) ([]*yaml.RNode, *openapi.ResourceSc
165173
field := l.Sources[i].Field(fieldName)
166174
f, s := l.valueIfPresent(field)
167175
fields = append(fields, f)
176+
if field != nil && field.Key != nil && field.Key.YNode() != nil {
177+
keyStyles[f] = field.Key.YNode().Style
178+
}
168179
if sch == nil && !s.IsMissingOrNull() {
169180
sch = s
170181
}
171182
}
172-
return fields, sch
183+
return fields, sch, keyStyles
173184
}

0 commit comments

Comments
 (0)