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

#6055 Add JMESPath support to imageExtractors #6183

Merged

Conversation

bdun1013
Copy link
Contributor

@bdun1013 bdun1013 commented Jan 31, 2023

Signed-off-by: Brian Dunnigan bdunnigan@clarityinnovates.com

Explanation

This PR adds an optional JMESPath to policy rule image extractors along with the new trim_prefix JMESPath function. This will allow verifyImages to work with KubeVirt registry image DataVolumes or any other CRD that includes a scheme with an image. A policy validation check has been added to ensure that if a JMESPath is used to extract an image, image digest mutation must be disabled.

Related issue

Closes #6055

@JimBugwadia
@eddycharly

Milestone of this PR

What type of PR is this

/kind feature

Proposed Changes

Added an optional spec.rules.*.imageExtractors.*.jmesPath to Policy and ClusterPolicy along with a trim_prefix JMESPath function to allow extracting images with schemes from CRDs for compatibility with verifyImages.

Proof Manifests

The following ClusterPolicy will fail admission validation because it uses an image extractor JMESPath and has mutateDigest set to true:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: verify-data-volume-image
spec:
  background: false
  validationFailureAction: Enforce
  rules:
    - name: verify-data-volume-image
      match:
        any:
        - resources:
            kinds:
              - DataVolume
      imageExtractors:
        DataVolume:
          - path: /spec/source/registry/url
            jmesPath: "trim_prefix(@, 'docker://')"
      verifyImages:
      - imageReferences:
        - "*"
        mutateDigest: true
        verifyDigest: true
        attestors:
        - entries:
          - keys:
              publicKeys: |
                -----BEGIN PUBLIC KEY-----
                ...
                -----END PUBLIC KEY-----

The ClusterPolicy will be admitted when mutateDigest is set to false:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: verify-data-volume-image
spec:
  background: false
  validationFailureAction: Enforce
  rules:
    - name: verify-data-volume-image
      match:
        any:
        - resources:
            kinds:
              - DataVolume
      imageExtractors:
        DataVolume:
          - path: /spec/source/registry/url
            jmesPath: "trim_prefix(@, 'docker://')"
      verifyImages:
      - imageReferences:
        - "*"
        mutateDigest: false
        verifyDigest: true
        attestors:
        - entries:
          - keys:
              publicKeys: |
                -----BEGIN PUBLIC KEY-----
                ...
                -----END PUBLIC KEY-----

A KubeVirt DataVolume will now have its image correctly extracted:

apiVersion: cdi.kubevirt.io/v1beta1
kind: DataVolume
metadata:
  name: registry-image-datavolume
spec:
  source:
    registry:
      url: "docker://kubevirt/fedora-cloud-registry-disk-demo"
  pvc:
    accessModes:
      - ReadWriteOnce
    resources:
      requests:
        storage: 5Gi

Checklist

Further Comments

@welcome
Copy link

welcome bot commented Jan 31, 2023

Thanks for opening your first Pull Request here! Please check out our Contributing guidelines and confirm that you Signed off.

@codecov
Copy link

codecov bot commented Jan 31, 2023

Codecov Report

Merging #6183 (22d61ff) into main (cfd4501) will increase coverage by 0.13%.
The diff coverage is 78.08%.

@@            Coverage Diff             @@
##             main    #6183      +/-   ##
==========================================
+ Coverage   37.15%   37.28%   +0.13%     
==========================================
  Files         201      201              
  Lines       20540    20604      +64     
==========================================
+ Hits         7632     7683      +51     
- Misses      12142    12151       +9     
- Partials      766      770       +4     
Impacted Files Coverage Δ
api/kyverno/v1/rule_types.go 54.66% <ø> (ø)
pkg/utils/api/image.go 66.10% <50.00%> (-3.81%) ⬇️
pkg/policy/validate.go 52.48% <85.18%> (+0.92%) ⬆️
pkg/engine/jmespath/functions.go 80.76% <100.00%> (+0.50%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bdun1013 bdun1013 force-pushed the 6055-image-extractors-jmespath-support branch from 33ff1cb to 3ce9fee Compare February 1, 2023 18:41
@bdun1013 bdun1013 marked this pull request as ready for review February 1, 2023 18:52
@bdun1013 bdun1013 force-pushed the 6055-image-extractors-jmespath-support branch from 3ce9fee to 2e1db36 Compare February 2, 2023 14:24
@JimBugwadia JimBugwadia self-assigned this Feb 2, 2023
@JimBugwadia JimBugwadia requested review from JimBugwadia and removed request for treydock, MarcelMue and realshuting February 2, 2023 14:52
@bdun1013 bdun1013 force-pushed the 6055-image-extractors-jmespath-support branch from 3b716af to 0b5612b Compare February 2, 2023 21:06
@eddycharly eddycharly added this to the Kyverno Release 1.10.0 milestone Feb 2, 2023
@eddycharly
Copy link
Member

Good work !
I will review in more depth tomorrow but could you add unit tests covering error cases (like in #6202) ?

pkg/utils/api/image.go Outdated Show resolved Hide resolved
pkg/utils/api/image.go Outdated Show resolved Hide resolved
Copy link
Member

@eddycharly eddycharly left a comment

Choose a reason for hiding this comment

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

The implementation in image.go looks wrong to me, see my comments.

@bdun1013 bdun1013 force-pushed the 6055-image-extractors-jmespath-support branch from 0b5612b to cfc8936 Compare February 6, 2023 16:47
@bdun1013
Copy link
Contributor Author

bdun1013 commented Feb 6, 2023

Good work ! I will review in more depth tomorrow but could you add unit tests covering error cases (like in #6202) ?

Added additional unit tests

@bdun1013 bdun1013 requested review from eddycharly and removed request for chipzoller and JimBugwadia February 7, 2023 17:13
@bdun1013
Copy link
Contributor Author

bdun1013 commented Feb 7, 2023

The implementation in image.go looks wrong to me, see my comments.

@eddycharly Updated with your suggestions

Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
@bdun1013 bdun1013 force-pushed the 6055-image-extractors-jmespath-support branch from cfc8936 to 3b46453 Compare February 7, 2023 17:18
@eddycharly
Copy link
Member

Nice work @bdun1013 !

@eddycharly eddycharly enabled auto-merge (squash) February 8, 2023 08:06
@eddycharly eddycharly merged commit d33e616 into kyverno:main Feb 8, 2023
@welcome
Copy link

welcome bot commented Feb 8, 2023

Congratulations! 🎉

Great job merging your first Pull Request here! How awesome! If you are new to this project, feel free to join our Slack community
200w

@bdun1013 bdun1013 deleted the 6055-image-extractors-jmespath-support branch February 8, 2023 18:31
vishal-chdhry pushed a commit to vishal-chdhry/kyverno that referenced this pull request Feb 9, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
Co-authored-by: bdunnigan <bdunnigan@clarityinnovates.com>
Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
sawanabhi157 pushed a commit to sawanabhi157/kyverno that referenced this pull request Feb 13, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
Co-authored-by: bdunnigan <bdunnigan@clarityinnovates.com>
Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
Signed-off-by: Abhishek Sawan <sawanabhi157@gmail.com>
sawanabhi157 pushed a commit to sawanabhi157/kyverno that referenced this pull request Feb 15, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
Co-authored-by: bdunnigan <bdunnigan@clarityinnovates.com>
Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
Signed-off-by: Abhishek Sawan <sawanabhi157@gmail.com>
vishal-chdhry pushed a commit to vishal-chdhry/kyverno that referenced this pull request Mar 30, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
Co-authored-by: bdunnigan <bdunnigan@clarityinnovates.com>
Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Add imageExtractors support for trimming image scheme
4 participants