[release-0.23] 🐛 Ensure DefaulterRemoveUnknownOrOmitableFields is still working even if objects are equal

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,

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{})