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

[Feature] Add imageExtractors support for trimming image scheme #6055

Closed
2 tasks done
bdun1013 opened this issue Jan 19, 2023 · 8 comments · Fixed by #6183
Closed
2 tasks done

[Feature] Add imageExtractors support for trimming image scheme #6055

bdun1013 opened this issue Jan 19, 2023 · 8 comments · Fixed by #6183
Assignees
Labels
enhancement New feature or request

Comments

@bdun1013
Copy link
Contributor

Problem Statement

I would like to verify a KubeVirt Containerized Data Importer DataVolume's source image from a registry using the Verify Images Policy.

A DataVolume's spec.source.registry.url is "the url of the registry source (starting with the scheme: docker, oci-archive)". The scheme (docker://) is the problem when verifying the DataVolume custom resource using this feature.

For example, with this ClusterPolicy in place:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: verify-cdi-image
spec:
  validationFailureAction: enforce
  rules:
  - name: verify-cdi-image
    match:
      any:
      - resources:
          kinds:
          - DataVolume
    imageExtractors:
      DataVolume:
      - path: /spec/source/registry/url
    verifyImages:
    - imageReferences:
      - "*"
      attestors:
      - entries:
        - keys: ...

applying the example registry image DataVolume

# This example assumes you are using a default storage class
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

produces the following error:

Error from server: error when creating "https://raw.githubusercontent.com/kubevirt/containerized-data-importer/main/manifests/example/registry-image-datavolume.yaml": admission webhook "mutate.kyverno.svc-fail" denied the request: 

policy DataVolume/gatekeeper-system/registry-image-datavolume for resource error: 

verify-cdi-image:
  verify-cdi-image: 'failed to extract images: failed to extract images: invalid image
    docker://kubevirt/fedora-cloud-registry-disk-demo'

Solution Description

I would like to see the the image extractor code updated to trim the scheme for an image if the image has a scheme. If the image value parses to a URI, then trim the scheme and return the rest of the image value.

Alternatives

Alternatives to trimming the scheme could be:

  • Trim a hardcoded docker:// from the image string if it starts with docker://
  • Add rules.imageExtractors.trimPrefix to the ClusterPolicy spec and optional trim that prefix it is there for the image

Additional Context

No response

Slack discussion

No response

Research

  • I have read and followed the documentation AND the troubleshooting guide.
  • I have searched other issues in this repository and mine is not recorded.
@bdun1013 bdun1013 added enhancement New feature or request triage Default label assigned to all new issues indicating label curation is needed to fully organize. labels Jan 19, 2023
@welcome
Copy link

welcome bot commented Jan 19, 2023

Thanks for opening your first issue here! Be sure to follow the issue template!

@bdun1013
Copy link
Contributor Author

Happy to work on this if approved.

@eddycharly
Copy link
Member

Could we do that with jmespath ?

@eddycharly
Copy link
Member

I guess we need to add the prefix back when mutating the resource though.

@JimBugwadia
Copy link
Member

I like the idea of adding an optional rules.imageExtractors.jmesPath to allow any transformation.

@bdun1013 - would that work?

However, we would still need to address @eddycharly's question on how to handle mutate digest, when the image reference is transformed.

@bdun1013
Copy link
Contributor Author

@JimBugwadia Adding an optional jmesPath works for me.

In regards to image digest mutation, we could add a caveat that if jmesPath is used, then mutateDigest must be set to false.

bdun1013 added a commit to bdun1013/kyverno that referenced this issue Jan 20, 2023
Signed-off-by: bdunnigan <bdunnigan@clarityinnovates.com>
@JimBugwadia
Copy link
Member

Sounds good. We should also be able to add a validation check for that.

I've assigned the issue to you - let us know if you need any help.

bdun1013 added a commit to bdun1013/kyverno that referenced this issue Jan 30, 2023
Signed-off-by: bdunnigan <bdunnigan@clarityinnovates.com>
bdun1013 added a commit to bdun1013/kyverno that referenced this issue Jan 30, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
bdun1013 added a commit to bdun1013/kyverno that referenced this issue Jan 30, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
bdun1013 added a commit to bdun1013/kyverno that referenced this issue Jan 31, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
bdun1013 added a commit to bdun1013/kyverno that referenced this issue Feb 1, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
@bdun1013
Copy link
Contributor Author

bdun1013 commented Feb 1, 2023

@JimBugwadia @eddycharly #6183 should be good to go.

bdun1013 added a commit to bdun1013/kyverno that referenced this issue Feb 2, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
bdun1013 added a commit to bdun1013/kyverno that referenced this issue Feb 2, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
@chipzoller chipzoller removed the triage Default label assigned to all new issues indicating label curation is needed to fully organize. label Feb 3, 2023
bdun1013 added a commit to bdun1013/kyverno that referenced this issue Feb 6, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
bdun1013 added a commit to bdun1013/kyverno that referenced this issue Feb 7, 2023
Signed-off-by: Brian Dunnigan <bdunnigan@clarityinnovates.com>
eddycharly added a commit that referenced this issue Feb 8, 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>
vishal-chdhry pushed a commit to vishal-chdhry/kyverno that referenced this issue 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 issue 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 issue 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 issue 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
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants