From b475b63e10e1e181ff853388a7e0007e911648a4 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Thu, 5 Mar 2026 14:46:26 +0100 Subject: [PATCH] Ensure DefaulterRemoveUnknownOrOmitableFields is still working even if objects are equal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- pkg/webhook/admission/defaulter_custom.go | 4 ++- .../admission/defaulter_custom_test.go | 25 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/pkg/webhook/admission/defaulter_custom.go b/pkg/webhook/admission/defaulter_custom.go index 9a6574bc64..9fec8003f2 100644 --- a/pkg/webhook/admission/defaulter_custom.go +++ b/pkg/webhook/admission/defaulter_custom.go @@ -145,7 +145,9 @@ func (h *defaulterForType[T]) Handle(ctx context.Context, req Request) Response // * json.Marshal that we use below sorts fields alphabetically // * for builtin types the apiserver also sorts alphabetically (but it seems like it adds an empty line at the end) // * for CRDs the apiserver uses the field order in the OpenAPI schema which very likely is not alphabetically sorted - if reflect.DeepEqual(originalObj, obj) { + // Note: If removeUnknownOrOmitableFields is set we have to compute a patch to remove unknown or omitable fields even + // if the objects are equal + if !h.removeUnknownOrOmitableFields && reflect.DeepEqual(originalObj, obj) { return Response{ AdmissionResponse: admissionv1.AdmissionResponse{ Allowed: true, diff --git a/pkg/webhook/admission/defaulter_custom_test.go b/pkg/webhook/admission/defaulter_custom_test.go index 58d360bdd0..8fcae2bed2 100644 --- a/pkg/webhook/admission/defaulter_custom_test.go +++ b/pkg/webhook/admission/defaulter_custom_test.go @@ -37,7 +37,7 @@ import ( var _ = Describe("Defaulter Handler", func() { - It("should remove unknown fields when DefaulterRemoveUnknownFields is passed", func(ctx SpecContext) { + It("should remove unknown fields when DefaulterRemoveUnknownFields is passed and some fields are defaulted", func(ctx SpecContext) { obj := &TestDefaulter{} handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{}, DefaulterRemoveUnknownOrOmitableFields) @@ -74,6 +74,29 @@ var _ = Describe("Defaulter Handler", func() { Expect(resp.Result.Code).Should(Equal(int32(http.StatusOK))) }) + It("should remove unknown fields when DefaulterRemoveUnknownFields is passed and no fields are defaulted", func(ctx SpecContext) { + obj := &TestDefaulter{} + handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{}, DefaulterRemoveUnknownOrOmitableFields) + + resp := handler.Handle(ctx, Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Object: runtime.RawExtension{ + Raw: []byte(`{"labels":{"foo": "bar"}, "replica": 2, "totalReplicas":0}`), + }, + }, + }) + Expect(resp.Allowed).Should(BeTrue()) + Expect(resp.Patches).To(HaveLen(1)) + Expect(resp.Patches).To(ContainElements( + jsonpatch.JsonPatchOperation{ + Operation: "remove", + Path: "/totalReplicas", + }, + )) + Expect(resp.Result.Code).Should(Equal(int32(http.StatusOK))) + }) + It("should preserve unknown fields by default", func(ctx SpecContext) { obj := &TestDefaulter{} handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{})