From 5c09e2b09984fe7c4d424a69a281e7d08859aa2c Mon Sep 17 00:00:00 2001
From: Aldo Culquicondor <acondor@google.com>
Date: Thu, 22 Aug 2024 20:39:05 +0000
Subject: [PATCH] Fix custom defaulter from deleting unknown fields

---
 pkg/webhook/admission/defaulter_custom.go     | 20 +++--
 .../admission/defaulter_custom_test.go        | 87 +++++++++++++++++++
 2 files changed, 101 insertions(+), 6 deletions(-)
 create mode 100644 pkg/webhook/admission/defaulter_custom_test.go

diff --git a/pkg/webhook/admission/defaulter_custom.go b/pkg/webhook/admission/defaulter_custom.go
index d15dec7a05..5687676527 100644
--- a/pkg/webhook/admission/defaulter_custom.go
+++ b/pkg/webhook/admission/defaulter_custom.go
@@ -71,13 +71,14 @@ 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())
@@ -85,10 +86,17 @@ func (h *defaulterForType) Handle(ctx context.Context, req Request) Response {
 		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)
 }
diff --git a/pkg/webhook/admission/defaulter_custom_test.go b/pkg/webhook/admission/defaulter_custom_test.go
new file mode 100644
index 0000000000..204cfb19af
--- /dev/null
+++ b/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 }