Use a zero-change patch to do find scheme caused fields removal.

Author: trasc

pkg/webhook/admission/defaulter_custom.go

@@ -21,11 +21,14 @@ import (
 "encoding/json"
 "errors"
 "net/http"
+"slices"
 
+"gomodules.xyz/jsonpatch/v2"
 admissionv1 "k8s.io/api/admission/v1"
 apierrors "k8s.io/apimachinery/pkg/api/errors"
 metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 "k8s.io/apimachinery/pkg/runtime"
+"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // CustomDefaulter defines functions for setting defaults on resources.
@@ -71,32 +74,59 @@ func (h *defaulterForType) Handle(ctx context.Context, req Request) Response {
 ctx = NewContextWithRequest(ctx, req)
 
 // Get the object in the request
-original := h.object.DeepCopyObject()
-if err := h.decoder.Decode(req, original); err != nil {
+obj := h.object.DeepCopyObject()
+if err := h.decoder.Decode(req, obj); err != nil {
 return Errored(http.StatusBadRequest, err)
 }
 
+// Keep a copy of the object
+originalObj := obj.DeepCopyObject()
+
 // Default the object
-updated := original.DeepCopyObject()
-if err := h.defaulter.Default(ctx, updated); err != nil {
+if err := h.defaulter.Default(ctx, obj); err != nil {
 var apiStatus apierrors.APIStatus
 if errors.As(err, &apiStatus) {
 return validationResponseFromStatus(false, apiStatus.Status())
 }
 return Denied(err.Error())
 }
 
-// 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)
+// Create the patch
+marshalled, err := json.Marshal(obj)
 if err != nil {
 return Errored(http.StatusInternalServerError, err)
 }
-marshalledUpdated, err := json.Marshal(updated)
+handlerResponse := PatchResponseFromRaw(req.Object.Raw, marshalled)
+
+return h.dropSchemeRemovals(handlerResponse, originalObj, req.Object.Raw)
+}
+
+func (h *defaulterForType) dropSchemeRemovals(r Response, original runtime.Object, raw []byte) Response {
+const opRemove = "remove"
+if !r.Allowed || r.PatchType == nil {
+return r
+}
+
+// If we don't have removals in the patch.
+if !slices.ContainsFunc(r.Patches, func(o jsonpatch.JsonPatchOperation) bool { return o.Operation == opRemove }) {
+return r
+}
+
+// Get the raw to original patch
+marshalledOriginal, err := json.Marshal(original)
 if err != nil {
 return Errored(http.StatusInternalServerError, err)
 }
-return PatchResponseFromRaw(marshalledOriginal, marshalledUpdated)
+
+patchOriginal := PatchResponseFromRaw(raw, marshalledOriginal).Patches
+removedByScheme := sets.New(slices.DeleteFunc(patchOriginal, func(p jsonpatch.JsonPatchOperation) bool { return p.Operation != opRemove })...)
+
+r.Patches = slices.DeleteFunc(r.Patches, func(p jsonpatch.JsonPatchOperation) bool {
+return removedByScheme.Has(p)
+})
+
+if len(r.Patches) == 0 {
+r.PatchType = nil
+}
+return r
 }

pkg/webhook/admission/defaulter_custom_test.go

@@ -29,24 +29,30 @@ import (
 
 var _ = Describe("Defaulter Handler", func() {
 
-It("should should not lose unknown fields", func() {
+It("should not lose unknown fields", func() {
 obj := &TestDefaulter{}
 handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{})
 
 resp := handler.Handle(context.TODO(), Request{
 AdmissionRequest: admissionv1.AdmissionRequest{
 Operation: admissionv1.Create,
 Object: runtime.RawExtension{
-Raw: []byte(`{"newField":"foo"}`),
+Raw: []byte(`{"newField":"foo", "totalReplicas":5}`),
 },
 },
 })
 Expect(resp.Allowed).Should(BeTrue())
-Expect(resp.Patches).To(Equal([]jsonpatch.JsonPatchOperation{{
-Operation: "add",
-Path: "/replica",
-Value: 2.0,
-}}))
+Expect(resp.Patches).To(Equal([]jsonpatch.JsonPatchOperation{
+{
+Operation: "add",
+Path: "/replica",
+Value: 2.0,
+},
+{
+Operation: "remove",
+Path: "/totalReplicas",
+},
+}))
 Expect(resp.Result.Code).Should(Equal(int32(http.StatusOK)))
 })
 
@@ -70,15 +76,17 @@ var _ = Describe("Defaulter Handler", func() {
 var _ runtime.Object = &TestDefaulter{}
 
 type TestDefaulter struct {
-Replica int `json:"replica,omitempty"`
+Replica int `json:"replica,omitempty"`
+TotalReplicas int `json:"totalReplicas,omitempty"`
 }
 
 var testDefaulterGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestDefaulter"}
 
 func (d *TestDefaulter) GetObjectKind() schema.ObjectKind { return d }
 func (d *TestDefaulter) DeepCopyObject() runtime.Object {
 return &TestDefaulter{
-Replica: d.Replica,
+Replica: d.Replica,
+TotalReplicas: d.TotalReplicas,
 }
 }
 
@@ -103,5 +111,6 @@ func (d *TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) e
 if o.Replica < 2 {
 o.Replica = 2
 }
+o.TotalReplicas = 0
 return nil
 }