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

Fix: handling unexpected global-anchor-variable for the apply command #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ansalamdaniel
Copy link

@ansalamdaniel ansalamdaniel commented Nov 28, 2022

Signed-off-by: ansalamdaniel [email protected]

Explanation

This PR aims to handle the global anchor variable disparity in apply command for mutation policy. Resource is mutated even if the global anchor match is false.

policy.yaml

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata: 
  name: add-safe-to-evict
  annotations:
    policies.kyverno.io/category: Workload Management
    policies.kyverno.io/description: The Kubernetes cluster autoscaler does not evict pods that 
      use hostPath or emptyDir volumes. To allow eviction of these pods, the annotation 
      cluster-autoscaler.kubernetes.io/safe-to-evict=true must be added to the pods. 
spec: 
  rules: 
  - name: annotate-empty-dir
    match:
      any:
      - resources: 
          kinds:
          - Pod
    mutate: 
      patchStrategicMerge:
        metadata:
          annotations:
            +(cluster-autoscaler.kubernetes.io/safe-to-evict): "true"
        spec:          
          volumes: 
          - <(emptyDir): {}

resource.yaml

apiVersion: v1
kind: Pod
metadata:
  name: pod-without-emptydir-hostpath
spec:
  containers:
  - name: nginx
    image: nginx

Shell Output

$ kc kyverno version                            
Version: 1.8.2
Time: 2022-11-21T08:37:03Z
Git commit ID: 540f133fdd1f78d394a2ec31658361b9cdc57a33
$ kc kyverno apply policy.yaml -r resources.yaml -v=4
I1128 19:11:57.416257 1436592 common.go:353]  "msg"="Defaulting request.operation to CREATE" 
I1128 19:11:57.684065 1436592 common.go:178]  "msg"="read policies" "errors"=0 "policies"=1
I1128 19:11:57.684594 1436592 rule.go:233] autogen "msg"="processing rule" "rulename"="annotate-empty-dir"

<snip>

Applying 1 policy rule to 1 resource...

<snip>

I1128 19:11:57.688111 1436592 common.go:454]  "msg"="applying policy on resource" "policy"="add-safe-to-evict" "resource"="default/Pod/pod-without-emptydir-hostpath"
I1128 19:11:57.688267 1436592 context.go:273]  "msg"="updated image info" "images"={"containers":{"nginx-123":{"registry":"docker.io","name":"nginx","path":"nginx","tag":"latest"}}}
I1128 19:11:57.688371 1436592 utils.go:30]  "msg"="applied JSON patch" "patch"=[{"op":"replace","path":"/spec/containers/0/image","value":"docker.io/nginx:latest"}]
I1128 19:11:57.688592 1436592 mutation.go:33] EngineMutate "msg"="start mutate policy processing" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "startTime"="2022-11-28T19:11:57.688556008+05:30"

<snip>

I1128 19:11:57.688696 1436592 mutation.go:61] EngineMutate "msg"="processing mutate rule" "applyRules"="All" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "rule"="annotate-empty-dir"
I1128 19:11:57.688898 1436592 mutation.go:108] EngineMutate "msg"="apply rule to resource" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "resource name"="pod-without-emptydir-hostpath" "resource namespace"="default" "rule"="annotate-empty-dir"
I1128 19:11:57.689503 1436592 strategicMergePatch.go:21] EngineMutate/ProcessStrategicMergePatch "msg"="started applying strategicMerge patch" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "rule"="annotate-empty-dir" "startTime"="2022-11-28T19:11:57.689484856+05:30"
I1128 19:11:57.689613 1436592 strategicMergePatch.go:100] EngineMutate/ProcessStrategicMergePatch "msg"="applying strategic merge patch" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "patch"="{\"metadata\": {\"annotations\": {\"cluster-autoscaler.kubernetes.io/safe-to-evict\": \"true\"}}}\n" "policy"="add-safe-to-evict" "rule"="annotate-empty-dir"
I1128 19:11:57.746998 1436592 strategicMergePatch.go:28] EngineMutate/ProcessStrategicMergePatch "msg"="finished applying strategicMerge patch" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "processingTime"="57.486111ms" "rule"="annotate-empty-dir"
I1128 19:11:57.748951 1436592 mutation.go:56] EngineMutate "msg"="rule not matched" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "reason"="rule autogen-annotate-empty-dir not matched:\n 1. kind does not match [DaemonSet Deployment Job StatefulSet]" "rule"="autogen-annotate-empty-dir"
I1128 19:11:57.749216 1436592 mutation.go:56] EngineMutate "msg"="rule not matched" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "reason"="rule autogen-cronjob-annotate-empty-dir not matched:\n 1. kind does not match [CronJob]" "rule"="autogen-cronjob-annotate-empty-dir"

<snip>

mutate policy add-safe-to-evict applied to default/Pod/pod-without-emptydir-hostpath:
apiVersion: v1
kind: Pod
metadata:
  annotations:
    cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
  name: pod-without-emptydir-hostpath
  namespace: default
spec:
  containers:
  - image: nginx
    name: nginx-123

---
<snip>

I1128 19:11:57.751803 1436592 imageVerify.go:76] EngineVerifyImages "msg"="processed image verification rules" "applied"=0 "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "successful"=true "time"="618.673µs"
I1128 19:11:57.751954 1436592 rule.go:233] autogen "msg"="processing rule" "rulename"="annotate-empty-dir"
I1128 19:11:57.752125 1436592 rule.go:279] autogen "msg"="generating rule for cronJob" 
I1128 19:11:57.752263 1436592 rule.go:233] autogen "msg"="processing rule" "rulename"="annotate-empty-dir"

pass: 1, fail: 0, warn: 0, error: 0, skip: 2 

Related issue

Closes: 2573

Milestone of this PR

What type of PR is this

/kind bug

Proposed Changes

Resource & pattern matching is handled if the resource is empty.

Proof Manifests

Applying the same policy and resource after the fix, the resource will be skipped.

Shell Output

$ go run ./cmd/cli/kubectl-kyverno/main.go apply ./2573-iss-apply/policy.yaml -r ./2573-iss-apply/resources.yaml -v=4
I1128 19:09:15.579476 1433018 common.go:333]  "msg"="Defaulting request.operation to CREATE" 
I1128 19:09:15.830490 1433018 common.go:191]  "msg"="read policies" "errors"=0 "policies"=1

<snip>

Applying 1 policy rule to 1 resource...
<snip>

I1128 19:09:15.835577 1433018 rule.go:286] autogen "msg"="generating rule for cronJob" 
I1128 19:09:15.835593 1433018 rule.go:233] autogen "msg"="processing rule" "rulename"="annotate-empty-dir"
I1128 19:09:15.835681 1433018 common.go:413]  "msg"="applying policy on resource" "policy"="add-safe-to-evict" "resource"="default/Pod/pod-without-emptydir-hostpath"
I1128 19:09:15.836014 1433018 context.go:273]  "msg"="updated image info" "images"={"containers":{"nginx-123":{"registry":"docker.io","name":"nginx","path":"nginx","tag":"latest"}}}
I1128 19:09:15.836188 1433018 utils.go:30]  "msg"="applied JSON patch" "patch"=[{"op":"replace","path":"/spec/containers/0/image","value":"docker.io/nginx:latest"}]
I1128 19:09:15.836507 1433018 mutation.go:33] EngineMutate "msg"="start mutate policy processing" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "startTime"="2022-11-28T19:09:15.836465624+05:30"

<snip>

I1128 19:09:15.836742 1433018 mutation.go:61] EngineMutate "msg"="processing mutate rule" "applyRules"="All" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "rule"="annotate-empty-dir"
I1128 19:09:15.837103 1433018 mutation.go:108] EngineMutate "msg"="apply rule to resource" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "resource name"="pod-without-emptydir-hostpath" "resource namespace"="default" "rule"="annotate-empty-dir"
I1128 19:09:15.837806 1433018 strategicMergePatch.go:21] EngineMutate/ProcessStrategicMergePatch "msg"="started applying strategicMerge patch" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "rule"="annotate-empty-dir" "startTime"="2022-11-28T19:09:15.837787419+05:30"
I1128 19:09:15.837914 1433018 strategicPreprocessing.go:168] EngineMutate/ProcessStrategicMergePatch "msg"="anchor mismatch" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "reason"="global condition failed: could not found \"emptyDir\" key in the resource" "rule"="annotate-empty-dir"
I1128 19:09:15.838002 1433018 strategicMergePatch.go:100] EngineMutate/ProcessStrategicMergePatch "msg"="applying strategic merge patch" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "patch"="{}\n" "policy"="add-safe-to-evict" "rule"="annotate-empty-dir"
I1128 19:09:15.913109 1433018 strategicMergePatch.go:28] EngineMutate/ProcessStrategicMergePatch "msg"="finished applying strategicMerge patch" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "processingTime"="75.290701ms" "rule"="annotate-empty-dir"
I1128 19:09:15.913208 1433018 mutation.go:56] EngineMutate "msg"="rule not matched" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "reason"="rule autogen-annotate-empty-dir not matched:\n 1. kind does not match [DaemonSet Deployment Job StatefulSet ReplicaSet ReplicationController]" "rule"="autogen-annotate-empty-dir"
I1128 19:09:15.913251 1433018 mutation.go:56] EngineMutate "msg"="rule not matched" "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "reason"="rule autogen-cronjob-annotate-empty-dir not matched:\n 1. kind does not match [CronJob]" "rule"="autogen-cronjob-annotate-empty-dir"

<snip>

skipped mutate policy add-safe-to-evict -> resource default/Pod/pod-without-emptydir-hostpathI1128 19:09:15.913522 1433018 rule.go:233] autogen "msg"="processing rule" "rulename"="annotate-empty-dir"

<snip>

I1128 19:09:15.913716 1433018 imageVerify.go:76] EngineVerifyImages "msg"="processed image verification rules" "applied"=0 "kind"="Pod" "name"="pod-without-emptydir-hostpath" "namespace"="default" "policy"="add-safe-to-evict" "successful"=true "time"="93.94µs"
I1128 19:09:15.913735 1433018 rule.go:233] autogen "msg"="processing rule" "rulename"="annotate-empty-dir"
I1128 19:09:15.913770 1433018 rule.go:286] autogen "msg"="generating rule for cronJob" 
I1128 19:09:15.913781 1433018 rule.go:233] autogen "msg"="processing rule" "rulename"="annotate-empty-dir"

pass: 0, fail: 0, warn: 0, error: 0, skip: 3 

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .
  • My PR contains new or altered behavior to Kyverno and
    • CLI support should be added and my PR doesn't contain that functionality.
    • I have added or changed the documentation myself in an existing PR and the link is:
    • I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

Note: Test cases are implemented for the kyverno test command.

Copy link

@shahpratikr shahpratikr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe change test/cli/test-mutate/global-anchor/patchedResource1.yaml to test/cli/test-mutate/global-anchor/patchedResourceWithVolume.yaml

@ansalamdaniel
Copy link
Author

Addressed review comment.

@ansalamdaniel ansalamdaniel force-pushed the issue-2573 branch 8 times, most recently from 31df790 to b51fdac Compare December 6, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants