fix: block mutation only when failurePolicy is set to fail (#8952) (#8986)

* fix: only block mutation when failurePolicy is set
to fail



* feat: kuttl test



* fix: add else check



* fix: update defaulting ns label policy's failure policy to be fail

based on readme, this test has nothing to do with failurePolicy and resource should not be blocked in case of ignore failurePolicy



* fix: there is another



* fix: update policy



* nit



* feat: add logs



* Update pkg/webhooks/resource/mutation/mutation.go



---------

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Signed-off-by: shuting <shuting@nirmata.com>
Co-authored-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Co-authored-by: shuting <shuting@nirmata.com>
Co-authored-by: shuting <shutting06@gmail.com>

pkg/webhooks/resource/mutation/mutation.go

@@ -82,6 +82,7 @@ func (v *mutationHandler) applyMutations(
 
 	var patches []jsonpatch.JsonPatchOperation
 	var engineResponses []engineapi.EngineResponse
+	failurePolicy := kyvernov1.Ignore
 
 	for _, policy := range policies {
 		spec := policy.GetSpec()
@@ -96,7 +97,11 @@ func (v *mutationHandler) applyMutations(
 			func(ctx context.Context, span trace.Span) error {
 				v.log.V(3).Info("applying policy mutate rules", "policy", policy.GetName())
 				currentContext := policyContext.WithPolicy(policy)
-				engineResponse, policyPatches, err := v.applyMutation(ctx, request, currentContext)
+				if policy.GetSpec().GetFailurePolicy(ctx) == kyvernov1.Fail {
+					failurePolicy = kyvernov1.Fail
+				}
+
+				engineResponse, policyPatches, err := v.applyMutation(ctx, request, currentContext, failurePolicy)
 				if err != nil {
 					return fmt.Errorf("mutation policy %s error: %v", policy.GetName(), err)
 				}
@@ -131,7 +136,7 @@ func (v *mutationHandler) applyMutations(
 	return jsonutils.JoinPatches(patch.ConvertPatches(patches...)...), engineResponses, nil
 }
 
-func (h *mutationHandler) applyMutation(ctx context.Context, request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext) (*engineapi.EngineResponse, []jsonpatch.JsonPatchOperation, error) {
+func (h *mutationHandler) applyMutation(ctx context.Context, request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext, failurePolicy kyvernov1.FailurePolicyType) (*engineapi.EngineResponse, []jsonpatch.JsonPatchOperation, error) {
 	if request.Kind.Kind != "Namespace" && request.Namespace != "" {
 		policyContext = policyContext.WithNamespaceLabels(engineutils.GetNamespaceSelectorsFromNamespaceLister(request.Kind.Kind, request.Namespace, h.nsLister, h.log))
 	}
@@ -140,7 +145,13 @@ func (h *mutationHandler) applyMutation(ctx context.Context, request admissionv1
 	policyPatches := engineResponse.GetPatches()
 
 	if !engineResponse.IsSuccessful() {
-		return nil, nil, fmt.Errorf("failed to apply policy %s rules %v", policyContext.Policy().GetName(), engineResponse.GetFailedRulesWithErrors())
+		if webhookutils.BlockRequest([]engineapi.EngineResponse{engineResponse}, failurePolicy, h.log) {
+			h.log.Info("failed to apply policy, blocking request", "policy", policyContext.Policy().GetName(), "rules", engineResponse.GetFailedRulesWithErrors())
+			return nil, nil, fmt.Errorf("failed to apply policy %s rules %v", policyContext.Policy().GetName(), engineResponse.GetFailedRulesWithErrors())
+		} else {
+			h.log.Info("ignoring unsuccessful engine responses", "policy", policyContext.Policy().GetName(), "rules", engineResponse.GetFailedRulesWithErrors())
+			return &engineResponse, nil, nil
+		}
 	}
 
 	return &engineResponse, policyPatches, nil

test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/01-manifests.yaml

@@ -3,7 +3,7 @@ kind: ClusterPolicy
 metadata:
   name: propagate-cost-labels-from-namespace
 spec:
-  failurePolicy: Ignore
+  failurePolicy: Fail
   rules:
   - name: add-cost-labels
     context:

test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/04-manifests.yaml

@@ -3,7 +3,7 @@ kind: ClusterPolicy
 metadata:
   name: propagate-cost-labels-from-namespace
 spec:
-  failurePolicy: Ignore
+  failurePolicy: Fail
   rules:
   - name: add-cost-labels
     context:

test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/01-policy.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- policy.yaml
+assert:
+- policy-assert.yaml

test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/02-pod.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- pod.yaml
+assert:
+- pod-assert.yaml

test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/README.md

@@ -0,0 +1,11 @@
+## Description
+
+This test checks that the mutate policy does not fail because of 404 in API Call when failure policy is set to `Ignore`.
+
+## Expected Behavior
+
+The failure policy in the policy is set to Ignore and the API Call refers to a non existent URL. Mutation should not happen and error should not be thrown. 
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/8936

test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod-assert.yaml

@@ -0,0 +1,5 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: mutate-404-api-call-example
+  namespace: default

test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: mutate-404-api-call-example
+  namespace: default
+spec:
+  containers:
+  - name: example
+    image: busybox
+    args: ["sleep", "infinity"]

test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy-assert.yaml

@@ -0,0 +1,4 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: mutate-404-api-call

test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy.yaml

@@ -0,0 +1,23 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: mutate-404-api-call
+spec:
+  failurePolicy: Ignore
+  rules:
+  - name: mutate-404-api-call
+    context:
+    - name: val
+      apiCall:
+        service:
+          url: "https://www.google.com/404"
+    match:
+      any:
+      - resources:
+          kinds:
+          - Pod
+    mutate:
+      patchStrategicMerge:
+        metadata:
+          labels:
+            foo: "{{ val }}"