From 8a06d9be64ad847232bd52aa45c04829ee67f894 Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Wed, 17 Apr 2019 17:33:41 -0700
Subject: [PATCH] :sparkles: use more reasonable status code for
 ValidationResponse

---
 pkg/builder/build_test.go              |  2 +-
 pkg/webhook/admission/response.go      | 11 ++++++++---
 pkg/webhook/admission/response_test.go | 24 ++++++++++++++++++++++--
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/pkg/builder/build_test.go b/pkg/builder/build_test.go
index a65890afa9..241f51c3f1 100644
--- 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
index 7f9b544712..bb625ed0d3 100644
--- 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
index 8e2dfde48e..eb7ec832b6 100644
--- 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,
+						},
 					},
 				},
 			))