Merge pull request #72751 from zhouhaibing089/no-internal-error-for-failure-webhook

webhook: respect the status error from webhook

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

@@ -72,9 +72,10 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr *generic.Version
 				continue
 			}
 			klog.Warningf("Failed calling webhook, failing closed %v: %v", hook.Name, err)
-		}
 			return apierrors.NewInternalError(err)
 		}
+		return err
+	}
 
 	// convert attr.VersionedObject to the internal version in the underlying admission.Attributes
 	if attr.VersionedObject != nil {

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

@@ -93,8 +93,12 @@ func TestAdmit(t *testing.T) {
 				t.Errorf("%s: expected an error saying %q, but got: %v", tt.Name, tt.ErrorContains, err)
 			}
 		}
-		if _, isStatusErr := err.(*errors.StatusError); err != nil && !isStatusErr {
+		if statusErr, isStatusErr := err.(*errors.StatusError); err != nil && !isStatusErr {
 			t.Errorf("%s: expected a StatusError, got %T", tt.Name, err)
+		} else if isStatusErr {
+			if statusErr.ErrStatus.Code != tt.ExpectStatusCode {
+				t.Errorf("%s: expected status code %d, got %d", tt.Name, tt.ExpectStatusCode, statusErr.ErrStatus.Code)
+			}
 		}
 		fakeAttr, ok := attr.(*webhooktesting.FakeAttributes)
 		if !ok {

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

@@ -17,6 +17,7 @@ limitations under the License.
 package testing
 
 import (
+	"net/http"
 	"net/url"
 	"sync"
 
@@ -191,6 +192,7 @@ type Test struct {
 	ExpectAllow       bool
 	ErrorContains     string
 	ExpectAnnotations map[string]string
+	ExpectStatusCode  int32
 }
 
 // NewNonMutatingTestCases returns test cases with a given base url.
@@ -237,6 +239,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 				NamespaceSelector:       &metav1.LabelSelector{},
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
+			ExpectStatusCode: http.StatusForbidden,
 			ErrorContains:    "without explanation",
 		},
 		{
@@ -248,7 +251,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 				NamespaceSelector:       &metav1.LabelSelector{},
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
-
+			ExpectStatusCode: http.StatusForbidden,
 			ErrorContains:    "you shall not pass",
 		},
 		{
@@ -334,6 +337,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 				Rules:                   matchEverythingRules,
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
+			ExpectStatusCode: http.StatusInternalServerError,
 			ExpectAllow:      false,
 		},
 		{
@@ -360,6 +364,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 				FailurePolicy:           &policyFail,
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
+			ExpectStatusCode: http.StatusInternalServerError,
 			ExpectAllow:      false,
 		},
 		{
@@ -383,6 +388,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 				NamespaceSelector:       &metav1.LabelSelector{},
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
+			ExpectStatusCode: http.StatusForbidden,
 			ErrorContains:    "without explanation",
 		}, {
 			Name: "absent response and fail open",
@@ -406,6 +412,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 				NamespaceSelector:       &metav1.LabelSelector{},
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
+			ExpectStatusCode: http.StatusInternalServerError,
 			ErrorContains:    "Webhook response was absent",
 		},
 		{
@@ -434,6 +441,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
 			IsDryRun:         true,
+			ExpectStatusCode: http.StatusBadRequest,
 			ErrorContains:    "does not support dry run",
 		},
 		{
@@ -461,6 +469,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
 			IsDryRun:         true,
+			ExpectStatusCode: http.StatusBadRequest,
 			ErrorContains:    "does not support dry run",
 		},
 		{
@@ -561,6 +570,7 @@ func NewMutatingTestCases(url *url.URL) []Test {
 				NamespaceSelector:       &metav1.LabelSelector{},
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
+			ExpectStatusCode: http.StatusInternalServerError,
 			ErrorContains:    "invalid character",
 		},
 		{
@@ -574,6 +584,7 @@ func NewMutatingTestCases(url *url.URL) []Test {
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
 			IsDryRun:         true,
+			ExpectStatusCode: http.StatusBadRequest,
 			ErrorContains:    "does not support dry run",
 		},
 		// No need to test everything with the url case, since only the

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

@@ -66,6 +66,9 @@ func webhookHandler(w http.ResponseWriter, r *http.Request) {
 		json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{
 			Response: &v1beta1.AdmissionResponse{
 				Allowed: false,
+				Result: &metav1.Status{
+					Code: http.StatusForbidden,
+				},
 			},
 		})
 	case "/disallowReason":
@@ -75,6 +78,7 @@ func webhookHandler(w http.ResponseWriter, r *http.Request) {
 				Allowed: false,
 				Result: &metav1.Status{
 					Message: "you shall not pass",
+					Code:    http.StatusForbidden,
 				},
 			},
 		})