Skip to content

Commit 7a3bab8

Browse files
authored
fix: preserve Secret stringData during generator merge (#6237)
* test: reproduce secretGenerator stringData merge failure * fix: preserve Secret stringData during generator merge
1 parent 3f00f0a commit 7a3bab8

3 files changed

Lines changed: 214 additions & 1 deletion

File tree

api/krusty/generatormergeandreplace_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,42 @@ metadata:
453453
`)
454454
}
455455

456+
// Regression test for https://github.com/kubernetes-sigs/kustomize/issues/5955
457+
func TestSecretGeneratorMergeStringData(t *testing.T) {
458+
th := kusttest_test.MakeHarness(t)
459+
th.WriteK(".", `
460+
resources:
461+
- secret.yaml
462+
secretGenerator:
463+
- name: test
464+
behavior: merge
465+
literals:
466+
- property2=value2
467+
options:
468+
disableNameSuffixHash: false
469+
`)
470+
th.WriteF("secret.yaml", `
471+
apiVersion: v1
472+
kind: Secret
473+
metadata:
474+
name: test
475+
type: Opaque
476+
stringData:
477+
property1: value1
478+
`)
479+
m := th.Run(".", th.MakeDefaultOptions())
480+
th.AssertActualEqualsExpected(m, `
481+
apiVersion: v1
482+
data:
483+
property1: dmFsdWUx
484+
property2: dmFsdWUy
485+
kind: Secret
486+
metadata:
487+
name: test
488+
type: Opaque
489+
`)
490+
}
491+
456492
func TestMergeAndReplaceDisableNameSuffixHashGenerators(t *testing.T) {
457493
th := kusttest_test.MakeHarness(t)
458494
th.WriteK("app", `

api/resmap/reswrangler.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,8 +588,15 @@ func (m *resWrangler) appendReplaceOrMerge(res *resource.Resource) error {
588588
if err != nil {
589589
return err
590590
}
591+
oldForData := old
592+
if res.GetApiVersion() == "v1" && res.GetKind() == "Secret" {
593+
oldForData, err = normalizeSecretStringData(old)
594+
if err != nil {
595+
return fmt.Errorf("failed to normalize stringData for %s: %w", id, err)
596+
}
597+
}
591598
res.CopyMergeMetaDataFieldsFrom(old)
592-
res.MergeDataMapFrom(old)
599+
res.MergeDataMapFrom(oldForData)
593600
res.MergeBinaryDataMapFrom(old)
594601
if orig != nil {
595602
res.SetOrigin(orig)
@@ -614,6 +621,38 @@ func (m *resWrangler) appendReplaceOrMerge(res *resource.Resource) error {
614621
}
615622
}
616623

624+
// normalizeSecretStringData returns a copy of res with stringData encoded into
625+
// data, matching the normalization performed by the Kubernetes API server.
626+
// Values in stringData take precedence over values with the same key in data.
627+
func normalizeSecretStringData(res *resource.Resource) (*resource.Resource, error) {
628+
normalized := res.DeepCopy()
629+
stringData, err := normalized.Pipe(kyaml.Lookup("stringData"))
630+
if err != nil {
631+
return nil, fmt.Errorf("lookup stringData: %w", err)
632+
}
633+
if !kyaml.IsMissingOrNull(stringData) {
634+
values := map[string]string{}
635+
if err := stringData.VisitFields(func(node *kyaml.MapNode) error {
636+
key := kyaml.GetValue(node.Key)
637+
value := node.Value
638+
if value == nil || value.YNode() == nil || value.YNode().Kind != kyaml.ScalarNode {
639+
return fmt.Errorf("stringData value for key %q must be a scalar", key)
640+
}
641+
values[key] = kyaml.GetValue(value)
642+
return nil
643+
}); err != nil {
644+
return nil, fmt.Errorf("read stringData: %w", err)
645+
}
646+
if err := normalized.LoadMapIntoSecretData(values); err != nil {
647+
return nil, fmt.Errorf("encode stringData: %w", err)
648+
}
649+
}
650+
if err := normalized.PipeE(kyaml.Clear("stringData")); err != nil {
651+
return nil, fmt.Errorf("clear stringData: %w", err)
652+
}
653+
return normalized, nil
654+
}
655+
617656
// AnnotateAll implements ResMap
618657
func (m *resWrangler) AnnotateAll(key string, value string) error {
619658
return m.ApplyFilter(annotations.Filter{

api/resmap/reswrangler_test.go

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -990,6 +990,144 @@ func TestAbsorbAll(t *testing.T) {
990990
t, strings.Contains(err.Error(), "behavior must be merge or replace"))
991991
}
992992

993+
func TestAbsorbAllMergeSecretStringData(t *testing.T) {
994+
tests := []struct {
995+
name string
996+
existing string
997+
incoming string
998+
expected string
999+
}{
1000+
{
1001+
name: "core v1 Secret normalizes stringData",
1002+
existing: `
1003+
apiVersion: v1
1004+
data:
1005+
dataOnly: ZGF0YQ==
1006+
generatorWins: b2xkLWRhdGE=
1007+
stringWins: b2xkLWRhdGE=
1008+
kind: Secret
1009+
metadata:
1010+
name: test
1011+
stringData:
1012+
generatorWins: string
1013+
long: abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
1014+
multiline: |
1015+
line one
1016+
line two
1017+
stringOnly: string
1018+
stringWins: string
1019+
type: Opaque
1020+
`,
1021+
incoming: `
1022+
apiVersion: v1
1023+
data:
1024+
generatedOnly: Z2VuZXJhdGVk
1025+
generatorWins: Z2VuZXJhdGVk
1026+
kind: Secret
1027+
metadata:
1028+
name: test
1029+
type: Opaque
1030+
`,
1031+
expected: `
1032+
apiVersion: v1
1033+
data:
1034+
dataOnly: ZGF0YQ==
1035+
generatedOnly: Z2VuZXJhdGVk
1036+
generatorWins: Z2VuZXJhdGVk
1037+
long: |
1038+
YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXpBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWj
1039+
AxMjM0NTY3ODk=
1040+
multiline: bGluZSBvbmUKbGluZSB0d28K
1041+
stringOnly: c3RyaW5n
1042+
stringWins: c3RyaW5n
1043+
kind: Secret
1044+
metadata:
1045+
name: test
1046+
type: Opaque
1047+
`,
1048+
},
1049+
{
1050+
name: "custom resource does not normalize stringData",
1051+
existing: `
1052+
apiVersion: example.com/v1
1053+
data:
1054+
oldOnly: b2xk
1055+
kind: Secret
1056+
metadata:
1057+
name: test
1058+
stringData:
1059+
customOnly: plain
1060+
`,
1061+
incoming: `
1062+
apiVersion: example.com/v1
1063+
data:
1064+
generatedOnly: Z2VuZXJhdGVk
1065+
kind: Secret
1066+
metadata:
1067+
name: test
1068+
`,
1069+
expected: `
1070+
apiVersion: example.com/v1
1071+
data:
1072+
generatedOnly: Z2VuZXJhdGVk
1073+
oldOnly: b2xk
1074+
kind: Secret
1075+
metadata:
1076+
name: test
1077+
`,
1078+
},
1079+
}
1080+
1081+
for _, tc := range tests {
1082+
t.Run(tc.name, func(t *testing.T) {
1083+
existing, err := rmF.NewResMapFromBytes([]byte(tc.existing))
1084+
require.NoError(t, err)
1085+
incoming, err := rmF.NewResMapFromBytes([]byte(tc.incoming))
1086+
require.NoError(t, err)
1087+
incoming.Resources()[0].SetBehavior(types.BehaviorMerge)
1088+
expected, err := rmF.NewResMapFromBytes([]byte(tc.expected))
1089+
require.NoError(t, err)
1090+
1091+
require.NoError(t, existing.AbsorbAll(incoming))
1092+
existing.RemoveBuildAnnotations()
1093+
require.NoError(t, expected.ErrorIfNotEqualLists(existing))
1094+
})
1095+
}
1096+
}
1097+
1098+
func TestAbsorbAllRejectsNonScalarSecretStringData(t *testing.T) {
1099+
existing, err := rmF.NewResMapFromBytes([]byte(`
1100+
apiVersion: v1
1101+
kind: Secret
1102+
metadata:
1103+
name: test
1104+
stringData:
1105+
invalid:
1106+
nested: value
1107+
type: Opaque
1108+
`))
1109+
require.NoError(t, err)
1110+
incoming, err := rmF.NewResMapFromBytes([]byte(`
1111+
apiVersion: v1
1112+
data:
1113+
generated: Z2VuZXJhdGVk
1114+
kind: Secret
1115+
metadata:
1116+
name: test
1117+
type: Opaque
1118+
`))
1119+
require.NoError(t, err)
1120+
incoming.Resources()[0].SetBehavior(types.BehaviorMerge)
1121+
before, err := existing.AsYaml()
1122+
require.NoError(t, err)
1123+
1124+
err = existing.AbsorbAll(incoming)
1125+
require.ErrorContains(t, err, `stringData value for key "invalid" must be a scalar`)
1126+
after, yamlErr := existing.AsYaml()
1127+
require.NoError(t, yamlErr)
1128+
assert.Equal(t, before, after)
1129+
}
1130+
9931131
func TestToRNodeSlice(t *testing.T) {
9941132
input := `apiVersion: rbac.authorization.k8s.io/v1
9951133
kind: ClusterRole

0 commit comments

Comments
 (0)