Fix custom defaulter from deleting unknown fields

Author: alculquicondor

pkg/webhook/admission/defaulter_custom.go

@@ -71,24 +71,32 @@ func (h *defaulterForType) Handle(ctx context.Context, req Request) Response {
 	ctx = NewContextWithRequest(ctx, req)
 
 	// Get the object in the request
-	obj := h.object.DeepCopyObject()
-	if err := h.decoder.Decode(req, obj); err != nil {
+	original := h.object.DeepCopyObject()
+	if err := h.decoder.Decode(req, original); err != nil {
 		return Errored(http.StatusBadRequest, err)
 	}
 
 	// Default the object
-	if err := h.defaulter.Default(ctx, obj); err != nil {
+	updated := original.DeepCopyObject()
+	if err := h.defaulter.Default(ctx, updated); err != nil {
 		var apiStatus apierrors.APIStatus
 		if errors.As(err, &apiStatus) {
 			return validationResponseFromStatus(false, apiStatus.Status())
 		}
 		return Denied(err.Error())
 	}
 
-	// Create the patch
-	marshalled, err := json.Marshal(obj)
+	// Create the patch.
+	// We need to decode and marshall the original because the type registered in the
+	// decoder might not match the latest version of the API.
+	// Creating a diff from the raw object might cause new fields to be dropped.
+	marshalledOriginal, err := json.Marshal(original)
 	if err != nil {
 		return Errored(http.StatusInternalServerError, err)
 	}
-	return PatchResponseFromRaw(req.Object.Raw, marshalled)
+	marshalledUpdated, err := json.Marshal(updated)
+	if err != nil {
+		return Errored(http.StatusInternalServerError, err)
+	}
+	return PatchResponseFromRaw(marshalledOriginal, marshalledUpdated)
 }

pkg/webhook/admission/defaulter_custom_test.go

@@ -0,0 +1,87 @@
+package admission
+
+import (
+	"context"
+	"net/http"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"gomodules.xyz/jsonpatch/v2"
+
+	admissionv1 "k8s.io/api/admission/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+var _ = Describe("CustomDefaulter Handler", func() {
+
+	It("should should not lose unknown fields", func() {
+		obj := &TestObject{}
+		handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{})
+
+		resp := handler.Handle(context.TODO(), Request{
+			AdmissionRequest: admissionv1.AdmissionRequest{
+				Operation: admissionv1.Create,
+				Object: runtime.RawExtension{
+					Raw: []byte(`{"newField":"foo"}`),
+				},
+			},
+		})
+		Expect(resp.Allowed).Should(BeTrue())
+		Expect(resp.Patches).To(Equal([]jsonpatch.JsonPatchOperation{{
+			Operation: "add",
+			Path:      "/replica",
+			Value:     2.0,
+		}}))
+		Expect(resp.Result.Code).Should(Equal(int32(http.StatusOK)))
+	})
+
+	It("should return ok if received delete verb in defaulter handler", func() {
+		obj := &TestObject{}
+		handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{})
+
+		resp := handler.Handle(context.TODO(), Request{
+			AdmissionRequest: admissionv1.AdmissionRequest{
+				Operation: admissionv1.Delete,
+				OldObject: runtime.RawExtension{
+					Raw: []byte("{}"),
+				},
+			},
+		})
+		Expect(resp.Allowed).Should(BeTrue())
+		Expect(resp.Result.Code).Should(Equal(int32(http.StatusOK)))
+	})
+
+})
+
+type TestCustomDefaulter struct{}
+
+func (d *TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+	tObj := obj.(*TestObject)
+	if tObj.Replica < 2 {
+		tObj.Replica = 2
+	}
+	return nil
+}
+
+type TestObject struct {
+	Replica int `json:"replica,omitempty"`
+}
+
+func (o *TestObject) GetObjectKind() schema.ObjectKind { return o }
+func (o *TestObject) DeepCopyObject() runtime.Object {
+	return &TestObject{
+		Replica: o.Replica,
+	}
+}
+
+func (o *TestObject) GroupVersionKind() schema.GroupVersionKind {
+	return testDefaulterGVK
+}
+
+func (o *TestObject) SetGroupVersionKind(gvk schema.GroupVersionKind) {}
+
+type TestObjectList struct{}
+
+func (*TestObjectList) GetObjectKind() schema.ObjectKind { return nil }
+func (*TestObjectList) DeepCopyObject() runtime.Object   { return nil }