fix mutate targets validation (#7387)

* fix mutate targets validation

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

* linter fixes

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

---------

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

pkg/validation/policy/validate.go

@@ -405,22 +405,22 @@ func hasInvalidVariables(policy kyvernov1.PolicyInterface, background bool) erro
 			}
 		}
 
-		// skip variable checks on mutate.targets, they will be validated separately
+		mutateTarget := false
-		withoutTargets := ruleCopy.DeepCopy()
+		if ruleCopy.Mutation.Targets != nil {
-		for i := range withoutTargets.Mutation.Targets {
+			mutateTarget = true
-			withoutTargets.Mutation.Targets[i].RawAnyAllConditions = nil
+			withTargetOnly := ruleWithoutPattern(ruleCopy)
-		}
+			for i := range ruleCopy.Mutation.Targets {
-		ctx := buildContext(withoutTargets, background, false, nil)
+				withTargetOnly.Mutation.Targets[i].ResourceSpec = ruleCopy.Mutation.Targets[i].ResourceSpec
-		if _, err := variables.SubstituteAllInRule(logging.GlobalLogger(), ctx, *withoutTargets); !variables.CheckNotFoundErr(err) {
+				ctx := buildContext(withTargetOnly, background, false)
-			return fmt.Errorf("variable substitution failed for rule %s: %s", withoutTargets.Name, err.Error())
+				if _, err := variables.SubstituteAllInRule(logging.GlobalLogger(), ctx, *withTargetOnly); !variables.CheckNotFoundErr(err) {
+					return fmt.Errorf("invalid variables defined at mutate.targets[%d]: %s", i, err.Error())
+				}
+			}
 		}
 
-		// perform variable checks with mutate.targets
+		ctx := buildContext(ruleCopy, background, mutateTarget)
-		for _, target := range r.Mutation.Targets {
+		if _, err := variables.SubstituteAllInRule(logging.GlobalLogger(), ctx, *ruleCopy); !variables.CheckNotFoundErr(err) {
-			ctx := buildContext(ruleCopy, background, true, target.Context)
+			return fmt.Errorf("variable substitution failed for rule %s: %s", ruleCopy.Name, err.Error())
-			if _, err := variables.SubstituteAllInRule(logging.GlobalLogger(), ctx, *ruleCopy); !variables.CheckNotFoundErr(err) {
-				return fmt.Errorf("variable substitution failed for rule target %s: %s", ruleCopy.Name, err.Error())
-			}
 		}
 	}
 
@@ -555,7 +555,15 @@ func imageRefHasVariables(verifyImages []kyvernov1.ImageVerification) error {
 	return nil
 }
 
-func buildContext(rule *kyvernov1.Rule, background bool, target bool, targetContext []kyvernov1.ContextEntry) *enginecontext.MockContext {
+func ruleWithoutPattern(ruleCopy *kyvernov1.Rule) *kyvernov1.Rule {
+	withTargetOnly := new(kyvernov1.Rule)
+	withTargetOnly.Mutation.Targets = make([]kyvernov1.TargetResourceSpec, len(ruleCopy.Mutation.Targets))
+	withTargetOnly.Context = ruleCopy.Context
+	withTargetOnly.RawAnyAllConditions = ruleCopy.RawAnyAllConditions
+	return withTargetOnly
+}
+
+func buildContext(rule *kyvernov1.Rule, background bool, target bool) *enginecontext.MockContext {
 	re := getAllowedVariables(background, target)
 	ctx := enginecontext.NewMockContext(re)
 	addContextVariables(rule.Context, ctx)

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/01-policy.yaml

@@ -0,0 +1,7 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- file: policy-bad.yaml
+  shouldFail: true
+- file: policy-good.yaml
+  shouldFail: false

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/README.md

@@ -0,0 +1,12 @@
+## Description
+
+This test ensures the variable `target` is allowed in a mutateExisting rule, except resource's spec definition under `mutate.targets`.
+
+## Expected Behavior
+
+The good policy should be allowed to create while the bad policy that contains `target.metadata.annotations.dns` cannot be created as it's invalid.
+
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/7379

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/policy-bad.yaml

@@ -0,0 +1,29 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: target-variable-validation-cpol
+spec:
+  mutateExistingOnPolicyUpdate: true
+  schemaValidation: false
+  background: true
+  rules:
+    - name: target-variable-validation-rule
+      match:
+        any:
+          - resources:
+              kinds:
+                - Secret
+      mutate:
+        targets:
+          - apiVersion: v1
+            kind: ConfigMap
+            name: server-external
+            namespace: "{{target.metadata.annotations.dns}}"
+          - apiVersion: v1
+            kind: ConfigMap
+            name: server-internal
+            namespace: external-dns
+        patchesJson6902: |-
+          - op: replace
+            path: "/spec/endpoints/1/targets/0"
+            value: "{{ request.object.data.prefix6 }}{{ target.metadata.annotations.dns_suffix }}"

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/policy-good.yaml

@@ -0,0 +1,30 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: target-variable-validation-cpol
+spec:
+  mutateExistingOnPolicyUpdate: true
+  schemaValidation: false
+  background: true
+  rules:
+    - name: target-variable-validation-rule
+      match:
+        any:
+          - resources:
+              kinds:
+                - Secret
+      mutate:
+        targets:
+          - apiVersion: v1
+            kind: ConfigMap
+            name: server-external
+            # namespace: "{{target.metadata.annotations.dns}}"
+            namespace: external-dns
+          - apiVersion: v1
+            kind: ConfigMap
+            name: server-internal
+            namespace: external-dns
+        patchesJson6902: |-
+          - op: replace
+            path: "/spec/endpoints/1/targets/0"
+            value: "{{ request.object.data.prefix6 }}{{ target.metadata.annotations.dns_suffix }}"

test/conformance/kuttl/policy-validation/cluster-policy/target-context/01-policies.yaml

@@ -1,7 +1,7 @@
 apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 apply:
-# - file: policy-1.yaml
+- file: policy-1.yaml
-#   shouldFail: true
+  shouldFail: true
 - file: policy-2.yaml
   shouldFail: true

test/conformance/kuttl/policy-validation/cluster-policy/target-context/policy-1.yaml

@@ -17,7 +17,7 @@ spec:
         jmesPath: request.object.data.content
     - name: targetContent
       variable:
-        jmesPath: target.data.content
+        jmesPath: "{{target.data.content}}"
     preconditions:
       all:
       - key: "{{ request.object.metadata.name }}"