feat: Removal of multiple elements using JSON patch in a foreach loop (#6335)

* handle mismatch of indices for removal operation while using foreach in mutate rule

Signed-off-by: Ashwin901 <ashwinprasanna9@gmail.com>

* Refactor elementIsRemoved function to pass 'gofumpt' check

Signed-off-by: Ashwin901 <ashwinprasanna9@gmail.com>

* traverse elements array in reverse order to handle removal of multiple elements

Signed-off-by: Ashwin901 <ashwinprasanna9@gmail.com>

* handle failing test case by changing assertion order

Signed-off-by: Ashwin901 <ashwinprasanna9@gmail.com>

* add kuttl tests

Signed-off-by: Ashwin901 <ashwinprasanna9@gmail.com>

* remove cleanup file from kuttl test

Signed-off-by: Ashwin901 <ashwinprasanna9@gmail.com>

* sort patches at the end

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

* fix policy

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

* fix

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

* fix

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

* move tests

Signed-off-by: ShutingZhao <shuting@nirmata.com>

---------

Signed-off-by: Ashwin901 <ashwinprasanna9@gmail.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Co-authored-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Co-authored-by: ShutingZhao <shuting@nirmata.com>

pkg/engine/handlers/mutation/common.go

@@ -2,6 +2,7 @@ package mutation
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 
 	"github.com/go-logr/logr"
@@ -9,8 +10,10 @@ import (
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"github.com/kyverno/kyverno/pkg/engine/internal"
 	"github.com/kyverno/kyverno/pkg/engine/mutate"
+	"github.com/kyverno/kyverno/pkg/engine/mutate/patch"
 	engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
 	"github.com/kyverno/kyverno/pkg/utils/api"
+	"github.com/mattbaird/jsonpatch"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -129,8 +132,24 @@ func (f *forEachMutator) mutateElements(ctx context.Context, foreach kyvernov1.F
 			allPatches = append(allPatches, mutateResp.Patches...)
 		}
 	}
-
-	return mutate.NewResponse(engineapi.RuleStatusPass, patchedResource.unstructured, allPatches, "")
+	var sortedPatches []jsonpatch.JsonPatchOperation
+	for _, p := range allPatches {
+		var jp jsonpatch.JsonPatchOperation
+		if err := json.Unmarshal(p, &jp); err != nil {
+			return mutate.NewErrorResponse("failed to convert patch", err)
+		}
+		sortedPatches = append(sortedPatches, jp)
+	}
+	sortedPatches = patch.FilterAndSortPatches(sortedPatches)
+	var finalPatches [][]byte
+	for _, p := range sortedPatches {
+		if data, err := p.MarshalJSON(); err != nil {
+			return mutate.NewErrorResponse("failed to marshal patch", err)
+		} else {
+			finalPatches = append(finalPatches, data)
+		}
+	}
+	return mutate.NewResponse(engineapi.RuleStatusPass, patchedResource.unstructured, finalPatches, "")
 }
 
 func buildRuleResponse(rule *kyvernov1.Rule, mutateResp *mutate.Response, info resourceInfo) *engineapi.RuleResponse {

pkg/engine/mutate/patch/patchesUtils.go

@@ -16,7 +16,7 @@ func generatePatches(src, dst []byte) ([][]byte, error) {
 		return nil, err
 	}
 
-	sortedPatches := filterAndSortPatches(pp)
+	sortedPatches := FilterAndSortPatches(pp)
 	for _, p := range sortedPatches {
 		pbytes, err := p.MarshalJSON()
 		if err != nil {
@@ -29,13 +29,13 @@ func generatePatches(src, dst []byte) ([][]byte, error) {
 	return patchesBytes, err
 }
 
-// filterAndSortPatches
+// FilterAndSortPatches
 // 1. filters out patches with the certain paths
 // 2. sorts the removal patches(with same path) by the key of index
 // in descending order. The sort is required as when removing multiple
 // elements from an array, the elements must be removed in descending
 // order to preserve each index value.
-func filterAndSortPatches(originalPatches []jsonpatch.JsonPatchOperation) []jsonpatch.JsonPatchOperation {
+func FilterAndSortPatches(originalPatches []jsonpatch.JsonPatchOperation) []jsonpatch.JsonPatchOperation {
 	patches := filterInvalidPatches(originalPatches)
 
 	result := make([]jsonpatch.JsonPatchOperation, len(patches))

pkg/engine/mutate/patch/patchesUtils_test.go

@@ -320,7 +320,7 @@ func Test_sortRemovalPatches(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		sortedPatches := filterAndSortPatches(test.patches)
+		sortedPatches := FilterAndSortPatches(test.patches)
 		assertnew.Equal(t, test.expected, sortedPatches, fmt.Sprintf("%dth test fails", i))
 	}
 }

test/conformance/kuttl/mutate/policy/cornercases/foreach-remove-elements/01-policy.yaml

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

test/conformance/kuttl/mutate/policy/cornercases/foreach-remove-elements/02-pod.yaml

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

test/conformance/kuttl/mutate/policy/cornercases/foreach-remove-elements/README.md

@@ -0,0 +1,12 @@
+## Description
+
+This test checks if multiple elements are successfully removed while using foreach.
+
+## Expected Behavior
+
+The two `hostPath` volumes should be removed from the `busybox` pod and only the `emptyDir` volume and service account volume should remain.
+
+
+## Reference Issue(s)
+
+5661

test/conformance/kuttl/mutate/policy/cornercases/foreach-remove-elements/pod-assert.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: busybox
+spec:
+  volumes:
+  - name: vault-secret
+    emptyDir:
+      medium: Memory
+  - projected: {}

test/conformance/kuttl/mutate/policy/cornercases/foreach-remove-elements/pod.yaml

@@ -0,0 +1,18 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: busybox
+spec:
+  containers:
+  - name: busybox
+    image: busybox:1.35
+  volumes:
+  - name: socket
+    hostPath:
+      path: "/var/run/foo"
+  - name: vault-secret
+    emptyDir:
+      medium: Memory
+  - name: bar
+    hostPath:
+      path: "/var/run/bar"

test/conformance/kuttl/mutate/policy/cornercases/foreach-remove-elements/policy-assert.yaml

@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: Policy
+metadata:
+  name: foreach-remove-elements
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready

test/conformance/kuttl/mutate/policy/cornercases/foreach-remove-elements/policy.yaml

@@ -0,0 +1,25 @@
+apiVersion: kyverno.io/v1
+kind: Policy
+metadata:
+  name: foreach-remove-elements
+spec:
+  background: false
+  schemaValidation: false
+  rules:
+    - name: remove-elements
+      match:
+        any:
+          - resources:
+              kinds:
+                - Pod
+      mutate:
+        foreach:
+          - list: request.object.spec.volumes[]
+            preconditions:
+              all:
+              - key: hostPath
+                operator: AnyIn
+                value: "{{ element.keys(@) }}"
+            patchesJson6902: |-
+              - path: /spec/volumes/{{elementIndex}}
+                op: remove