opendatahub-io / notebooks

Notebook images for ODH
Apache License 2.0
15 stars 51 forks source link

Add daily check for vulnerability issues using Trivy #600

Closed caponetto closed 1 week ago

caponetto commented 1 week ago

https://issues.redhat.com/browse/RHOAIENG-8779

Description

This PR introduces the usage of Trivy to scan the container images and consolidate vulnerability reports. I've configured the Build Notebooks workflow to be triggered daily at 2 am UTC to generate the report (the scan runs after each image is built). I didn't see the need to run it on each push but we can enable it if it makes sense. Each report is uploaded to the workflow summary as soon as each individual job is completed. Finally, reports follow a markdown template.

Demo

https://github.com/opendatahub-io/notebooks/assets/638737/68f92ecb-40a2-4fa9-8f19-c5037f98458d

Here is an example of workflow run where you can see real reports.

Note: I haven't added the checks for PRs because the scan operation adds an extra ~10 minutes, which would slow our PR jobs down.

How Has This Been Tested?

I've executed the workflow on my fork.

Merge criteria:

caponetto commented 1 week ago

@jiridanek thanks for the review!

We can't fail the build when vulnerabilities are detected, that way we'd never have a pass. But these inforeports are not very practical to work with either, there is too much info to scan. We'd need to either reduce the number of findings the tool produces, so it is possible to read through it easily.

Yeah, I agree. I've reduced the scope to only HIGH and CRITICAL issues because the list was getting bigger if we allowed all of them. But let's see if it helps otherwise we can revisit it and try to personalize more.

Currently, this is mainly useful to check that some vulnerability we intended to fix was indeed fixed. But, then I'd have to first merge the PR to get scan on it. (Or run scan on my machine.)

We can't enable it to run for all PRs due to the amount of time it adds but now I'm thinking if we can come up with some strategy like running it if the PR has a certain label or something like that.

caponetto commented 1 week ago

Ok, this PR is ready.

jiridanek commented 1 week ago

Do we push the images to ghcr after completing the report? If so, I would suggest not doing it because we will fill up the registry on a daily basis.

we have to push, the makefile is written in such a way that it builds and pushes, without the possibility to skip a push

https://github.com/opendatahub-io/notebooks/blob/3f93529616cf01bd8305a16e6671f6253ee27ba8/Makefile#L61-L65

regarding filling up space, first, there does not seem to be a limit that applies to us, and second, please review this PR that sets up a cleaning job to free up the space daily

caponetto commented 1 week ago

@atheo89 Thanks for raising this concern! Another option is to use the approach we have for pull_request, which publishes the image to localhost (ref) but I think it won't be an issue anymore after @jiridanek's PR that cleans up the registry periodically. Plus, it could be helpful to have them published somewhere if someone needs to debug the associated image with the report.

atheo89 commented 1 week ago

Great! Let's make sure enough time passes before Jiri's prune workflow starts! :slightly_smiling_face:

https://github.com/opendatahub-io/notebooks/pull/601/files#diff-6a2b18c5159aa29ecd02344b37be379eac595c441858bd84f754a0353e0bb5caR12

/lgtm /approve

openshift-ci[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atheo89, jiridanek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/opendatahub-io/notebooks/blob/main/OWNERS)~~ [atheo89] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
atheo89 commented 1 week ago

we have to push, the makefile is written in such a way that it builds and pushes, without the possibility to skip a push

That's true; but it's not a big deal to make recipes more independent if that makes our lives easier. Added a tracker here