Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] - Safe to evict policy fails on cronjobs without volumes #1915

Closed
nomeelnoj opened this issue May 17, 2021 · 1 comment · Fixed by #2156
Closed

[BUG] - Safe to evict policy fails on cronjobs without volumes #1915

nomeelnoj opened this issue May 17, 2021 · 1 comment · Fixed by #2156
Assignees
Labels
bug Something isn't working mutation Issues pertaining to the mutate ability.

Comments

@nomeelnoj
Copy link

Software version numbers
State the version numbers of applications involved in the bug.

  • Kubernetes version: 1.19
  • Kyverno version: 1.3.5

Describe the bug
When using the recommended safe-to-evict policy from the docs, it fails to properly validate on cronjobs that do not have a volume present

To Reproduce
Steps to reproduce the behavior:

  1. Create cronjob manifest without a volume definition:
apiVersion: batch/v1beta1
kind: CronJob
metadata:
  name: mycronjob
  namespace: default
spec:
  concurrencyPolicy: Forbid
  jobTemplate:
    spec:
      template:
        spec:
          containers:
          - envFrom:
            - configMapRef:
                name: myconfigmap
            image: myimage:latest
            imagePullPolicy: Always
            name: myjob
          restartPolicy: OnFailure
  schedule: 0 0 * * *
  1. Use the default policy from the website
  2. Run the policy in cluster or check it with the kyverno CLI:
kyverno apply add-safe-to-evict.yaml --resource mycron.yaml
  1. Observe the error
╰─ kyverno apply add-safe-to-evict.yaml --resource mycron.yaml

applying 1 policy to 1 resource...
E0517 12:43:22.654826   47224 strategicMergePatch.go:69] EngineMutate "msg"="failed to apply patchStrategicMerge" "error"="failed to preProcess rule: wrong Node Kind for  expected: SequenceNode was MappingNode: value: {{\"containers\": [{\"envFrom\": [{\"configMapRef\": {\"name\": \"myconfigmap\"}}], \"image\": \"myimage:latest\", \"imagePullPolicy\": \"Always\", \"name\": \"myjob\"}], \"restartPolicy\": \"OnFailure\"}}" "kind"="CronJob" "name"="mycronjob" "namespace"="default" "policy"="add-safe-to-evict" "rule"="autogen-cronjob-annotate-empty-dir"
E0517 12:43:22.655304   47224 strategicMergePatch.go:69] EngineMutate "msg"="failed to apply patchStrategicMerge" "error"="failed to preProcess rule: wrong Node Kind for  expected: SequenceNode was MappingNode: value: {{\"containers\": [{\"envFrom\": [{\"configMapRef\": {\"name\": \"myconfigmap\"}}], \"image\": \"myimage:latest\", \"imagePullPolicy\": \"Always\", \"name\": \"myjob\"}], \"restartPolicy\": \"OnFailure\"}}" "kind"="CronJob" "name"="mycronjob" "namespace"="default" "policy"="add-safe-to-evict" "rule"="autogen-cronjob-annotate-host-path"
Failed to apply mutate policy add-safe-to-evict -> resource default/CronJob/mycronjob
1. failed to apply patchStrategicMerge: failed to preProcess rule: wrong Node Kind for  expected: SequenceNode was MappingNode: value: {{"containers": [{"envFrom": [{"configMapRef": {"name": "myconfigmap"}}], "image": "myimage:latest", "imagePullPolicy": "Always", "name": "myjob"}], "restartPolicy": "OnFailure"}}
2. failed to apply patchStrategicMerge: failed to preProcess rule: wrong Node Kind for  expected: SequenceNode was MappingNode: value: {{"containers": [{"envFrom": [{"configMapRef": {"name": "myconfigmap"}}], "image": "myimage:latest", "imagePullPolicy": "Always", "name": "myjob"}], "restartPolicy": "OnFailure"}}
pass: 0, fail: 1, warn: 0, error: 0, skip: 0

Expected behavior
The policy applies the annotation if an emptydir or host path is present, or passes without error if not.

You can get it to work by changing the policy mutate spec from:

spec:
  volumes:
  - (emptyDir): {}

to

spec:
  (volumes):
  - (emptyDir): {}

Opening per request of @realshuting in kyverno/policies#61

@realshuting
Copy link
Member

@kacejot - are you planning to take this issue? Is this the same issue as what we have in #1658? If so, I can re-assign this issue to you and let's close both issues with fixing PR.

@chipzoller chipzoller added the mutation Issues pertaining to the mutate ability. label Jul 15, 2021
kacejot added a commit to kacejot/kyverno that referenced this issue Jul 16, 2021
Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>
@realshuting realshuting assigned kacejot and unassigned vyankyGH Jul 22, 2021
realshuting pushed a commit that referenced this issue Jul 23, 2021
* finished walkMap

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* added validation to the patchStrategicMerge

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* finished fixing tests

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* fixed part of old tests

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* patchStrategicMerge anchor preprocessing is finished

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* fix #1915 and #1896

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* fix lint errors

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* removed debug logs

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* added failing test

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>

* Fix unnecessary deletion

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mutation Issues pertaining to the mutate ability.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants