v0.7.0: Handling empty patch in PatchResponseFromRaw

[tatsuhiro-t]

In admission v1, api server requires that patch and patchType are both provided or none are provided:

https://github.com/kubernetes/apiserver/blob/6d407d18a284aae4db693a8a50ca3647cc3ad429/pkg/admission/plugin/webhook/request/admissionreview.go#L67-L73

admission v1beta1 does not have this kind of requirement.

In controller-runtime, PatchResponseFromRaw sets patchType regardless of existence of patch:

controller-runtime/pkg/webhook/admission/response.go

Line 94 in 5720da0

PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),

So if patch is empty, a response contains only patchType and api server does not like it. Webhook call fails.

To make transition from admission v1beta1 to v1 smooth, it would be nice to set patchType only when patch is not empty, like so:

--- a/pkg/webhook/admission/response.go
+++ b/pkg/webhook/admission/response.go
@@ -91,7 +91,13 @@ func PatchResponseFromRaw(original, current []byte) Response {
                Patches: patches,
                AdmissionResponse: admissionv1.AdmissionResponse{
                        Allowed:   true,
-                       PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),
+                       PatchType: func() *admissionv1.PatchType {
+                               if len(patches) == 0 {
+                                       return nil
+                               }
+                               pt := admissionv1.PatchTypeJSONPatch;
+                               return &pt
+                       }(),
                },
        }
 }

[alvaroaleman]

@tatsuhiro-t can you send a patch for this?

[tatsuhiro-t]

sure.