Merge pull request kubernetes#131627 from dims/mutating-webhook-error

[Attempt #2] apiserver: Treat error decoding a mutating webhook patch as error calling the webhook

Author: k8s-ci-robot

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go

@@ -335,7 +335,7 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *admiss
 	}
 	patchObj, err := jsonpatch.DecodePatch(result.Patch)
 	if err != nil {
-		return false, apierrors.NewInternalError(err)
+		return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("received undecodable patch in webhook response: %w", err), Status: apierrors.NewServiceUnavailable("error decoding patch in webhook response")}
 	}
 
 	if len(patchObj) == 0 {

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go

@@ -38,6 +38,7 @@ import (
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	fakeclientset "k8s.io/client-go/kubernetes/fake"
+	"k8s.io/utils/ptr"
 )
 
 var matchEverythingRules = []registrationv1.RuleWithOperations{{
@@ -897,6 +898,40 @@ func NewMutatingTestCases(url *url.URL, configurationName string) []MutatingTest
 				"mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "invalidMutation", false),
 			},
 		},
+		{
+			Name: "match & invalid patch",
+			Webhooks: []registrationv1.MutatingWebhook{{
+				Name:                    "invalidPatch",
+				ClientConfig:            ccfgSVC("invalidPatch"),
+				Rules:                   matchEverythingRules,
+				NamespaceSelector:       &metav1.LabelSelector{},
+				ObjectSelector:          &metav1.LabelSelector{},
+				AdmissionReviewVersions: []string{"v1beta1"},
+			}},
+			ExpectStatusCode: http.StatusInternalServerError,
+			ErrorContains:    "unexpected end of JSON input",
+			ExpectAnnotations: map[string]string{
+				"mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "invalidPatch", false),
+			},
+		},
+		{
+			Name: "match & invalid patch fail open",
+			Webhooks: []registrationv1.MutatingWebhook{{
+				Name:                    "invalidPatch",
+				ClientConfig:            ccfgSVC("invalidPatch"),
+				Rules:                   matchEverythingRules,
+				NamespaceSelector:       &metav1.LabelSelector{},
+				ObjectSelector:          &metav1.LabelSelector{},
+				AdmissionReviewVersions: []string{"v1beta1"},
+				FailurePolicy:           ptr.To(registrationv1.Ignore),
+			}},
+			ExpectAllow:      true,
+			ExpectStatusCode: http.StatusOK,
+			ExpectAnnotations: map[string]string{
+				"failed-open.mutation.webhook.admission.k8s.io/round_0_index_0": "invalidPatch",
+				"mutation.webhook.admission.k8s.io/round_0_index_0":             mutationAnnotationValue(configurationName, "invalidPatch", false),
+			},
+		},
 		{
 			Name: "match & remove label dry run unsupported",
 			Webhooks: []registrationv1.MutatingWebhook{{

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go

@@ -137,6 +137,16 @@ func webhookHandler(w http.ResponseWriter, r *http.Request) {
 				Patch:     []byte(`[{"op": "add", "path": "/metadata/labels/added", "value": "test"}]`),
 			},
 		})
+	case "/invalidPatch":
+		w.Header().Set("Content-Type", "application/json")
+		pt := v1beta1.PatchTypeJSONPatch
+		json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{
+			Response: &v1beta1.AdmissionResponse{
+				Allowed:   true,
+				PatchType: &pt,
+				Patch:     []byte(`[{`),
+			},
+		})
 	case "/invalidMutation":
 		w.Header().Set("Content-Type", "application/json")
 		pt := v1beta1.PatchTypeJSONPatch