✨ use more reasonable status code for ValidationResponse

kubernetes-sigs · May 6, 2019 · 8a06d9b · 8a06d9b
1 parent 8c0b805
commit 8a06d9b
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 6 deletions.
diff --git a/pkg/builder/build_test.go b/pkg/builder/build_test.go
@@ -271,7 +271,7 @@ var _ = Describe("application", func() {
 			Expect(w.Code).To(Equal(http.StatusOK))
 			By("sanity checking the response contains reasonable field")
 			Expect(w.Body).To(ContainSubstring(`"allowed":false`))
-			Expect(w.Body).To(ContainSubstring(`"code":200`))
+			Expect(w.Body).To(ContainSubstring(`"code":403`))
 		})
 
 		It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() {

diff --git a/pkg/webhook/admission/response.go b/pkg/webhook/admission/response.go
@@ -62,15 +62,20 @@ func Errored(code int32, err error) Response {
 
 // ValidationResponse returns a response for admitting a request.
 func ValidationResponse(allowed bool, reason string) Response {
+	code := http.StatusForbidden
+	if allowed {
+		code = http.StatusOK
+	}
 	resp := Response{
 		AdmissionResponse: admissionv1beta1.AdmissionResponse{
 			Allowed: allowed,
+			Result: &metav1.Status{
+				Code: int32(code),
+			},
 		},
 	}
 	if len(reason) > 0 {
-		resp.Result = &metav1.Status{
-			Reason: metav1.StatusReason(reason),
-		}
+		resp.Result.Reason = metav1.StatusReason(reason)
 	}
 	return resp
 }

diff --git a/pkg/webhook/admission/response_test.go b/pkg/webhook/admission/response_test.go
@@ -35,6 +35,9 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 				Response{
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: true,
+						Result: &metav1.Status{
+							Code: http.StatusOK,
+						},
 					},
 				},
 			))
@@ -46,6 +49,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: true,
 						Result: &metav1.Status{
+							Code:   http.StatusOK,
 							Reason: "acceptable",
 						},
 					},
@@ -60,6 +64,9 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 				Response{
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: false,
+						Result: &metav1.Status{
+							Code: http.StatusForbidden,
+						},
 					},
 				},
 			))
@@ -71,6 +78,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: false,
 						Result: &metav1.Status{
+							Code:   http.StatusForbidden,
 							Reason: "UNACCEPTABLE!",
 						},
 					},
@@ -96,6 +104,9 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 				Response{
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: true,
+						Result: &metav1.Status{
+							Code: http.StatusOK,
+						},
 					},
 					Patches: ops,
 				},
@@ -107,6 +118,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: true,
 						Result: &metav1.Status{
+							Code:   http.StatusOK,
 							Reason: "some changes",
 						},
 					},
@@ -141,6 +153,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: true,
 						Result: &metav1.Status{
+							Code:   http.StatusOK,
 							Reason: "acceptable",
 						},
 					},
@@ -153,6 +166,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: false,
 						Result: &metav1.Status{
+							Code:   http.StatusForbidden,
 							Reason: "UNACCEPTABLE!",
 						},
 					},
@@ -161,20 +175,26 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
 		})
 
 		It("should return an admission decision", func() {
-			By("checking that it returns a 'denied' response when allowed is false")
+			By("checking that it returns an 'allowed' response when allowed is true")
 			Expect(ValidationResponse(true, "")).To(Equal(
 				Response{
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: true,
+						Result: &metav1.Status{
+							Code: http.StatusOK,
+						},
 					},
 				},
 			))
 
-			By("checking that it returns an 'allowed' response when allowed is true")
+			By("checking that it returns an 'denied' response when allowed is false")
 			Expect(ValidationResponse(false, "")).To(Equal(
 				Response{
 					AdmissionResponse: admissionv1beta1.AdmissionResponse{
 						Allowed: false,
+						Result: &metav1.Status{
+							Code: http.StatusForbidden,
+						},
 					},
 				},
 			))