diff --git a/pkg/validation/policy/validate.go b/pkg/validation/policy/validate.go index 193ddfdd3a..5e8dd0b818 100644 --- a/pkg/validation/policy/validate.go +++ b/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 - withoutTargets := ruleCopy.DeepCopy() - for i := range withoutTargets.Mutation.Targets { - withoutTargets.Mutation.Targets[i].RawAnyAllConditions = nil - } - ctx := buildContext(withoutTargets, background, false, nil) - if _, err := variables.SubstituteAllInRule(logging.GlobalLogger(), ctx, *withoutTargets); !variables.CheckNotFoundErr(err) { - return fmt.Errorf("variable substitution failed for rule %s: %s", withoutTargets.Name, err.Error()) + mutateTarget := false + if ruleCopy.Mutation.Targets != nil { + mutateTarget = true + withTargetOnly := ruleWithoutPattern(ruleCopy) + for i := range ruleCopy.Mutation.Targets { + withTargetOnly.Mutation.Targets[i].ResourceSpec = ruleCopy.Mutation.Targets[i].ResourceSpec + ctx := buildContext(withTargetOnly, background, false) + 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 - for _, target := range r.Mutation.Targets { - ctx := buildContext(ruleCopy, background, true, target.Context) - 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()) - } + ctx := buildContext(ruleCopy, background, mutateTarget) + if _, err := variables.SubstituteAllInRule(logging.GlobalLogger(), ctx, *ruleCopy); !variables.CheckNotFoundErr(err) { + return fmt.Errorf("variable substitution failed for rule %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) diff --git a/test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/01-policy.yaml b/test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/01-policy.yaml new file mode 100644 index 0000000000..ac880f35d2 --- /dev/null +++ b/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 diff --git a/test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/README.md b/test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/README.md new file mode 100644 index 0000000000..41cbef57c8 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/policy-bad.yaml b/test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/policy-bad.yaml new file mode 100644 index 0000000000..208c1b9f7f --- /dev/null +++ b/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 }}" \ No newline at end of file diff --git a/test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/policy-good.yaml b/test/conformance/kuttl/mutate/clusterpolicy/standard/existing/validation/target-variable-validation/policy-good.yaml new file mode 100644 index 0000000000..4e18e9e385 --- /dev/null +++ b/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 }}" \ No newline at end of file diff --git a/test/conformance/kuttl/policy-validation/cluster-policy/target-context/01-policies.yaml b/test/conformance/kuttl/policy-validation/cluster-policy/target-context/01-policies.yaml index 22a4c123cf..f5e78e4071 100644 --- a/test/conformance/kuttl/policy-validation/cluster-policy/target-context/01-policies.yaml +++ b/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 -# shouldFail: true +- file: policy-1.yaml + shouldFail: true - file: policy-2.yaml shouldFail: true diff --git a/test/conformance/kuttl/policy-validation/cluster-policy/target-context/policy-1.yaml b/test/conformance/kuttl/policy-validation/cluster-policy/target-context/policy-1.yaml index b0519c4b74..c0aa7e93fa 100644 --- a/test/conformance/kuttl/policy-validation/cluster-policy/target-context/policy-1.yaml +++ b/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 }}"