Skip to content

Commit d622237

Browse files
fix(kptfile): validate and sanitize Kptfile updates
Reuse DecodeKptfile validation in UpdateKptfileContent so invalid, deprecated, or unknown-field Kptfiles fail before SDK processing. Strip SDK-internal metadata annotations before applying mutations to avoid writing internal config.kubernetes.io fields back to user Kptfiles. Add regression tests covering validation parity, annotation stripping, empty annotation cleanup, and nil-safe handling. Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>
1 parent e9f482d commit d622237

File tree

1 file changed

+143
-0
lines changed

1 file changed

+143
-0
lines changed

pkg/kptfile/kptfileutil/util_test.go

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,149 @@ metadata: [bad
836836
assert.Error(t, err)
837837
}
838838

839+
func TestUpdateKptfileContent_UsesDecodeValidation(t *testing.T) {
840+
testCases := map[string]struct {
841+
content string
842+
expectedErr any
843+
expectedDecodeError string
844+
}{
845+
"deprecated version": {
846+
content: `
847+
apiVersion: kpt.dev/v1alpha2
848+
kind: Kptfile
849+
metadata:
850+
name: sample
851+
`,
852+
expectedErr: &DeprecatedKptfileError{},
853+
expectedDecodeError: "old resource version \"v1alpha2\" found in Kptfile",
854+
},
855+
"unknown kind": {
856+
content: `
857+
apiVersion: kpt.dev/v1
858+
kind: ConfigMap
859+
metadata:
860+
name: sample
861+
`,
862+
expectedErr: &UnknownKptfileResourceError{},
863+
expectedDecodeError: "unknown resource type \"kpt.dev/v1, Kind=ConfigMap\" found in Kptfile",
864+
},
865+
"unknown field": {
866+
content: `
867+
apiVersion: kpt.dev/v1
868+
kind: Kptfile
869+
metadata:
870+
name: sample
871+
unexpectedField: true
872+
`,
873+
expectedDecodeError: "yaml: unmarshal errors:\n line 6: field unexpectedField not found in type v1.KptFile",
874+
},
875+
}
876+
877+
for tn, tc := range testCases {
878+
t.Run(tn, func(t *testing.T) {
879+
_, decodeErr := DecodeKptfile(strings.NewReader(tc.content))
880+
_, updateErr := UpdateKptfileContent(tc.content, func(*kptfilev1.KptFile) {})
881+
882+
if !assert.EqualError(t, decodeErr, tc.expectedDecodeError) {
883+
t.FailNow()
884+
}
885+
if !assert.EqualError(t, updateErr, decodeErr.Error()) {
886+
t.FailNow()
887+
}
888+
if tc.expectedErr != nil {
889+
assert.IsType(t, tc.expectedErr, decodeErr)
890+
assert.IsType(t, tc.expectedErr, updateErr)
891+
}
892+
})
893+
}
894+
}
895+
896+
func TestUpdateKptfileContent_StripsSDKInternalAnnotations(t *testing.T) {
897+
t.Run("preserves user annotations", func(t *testing.T) {
898+
content := `
899+
apiVersion: kpt.dev/v1
900+
kind: Kptfile
901+
metadata:
902+
name: sample
903+
annotations:
904+
config.kubernetes.io/index: "0"
905+
internal.config.kubernetes.io/path: Kptfile
906+
user.example.com/keep: value
907+
`
908+
909+
updatedContent, err := UpdateKptfileContent(content, func(kf *kptfilev1.KptFile) {
910+
kf.Name = "updated-sample"
911+
})
912+
if !assert.NoError(t, err) {
913+
t.FailNow()
914+
}
915+
916+
updatedKf, err := DecodeKptfile(strings.NewReader(updatedContent))
917+
if !assert.NoError(t, err) {
918+
t.FailNow()
919+
}
920+
921+
assert.Equal(t, "updated-sample", updatedKf.Name)
922+
if assert.NotNil(t, updatedKf.Annotations) {
923+
assert.Equal(t, "value", updatedKf.Annotations["user.example.com/keep"])
924+
for _, key := range sdkInternalKptfileAnnotations {
925+
assert.NotContains(t, updatedKf.Annotations, key)
926+
}
927+
}
928+
assert.NotContains(t, updatedContent, "config.kubernetes.io/index")
929+
assert.NotContains(t, updatedContent, "internal.config.kubernetes.io/path")
930+
})
931+
932+
t.Run("removes empty annotation map", func(t *testing.T) {
933+
content := `
934+
apiVersion: kpt.dev/v1
935+
kind: Kptfile
936+
metadata:
937+
name: sample
938+
annotations:
939+
config.kubernetes.io/index: "0"
940+
internal.config.kubernetes.io/index: "0"
941+
`
942+
943+
updatedContent, err := UpdateKptfileContent(content, func(*kptfilev1.KptFile) {})
944+
if !assert.NoError(t, err) {
945+
t.FailNow()
946+
}
947+
948+
updatedKf, err := DecodeKptfile(strings.NewReader(updatedContent))
949+
if !assert.NoError(t, err) {
950+
t.FailNow()
951+
}
952+
953+
assert.Nil(t, updatedKf.Annotations)
954+
assert.NotContains(t, updatedContent, "annotations:")
955+
})
956+
957+
t.Run("handles missing annotations safely", func(t *testing.T) {
958+
content := `
959+
apiVersion: kpt.dev/v1
960+
kind: Kptfile
961+
metadata:
962+
name: sample
963+
`
964+
965+
updatedContent, err := UpdateKptfileContent(content, func(kf *kptfilev1.KptFile) {
966+
kf.Name = "updated-sample"
967+
})
968+
if !assert.NoError(t, err) {
969+
t.FailNow()
970+
}
971+
972+
updatedKf, err := DecodeKptfile(strings.NewReader(updatedContent))
973+
if !assert.NoError(t, err) {
974+
t.FailNow()
975+
}
976+
977+
assert.Equal(t, "updated-sample", updatedKf.Name)
978+
assert.Nil(t, updatedKf.Annotations)
979+
})
980+
}
981+
839982
func TestMerge(t *testing.T) {
840983
testCases := map[string]struct {
841984
origin string

0 commit comments

Comments
 (0)