Add support for custom image extractors #3596

sambhav · 2022-04-13T13:24:22Z

Milestone of this PR

/milestone 1.7.0

What type of PR is this

/kind feature

Proposed Changes

This PR adds a new imageExtractor key to the kyverno configmap which allows users to define controller level configs for extracting images from custom resources. The configmap load/update is dynamic and doesn't require a restart of the kyverno controller to update the image extractors.

This allows users to use imageVerify with custom resources like tekton tasks or more.

Since CRDs are a cluster level resource, I wanted to initially go with a controller level config that can easily be configured via the kyverno helm chart. If needs be, we can figure out how to expose this config at a policy level.

Proof Manifests

Added as automated tests

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: [Enhancement] Document the structure and configuration for custom image extractors website#512

Further Comments

test/e2e/validate/validate_test.go

pkg/utils/kube/image_test.go

sambhav · 2022-04-13T13:30:19Z

I added this as a controller level config. Per the discussion in #2938 I tried to put it in the context but it didn't really feel right since the context can be present in multiple places in a policy, where as this config should only be defined at the cluster level. I really don't think users should be able to use the context inside foreach to define this image config variable.

sambhav · 2022-04-13T13:49:58Z

Structure of the config is as described at kyverno/website#512

JimBugwadia · 2022-04-13T15:27:46Z

@samj1912 - I understand the concern on the context declaration, but to me this does not seem right as a Kyverno controller configuration. Seems like it belongs somewhere in the policy.

What would be the downside of allowing this in a foreach? One use case I can envision would be if the user wants to selectively loop and load some paths, but not all paths within a foreach. I am not sure if this is a strong requirement, but does not seem like it would cause any problems if supported.

      mutate:
        foreach: 
        - list: "request.object.spec.steps"
          context:
            imagePaths:
            - path: "."

The other option we had discussed was a new element in the rule:

imageVerify:
  imagePaths:
  - path: "/spec/steps/*"

If we are sure there is no use case where the context could be expanded in a foreach we can consider introducing at the rule level.

Thoughts?

sambhav · 2022-04-13T18:21:34Z

@JimBugwadia what makes it really hard to make it work with foreach or other sub-contexts is that we don't track the exact node/path to the variable when evaluating JMESPath expressions. And we require this information for patching + checking if variables have changed.

We would have to dramatically change how jmespath expressions are evaluated to support something like this (Related jmespath/jmespath.js#22 )

We could introduce the current config structure at any of the scopes (controller, policy, rule).

I wanted to start with something at a controller level to avoid redeclaration of the resource paths in multiple policies but also happy to switch it to a policy level key or a rule level key. In case we do it at a rule level, would we also want it for other rule types or image verify only?

JimBugwadia · 2022-04-13T18:24:04Z

I wanted to start with something at a controller level to avoid redeclaration of the resource paths in multiple policies but also happy to switch it to a policy level key or a rule level key. In case we do it at a rule level, would we also want it for other rule types or image verify only?

I was thinking within the imageVerify rule only.

sambhav · 2022-04-14T01:21:25Z

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: tasks
spec:
  validationFailureAction: enforce
  rules:
  - name: verify-images
    match:
      resources:
        kinds:
        - Task
    preconditions:
    - key: '{{request.operation}}'
      operator: NotEquals
      value: DELETE
    # this schema for config can be modified but we will need to express the config here in order for it to be applicable at a rule level. This is the only extension point possible for rule level configuration for image verify 
    config:
      imageExtractors:
        Task:
          - name: steps
            path: spec.steps.*
            value: image
    verifyImages:
    # imagePaths key cannot be inserted here since verifyImages is not a map but an array 
    - image: "*"
      # imagePaths can't be inserted here since the implementation has to first extract the images and then verify them as a whole. Adding it here would also lead to a lot of redundancy. 
      key: |-
        -----BEGIN PUBLIC KEY-----
        MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM
        5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==
        -----END PUBLIC KEY-----

Here's what the current policy config looks like

api/kyverno/v1/zz_generated.deepcopy.go

JimBugwadia

@samj1912 - this looks good overall! My only suggestion would be to remove "config" and directly add "imageExtractors" to the rule. If we see other common configuration in the future, we can always group then.

api/kyverno/v1/rule_types.go

sambhav · 2022-04-14T08:31:29Z

@JimBugwadia made some other enhancements as well to the CRD, the minimal policy may now look like

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: tasks
spec:
  validationFailureAction: enforce
  rules:
  - name: verify-images
    match:
      resources:
        kinds:
        - Task
    preconditions:
    - key: '{{request.operation}}'
      operator: NotEquals
      value: DELETE
    imageExtractors:
      Task:
        - path: /spec/steps/*/image
    verifyImages:
    - image: "*"
      key: |-
        -----BEGIN PUBLIC KEY-----
        MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM
        5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==
        -----END PUBLIC KEY-----

And if users want more control over output context structure they can also define the other optional fields.

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: tasks
spec:
  validationFailureAction: enforce
  rules:
  - name: verify-images
    match:
      resources:
        kinds:
        - Task
    preconditions:
    - key: '{{request.operation}}'
      operator: NotEquals
      value: DELETE
    imageExtractors:
      Task:
        - path: /spec/steps/*
          name: steps
          value: image
          key: name
    verifyImages:
    - image: "*"
      key: |-
        -----BEGIN PUBLIC KEY-----
        MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM
        5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==
        -----END PUBLIC KEY-----

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

pkg/utils/kube/image.go

api/kyverno/v1/zz_generated.deepcopy.go

codecov-commenter · 2022-04-14T15:45:34Z

Codecov Report

Merging #3596 (8a141c3) into main (1714a32) will increase coverage by 0.07%.
The diff coverage is 51.94%.

@@            Coverage Diff             @@
##             main    #3596      +/-   ##
==========================================
+ Coverage   26.74%   26.82%   +0.07%     
==========================================
  Files         137      137              
  Lines       19139    19202      +63     
==========================================
+ Hits         5119     5150      +31     
- Misses      13390    13420      +30     
- Partials      630      632       +2

Impacted Files	Coverage Δ
api/kyverno/v1/rule_types.go	`53.61% <0.00%> (-0.99%)`	⬇️
api/kyverno/v1/zz_generated.deepcopy.go	`0.00% <0.00%> (ø)`
pkg/config/dynamicconfig.go	`0.00% <ø> (ø)`
pkg/engine/context/context.go	`33.09% <0.00%> (-2.30%)`	⬇️
pkg/engine/imageVerify.go	`45.00% <40.00%> (+0.12%)`	⬆️
pkg/utils/kube/image.go	`71.42% <97.29%> (+11.42%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1714a32...8a141c3. Read the comment docs.

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

* feat: mutate existing, replace GR by UR in webhook server (#3601) * add attributes for post mutation Signed-off-by: ShutingZhao <shuting@nirmata.com> * add UR informer to webhook server Signed-off-by: ShutingZhao <shuting@nirmata.com> * - replace gr with ur in the webhook server; - create ur for mutateExsiting policies Signed-off-by: ShutingZhao <shuting@nirmata.com> * replace gr by ur across entire packages Signed-off-by: ShutingZhao <shuting@nirmata.com> * add YAMLs Signed-off-by: ShutingZhao <shuting@nirmata.com> * update api docs & fix unit tests Signed-off-by: ShutingZhao <shuting@nirmata.com> * add UR deletion handler Signed-off-by: ShutingZhao <shuting@nirmata.com> * add api docs for v1beta1 Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix clientset method Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix v1beta1 client registration Signed-off-by: ShutingZhao <shuting@nirmata.com> * feat: mutate existing - generates UR for admission requests (#3623) Signed-off-by: ShutingZhao <shuting@nirmata.com> * replace with UR in policy controller generate rules (#3635) Signed-off-by: prateekpandey14 <prateek.pandey@nirmata.com> * - enable mutate engine to process mutateExisting rules; - add unit tests Signed-off-by: ShutingZhao <shuting@nirmata.com> * implemented ur background reconciliation for mutateExisting policies Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix webhook update error Signed-off-by: ShutingZhao <shuting@nirmata.com> * temporary comment out new unit tests Signed-off-by: ShutingZhao <shuting@nirmata.com> * feat: mutate existing, replace GR by UR in webhook server (#3601) * add attributes for post mutation Signed-off-by: ShutingZhao <shuting@nirmata.com> * add UR informer to webhook server Signed-off-by: ShutingZhao <shuting@nirmata.com> * - replace gr with ur in the webhook server; - create ur for mutateExsiting policies Signed-off-by: ShutingZhao <shuting@nirmata.com> * replace gr by ur across entire packages Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix missing policy.kyverno.io/policy-name label (#3599) Signed-off-by: prateekpandey14 <prateek.pandey@nirmata.com> * refactor cli code from pkg to cmd (#3591) * refactor cli code from pkg to cmd Signed-off-by: Mritunjay Sharma <mritunjaysharma394@gmail.com> * fixes in imports Signed-off-by: Mritunjay Sharma <mritunjaysharma394@gmail.com> * fixes tests Signed-off-by: Mritunjay Sharma <mritunjaysharma394@gmail.com> * fixed conflicts Signed-off-by: Mritunjay Sharma <mritunjaysharma394@gmail.com> * moved non-commands to utils Signed-off-by: Mritunjay Sharma <mritunjaysharma394@gmail.com> Co-authored-by: Vyankatesh Kudtarkar <vyankateshkd@gmail.com> * add YAMLs Signed-off-by: ShutingZhao <shuting@nirmata.com> * update api docs & fix unit tests Signed-off-by: ShutingZhao <shuting@nirmata.com> * add UR deletion handler Signed-off-by: ShutingZhao <shuting@nirmata.com> * add api docs for v1beta1 Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix clientset method Signed-off-by: ShutingZhao <shuting@nirmata.com> * add-kms-libraries for cosign (#3603) * add-kms-libraries Signed-off-by: anushkamittal20 <anumittal4641@gmail.com> * Shifted providers to cosign package Signed-off-by: anushkamittal20 <anumittal4641@gmail.com> Signed-off-by: ShutingZhao <shuting@nirmata.com> * Add support for custom image extractors (#3596) Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net> * Update vulnerable dependencies (#3577) Signed-off-by: Shubham Gupta <shubham.gupta2956@gmail.com> Co-authored-by: Jim Bugwadia <jim@nirmata.com> Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix v1beta1 client registration Signed-off-by: ShutingZhao <shuting@nirmata.com> * feat: mutate existing - generates UR for admission requests (#3623) Signed-off-by: ShutingZhao <shuting@nirmata.com> * updating version in Chart.yaml (#3618) * updatimg version in Chart.yaml Signed-off-by: Prateeknandle <prateeknandle@gmail.com> * changes from, make gen-helm Signed-off-by: Prateeknandle <prateeknandle@gmail.com> Co-authored-by: Vyankatesh Kudtarkar <vyankateshkd@gmail.com> Signed-off-by: ShutingZhao <shuting@nirmata.com> * Allow kyverno-policies to have preconditions defined (#3606) * Allow kyverno-policies to have preconditions defined Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu> * Fix docs Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu> Signed-off-by: ShutingZhao <shuting@nirmata.com> * replace with UR in policy controller generate rules (#3635) Signed-off-by: prateekpandey14 <prateek.pandey@nirmata.com> Signed-off-by: ShutingZhao <shuting@nirmata.com> * - enable mutate engine to process mutateExisting rules; - add unit tests Signed-off-by: ShutingZhao <shuting@nirmata.com> * implemented ur background reconciliation for mutateExisting policies Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix webhook update error Signed-off-by: ShutingZhao <shuting@nirmata.com> * temporary comment out new unit tests Signed-off-by: ShutingZhao <shuting@nirmata.com> * Image verify attestors (#3614) * fix logs Signed-off-by: Jim Bugwadia <jim@nirmata.com> * fix logs Signed-off-by: Jim Bugwadia <jim@nirmata.com> * support multiple attestors Signed-off-by: Jim Bugwadia <jim@nirmata.com> * rm CLI tests (not currently supported) Signed-off-by: Jim Bugwadia <jim@nirmata.com> * apply attestor repo Signed-off-by: Jim Bugwadia <jim@nirmata.com> * fix linter issues Signed-off-by: Jim Bugwadia <jim@nirmata.com> * fix entryError assignment Signed-off-by: Jim Bugwadia <jim@nirmata.com> * fix tests Signed-off-by: Jim Bugwadia <jim@nirmata.com> * format Signed-off-by: Jim Bugwadia <jim@nirmata.com> * add intermediary certs Signed-off-by: Jim Bugwadia <jim@nirmata.com> * Allow defining imagePullSecrets (#3633) * Allow defining imagePullSecrets Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu> * Use dict for imagePullSecrets Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu> * Simplify how imagePullSecrets is defined Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu> Signed-off-by: ShutingZhao <shuting@nirmata.com> * Fix race condition in pCache (#3632) * fix race condition in pCache Signed-off-by: ShutingZhao <shuting@nirmata.com> * refact: remove unused Run function from generate (#3638) Signed-off-by: prateekpandey14 <prateek.pandey@nirmata.com> * Remove helm mode setting (#3628) Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> Signed-off-by: ShutingZhao <shuting@nirmata.com> * refactor: image utils (#3630) Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> Signed-off-by: ShutingZhao <shuting@nirmata.com> * -resolve lift comments; -fix informer sync issue Signed-off-by: ShutingZhao <shuting@nirmata.com> * refact the update request cleanup controller Signed-off-by: prateekpandey14 <prateek.pandey@nirmata.com> * - fix delete request for mutateExisting; - fix context variable substitution; - improve logging Signed-off-by: ShutingZhao <shuting@nirmata.com> * - enable events; - add last applied annotation Signed-off-by: ShutingZhao <shuting@nirmata.com> * enable mutate existing on policy creation Signed-off-by: ShutingZhao <shuting@nirmata.com> * update autogen code Signed-off-by: ShutingZhao <shuting@nirmata.com> * merge main Signed-off-by: ShutingZhao <shuting@nirmata.com> * add unit tests Signed-off-by: ShutingZhao <shuting@nirmata.com> * address list comments Signed-off-by: ShutingZhao <shuting@nirmata.com> * update api docs Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix "Implicit memory aliasing in for loop" Signed-off-by: ShutingZhao <shuting@nirmata.com> * remove unused definitions Signed-off-by: ShutingZhao <shuting@nirmata.com> * update api docs Signed-off-by: ShutingZhao <shuting@nirmata.com> Co-authored-by: Prateek Pandey <prateek.pandey@nirmata.com> Co-authored-by: Mritunjay Kumar Sharma <mritunjaysharma394@gmail.com> Co-authored-by: Vyankatesh Kudtarkar <vyankateshkd@gmail.com> Co-authored-by: Anushka Mittal <55237170+anushkamittal20@users.noreply.github.com> Co-authored-by: Sambhav Kothari <sambhavs.email@gmail.com> Co-authored-by: Shubham Gupta <shubham.gupta2956@gmail.com> Co-authored-by: Jim Bugwadia <jim@nirmata.com> Co-authored-by: Prateek Nandle <56027872+Prateeknandle@users.noreply.github.com> Co-authored-by: treydock <tdockendorf@osc.edu> Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

sambhav requested a review from vyankyGH as a code owner April 13, 2022 13:24

sambhav requested a review from JimBugwadia April 13, 2022 13:24

sambhav force-pushed the image-custom-resources branch from bc57ec2 to 66269c1 Compare April 13, 2022 13:25

sambhav commented Apr 13, 2022

View reviewed changes

test/e2e/validate/validate_test.go Outdated Show resolved Hide resolved

test/e2e/validate/validate_test.go Outdated Show resolved Hide resolved

pkg/utils/kube/image_test.go Outdated Show resolved Hide resolved

sambhav force-pushed the image-custom-resources branch from 66269c1 to b857acd Compare April 13, 2022 13:32

sambhav mentioned this pull request Apr 13, 2022

[Enhancement] Document the structure and configuration for custom image extractors kyverno/website#512

Closed

sambhav force-pushed the image-custom-resources branch from b857acd to db43a78 Compare April 13, 2022 14:56

sambhav force-pushed the image-custom-resources branch from db43a78 to 3567cef Compare April 14, 2022 01:14

sambhav requested a review from prateekpandey14 as a code owner April 14, 2022 01:14

sambhav force-pushed the image-custom-resources branch 3 times, most recently from a7b1bd1 to bb37a5e Compare April 14, 2022 01:20

sonatype-lift bot reviewed Apr 14, 2022

View reviewed changes

api/kyverno/v1/zz_generated.deepcopy.go Outdated Show resolved Hide resolved

JimBugwadia requested changes Apr 14, 2022

View reviewed changes

api/kyverno/v1/rule_types.go Outdated Show resolved Hide resolved

sambhav force-pushed the image-custom-resources branch from bb37a5e to 6f8963b Compare April 14, 2022 08:28

sambhav force-pushed the image-custom-resources branch from 6f8963b to f4a33a9 Compare April 14, 2022 08:31

sambhav requested a review from JimBugwadia April 14, 2022 08:32

sambhav force-pushed the image-custom-resources branch from f4a33a9 to 0cd4077 Compare April 14, 2022 08:41

Add support for custom image extractors

945e00f

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

sambhav force-pushed the image-custom-resources branch from 0cd4077 to 945e00f Compare April 14, 2022 09:01

prateekpandey14 reviewed Apr 14, 2022

View reviewed changes

pkg/utils/kube/image.go Show resolved Hide resolved

sonatype-lift bot reviewed Apr 14, 2022

View reviewed changes

api/kyverno/v1/zz_generated.deepcopy.go Show resolved Hide resolved

sambhav mentioned this pull request Apr 14, 2022

refactor: dependencies between api types and kube utils #3600

Closed

sambhav requested a review from prateekpandey14 April 14, 2022 10:54

sambhav mentioned this pull request Apr 14, 2022

use JSON Pointer for patches #3605

Closed

9 tasks

Merge branch 'main' into image-custom-resources

8a141c3

JimBugwadia approved these changes Apr 14, 2022

View reviewed changes

JimBugwadia merged commit ec4e4ba into kyverno:main Apr 14, 2022

sambhav deleted the image-custom-resources branch April 14, 2022 16:09

JimBugwadia added this to the Kyverno Release 1.7.0 milestone Apr 14, 2022

sambhav added the milestone 1.7.0 label Apr 14, 2022

realshuting pushed a commit that referenced this pull request Apr 21, 2022

Add support for custom image extractors (#3596)

f733788

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

dkulchinsky mentioned this pull request Apr 29, 2022

webhookconfig: include services/status if services resource configured #3740

Merged

9 tasks

realshuting added the release notes This label is added to the PR without a corresponding issue. It is used to create release notes. label Jun 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for custom image extractors #3596

Add support for custom image extractors #3596

Uh oh!

sambhav commented Apr 13, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sambhav commented Apr 13, 2022

Uh oh!

sambhav commented Apr 13, 2022

Uh oh!

JimBugwadia commented Apr 13, 2022

Uh oh!

sambhav commented Apr 13, 2022

Uh oh!

JimBugwadia commented Apr 13, 2022

Uh oh!

sambhav commented Apr 14, 2022 •

edited

Loading

Uh oh!

Uh oh!

JimBugwadia left a comment

Uh oh!

Uh oh!

sambhav commented Apr 14, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Apr 14, 2022

Uh oh!

Uh oh!

Add support for custom image extractors #3596

Add support for custom image extractors #3596

Uh oh!

Conversation

sambhav commented Apr 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Milestone of this PR

What type of PR is this

Proposed Changes

Proof Manifests

Checklist

Further Comments

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sambhav commented Apr 13, 2022

Uh oh!

sambhav commented Apr 13, 2022

Uh oh!

JimBugwadia commented Apr 13, 2022

Uh oh!

sambhav commented Apr 13, 2022

Uh oh!

JimBugwadia commented Apr 13, 2022

Uh oh!

sambhav commented Apr 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JimBugwadia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sambhav commented Apr 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Apr 14, 2022

Codecov Report

Uh oh!

Uh oh!

sambhav commented Apr 13, 2022 •

edited

Loading

sambhav commented Apr 14, 2022 •

edited

Loading

sambhav commented Apr 14, 2022 •

edited

Loading