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

Add support for CRI "pinned" images #380

Open
cpuguy83 opened this issue Aug 19, 2022 · 6 comments · May be fixed by #920
Open

Add support for CRI "pinned" images #380

cpuguy83 opened this issue Aug 19, 2022 · 6 comments · May be fixed by #920
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@cpuguy83
Copy link
Contributor

The CRI API has a field on images Pinned.
This will be used for images like pause and should be used as as a filter for image removal.

ref: containerd/containerd#6456

@cpuguy83 cpuguy83 added the enhancement New feature or request label Aug 19, 2022
@sozercan
Copy link
Member

tracked in containerd/containerd#7944 now

@sozercan sozercan added this to the v1.1.0 milestone Feb 23, 2023
@sozercan sozercan modified the milestones: v1.1.0, v1.2.0 Mar 16, 2023
@sozercan sozercan modified the milestones: v1.2.0, v1.3.0 May 25, 2023
@sozercan
Copy link
Member

looks like containerd pr got merged

@inFocus7
Copy link
Contributor

inFocus7 commented Sep 5, 2023

So we'll want to allow toggling if we want to remove/keep Pinned images?

If a user decides to keep pinned images (ex. removePinnedImages: false or whatever a toggle would be): do we still want to scan those images? Or skip them from being scanned, since it doesn't matter as we're keeping them?

The main, or maybe only(?), reason I can think of scanning them is if we do the following issue #356 and it would be nice to raise any CVEs caught. Although since that hasn't been done, not sure if it's worth scanning (for now).

@sozercan sozercan modified the milestones: v1.3.0, v1.4.0 Nov 29, 2023
@inFocus7
Copy link
Contributor

inFocus7 commented Nov 29, 2023

Is anyone in the eraser team planned to pick this up? If not, I could work on this 👀
(I have a WIP branch I was trying stuff out in a few months ago. I pushed to my forked repo, because I forgot most of my changes made lol)

@sozercan
Copy link
Member

@inFocus7 sounds great! i don't think anyone has been working on this. assigned to you. thanks!

@inFocus7 inFocus7 linked a pull request Nov 29, 2023 that will close this issue
@inFocus7
Copy link
Contributor

inFocus7 commented Nov 30, 2023

@sozercan Awesome, thanks! I wrote some thoughts in this Google Docs Design/Thoughts Docs.

If you don't want to click into the link (understandable), it boils down to:

  1. Will we want to be strict and never delete Pinned images? Or should we allow this to be user-configurable, so they can decide if should delete or keep them?
  2. Do we even care about scanning Pinned images when we won't delete them?
    • Their vulnerabilities technically won't matter if we don't delete them, as the results would essentially be ignored.
    • From my understanding, scanning them would only be useful if we did any reports to make users aware, but as far as I know I don't believe we do something like this at the moment.

Actually, from looking over the architecture, I think it makes more sense to handle Pinned images through the collector and remover. Pinned statuses have nothing to do with vulnerabilities, and the scanner can be bypassed, so doing so in the scanner wouldn't be too helpful.

I'm assuming it should be possible to handle pinned images in those pods, since they have flags, so we should hopefully be able to do add a bool flag for skip-pinned (or similar).

The question of: "Do we want to still scan Pinned images if we want to keep them no matter what?" still remains.

I updated the Google doc with a section at the end with explaining this.

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
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants