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

Implement global anchor #2311

Merged
merged 13 commits into from
Sep 13, 2021
Merged

Implement global anchor #2311

merged 13 commits into from
Sep 13, 2021

Conversation

kacejot
Copy link
Contributor

@kacejot kacejot commented Aug 25, 2021

Related issue

closes #2201

Milestone of this PR

/kind feature

Proposed Changes

Added global anchor logic for both validation and strategic merge patch.
Changed e2e tests and updated add-safe-to-evict policy with global anchor.
Global anchor is a key wrapped with <( and ) symbols. For example <(image).
When global anchor condition fails, entire policy application is skipped.

Proof Manifests

Resource:

apiVersion: v1
kind: Pod
metadata:
  name: static-web
  labels:
    role: myrole
spec:
  containers:
    - name: web
      image: 1nginx
      ports:
        - name: web
          containerPort: 80
          protocol: TCP

Validation:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: sample
spec:
  validationFailureAction: enforce
  rules:
  - name: check-container-image
    match:
      resources:
        kinds:
        - Pod
    validate:
      pattern:
        spec:
          containers:
          - name: "*"
            <(image): "nginx"

Strategic merge patch:

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: 
      resources: 
        kinds: 
        - Pod
    mutate: 
      patchStrategicMerge:
        metadata:
          annotations:
            +(cluster-autoscaler.kubernetes.io/safe-to-evict): "true"
        spec:          
          volumes: 
          - <(emptyDir): {}
  - name: annotate-host-path
    match: 
      resources: 
        kinds: 
        - Pod
    mutate: 
      patchStrategicMerge:
        metadata:
          annotations:
            +(cluster-autoscaler.kubernetes.io/safe-to-evict): "true"
        spec:          
          volumes: 
          - hostPath:
              <(path): "*"

^ Here is the updated policy variant

Checklist

kacejot and others added 9 commits July 27, 2021 18:20
Signed-off-by: Max Goncharenko <kacejot@fex.net>
Signed-off-by: Max Goncharenko <kacejot@fex.net>
Signed-off-by: Max Goncharenko <kacejot@fex.net>
Signed-off-by: Max Goncharenko <kacejot@fex.net>
Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>
Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>
Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>
Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>
Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>
@chipzoller
Copy link
Member

How would or does this work with other anchors in a policy? I assume there can't be two global anchors, correct? It would be good to better understand this change and what impact it may have to both existing policies in the docs/policies page and for users.

@chipzoller
Copy link
Member

I'd really like to see this presented at a contributors/community meeting so as to ask some live Q&A without having to go back and forth over GitHub and take two weeks.

@kacejot
Copy link
Contributor Author

kacejot commented Aug 26, 2021

@chipzoller, unfortunately I can't participate today. The reason is that I need some time to prepare manifests and test them. We could have a call tomorrow.

@realshuting
Copy link
Member

Hi @kacejot - got a few questions regarding the proof manifests:

  1. what does the following policy validate?
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: sample
spec:
  validationFailureAction: enforce
  rules:
  - name: check-container-image
    match:
      resources:
        kinds:
        - Pod
    validate:
      pattern:
        spec:
          containers:
          - name: "*"
            <(image): "nginx"
  1. I tested the above mutate policy with the following pod, but it doesn't seem to work:
apiVersion: v1
kind: Pod
metadata:
  name: static-web
  labels:
    role: myrole
spec:
  containers:
    - name: web
      image: 1nginx
      ports:
        - name: web
          containerPort: 80
          protocol: TCP
      volumeMounts:
      - mountPath: /cache
        name: cache-volume
  volumes: 
  - emptyDir: {}
    name: cache-volume
✗ k get pod static-web -o yaml | grep "annotations:" -A5
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"v1","kind":"Pod","metadata":{"annotations":{},"labels":{"role":"myrole"},"name":"static-web","namespace":"default"},"spec":{"containers":[{"image":"1nginx","name":"web","ports":[{"containerPort":80,"name":"web","protocol":"TCP"}],"volumeMounts":[{"mountPath":"/cache","name":"cache-volume"}]}],"volumes":[{"emptyDir":{},"name":"cache-volume"}]}}
  creationTimestamp: "2021-09-07T06:37:33Z"
  labels:
    role: myrole
...

@kacejot
Copy link
Contributor Author

kacejot commented Sep 7, 2021

@realshuting, in your case global condition fails and policy application is skipped.

Signed-off-by: Maxim Goncharenko <goncharenko.maxim@apriorit.com>
This reverts commit 08e176a.

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

@realshuting realshuting left a comment

Choose a reason for hiding this comment

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

@kacejot - the mutate policy works as expected now.

Having a question about validate:
Testing a validate policy, I would expect the pod creation to be blocked while it didn't:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: sample
spec:
  validationFailureAction: enforce
  rules:
  - name: check-container-image
    match:
      resources:
        kinds:
        - Pod
    validate:
      pattern:
        spec:
          containers:
          - name: "*"
            <(image): "nginx"
          imagePullSecrets:
          - name: my-registry-secret
apiVersion: v1
kind: Pod
metadata:
  name: static-web
  labels:
    role: myrole
spec:
  containers:
    - name: web
      image: nginx
      ports:
        - name: web
          containerPort: 80
          protocol: TCP
  imagePullSecrets:
  - name: other-registry
✗ k apply -f pod.yaml 
pod/static-web created

Comment on lines +85 to +90
"imagePullSecrets": [
{
"name": "regcred"
}
]
}
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the conditional anchor works only on its child tags? Why the imagePullSecrets is added to the Pod? Same for the next unit test.

Copy link
Contributor Author

@kacejot kacejot Sep 10, 2021

Choose a reason for hiding this comment

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

Because imagePullSecrets is not a child tag for image.
image condition applies only to its list element, but not the elements from the other list.

@kacejot
Copy link
Contributor Author

kacejot commented Sep 10, 2021

@realshuting, OK, I will take a look on Monday.

@kacejot
Copy link
Contributor Author

kacejot commented Sep 13, 2021

I re-tested several times, I have global anchor logic working here. Here are my manifests:

Resource:

apiVersion: v1
kind: Pod
metadata:
  name: static-web
  labels:
    role: myrole
spec:
  containers:
    - name: web
      image: nginx
      ports:
        - name: web
          containerPort: 80
          protocol: TCP
  imagePullSecrets:
  - name: other-registry

Resource is blocked:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: sample
spec:
  validationFailureAction: enforce
  rules:
  - name: check-container-image
    match:
      resources:
        kinds:
        - Pod
    validate:
      pattern:
        spec:
          containers:
          - name: "*"
            <(image): "nginx"
          imagePullSecrets:
          - name: my-registry-secret

Resource is validated:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: sample
spec:
  validationFailureAction: enforce
  rules:
  - name: check-container-image
    match:
      resources:
        kinds:
        - Pod
    validate:
      pattern:
        spec:
          containers:
          - name: "*"
            <(image): "nginx1"
          imagePullSecrets:
          - name: my-registry-secret

In last policy global anchor condition fails and the policy application is skipped.

P.S. I see there some merge conflict appeared. I'm going to resolve them immediately.

Signed-off-by: Max Goncharenko <kacejot@fex.net>
@kacejot
Copy link
Contributor Author

kacejot commented Sep 13, 2021

Here is the output for the first case where the resource should be blocked:

umka@polarbear ~/Workspace/kyverno/global-anchor-validation                                                                      [16:38:03] 
> $ kubectl create -f ./resource.yaml                                                                                                      
Error from server: error when creating "./resource.yaml": admission webhook "validate.kyverno.svc" denied the request: 

resource Pod/default/static-web was blocked due to the following policies

sample:
  check-container-image: 'validation error: rule check-container-image failed at path
    /spec/imagePullSecrets/0/name/'                                                                                   
umka@polarbear ~/go/src/github.com/kyverno/kyverno                                                                               [18:02:32] 
> $ git status                                                                                                           [±global-anchor ✓]
On branch global-anchor
Your branch is up to date with 'kacejot/global-anchor'.

nothing to commit, working tree clean

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.

Global anchor behavior in validation and mutation
3 participants