fix: updaterequests stuck in pending/fail infinite loop (#9119)

* fix: updaterequests stuck in pending/fail infinite loop

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>

* fix: prevent creating URs upon DELETE unless it is specified

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>

* fix chainsaw test

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>

---------

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>

pkg/webhooks/resource/generation/handler.go

@@ -21,6 +21,7 @@ import (
 	"github.com/kyverno/kyverno/pkg/metrics"
 	utils "github.com/kyverno/kyverno/pkg/utils/engine"
 	webhookgenerate "github.com/kyverno/kyverno/pkg/webhooks/updaterequest"
+	webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils"
 	admissionv1 "k8s.io/api/admission/v1"
 	corev1listers "k8s.io/client-go/listers/core/v1"
 )
@@ -203,7 +204,7 @@ func (h *generationHandler) syncTriggerAction(
 	rules := getAppliedRules(policy, failedRules)
 	for _, rule := range rules {
 		// fire generation on trigger deletion
-		if (request.Operation == admissionv1.Delete) && matchDeleteOperation(rule) {
+		if (request.Operation == admissionv1.Delete) && webhookutils.MatchDeleteOperation(rule) {
 			h.log.V(4).Info("creating the UR to generate downstream on trigger's deletion", "operation", request.Operation, "rule", rule.Name)
 			ur := buildURSpec(kyvernov1beta1.Generate, pKey, rule.Name, urSpec, false)
 			ur.Context = buildURContext(request, policyContext)

pkg/webhooks/resource/generation/utils.go

@@ -4,7 +4,6 @@ import (
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1"
 	"github.com/kyverno/kyverno/pkg/engine"
-	datautils "github.com/kyverno/kyverno/pkg/utils/data"
 	admissionv1 "k8s.io/api/admission/v1"
 )
 
@@ -27,12 +26,3 @@ func buildURContext(request admissionv1.AdmissionRequest, policyContext *engine.
 		},
 	}
 }
-
-func matchDeleteOperation(rule kyvernov1.Rule) bool {
-	ops := rule.MatchResources.GetOperations()
-	for _, rscFilters := range append(rule.MatchResources.All, rule.MatchResources.Any...) {
-		ops = append(ops, rscFilters.ResourceDescription.GetOperations()...)
-	}
-
-	return datautils.SliceContains(ops, string(admissionv1.Delete))
-}

pkg/webhooks/resource/updaterequest.go

@@ -8,10 +8,13 @@ import (
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1"
+	"github.com/kyverno/kyverno/pkg/autogen"
 	"github.com/kyverno/kyverno/pkg/engine"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"github.com/kyverno/kyverno/pkg/event"
+	datautils "github.com/kyverno/kyverno/pkg/utils/data"
 	"github.com/kyverno/kyverno/pkg/webhooks/resource/generation"
+	webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils"
 	admissionv1 "k8s.io/api/admission/v1"
 )
 
@@ -36,12 +39,20 @@ func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr
 		}
 		logger.V(4).Info("update request for mutateExisting policy")
 
+		// skip rules that don't specify the DELETE operation in case the admission request is of type DELETE
+		var skipped []string
+		for _, rule := range autogen.ComputeRules(policy) {
+			if request.Operation == admissionv1.Delete && !webhookutils.MatchDeleteOperation(rule) {
+				skipped = append(skipped, rule.Name)
+			}
+		}
+
 		var rules []engineapi.RuleResponse
 		policyContext := policyContext.WithPolicy(policy)
 		engineResponse := h.engine.ApplyBackgroundChecks(ctx, policyContext)
 
 		for _, rule := range engineResponse.PolicyResponse.Rules {
-			if rule.Status() == engineapi.RuleStatusPass {
+			if rule.Status() == engineapi.RuleStatusPass && !datautils.SliceContains(skipped, rule.Name()) {
 				rules = append(rules, rule)
 			}
 		}

pkg/webhooks/utils/match.go

@@ -0,0 +1,17 @@
+package utils
+
+import (
+	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	datautils "github.com/kyverno/kyverno/pkg/utils/data"
+	admissionv1 "k8s.io/api/admission/v1"
+)
+
+// MatchDeleteOperation checks if the rule specifies the DELETE operation.
+func MatchDeleteOperation(rule kyvernov1.Rule) bool {
+	ops := rule.MatchResources.GetOperations()
+	for _, rscFilters := range append(rule.MatchResources.All, rule.MatchResources.Any...) {
+		ops = append(ops, rscFilters.ResourceDescription.GetOperations()...)
+	}
+
+	return datautils.SliceContains(ops, string(admissionv1.Delete))
+}

test/conformance/chainsaw/mutate/clusterpolicy/standard/existing/basic-delete/chainsaw-step-01-apply-1-4.yaml

@@ -14,6 +14,8 @@ spec:
           - dictionary-2
           namespaces:
           - staging-2
+          operations:
+          - DELETE
     mutate:
       patchStrategicMerge:
         metadata:
@@ -25,8 +27,4 @@ spec:
         name: test-secret-2
         namespace: '{{ request.object.metadata.namespace }}'
     name: mutate-secret-on-configmap-delete
-    preconditions:
+
-      any:
-      - key: '{{ request.operation }}'
-        operator: Equals
-        value: DELETE

test/conformance/chainsaw/mutate/clusterpolicy/standard/existing/delete-trigger-namespace/chainsaw-test.yaml

@@ -0,0 +1,41 @@
+apiVersion: chainsaw.kyverno.io/v1alpha1
+kind: Test
+metadata:
+  creationTimestamp: null
+  name: delete-trigger-namespace
+spec:
+  steps:
+  - name: step-01
+    try:
+    - apply:
+        file: namespace.yaml
+  - name: step-02
+    try:
+    - apply:
+        file: secret.yaml
+  - name: step-03
+    try:
+    - apply:
+        file: policy.yaml
+    - assert:
+        file: policy-ready.yaml
+  - name: step-04
+    try:
+    - apply:
+        file: configmap.yaml
+  - name: step-05
+    try:
+    - assert:
+        file: patched-secret.yaml
+  - name: step-06
+    try:
+    - delete:
+        ref:
+          apiVersion: v1
+          kind: Namespace
+          name: staging
+  - name: step-07
+    try:
+    - script:
+        content: "if kubectl get updaterequests -n kyverno 2>&1 | grep -q 'No resources found in kyverno namespace.'\nthen \n  exit 0 \nelse \n  exit
+          1\nfi\n"

test/conformance/chainsaw/mutate/clusterpolicy/standard/existing/delete-trigger-namespace/configmap.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: dictionary-1
+  namespace: staging
+data:
+  key: "some value"

test/conformance/chainsaw/mutate/clusterpolicy/standard/existing/delete-trigger-namespace/namespace.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: staging

test/conformance/chainsaw/mutate/clusterpolicy/standard/existing/delete-trigger-namespace/patched-secret.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  labels:
+    foo: bar
+  name: secret-1
+  namespace: staging

test/conformance/chainsaw/mutate/clusterpolicy/standard/existing/delete-trigger-namespace/policy-ready.yaml

@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: mutate-existing-secret
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready

test/conformance/chainsaw/mutate/clusterpolicy/standard/existing/delete-trigger-namespace/policy.yaml

@@ -0,0 +1,27 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: mutate-existing-secret
+spec:
+  mutateExistingOnPolicyUpdate: true
+  rules:
+    - name: mutate-secret-on-configmap-event
+      match:
+        any:
+        - resources:
+            kinds:
+            - ConfigMap
+            names:
+            - dictionary-1
+            namespaces:
+            - staging
+      mutate:
+        targets:
+        - apiVersion: v1
+          kind: Secret
+          name: secret-1
+          namespace: "{{ request.object.metadata.namespace }}"
+        patchStrategicMerge:
+          metadata:
+            labels:
+              foo: bar

test/conformance/chainsaw/mutate/clusterpolicy/standard/existing/delete-trigger-namespace/secret.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: secret-1
+  namespace: staging
+data:
+  key: dmFsdWUtMg0KDQo=