fix: cascading mutations (#6411)

* fix: cascading mutations

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* remove explicit AddResource call

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* update resource in context

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

---------

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

pkg/engine/mutation.go

@@ -30,7 +30,6 @@ func (e *engine) mutate(
 	policy := policyContext.Policy()
 	resp = engineapi.NewEngineResponseFromPolicyContext(policyContext, nil)
 	matchedResource := policyContext.NewResource()
-	enginectx := policyContext.JSONContext()
 	var skippedRules []string
 
 	logger.V(4).Info("start mutate policy processing", "startTime", startTime)
@@ -76,15 +75,6 @@ func (e *engine) mutate(
 				}
 
 				logger.V(3).Info("processing mutate rule")
-				resource, err := policyContext.JSONContext().Query("request.object")
-				policyContext.JSONContext().Reset()
-				if err == nil && resource != nil {
-					if err := enginectx.AddResource(resource.(map[string]interface{})); err != nil {
-						logger.Error(err, "unable to update resource object")
-					}
-				} else {
-					logger.Error(err, "failed to query resource object")
-				}
 
 				if err := internal.LoadContext(ctx, e, policyContext, rule); err != nil {
 					if _, ok := err.(gojmespath.NotFoundError); ok {
@@ -233,6 +223,10 @@ func (f *forEachMutator) mutateForEach(ctx context.Context) *mutate.Response {
 				f.resource.unstructured = mutateResp.PatchedResource
 				allPatches = append(allPatches, mutateResp.Patches...)
 			}
+			f.log.Info("mutateResp.PatchedResource", "resource", mutateResp.PatchedResource)
+			if err := f.policyContext.JSONContext().AddResource(mutateResp.PatchedResource.Object); err != nil {
+				f.log.Error(err, "failed to update resource in context")
+			}
 		}
 	}
 

test/conformance/kuttl/mutate/cascading/first-rule-is-foreach/01-policy.yaml

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

test/conformance/kuttl/mutate/cascading/first-rule-is-foreach/02-configmap.yaml

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

test/conformance/kuttl/mutate/cascading/first-rule-is-foreach/README.md

@@ -0,0 +1,9 @@
+## Description
+
+This test creates a policy with two mutation rules.
+The second rule depends on the mutation in the first rule.
+To succeed, the changes in the first mutation rule need to cascade correctly to get the second rule to execute correctly.
+
+## Related issue
+
+https://github.com/kyverno/kyverno/issues/6210

test/conformance/kuttl/mutate/cascading/first-rule-is-foreach/configmap-assert.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: cm
+  annotations:
+    mutation1: '1'
+    mutation2: 'found mutation1: 1'

test/conformance/kuttl/mutate/cascading/first-rule-is-foreach/configmap.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: cm

test/conformance/kuttl/mutate/cascading/first-rule-is-foreach/policy-assert.yaml

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

test/conformance/kuttl/mutate/cascading/first-rule-is-foreach/policy.yaml

@@ -0,0 +1,33 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: mutate-chain
+spec:
+  background: false
+  validationFailureAction: Enforce
+  rules:
+    - name: mutation1
+      match:
+        all:
+          - resources:
+              kinds:
+                - v1/ConfigMap
+      mutate:
+        foreach:
+          - list: "['dummy']"
+            patchStrategicMerge:
+              metadata:
+                annotations:
+                  # value is a counter in case K8s decides for multiple mutation rounds
+                  mutation1: "{{ not_null(request.object.metadata.annotations.mutation1, '0') | add(@, '1') }}"
+    - name: mutation2
+      match:
+        all:
+          - resources:
+              kinds:
+                - v1/ConfigMap
+      mutate:
+        patchStrategicMerge:
+          metadata:
+            annotations:
+              mutation2: "found mutation1: {{ request.object.metadata.annotations.mutation1 || '<nothing>' }}"

test/conformance/kuttl/mutate/cascading/no-foreach/01-policy.yaml

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

test/conformance/kuttl/mutate/cascading/no-foreach/02-configmap.yaml

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

test/conformance/kuttl/mutate/cascading/no-foreach/README.md

@@ -0,0 +1,9 @@
+## Description
+
+This test creates a policy with two mutation rules.
+The second rule depends on the mutation in the first rule.
+To succeed, the changes in the first mutation rule need to cascade correctly to get the second rule to execute correctly.
+
+## Related issue
+
+https://github.com/kyverno/kyverno/issues/6210

test/conformance/kuttl/mutate/cascading/no-foreach/configmap-assert.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: cm
+  annotations:
+    mutation1: '1'
+    mutation2: 'found mutation1: 1'

test/conformance/kuttl/mutate/cascading/no-foreach/configmap.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: cm

test/conformance/kuttl/mutate/cascading/no-foreach/policy-assert.yaml

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

test/conformance/kuttl/mutate/cascading/no-foreach/policy.yaml

@@ -0,0 +1,31 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: mutate-chain
+spec:
+  background: false
+  validationFailureAction: Enforce
+  rules:
+    - name: mutation1
+      match:
+        all:
+          - resources:
+              kinds:
+                - v1/ConfigMap
+      mutate:
+        patchStrategicMerge:
+          metadata:
+            annotations:
+              # value is a counter in case K8s decides for multiple mutation rounds
+              mutation1: "{{ not_null(request.object.metadata.annotations.mutation1, '0') | add(@, '1') }}"
+    - name: mutation2
+      match:
+        all:
+          - resources:
+              kinds:
+                - v1/ConfigMap
+      mutate:
+        patchStrategicMerge:
+          metadata:
+            annotations:
+              mutation2: "found mutation1: {{ request.object.metadata.annotations.mutation1 || '<nothing>' }}"