Update the status of test results to error [Bug]

viveksahu26 commented 2 years ago

Kyverno Version

1.7.x

Kubernetes Version

1.24.x

Kubernetes Platform

KinD

Description

Earlier when the values of preconditions are not found they are assigned as empty string, whereas for variables present outside the block of precondition or normal variables whose value is not found they are assigned as nil and error is thrown. This partiality b/w normal and preconditions variables has to remove. And from now onwards, any variables whether it's a normal or precondition variables, if there value not found then assigned them with a nil value.

The status of the policy earlier when values were not found for preconditions was assigned as empty therefore precondition block values is compared against the empty string with the values found in the resource. Which means that the possible results would be skip/fail/pass. But from now onwards if the values is assigned as nil, which means values for the variables are not assigned or not provided. Therefore the status of the result when value is not provided by the user won't be either skip/pass/fail. Simply it will be error.

Conclusion: When value for variable in not provided by the user either through resource or values.yaml or configMaps, therefore nil value is assigned. Status ---> error

Steps to reproduce

Apply policy


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


Resource

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
```
kyverno-test.yaml
```
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: skip


Check it with:  kyverno test ./

Currently, we can see that result is skip inspite of values of the variable for `request.object.spec.duration` in the policy.  is not provided. Since the issue 3982 is logged in kyverno repo. After the PR gets merged the status will get changed from skip to error. The reason why I log this issue earlier before PR  merged for 3982. Because cli test is failing on PR. On updating the status from skip to error,  cli test cases will pass.

Expected behavior

When value for variable in not provided by the user either through resource or values.yaml or configMaps, therefore nil value is assigned. Status should be error instead of skip/pass/fail.

Screenshots

No response

Kyverno logs

No response

Slack discussion

No response

Troubleshooting

[X] I have read and followed the documentation AND the troubleshooting guide.
[X] I have searched other issues in this repository and mine is not recorded.

viveksahu26 commented 2 years ago

Hey @chipzoller , could you please assign this to me.

chipzoller commented 2 years ago

Thanks for taking this up, Vivek. Getting these changes made will be very helpful. We may also need to update some e2e tests on kyverno/kyverno as well.

viveksahu26 commented 2 years ago

Thanks for taking this up, Vivek. Getting these changes made will be very helpful. We may also need to update some e2e tests on kyverno/kyverno as well.

Not sure about e2e test cases are present specifically for it or not. But I will look to it, if present then ok. Otherwise need to log an issue for the same to create new e2e test cases.

kyverno / policies