fix mutate preprocessing for anchors (#3052)

* fix mutate preprocessing for anchors

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* make fmt

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

Co-authored-by: shuting <shutting06@gmail.com>

api/kyverno/v1alpha1/zz_generated.deepcopy.go

@@ -21,7 +21,7 @@ package v1alpha1
 
 import (
 	policyreportv1alpha1 "github.com/kyverno/kyverno/api/policyreport/v1alpha1"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )

api/kyverno/v1alpha2/zz_generated.deepcopy.go

@@ -21,7 +21,7 @@ package v1alpha2
 
 import (
 	policyreportv1alpha2 "github.com/kyverno/kyverno/api/policyreport/v1alpha2"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )

api/policyreport/v1alpha1/zz_generated.deepcopy.go

@@ -20,7 +20,7 @@ limitations under the License.
 package v1alpha1
 
 import (
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )

api/policyreport/v1alpha2/zz_generated.deepcopy.go

@@ -20,7 +20,7 @@ limitations under the License.
 package v1alpha2
 
 import (
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )

go.mod

@@ -56,6 +56,7 @@ require (
 require (
 	github.com/aquilax/truncate v1.0.0
 	github.com/blang/semver/v4 v4.0.0
+	github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249
 	gopkg.in/inf.v0 v0.9.1
 )
 

go.sum

@@ -1268,6 +1268,8 @@ github.com/nats-io/nkeys v0.1.3/go.mod h1:xpnFELMwJABBLVhffcfd1MZx6VsNRFpEugbxzi
 github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 github.com/nishanths/predeclared v0.0.0-20200524104333-86fad755b4d3/go.mod h1:nt3d53pc1VYcphSCIaYAJtnPYnr3Zyn8fMq2wvPGPso=
+github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249 h1:NHrXEjTNQY7P0Zfx1aMrNhpgxHmow66XQtm0aQLY0AE=
+github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249/go.mod h1:mpRZBD8SJ55OIICQ3iWH0Yz3cjzA61JdqMLoWXeB2+8=
 github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
 github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
 github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=

pkg/engine/anchor/common.go

@@ -25,8 +25,9 @@ func IsGlobalAnchor(str string) bool {
 		return false
 	}
 
-	//TODO: trim spaces ?
-	return (str[:len(left)] == left && str[len(str)-len(right):] == right)
+	leftMatch := strings.TrimSpace(str[:len(left)]) == left
+	rightMatch := strings.TrimSpace(str[len(str)-len(right):]) == right
+	return leftMatch && rightMatch
 }
 
 //ContainsCondition returns true, if str is either condition anchor or
@@ -46,8 +47,8 @@ func IsNegationAnchor(str string) bool {
 	return (str[:len(left)] == left && str[len(str)-len(right):] == right)
 }
 
-// IsAddingAnchor checks for addition anchor
-func IsAddingAnchor(key string) bool {
+// IsAddIfNotPresentAnchor checks for addition anchor
+func IsAddIfNotPresentAnchor(key string) bool {
 	const left = "+("
 	const right = ")"
 
@@ -94,7 +95,7 @@ func RemoveAnchor(key string) (string, string) {
 		return key[1 : len(key)-1], key[0:1]
 	}
 
-	if IsExistenceAnchor(key) || IsAddingAnchor(key) || IsEqualityAnchor(key) || IsNegationAnchor(key) || IsGlobalAnchor(key) {
+	if IsExistenceAnchor(key) || IsAddIfNotPresentAnchor(key) || IsEqualityAnchor(key) || IsNegationAnchor(key) || IsGlobalAnchor(key) {
 		return key[2 : len(key)-1], key[0:2]
 	}
 

pkg/engine/common/pattern.go

@@ -151,6 +151,10 @@ func validateValueWithNilPattern(log logr.Logger, value interface{}) bool {
 
 // Handler for pattern values during validation process
 func validateValueWithStringPatterns(log logr.Logger, value interface{}, pattern string) bool {
+	if value == pattern {
+		return true
+	}
+
 	conditions := strings.Split(pattern, "|")
 	for _, condition := range conditions {
 		condition = strings.Trim(condition, " ")

pkg/engine/mutate/patch/strategicMergePatch.go

@@ -97,6 +97,8 @@ func strategicMergePatch(logger logr.Logger, base, overlay string) ([]byte, erro
 		}
 	}
 
+	patchStr, _ := preprocessedYaml.String()
+	logger.V(3).Info("applying strategic merge patch", "patch", patchStr)
 	f := patchstrategicmerge.Filter{
 		Patch: preprocessedYaml,
 	}

pkg/engine/mutate/patch/strategicMergePatch_test.go

@@ -164,9 +164,7 @@ func TestMergePatch(t *testing.T) {
 		t.Logf("Running test %d...", i+1)
 		out, err := strategicMergePatch(log.Log, string(test.rawResource), string(test.rawPolicy))
 		assert.NilError(t, err)
-
-		// has assertions inside
-		areEqualJSONs(t, test.expected, out)
+		assert.DeepEqual(t, toJSON(t, test.expected), toJSON(t, out))
 	}
 }
 

pkg/engine/mutate/patch/strategicPreprocessing.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/pkg/errors"
+
 	"github.com/go-logr/logr"
 	"github.com/kyverno/kyverno/pkg/engine/anchor"
 	"github.com/kyverno/kyverno/pkg/engine/validate"
@@ -65,31 +67,26 @@ func preProcessRecursive(logger logr.Logger, pattern, resource *yaml.RNode) erro
 }
 
 func walkMap(logger logr.Logger, pattern, resource *yaml.RNode) error {
-	var err error
-
-	err = validateConditions(logger, pattern, resource)
-	if err != nil {
-		return err
+	if _, err := handleAddIfNotPresentAnchor(pattern, resource); err != nil {
+		return errors.Wrap(err, "failed to process addIfNotPresent anchor")
 	}
 
-	err = handleAddings(pattern, resource)
-	if err != nil {
-		return err
+	if err := validateConditions(logger, pattern, resource); err != nil {
+		return err // do not wrap condition errors
 	}
 
-	nonAnchors, err := filterKeys(pattern, func(key string) bool {
+	isNotAnchor := func(key string) bool {
 		return !hasAnchor(key)
-	})
+	}
+
+	nonAnchors, err := filterKeys(pattern, isNotAnchor)
 	if err != nil {
 		return err
 	}
 
-	var resourceValue *yaml.RNode
-
 	for _, field := range nonAnchors {
-		if resource == nil || resource.Field(field) == nil {
-			resourceValue = nil
-		} else {
+		var resourceValue *yaml.RNode
+		if resource != nil && resource.Field(field) != nil {
 			resourceValue = resource.Field(field).Value
 		}
 
@@ -139,57 +136,30 @@ func processListOfMaps(logger logr.Logger, pattern, resource *yaml.RNode) error
 		hasAnyAnchor := hasAnchors(patternElement, hasAnchor)
 		hasGlobalConditions := hasAnchors(patternElement, anchor.IsGlobalAnchor)
 		if hasAnyAnchor {
-
 			anyGlobalConditionPassed := false
 			var lastGlobalAnchorError error = nil
 
 			for _, resourceElement := range resourceElements {
-				err := preProcessRecursive(logger, patternElement, resourceElement)
-				if err != nil {
-					switch err.(type) {
-					case ConditionError:
-						// Skip element, if condition has failed
+				if err := preProcessRecursive(logger, patternElement, resourceElement); err != nil {
+					logger.V(3).Info("anchor mismatch", "reason", err.Error())
+					if isConditionError(err) {
 						continue
-					case GlobalConditionError:
+					}
+
+					if isGlobalConditionError(err) {
 						lastGlobalAnchorError = err
 						continue
 					}
 
 					return err
+				}
+
+				if hasGlobalConditions {
+					// global anchor has passed, there is no need to return an error
+					anyGlobalConditionPassed = true
 				} else {
-					if hasGlobalConditions {
-						// global anchor has passed, there is no need to return an error
-						anyGlobalConditionPassed = true
-					}
-
-					// If condition is satisfied, create new pattern list element based on patternElement
-					// but related with current resource element by name.
-					// Resource element must have name. Without name kustomize won't be able to update this element.
-					// In case if element does not have name, skip it.
-					resourceElementName := resourceElement.Field("name")
-					if resourceElementName.IsNilOrEmpty() {
-						continue
-					}
-
-					newNode := patternElement.Copy()
-					empty, err := deleteConditionsFromNestedMaps(newNode)
-					if err != nil {
-						return err
-					}
-
-					// Do not add an empty element to the patch
-					if empty {
-						continue
-					}
-
-					err = newNode.PipeE(yaml.SetField("name", resourceElementName.Value))
-					if err != nil {
-						return err
-					}
-
-					err = pattern.PipeE(yaml.Append(newNode.YNode()))
-					if err != nil {
-						return err
+					if err := handlePatternName(pattern, patternElement, resourceElement); err != nil {
+						return errors.Wrap(err, "failed to update name in pattern")
 					}
 				}
 			}
@@ -203,6 +173,50 @@ func processListOfMaps(logger logr.Logger, pattern, resource *yaml.RNode) error
 	return nil
 }
 
+func handlePatternName(pattern, patternElement, resourceElement *yaml.RNode) error {
+	// If condition is satisfied, create new pattern list element based on patternElement
+	// but related with current resource element by name.
+	// Resource element must have name. Without name kustomize won't be able to update this element.
+	// In case if element does not have name, skip it.
+	resourceElementName := resourceElement.Field("name")
+	if resourceElementName.IsNilOrEmpty() {
+		return nil
+	}
+
+	newNode := patternElement.Copy()
+	empty, err := deleteAnchors(newNode, true, false)
+	if err != nil {
+		return err
+	}
+
+	// Do not add an empty element to the patch
+	if empty {
+		return nil
+	}
+
+	err = newNode.PipeE(yaml.SetField("name", resourceElementName.Value))
+	if err != nil {
+		return err
+	}
+
+	err = pattern.PipeE(yaml.Append(newNode.YNode()))
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func isConditionError(err error) bool {
+	_, ok := err.(ConditionError)
+	return ok
+}
+
+func isGlobalConditionError(err error) bool {
+	_, ok := err.(GlobalConditionError)
+	return ok
+}
+
 // validateConditions checks all conditions from current map.
 // If at least one condition fails, return error.
 // If caller handles list of maps and gets an error, it must skip element.
@@ -223,35 +237,38 @@ func validateConditions(logger logr.Logger, pattern, resource *yaml.RNode) error
 	return nil
 }
 
-// handleAddings handles adding anchors.
+// handleAddIfNotPresentAnchor handles adding anchors.
 // Remove anchor from pattern, if field already exists.
 // Remove anchor wrapping from key, if field does not exist in the resource.
-func handleAddings(pattern, resource *yaml.RNode) error {
-	addings, err := filterKeys(pattern, anchor.IsAddingAnchor)
+func handleAddIfNotPresentAnchor(pattern, resource *yaml.RNode) (int, error) {
+	anchors, err := filterKeys(pattern, anchor.IsAddIfNotPresentAnchor)
 	if err != nil {
-		return err
+		return 0, err
 	}
 
-	for _, adding := range addings {
-		key, _ := anchor.RemoveAnchor(adding)
+	for _, a := range anchors {
+		key, _ := anchor.RemoveAnchor(a)
 		if resource != nil && resource.Field(key) != nil {
 			// Resource already has this field.
-			// Delete the field with adding anchor from patch.
-			err = pattern.PipeE(yaml.Clear(adding))
+			// Delete the field with addIfNotPresent anchor from patch.
+			err = pattern.PipeE(yaml.Clear(a))
 			if err != nil {
-				return err
+				return 0, err
 			}
-			continue
+		} else {
+			// Remove anchor tags from patch field key.
+			renameField(a, key, pattern)
 		}
-
-		// Remove anchor wrap from patch field.
-		renameField(adding, key, pattern)
 	}
 
-	return nil
+	return len(anchors), nil
 }
 
 func filterKeys(pattern *yaml.RNode, condition func(string) bool) ([]string, error) {
+	if !isMappingNode(pattern) {
+		return nil, nil
+	}
+
 	keys := make([]string, 0)
 	fields, err := pattern.Fields()
 	if err != nil {
@@ -267,12 +284,23 @@ func filterKeys(pattern *yaml.RNode, condition func(string) bool) ([]string, err
 	return keys, nil
 }
 
+func isMappingNode(node *yaml.RNode) bool {
+	if err := yaml.ErrorIfInvalid(node, yaml.MappingNode); err != nil {
+		return false
+	}
+
+	return true
+}
+
 func hasAnchor(key string) bool {
-	return anchor.ContainsCondition(key) || anchor.IsAddingAnchor(key)
+	return anchor.ContainsCondition(key) || anchor.IsAddIfNotPresentAnchor(key)
 }
 
 func hasAnchors(pattern *yaml.RNode, isAnchor func(key string) bool) bool {
-	if yaml.MappingNode == pattern.YNode().Kind {
+	ynode := pattern.YNode()
+	kind := ynode.Kind
+
+	if kind == yaml.MappingNode {
 		fields, err := pattern.Fields()
 		if err != nil {
 			return false
@@ -290,6 +318,19 @@ func hasAnchors(pattern *yaml.RNode, isAnchor func(key string) bool) bool {
 				}
 			}
 		}
+	} else if kind == yaml.ScalarNode {
+		v := ynode.Value
+		return anchor.ContainsCondition(v)
+
+	} else if kind == yaml.SequenceNode {
+		elements, _ := pattern.Elements()
+		for _, e := range elements {
+			if hasAnchors(e, isAnchor) {
+				return true
+			}
+		}
+
+		return false
 	}
 
 	return false
@@ -343,52 +384,6 @@ func checkCondition(logger logr.Logger, pattern *yaml.RNode, resource *yaml.RNod
 	return nil
 }
 
-func deleteConditionsFromNestedMaps(pattern *yaml.RNode) (bool, error) {
-	if pattern.YNode().Kind != yaml.MappingNode {
-		return false, nil
-	}
-
-	fields, err := pattern.Fields()
-	if err != nil {
-		return false, err
-	}
-
-	for _, field := range fields {
-		if anchor.ContainsCondition(field) {
-			err = pattern.PipeE(yaml.Clear(field))
-			if err != nil {
-				return false, err
-			}
-		} else {
-			child := pattern.Field(field).Value
-			if child != nil {
-				empty, err := deleteConditionsFromNestedMaps(child)
-				if err != nil {
-					return false, err
-				}
-
-				if empty {
-					err = pattern.PipeE(yaml.Clear(field))
-					if err != nil {
-						return false, err
-					}
-				}
-			}
-		}
-	}
-
-	fields, err = pattern.Fields()
-	if err != nil {
-		return false, err
-	}
-
-	if len(fields) == 0 {
-		return true, nil
-	}
-
-	return false, nil
-}
-
 func deleteConditionElements(pattern *yaml.RNode) error {
 	fields, err := pattern.Fields()
 	if err != nil {
@@ -396,11 +391,12 @@ func deleteConditionElements(pattern *yaml.RNode) error {
 	}
 
 	for _, field := range fields {
-		ok, err := deleteAnchors(pattern.Field(field).Value)
+		deleteScalar := anchor.ContainsCondition(field)
+		canDelete, err := deleteAnchors(pattern.Field(field).Value, deleteScalar, false)
 		if err != nil {
 			return err
 		}
-		if ok {
+		if canDelete {
 			err = pattern.PipeE(yaml.Clear(field))
 			if err != nil {
 				return err
@@ -414,32 +410,50 @@ func deleteConditionElements(pattern *yaml.RNode) error {
 // deleteAnchors deletes all the anchors and returns true,
 // if this node must be deleted from patch.
 // Node is considered to be deleted, if there were only
-// anchors elemets. After anchors elements are removed,
-// we have patch with nil values which could cause
+// anchors elements. After anchors elements are removed,
+// A patch with nil values which could cause
 // unnecessary resource elements deletion.
-func deleteAnchors(node *yaml.RNode) (bool, error) {
+func deleteAnchors(node *yaml.RNode, deleteScalar, traverseMappingNodes bool) (bool, error) {
 	switch node.YNode().Kind {
 	case yaml.MappingNode:
-		return deleteAnchorsInMap(node)
+		return deleteAnchorsInMap(node, traverseMappingNodes)
 	case yaml.SequenceNode:
-		return deleteAnchorsInList(node)
+		return deleteAnchorsInList(node, traverseMappingNodes)
+	case yaml.ScalarNode:
+		return deleteScalar, nil
 	}
 
 	return false, nil
 }
 
-func deleteAnchorsInMap(node *yaml.RNode) (bool, error) {
+func deleteAnchorsInMap(node *yaml.RNode, traverseMappingNodes bool) (bool, error) {
 	conditions, err := filterKeys(node, anchor.ContainsCondition)
 	if err != nil {
 		return false, err
 	}
 
-	// Remove all conditions first.
+	// remove all conditional anchors with no child nodes first
+	anchorsExist := false
 	for _, condition := range conditions {
-		err = node.PipeE(yaml.Clear(condition))
+		field := node.Field(condition)
+		shouldDelete, err := deleteAnchors(field.Value, true, traverseMappingNodes)
 		if err != nil {
 			return false, err
 		}
+
+		if shouldDelete {
+			if err := node.PipeE(yaml.Clear(condition)); err != nil {
+				return false, err
+			}
+		} else {
+			anchorsExist = true
+		}
+	}
+
+	if anchorsExist {
+		if err := stripAnchorsFromNode(node, ""); err != nil {
+			return false, errors.Wrap(err, "failed to remove anchor tags")
+		}
 	}
 
 	fields, err := node.Fields()
@@ -448,15 +462,13 @@ func deleteAnchorsInMap(node *yaml.RNode) (bool, error) {
 	}
 
 	needToDelete := true
-
-	// Go further through the map elements.
 	for _, field := range fields {
-		ok, err := deleteAnchors(node.Field(field).Value)
+		canDelete, err := deleteAnchors(node.Field(field).Value, false, traverseMappingNodes)
 		if err != nil {
 			return false, err
 		}
 
-		if ok {
+		if canDelete {
 			err = node.PipeE(yaml.Clear(field))
 			if err != nil {
 				return false, err
@@ -471,26 +483,53 @@ func deleteAnchorsInMap(node *yaml.RNode) (bool, error) {
 	return needToDelete, nil
 }
 
-func deleteAnchorsInList(node *yaml.RNode) (bool, error) {
+// stripAnchorFromNode strips one or more anchor fields from the node.
+// If key is "" all anchor fields are stripped. Otherwise, only the matching
+// field is stripped.
+func stripAnchorsFromNode(node *yaml.RNode, key string) error {
+	anchors, err := filterKeys(node, anchor.ContainsCondition)
+	if err != nil {
+		return err
+	}
+
+	for _, a := range anchors {
+		k, _ := anchor.RemoveAnchor(a)
+		if key == "" || k == key {
+			renameField(a, k, node)
+		}
+	}
+
+	return nil
+}
+
+func deleteAnchorsInList(node *yaml.RNode, traverseMappingNodes bool) (bool, error) {
 	elements, err := node.Elements()
 	if err != nil {
 		return false, err
 	}
 
 	wasEmpty := len(elements) == 0
-
 	for i, element := range elements {
 		if hasAnchors(element, hasAnchor) {
-			deleteListElement(node, i)
+			shouldDelete := true
+			if traverseMappingNodes && isMappingNode(element) {
+				shouldDelete, err = deleteAnchors(element, true, traverseMappingNodes)
+				if err != nil {
+					return false, errors.Wrap(err, "failed to delete anchors")
+				}
+			}
+
+			if shouldDelete {
+				deleteListElement(node, i)
+			}
 		} else {
 			// This element also could have some conditions
 			// inside sub-arrays. Delete them too.
-
-			ok, err := deleteAnchors(element)
+			canDelete, err := deleteAnchors(element, false, traverseMappingNodes)
 			if err != nil {
-				return false, err
+				return false, errors.Wrap(err, "failed to delete anchors")
 			}
-			if ok {
+			if canDelete {
 				deleteListElement(node, i)
 			}
 		}
@@ -524,8 +563,15 @@ func validateConditionsInternal(logger logr.Logger, pattern, resource *yaml.RNod
 			return fmt.Errorf("could not found \"%s\" key in the resource", conditionKey)
 		}
 
-		err = checkCondition(logger, pattern.Field(condition).Value, resource.Field(conditionKey).Value)
-		if err != nil {
+		patternValue := pattern.Field(condition).Value
+		resourceValue := resource.Field(conditionKey).Value
+		if count, err := handleAddIfNotPresentAnchor(patternValue, resourceValue); err != nil {
+			return err
+		} else if count > 0 {
+			continue
+		}
+
+		if err := checkCondition(logger, patternValue, resourceValue); err != nil {
 			return err
 		}
 	}

pkg/engine/mutate/patch/strategicPreprocessing_test.go

@@ -5,25 +5,11 @@ import (
 	"testing"
 
 	"github.com/kyverno/kyverno/pkg/engine/anchor"
-
 	"gotest.tools/assert"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	yaml "sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
-func areEqualJSONs(t *testing.T, s1, s2 []byte) {
-	var o1 interface{}
-	var o2 interface{}
-
-	var err error
-	err = json.Unmarshal(s1, &o1)
-	assert.NilError(t, err)
-
-	err = json.Unmarshal(s2, &o2)
-	assert.NilError(t, err)
-	assert.DeepEqual(t, o1, o2)
-}
-
 func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) {
 	testCases := []struct {
 		rawPolicy     []byte
@@ -341,7 +327,7 @@ func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) {
 				"spec": {
 				  "volumes": [
 					{
-					  "(hostPath)": {
+					  "<(hostPath)": {
 						"path": "*"
 					  }
 					}
@@ -400,7 +386,7 @@ func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) {
 				"spec": {
 				  "volumes": [
 					{
-					  "(hostPath)": {
+					  "<(hostPath)": {
 						"path": "*"
 					  }
 					}
@@ -855,19 +841,25 @@ func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) {
 	}
 
 	for i, test := range testCases {
-
-		t.Logf("Running test %d...", i+1)
+		t.Logf("Running test %d...", i)
 		preProcessedPolicy, err := preProcessStrategicMergePatch(log.Log, string(test.rawPolicy), string(test.rawResource))
 		assert.NilError(t, err)
 
 		output, err := preProcessedPolicy.MarshalJSON()
 		assert.NilError(t, err)
 
-		// has assertions inside
-		areEqualJSONs(t, test.expectedPatch, output)
+		assert.DeepEqual(t, toJSON(t, []byte(test.expectedPatch)), toJSON(t, output))
 	}
 }
 
+func toJSON(t *testing.T, b []byte) interface{} {
+	var i interface{}
+	var err error
+	err = json.Unmarshal(b, &i)
+	assert.NilError(t, err)
+	return i
+}
+
 func Test_FilterKeys_NoConditions(t *testing.T) {
 	patternRaw := []byte(`{
 		"key1": "value1",
@@ -1056,7 +1048,7 @@ func Test_DeleteConditions(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Equal(t, len(containers), 2)
 
-	_, err = deleteAnchors(pattern)
+	_, err = deleteAnchors(pattern, false, false)
 	assert.NilError(t, err)
 
 	containers, err = pattern.Field("spec").Value.Field("containers").Value.Elements()
@@ -1150,7 +1142,20 @@ func Test_NonExistingKeyMustFailPreprocessing(t *testing.T) {
 
 	pattern := yaml.MustParse(string(rawPattern))
 	resource := yaml.MustParse(string(rawResource))
-
 	err := preProcessPattern(log.Log, pattern, resource)
 	assert.Error(t, err, "condition failed: could not found \"key1\" key in the resource")
 }
+
+func Test_NestedConditionals(t *testing.T) {
+	rawPattern := `{"spec":{"template":{"spec":{"volumes":[{"(emptyDir)":{"+(sizeLimit)":"20Mi"},"name":"*"}]}}}}`
+	rawResource := `{"spec":{"template":{"spec":{"volumes":[{"emptyDir":{},"name":"vol02"}]}}}}`
+	expectedPattern := `{"spec":{"template":{"spec":{"volumes":[{"emptyDir":{"sizeLimit":"20Mi"},"name":"vol02"}]}}}}`
+
+	pattern := yaml.MustParse(rawPattern)
+	resource := yaml.MustParse(rawResource)
+	err := preProcessPattern(log.Log, pattern, resource)
+	assert.NilError(t, err)
+	resultPattern, _ := pattern.String()
+
+	assert.DeepEqual(t, toJSON(t, []byte(expectedPattern)), toJSON(t, []byte(resultPattern)))
+}

pkg/engine/mutate/utils.go

@@ -11,7 +11,7 @@ func getAnchorAndElementsFromMap(anchorsMap map[string]interface{}) (map[string]
 	for key, value := range anchorsMap {
 		if commonAnchors.IsConditionAnchor(key) {
 			anchors[key] = value
-		} else if !commonAnchors.IsAddingAnchor(key) {
+		} else if !commonAnchors.IsAddIfNotPresentAnchor(key) {
 			elementsWithoutanchor[key] = value
 		}
 	}

pkg/engine/validation.go

@@ -277,7 +277,7 @@ func (v *validator) validateElements(foreach *kyverno.ForEachValidation, element
 			v.log.Info("skip rule", "reason", r.Message)
 			continue
 		} else if r.Status != response.RuleStatusPass {
-			msg := fmt.Sprintf("validation failed in foreach rule for %v", r.Message)
+			msg := fmt.Sprintf("validation failure: %v", r.Message)
 			return ruleResponse(v.rule, utils.Validation, msg, r.Status), applyCount
 		}
 

pkg/kyverno/test/test_command.go

@@ -15,7 +15,6 @@ import (
 	"github.com/fatih/color"
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-billy/v5/memfs"
-	"github.com/go-logr/logr"
 	"github.com/kataras/tablewriter"
 	report "github.com/kyverno/kyverno/api/policyreport/v1alpha2"
 	client "github.com/kyverno/kyverno/pkg/dclient"
@@ -391,11 +390,11 @@ func getLocalDirTestFiles(fs billy.Filesystem, path, fileName, valuesFile string
 	return errors
 }
 
-func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResults, infos []policyreport.Info, policyResourcePath string, fs billy.Filesystem, isGit bool) (map[string]report.PolicyReportResult, []TestResults) {
+func buildPolicyResults(engineResponses []*response.EngineResponse, testResults []TestResults, infos []policyreport.Info, policyResourcePath string, fs billy.Filesystem, isGit bool) (map[string]report.PolicyReportResult, []TestResults) {
 	results := make(map[string]report.PolicyReportResult)
 	now := metav1.Timestamp{Seconds: time.Now().Unix()}
 
-	for _, resp := range resps {
+	for _, resp := range engineResponses {
 		policyName := resp.PolicyResponse.Policy.Name
 		resourceName := resp.PolicyResponse.Resource.Name
 		resourceKind := resp.PolicyResponse.Resource.Kind
@@ -417,7 +416,7 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu
 			Message: buildMessage(resp),
 		}
 
-		var patcheResourcePath []string
+		var patchedResourcePath []string
 		for i, test := range testResults {
 			var userDefinedPolicyNamespace string
 			var userDefinedPolicyName string
@@ -457,7 +456,7 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu
 					}
 				}
 
-				patcheResourcePath = append(patcheResourcePath, test.PatchedResource)
+				patchedResourcePath = append(patchedResourcePath, test.PatchedResource)
 				if _, ok := results[resultsKey]; !ok {
 					results[resultsKey] = result
 				}
@@ -472,13 +471,12 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu
 
 			var resultsKey []string
 			var resultKey string
-
 			var result report.PolicyReportResult
 			resultsKey = GetAllPossibleResultsKey(policyNamespace, policyName, rule.Name, resourceNamespace, resourceKind, resourceName)
-			for _, resultK := range resultsKey {
-				if val, ok := results[resultK]; ok {
+			for _, key := range resultsKey {
+				if val, ok := results[key]; ok {
 					result = val
-					resultKey = resultK
+					resultKey = key
 				} else {
 					continue
 				}
@@ -491,7 +489,7 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu
 
 				} else {
 					var x string
-					for _, path := range patcheResourcePath {
+					for _, path := range patchedResourcePath {
 						result.Result = report.StatusFail
 						x = getAndComparePatchedResource(path, resp.PatchedResource, isGit, policyResourcePath, fs)
 						if x == "pass" {
@@ -538,12 +536,12 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu
 	return results, testResults
 }
 
-func GetAllPossibleResultsKey(policyNs, policy, rule, resourceNsnamespace, kind, resource string) []string {
+func GetAllPossibleResultsKey(policyNamespace, policy, rule, resourceNamespace, kind, resource string) []string {
 	var resultsKey []string
 	resultKey1 := fmt.Sprintf("%s-%s-%s-%s", policy, rule, kind, resource)
-	resultKey2 := fmt.Sprintf("%s-%s-%s-%s-%s", policy, rule, resourceNsnamespace, kind, resource)
-	resultKey3 := fmt.Sprintf("%s-%s-%s-%s-%s", policyNs, policy, rule, kind, resource)
-	resultKey4 := fmt.Sprintf("%s-%s-%s-%s-%s-%s", policyNs, policy, rule, resourceNsnamespace, kind, resource)
+	resultKey2 := fmt.Sprintf("%s-%s-%s-%s-%s", policy, rule, resourceNamespace, kind, resource)
+	resultKey3 := fmt.Sprintf("%s-%s-%s-%s-%s", policyNamespace, policy, rule, kind, resource)
+	resultKey4 := fmt.Sprintf("%s-%s-%s-%s-%s-%s", policyNamespace, policy, rule, resourceNamespace, kind, resource)
 	resultsKey = append(resultsKey, resultKey1, resultKey2, resultKey3, resultKey4)
 	return resultsKey
 }
@@ -584,12 +582,12 @@ func getAndComparePatchedResource(path string, enginePatchedResource unstructure
 	if err != nil {
 		os.Exit(1)
 	}
-	var log logr.Logger
-	matched, err := generate.ValidateResourceWithPattern(log, enginePatchedResource.UnstructuredContent(), patchedResources.UnstructuredContent())
-
+	matched, err := generate.ValidateResourceWithPattern(log.Log, enginePatchedResource.UnstructuredContent(), patchedResources.UnstructuredContent())
 	if err != nil {
+		log.Log.Info("patched resource mismatch", "error", err.Error())
 		status = "fail"
 	}
+
 	if matched == "" {
 		status = "pass"
 	}

test/cli/test-mutate/foreach/addIfNotPresent/deploy-patched.yaml

@@ -0,0 +1,35 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: svc-sizelimit-test
+  namespace: default
+spec:
+  selector:
+    matchLabels:
+      test: svc-sizelimit-test
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        test: svc-sizelimit-test
+    spec:
+      securityContext:
+        runAsUser: 65000
+        runAsGroup: 65000
+      containers:
+        - name: pause
+          image: k8s.gcr.io/pause:3.3
+          resources:
+            limits:
+              cpu: 10m
+              memory: 32Mi
+            requests:
+              cpu: 10m
+              memory: 32Mi
+          volumeMounts:
+            - name: vol02
+              mountPath: /opt02
+      volumes:
+        - name: vol02
+          emptyDir:
+            sizeLimit: 20Mi

test/cli/test-mutate/foreach/addIfNotPresent/kyverno-test.yaml

@@ -0,0 +1,12 @@
+name: foreach-mutate
+policies:
+  - policies.yaml
+resources:
+  - resources.yaml
+results:
+  - policy: mutate-emptydir
+    rule: setDefault
+    resource: svc-sizelimit-test
+    patchedResource: deploy-patched.yaml
+    kind: Deployment
+    result: pass

test/cli/test-mutate/foreach/addIfNotPresent/policies.yaml

@@ -0,0 +1,22 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: mutate-emptydir
+spec:
+  rules:
+    - name: setDefault
+      match:
+        resources:
+          kinds:
+            - Deployment
+      mutate:
+        foreach:
+          - list: "request.object.spec.template.spec.volumes"
+            patchStrategicMerge:
+              spec:
+                template:
+                  spec:
+                    volumes:
+                      - name: "{{ element.name }}"
+                        (emptyDir):
+                          +(sizeLimit): "20Mi"

test/cli/test-mutate/foreach/addIfNotPresent/resources.yaml

@@ -0,0 +1,35 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: svc-sizelimit-test
+  namespace: default
+spec:
+  selector:
+    matchLabels:
+      test: svc-sizelimit-test
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        test: svc-sizelimit-test
+    spec:
+      securityContext:
+        runAsUser: 65000
+        runAsGroup: 65000
+      containers:
+        - name: pause
+          image: k8s.gcr.io/pause:3.3
+          resources:
+            limits:
+              cpu: 10m
+              memory: 32Mi
+            requests:
+              cpu: 10m
+              memory: 32Mi
+          volumeMounts:
+            - name: vol02
+              mountPath: /opt02
+      volumes:
+        - name: vol02
+          emptyDir: {}
+   

test/cli/test-mutate/foreach/replaceRegistry/kyverno-test.yaml

@@ -0,0 +1,12 @@
+name: foreach-mutate
+policies:
+  - policies.yaml
+resources:
+  - resources.yaml
+results:
+  - policy: replace-image-registry-containers
+    rule: set-default
+    resource: test-patched-image
+    patchedResource: pod-patched.yaml
+    kind: Pod
+    result: pass

test/cli/test-mutate/foreach/replaceRegistry/pod-patched.yaml

@@ -0,0 +1,27 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  namespace: default
+  name: test-patched-image
+spec:
+  containers:
+    - args:
+        - --web.listen-address=127.0.0.1:9100
+        - --path.procfs=/host/proc
+        - --path.sysfs=/host/sys
+        - --path.rootfs=/host/root
+        - --no-collector.wifi
+        - --no-collector.hwmon
+        - --collector.filesystem.ignored-mount-points=^/(dev|proc|sys|var/lib/docker/.+)($|/)
+        - --collector.filesystem.ignored-fs-types=^(autofs|binfmt_misc|cgroup|configfs|debugfs|devpts|devtmpfs|fusectl|hugetlbfs|mqueue|overlay|proc|procfs|pstore|rpc_pipefs|securityfs|sysfs|tracefs)$
+      image: test/test3.2
+      imagePullPolicy: IfNotPresent
+      name: node-exporter
+    - args:
+        - --logtostderr
+        - --secure-listen-address=[$(IP)]:9100
+        - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_RSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
+        - --upstream=http://127.0.0.1:9100/
+      image: test/test3.2
+      imagePullPolicy: IfNotPresent
+      name: kube-rbac-proxy

test/cli/test-mutate/foreach/replaceRegistry/policies.yaml

@@ -0,0 +1,22 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: replace-image-registry-containers
+  annotations:
+    pod-policies.kyverno.io/autogen-controllers: "none"
+spec:
+  rules:
+    - name: set-default
+      match:
+        all:
+          - resources:
+              kinds:
+                - Pod
+      mutate:
+        foreach:
+          - list: "request.object.spec.containers"
+            patchStrategicMerge:
+              spec:
+                containers:
+                  - (name): "{{ element.name }}"
+                    image: test/test3.2

test/cli/test-mutate/foreach/replaceRegistry/resources.yaml

@@ -0,0 +1,27 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  namespace: default
+  name: test-patched-image
+spec:
+  containers:
+    - args:
+        - --web.listen-address=127.0.0.1:9100
+        - --path.procfs=/host/proc
+        - --path.sysfs=/host/sys
+        - --path.rootfs=/host/root
+        - --no-collector.wifi
+        - --no-collector.hwmon
+        - "--collector.filesystem.ignored-mount-points=^/(dev|proc|sys|var/lib/docker/.+)($|/)"
+        - "--collector.filesystem.ignored-fs-types=^(autofs|binfmt_misc|cgroup|configfs|debugfs|devpts|devtmpfs|fusectl|hugetlbfs|mqueue|overlay|proc|procfs|pstore|rpc_pipefs|securityfs|sysfs|tracefs)$"
+      image: docker.io/prom/node-exporter:v0.18.1
+      imagePullPolicy: IfNotPresent
+      name: node-exporter
+    - args:
+        - --logtostderr
+        - --secure-listen-address=[$(IP)]:9100
+        - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_RSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
+        - --upstream=http://127.0.0.1:9100/
+      image: kubesphere/kube-rbac-proxy:v0.8.0
+      imagePullPolicy: IfNotPresent
+      name: kube-rbac-proxy