fix: account for error rules in mutation webhook (kyverno#5264)

* fix: account for error rules in mutation webhook

Signed-off-by: Charles-Edouard Brétéché <[email protected]>

* add test

Signed-off-by: Charles-Edouard Brétéché <[email protected]>

Signed-off-by: Charles-Edouard Brétéché <[email protected]>

pkg/engine/response/response.go

@@ -196,12 +196,12 @@ func (er EngineResponse) GetPatches() [][]byte {
 
 // GetFailedRules returns failed rules
 func (er EngineResponse) GetFailedRules() []string {
-	return er.getRules(RuleStatusFail)
+	return er.getRules(func(status RuleStatus) bool { return status == RuleStatusFail || status == RuleStatusError })
 }
 
 // GetSuccessRules returns success rules
 func (er EngineResponse) GetSuccessRules() []string {
-	return er.getRules(RuleStatusPass)
+	return er.getRules(func(status RuleStatus) bool { return status == RuleStatusPass })
 }
 
 // GetResourceSpec returns resourceSpec of er
@@ -215,10 +215,10 @@ func (er EngineResponse) GetResourceSpec() ResourceSpec {
 	}
 }
 
-func (er EngineResponse) getRules(status RuleStatus) []string {
+func (er EngineResponse) getRules(predicate func(RuleStatus) bool) []string {
 	var rules []string
 	for _, r := range er.PolicyResponse.Rules {
-		if r.Status == status {
+		if predicate(r.Status) {
 			rules = append(rules, r.Name)
 		}
 	}

pkg/webhooks/resource/mutation/mutation.go

@@ -152,7 +152,7 @@ func (h *mutationHandler) applyMutation(request *admissionv1.AdmissionRequest, p
 	engineResponse := engine.Mutate(policyContext)
 	policyPatches := engineResponse.GetPatches()
 
-	if !engineResponse.IsSuccessful() && len(engineResponse.GetFailedRules()) > 0 {
+	if !engineResponse.IsSuccessful() {
 		return nil, nil, fmt.Errorf("failed to apply policy %s rules %v", policyContext.Policy.GetName(), engineResponse.GetFailedRules())
 	}
 

test/conformance/kuttl/issues/5136/01-assert.yaml

@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: propagate-cost-labels-from-namespace
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready

test/conformance/kuttl/issues/5136/01-manifests.yaml

@@ -0,0 +1,29 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: propagate-cost-labels-from-namespace
+spec:
+  failurePolicy: Ignore
+  rules:
+  - name: add-cost-labels
+    context:
+    - name: namespaceLabels
+      apiCall:
+        urlPath: "/api/v1/namespaces/{{request.namespace}}"
+        jmesPath: metadata.labels
+    match:
+      any:
+      - resources:
+          kinds:
+          - Pod
+          - Deployment
+          - StatefulSet
+          - DaemonSet
+          - Job
+          - CronJob
+    mutate:
+      patchStrategicMerge:
+        metadata:
+          labels:
+            cost.starfleet.evtech/project: "{{namespaceLabels.\"cost.starfleet.evtech/project\"}}"
+            cost.starfleet.evtech/application: "{{request.object.metadata.labels.\"cost.starfleet.evtech/application\" || namespaceLabels.\"cost.starfleet.evtech/application\"}}"

test/conformance/kuttl/issues/5136/02-script.yaml

@@ -0,0 +1,14 @@
+## Checks that the manifests.yaml file CANNOT be successfully created. If it can, fail the test as this is incorrect.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+- script: |
+    if kubectl apply -f resource.yaml
+    then 
+      echo "Tested failed. Resource was allowed."
+      exit 1 
+    else 
+      echo "Test succeeded. Resource was blocked."
+      exit 0
+    fi

test/conformance/kuttl/issues/5136/03-errors.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: webserver

test/conformance/kuttl/issues/5136/04-manifests.yaml

@@ -0,0 +1,29 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: propagate-cost-labels-from-namespace
+spec:
+  failurePolicy: Ignore
+  rules:
+  - name: add-cost-labels
+    context:
+    - name: namespaceLabels
+      apiCall:
+        urlPath: "/api/v1/namespaces/{{request.namespace}}"
+        jmesPath: metadata.labels
+    match:
+      any:
+      - resources:
+          kinds:
+          - Pod
+          - Deployment
+          - StatefulSet
+          - DaemonSet
+          - Job
+          - CronJob
+    mutate:
+      patchStrategicMerge:
+        metadata:
+          labels:
+            cost.starfleet.evtech/project: "{{namespaceLabels.\"cost.starfleet.evtech/project\" || 'empty'}}"
+            cost.starfleet.evtech/application: "{{request.object.metadata.labels.\"cost.starfleet.evtech/application\" || namespaceLabels.\"cost.starfleet.evtech/application\" || 'empty'}}"

test/conformance/kuttl/issues/5136/05-assert.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: webserver
+  labels:
+    cost.starfleet.evtech/project: empty
+    cost.starfleet.evtech/application: empty

test/conformance/kuttl/issues/5136/05-pod.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: webserver
+spec:
+  containers:
+    - name: webserver
+      image: nginx:latest
+      ports:
+        - containerPort: 80

test/conformance/kuttl/issues/5136/99-cleanup.yaml

@@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: kubectl delete -f 01-manifests.yaml,resource.yaml,05-pod.yaml --force --wait=true --ignore-not-found=true

test/conformance/kuttl/issues/5136/resource.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: webserver
+spec:
+  containers:
+    - name: webserver
+      image: nginx:latest
+      ports:
+        - containerPort: 80