From 7e59e8e484c9cad3e629e10716f0d8a6e772a87c Mon Sep 17 00:00:00 2001
From: Shuting Zhao <shutting06@gmail.com>
Date: Wed, 15 Jan 2020 21:46:58 -0800
Subject: [PATCH] mutation failure to not block resource creation

---
 pkg/engine/mutate/overlay.go |  1 -
 pkg/webhooks/common.go       |  1 +
 pkg/webhooks/mutation.go     | 24 ++++++++++++++++--------
 pkg/webhooks/server.go       | 16 ++++------------
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/pkg/engine/mutate/overlay.go b/pkg/engine/mutate/overlay.go
index 59d1f50865..3c93e33b27 100644
--- a/pkg/engine/mutate/overlay.go
+++ b/pkg/engine/mutate/overlay.go
@@ -374,7 +374,6 @@ func processSubtree(overlay interface{}, path string, op string) ([]byte, error)
 	// check the patch
 	_, err := jsonpatch.DecodePatch([]byte("[" + patchStr + "]"))
 	if err != nil {
-		glog.V(3).Info(err)
 		return nil, fmt.Errorf("Failed to make '%s' patch from an overlay '%s' for path %s, err: %v", op, value, path, err)
 	}
 
diff --git a/pkg/webhooks/common.go b/pkg/webhooks/common.go
index d8c57ed7f2..d2c1a9720f 100644
--- a/pkg/webhooks/common.go
+++ b/pkg/webhooks/common.go
@@ -13,6 +13,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
+// isResponseSuccesful return true if all responses are successful
 func isResponseSuccesful(engineReponses []response.EngineResponse) bool {
 	for _, er := range engineReponses {
 		if !er.IsSuccesful() {
diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go
index f10289eced..db64a1b6d1 100644
--- a/pkg/webhooks/mutation.go
+++ b/pkg/webhooks/mutation.go
@@ -15,7 +15,8 @@ import (
 )
 
 // HandleMutation handles mutating webhook admission request
-func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resource unstructured.Unstructured, policies []kyverno.ClusterPolicy, roles, clusterRoles []string) (bool, []byte, string) {
+// return generated patches
+func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resource unstructured.Unstructured, policies []kyverno.ClusterPolicy, roles, clusterRoles []string) []byte {
 	glog.V(4).Infof("Receive request in mutating webhook: Kind=%s, Namespace=%s Name=%s UID=%s patchOperation=%s",
 		request.Kind.Kind, request.Namespace, request.Name, request.UID, request.Operation)
 
@@ -108,13 +109,20 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou
 	events := generateEvents(engineResponses, (request.Operation == v1beta1.Update))
 	ws.eventGen.Add(events...)
 
-	if isResponseSuccesful(engineResponses) {
-		sendStat(false)
-		patch := engineutils.JoinPatches(patches)
-		return true, patch, ""
+	sendStat(false)
+
+	// debug info
+	if len(patches) != 0 {
+		glog.V(3).Infof("Patches generated for %s/%s/%s, operation=%v:\n %v",
+			resource.GetKind(), resource.GetNamespace(), resource.GetName(), request.Operation, string(engineutils.JoinPatches(patches)))
 	}
 
-	sendStat(true)
-	glog.Errorf("Failed to mutate the resource, %s\n", getErrorMsg(engineResponses))
-	return false, nil, getErrorMsg(engineResponses)
+	// if any of the policies fails, print out the error
+	if !isResponseSuccesful(engineResponses) {
+		glog.Errorf("Failed to mutate the resource, report as violation: %s\n", getErrorMsg(engineResponses))
+	}
+
+	// patches holds all the successful patches
+	// if no patch is created, it returns nil
+	return engineutils.JoinPatches(patches)
 }
diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go
index 1e7c1eb8d8..c9704ff160 100644
--- a/pkg/webhooks/server.go
+++ b/pkg/webhooks/server.go
@@ -235,23 +235,15 @@ func (ws *WebhookServer) handleAdmissionRequest(request *v1beta1.AdmissionReques
 	}
 
 	// MUTATION
-	ok, patches, msg := ws.HandleMutation(request, resource, policies, roles, clusterRoles)
-	if !ok {
-		glog.V(4).Infof("Deny admission request:  %v/%s/%s", request.Kind, request.Namespace, request.Name)
-		return &v1beta1.AdmissionResponse{
-			Allowed: false,
-			Result: &metav1.Status{
-				Status:  "Failure",
-				Message: msg,
-			},
-		}
-	}
+	// mutation failure should not block the resource creation
+	// any mutation failure is reported as the violation
+	patches := ws.HandleMutation(request, resource, policies, roles, clusterRoles)
 
 	// patch the resource with patches before handling validation rules
 	patchedResource := processResourceWithPatches(patches, request.Object.Raw)
 
 	// VALIDATION
-	ok, msg = ws.HandleValidation(request, policies, patchedResource, roles, clusterRoles)
+	ok, msg := ws.HandleValidation(request, policies, patchedResource, roles, clusterRoles)
 	if !ok {
 		glog.V(4).Infof("Deny admission request: %v/%s/%s", request.Kind, request.Namespace, request.Name)
 		return &v1beta1.AdmissionResponse{