Treat normal and precondition variable equally (#4217)

* When  the value of the variables not present will assigned as nil

Signed-off-by: viveksahu26 <vivekkumarsahu650@gmail.com>

* Added cli test cases

Signed-off-by: viveksahu26 <vivekkumarsahu650@gmail.com>

* fixed failing test cases

Signed-off-by: viveksahu26 <vivekkumarsahu650@gmail.com>

* remove extra line

Signed-off-by: viveksahu26 <vivekkumarsahu650@gmail.com>

Signed-off-by: viveksahu26 <vivekkumarsahu650@gmail.com>
Co-authored-by: Vyankatesh Kudtarkar <vyankateshkd@gmail.com>

pkg/engine/validation.go

@@ -318,6 +318,13 @@ func (v *validator) validateElements(foreach kyvernov1.ForEachValidation, elemen
 			v.log.Info("skip rule", "reason", r.Message)
 			continue
 		} else if r.Status != response.RuleStatusPass {
+			if r.Status == response.RuleStatusError {
+				if i < len(elements)-1 {
+					continue
+				}
+				msg := fmt.Sprintf("validation failure: %v", r.Message)
+				return ruleResponse(*v.rule, response.Validation, msg, r.Status, nil), applyCount
+			}
 			msg := fmt.Sprintf("validation failure: %v", r.Message)
 			return ruleResponse(*v.rule, response.Validation, msg, r.Status, nil), applyCount
 		}

pkg/engine/variables/vars.go

@@ -67,8 +67,8 @@ func newPreconditionsVariableResolver(log logr.Logger) VariableResolver {
 	return func(ctx context.EvalInterface, variable string) (interface{}, error) {
 		value, err := DefaultVariableResolver(ctx, variable)
 		if err != nil {
-			log.V(4).Info(fmt.Sprintf("using empty string for unresolved variable \"%s\" in preconditions", variable))
-			return "", nil
+			log.V(4).Info(fmt.Sprintf("Variable substitution failed in preconditions, therefore nil value assigned to variable,  \"%s\" ", variable))
+			return value, err
 		}
 
 		return value, nil

test/cli/test/nil-values-in-variables/exclude_namespaces_dynamically/exclude_namespaces_dynamically.yaml

@@ -0,0 +1,45 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: exclude-namespaces-example
+  annotations:
+    policies.kyverno.io/title: Exclude Namespaces Dynamically
+    policies.kyverno.io/category: Sample
+    policies.kyverno.io/severity: medium
+    policies.kyverno.io/subject: Namespace, Pod
+    policies.kyverno.io/minversion: 1.6.0
+    kyverno.io/kyverno-version: 1.6.0
+    kyverno.io/kubernetes-version: "1.23"
+    policies.kyverno.io/description: >-
+      It's common where policy lookups need to consider a mapping to many possible values rather than a
+      static mapping. This is a sample which demonstrates how to dynamically look up an allow list of Namespaces from a ConfigMap
+      where the ConfigMap stores an array of strings. This policy validates that any Pods created
+      outside of the list of Namespaces have the label `foo` applied.
+spec:
+  validationFailureAction: audit
+  background: true
+  rules:
+  - name: exclude-namespaces-dynamically
+    context:
+      - name: namespacefilters
+        configMap:
+          name: namespace-filters
+          namespace: default
+    match:
+      resources:
+        kinds:
+        - Pod
+    preconditions:
+      all:
+      - key: "{{request.object.metadata.namespace}}"
+        operator: AnyNotIn
+        value: "{{namespacefilters.data.exclude}}"
+    validate:
+      message: >
+        Creating Pods in the  namespace,
+        which is not in the excluded list of namespaces {{ namespacefilters.data.exclude }},
+        is forbidden unless it carries the label `foo`.
+      pattern:
+        metadata:
+          labels:
+            foo: "*"

test/cli/test/nil-values-in-variables/exclude_namespaces_dynamically/kyverno-test.yaml

@@ -0,0 +1,17 @@
+name: exclude-namespaces-example
+policies:
+  - exclude_namespaces_dynamically.yaml
+resources:
+  - resource.yaml
+variables: values.yaml
+results:
+  - policy: exclude-namespaces-example
+    rule: exclude-namespaces-dynamically
+    resource: bad-pod01
+    kind: Pod
+    result: pass
+  - policy: exclude-namespaces-example
+    rule: exclude-namespaces-dynamically
+    resource: bad-pod02
+    kind: Pod
+    result: error

test/cli/test/nil-values-in-variables/exclude_namespaces_dynamically/resource.yaml

@@ -0,0 +1,23 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: bad-pod01
+  namespace: vivek
+  labels:
+    foo: bar
+spec:
+  containers:
+  - name: nginx
+    image: nginx:alpine 
+---
+
+apiVersion: v1
+kind: Pod
+metadata:
+  name: bad-pod02
+  labels:
+    foo: bar
+spec:
+  containers:
+  - name: nginx
+    image: nginx:alpine 

test/cli/test/nil-values-in-variables/exclude_namespaces_dynamically/values.yaml

@@ -0,0 +1,10 @@
+policies:
+  - name: exclude-namespaces-example
+    rules:
+      - name: exclude-namespaces-dynamically
+    resources:
+    - name: bad-pod01
+      values:
+        namespacefilters.data.exclude: "[\"default\", \"test\"]"
+        request.namespace: default
+    - name: bad-pod02

test/cli/test/nil-values-in-variables/limit-duration/kyverno-test.yaml

@@ -0,0 +1,16 @@
+name: limit-duration
+policies:
+  -  limit-duration.yaml
+resources:
+  -  resource.yaml
+results:
+  - policy: cert-manager-limit-duration
+    rule: certificate-duration-max-100days 
+    resource: letsencrypt-crt
+    kind: Certificate
+    result: error
+  - policy: cert-manager-limit-duration
+    rule: certificate-duration-max-100days 
+    resource: acme-crt
+    kind: Certificate
+    result: error

test/cli/test/nil-values-in-variables/limit-duration/limit-duration.yaml

@@ -0,0 +1,36 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: cert-manager-limit-duration
+  annotations:
+    policies.kyverno.io/title: Certificate max duration 100 days
+    policies.kyverno.io/category: Cert-Manager
+    policies.kyverno.io/severity: medium
+    policies.kyverno.io/minversion: 1.3.6
+    policies.kyverno.io/subject: Certificate
+    policies.kyverno.io/description: >-
+      Kubernetes managed non-letsencrypt certificates have to be renewed in every 100 days.
+spec:
+  validationFailureAction: audit
+  background: false
+  rules:
+  - name: certificate-duration-max-100days 
+    match:
+      resources:
+        kinds:
+        - Certificate
+    preconditions:
+      all:
+      - key: "{{ contains(request.object.spec.issuerRef.name, 'letsencrypt') }}"
+        operator: Equals
+        value: False
+      - key: "{{ request.object.spec.duration }}"
+        operator: NotEquals
+        value: ""
+    validate:
+      message: "certificate duration must be < than 2400h (100 days)"
+      deny:
+        conditions:
+        - key: "{{ max( [ to_number(regex_replace_all('h.*',request.object.spec.duration,'')), to_number('2400') ] ) }}"
+          operator: NotEquals
+          value: 2400

test/cli/test/nil-values-in-variables/limit-duration/resource.yaml

@@ -0,0 +1,28 @@
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: letsencrypt-crt
+spec:
+  secretName: letsencrypt-crt-secret
+  dnsNames:
+  - example.com
+  - foo.example.com
+  issuerRef:
+    name: letsencrypt-prod
+    kind: Issuer
+    group: cert-manager.io
+  # duration  field is not present, therefore nil be assigned.
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: acme-crt
+spec:
+  secretName: acme-crt-secret
+  dnsNames:
+  - example.com
+  issuerRef:
+    name: acme-prod
+    kind: Issuer
+    group: cert-manager.io
+  # duration  field is not present, therefore nil be assigned.