From d566e9886cf8d351722960e7c9d2ea5f3421656c Mon Sep 17 00:00:00 2001 From: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> Date: Wed, 21 Feb 2024 12:50:43 +0530 Subject: [PATCH] Fix :variables are not getting processed in validation message for "anyPattern" (#9713) * Update validate_resource.go Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Create pod.yaml Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Create chainsaw-test.yaml Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Create policy.yaml Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Update validate_resource.go Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * test Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * test Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * test Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * test Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Update chainsaw-test.yaml Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Create README.md Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Update test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/README.md Co-authored-by: Mariam Fahmy Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Update test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/README.md Co-authored-by: Mariam Fahmy Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Update test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/chainsaw-test.yaml Co-authored-by: Mariam Fahmy Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> * Update test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/README.md Co-authored-by: Mariam Fahmy Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> --------- Signed-off-by: mohamedasifs123 <142201466+mohamedasifs123@users.noreply.github.com> Co-authored-by: Mariam Fahmy --- .../handlers/validation/validate_resource.go | 21 ++++---- .../README.md | 11 +++++ .../chainsaw-test.yaml | 33 +++++++++++++ .../pod.yaml | 14 ++++++ .../policy-assert.yaml | 9 ++++ .../policy.yaml | 48 +++++++++++++++++++ 6 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/README.md create mode 100644 test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/chainsaw-test.yaml create mode 100644 test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/pod.yaml create mode 100644 test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/policy-assert.yaml create mode 100644 test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/policy.yaml diff --git a/pkg/engine/handlers/validation/validate_resource.go b/pkg/engine/handlers/validation/validate_resource.go index d1a45e7b65..ef952ed8cc 100644 --- a/pkg/engine/handlers/validation/validate_resource.go +++ b/pkg/engine/handlers/validation/validate_resource.go @@ -389,7 +389,7 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *engine } v.log.V(4).Info(fmt.Sprintf("Validation rule '%s' failed. %s", v.rule.Name, errorStr)) - msg := buildAnyPatternErrorMessage(v.rule, errorStr) + msg := v.buildAnyPatternErrorMessage(errorStr) return engineapi.RuleFail(v.rule.Name, engineapi.Validation, msg) } } @@ -440,17 +440,22 @@ func (v *validator) buildErrorMessage(err error, path string) string { } } -func buildAnyPatternErrorMessage(rule kyvernov1.Rule, errors []string) string { +func (v *validator) buildAnyPatternErrorMessage(errors []string) string { errStr := strings.Join(errors, " ") - if rule.Validation.Message == "" { + if v.rule.Validation.Message == "" { return fmt.Sprintf("validation error: %s", errStr) } - - if strings.HasSuffix(rule.Validation.Message, ".") { - return fmt.Sprintf("validation error: %s %s", rule.Validation.Message, errStr) + msgRaw, sErr := variables.SubstituteAll(v.log, v.policyContext.JSONContext(), v.rule.Validation.Message) + if sErr != nil { + v.log.V(2).Info("failed to substitute variables in message", "error", sErr) + return fmt.Sprintf("validation error: variables substitution error in rule %s execution error: %s", v.rule.Name, errStr) + } else { + msg := msgRaw.(string) + if strings.HasSuffix(msg, ".") { + return fmt.Sprintf("validation error: %s %s", msg, errStr) + } + return fmt.Sprintf("validation error: %s. %s", msg, errStr) } - - return fmt.Sprintf("validation error: %s. %s", rule.Validation.Message, errStr) } func (v *validator) substitutePatterns() error { diff --git a/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/README.md b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/README.md new file mode 100644 index 0000000000..85be009d4f --- /dev/null +++ b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/README.md @@ -0,0 +1,11 @@ +## Description + +This test ensures that variables are substituted correctly in the validation messages for `anyPattern`. + +## Expected Behavior + +The variable `allowedUIDs` will be successfully substituted by `9999 | 4000` in the validation message. + +## Reference Issue(s) + +#8095 diff --git a/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/chainsaw-test.yaml b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/chainsaw-test.yaml new file mode 100644 index 0000000000..0a99116d08 --- /dev/null +++ b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/chainsaw-test.yaml @@ -0,0 +1,33 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: variable-substitution-failure-messages +spec: + steps: + - name: step-01 + try: + - apply: + file: policy.yaml + - assert: + file: policy-assert.yaml + - name: step-02 + try: + - script: + content: kubectl apply -f pod.yaml + check: + ($error != null): true + # This check ensures the contents of stderr are exactly as shown. + ($stderr): |- + Error from server: error when creating "pod.yaml": admission webhook "validate.kyverno.svc-fail" denied the request: + + resource Pod/default/ba was blocked due to the following policies + + uid-groups-fsgroup-validate: + check-runasuser: 'validation error: Running with specific user IDs 9999 | 4000. + The fields spec.securityContext.runAsGroup, spec.containers[*].securityContext.runAsGroup, + spec.initContainers[*].securityContext.runAsGroup, and spec.ephemeralContainers[*].securityContext.runAsGroup + must be set to one of the 9999 | 4000 values. rule check-runasuser[0] failed at + path /spec/containers/0/securityContext/runAsUser/ rule check-runasuser[1] failed + at path /spec/containers/0/securityContext/runAsUser/' + diff --git a/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/pod.yaml b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/pod.yaml new file mode 100644 index 0000000000..f7e036cd5c --- /dev/null +++ b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/pod.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: Pod +metadata: + name: ba + labels: + app: nginx-users +spec: + securityContext: + runAsUser: 115 + containers: + - name: notnginx + image: nothingherenginx + securityContext: + runAsUser: 250 \ No newline at end of file diff --git a/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/policy-assert.yaml b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/policy-assert.yaml new file mode 100644 index 0000000000..efa97035d5 --- /dev/null +++ b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/policy-assert.yaml @@ -0,0 +1,9 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: uid-groups-fsgroup-validate +status: + conditions: + - reason: Succeeded + status: "True" + type: Ready \ No newline at end of file diff --git a/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/policy.yaml b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/policy.yaml new file mode 100644 index 0000000000..6cbfcd87cb --- /dev/null +++ b/test/conformance/chainsaw/validate/clusterpolicy/cornercases/variable-substitution-failure-messages/policy.yaml @@ -0,0 +1,48 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: uid-groups-fsgroup-validate +spec: + validationFailureAction: enforce + background: true + rules: + - name: check-runasuser + context: + - name: allowedUIDs + variable: + value: "9999 | 4000" + match: + any: + - resources: + kinds: + - Pod + validate: + message: >- + Running with specific user IDs {{ allowedUIDs }}. The fields + spec.securityContext.runAsGroup, spec.containers[*].securityContext.runAsGroup, + spec.initContainers[*].securityContext.runAsGroup, and + spec.ephemeralContainers[*].securityContext.runAsGroup must be + set to one of the {{ allowedUIDs }} values. + anyPattern: + - spec: + securityContext: + runAsUser: "{{ allowedUIDs }}" + =(ephemeralContainers): + - =(securityContext): + =(runAsUser): "{{ allowedUIDs }}" + =(initContainers): + - =(securityContext): + =(runAsUser): "{{ allowedUIDs }}" + containers: + - =(securityContext): + =(runAsUser): "{{ allowedUIDs }}" + - spec: + =(ephemeralContainers): + - securityContext: + runAsUser: "{{ allowedUIDs }}" + =(initContainers): + - securityContext: + runAsUser: "{{ allowedUIDs }}" + containers: + - securityContext: + runAsUser: "{{ allowedUIDs }}" \ No newline at end of file