Skip to content

Commit f9589b9

Browse files
authored
Merge pull request kubernetes-sigs#3469 from k8s-infra-cherrypick-robot/cherry-pick-3468-to-release-0.23
[release-0.23] 🐛 Ensure DefaulterRemoveUnknownOrOmitableFields is still working even if objects are equal
2 parents 8122a62 + 25615ad commit f9589b9

File tree

2 files changed

+27
-2
lines changed

2 files changed

+27
-2
lines changed

pkg/webhook/admission/defaulter_custom.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,9 @@ func (h *defaulterForType[T]) Handle(ctx context.Context, req Request) Response
145145
// * json.Marshal that we use below sorts fields alphabetically
146146
// * for builtin types the apiserver also sorts alphabetically (but it seems like it adds an empty line at the end)
147147
// * for CRDs the apiserver uses the field order in the OpenAPI schema which very likely is not alphabetically sorted
148-
if reflect.DeepEqual(originalObj, obj) {
148+
// Note: If removeUnknownOrOmitableFields is set we have to compute a patch to remove unknown or omitable fields even
149+
// if the objects are equal
150+
if !h.removeUnknownOrOmitableFields && reflect.DeepEqual(originalObj, obj) {
149151
return Response{
150152
AdmissionResponse: admissionv1.AdmissionResponse{
151153
Allowed: true,

pkg/webhook/admission/defaulter_custom_test.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import (
3737

3838
var _ = Describe("Defaulter Handler", func() {
3939

40-
It("should remove unknown fields when DefaulterRemoveUnknownFields is passed", func(ctx SpecContext) {
40+
It("should remove unknown fields when DefaulterRemoveUnknownFields is passed and some fields are defaulted", func(ctx SpecContext) {
4141
obj := &TestDefaulter{}
4242
handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{}, DefaulterRemoveUnknownOrOmitableFields)
4343

@@ -74,6 +74,29 @@ var _ = Describe("Defaulter Handler", func() {
7474
Expect(resp.Result.Code).Should(Equal(int32(http.StatusOK)))
7575
})
7676

77+
It("should remove unknown fields when DefaulterRemoveUnknownFields is passed and no fields are defaulted", func(ctx SpecContext) {
78+
obj := &TestDefaulter{}
79+
handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{}, DefaulterRemoveUnknownOrOmitableFields)
80+
81+
resp := handler.Handle(ctx, Request{
82+
AdmissionRequest: admissionv1.AdmissionRequest{
83+
Operation: admissionv1.Create,
84+
Object: runtime.RawExtension{
85+
Raw: []byte(`{"labels":{"foo": "bar"}, "replica": 2, "totalReplicas":0}`),
86+
},
87+
},
88+
})
89+
Expect(resp.Allowed).Should(BeTrue())
90+
Expect(resp.Patches).To(HaveLen(1))
91+
Expect(resp.Patches).To(ContainElements(
92+
jsonpatch.JsonPatchOperation{
93+
Operation: "remove",
94+
Path: "/totalReplicas",
95+
},
96+
))
97+
Expect(resp.Result.Code).Should(Equal(int32(http.StatusOK)))
98+
})
99+
77100
It("should preserve unknown fields by default", func(ctx SpecContext) {
78101
obj := &TestDefaulter{}
79102
handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{})

0 commit comments

Comments
 (0)