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

Split published and example images vulnerability scanning #2914

Merged
merged 5 commits into from
Jun 21, 2024

Conversation

hairyhum
Copy link
Contributor

@hairyhum hairyhum commented May 31, 2024

Change Overview

The idea behind this change is to separate critical and non-critical images scanning. So we can set up notifications when critical images have vulnerabilities.

This PR just separates the scanning workflow. #2829 will separate the image build workflow.

Future improvement: after #2829 is merged, we can trigger example image scanning on example images build workflow instead of main.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test
  • 🏗️ Build

Issues

  • fixes #issue-number

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@hairyhum hairyhum force-pushed the image-scanning-split branch 3 times, most recently from b59a741 to 7a282cf Compare May 31, 2024 21:03
@hairyhum hairyhum marked this pull request as ready for review May 31, 2024 21:07
Copy link
Contributor

@julio-lopez julio-lopez left a comment

Choose a reason for hiding this comment

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

At a high-level, LGTM.

See nitty comments

build/push_images.sh Outdated Show resolved Hide resolved
build/published_images.json Outdated Show resolved Hide resolved
@@ -0,0 +1,17 @@
name: Published images vulnerability scanning
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should the name for this workflow be different in order to more easily tell it apart when looking at all the (action) jobs.

Also, a shorter name would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It's supposed to be different.

Comment on lines +19 to +23
EOF=$(dd if=/dev/urandom bs=15 count=1 status=none | base64)
echo "images_json<<$EOF" >> $GITHUB_OUTPUT
cat ${{ inputs.images_file }} >> $GITHUB_OUTPUT
echo "$EOF" >> $GITHUB_OUTPUT
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this code doing actually? (Maybe a comment would be good)

Copy link
Contributor

@psilva-veeam psilva-veeam left a comment

Choose a reason for hiding this comment

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

I left some comments. Generally, it would be good to have a more descriptive name than example-images. What would be the use for the split? It seems the listed images are possibly 3rd party images

@hairyhum
Copy link
Contributor Author

Generally, it would be good to have a more descriptive name than example-images

Example images are images that we use in the examples. I don't have a better description for that honestly.

Will it still be possible to notice issues that aren't fixed by upstream? In many cases it might be possible to provide a mitigation

Maybe, but it's not clear how to address those. We can change the flag and see how many issues we get then.

@julio-lopez
Copy link
Contributor

The idea behind this change is to separate critical and non-critical images scanning. So we can set up notifications when critical images have vulnerabilities.

@psilva-veeam IOW: the objective is to facilitate or improve the identification of (vulnerability) notifications that are in the packaged (released "product") Kanister images in order to prioritize those first, over vulnerabilities that are only present in example images.

@julio-lopez
Copy link
Contributor

@hairyhum @psilva-veeam Let's merge these changes as they are. We can iterate and address the comments as separate PRs. Thanks.

@hairyhum
Copy link
Contributor Author

I'm still enabling medium severity here to highlight existing issues (currently in restic and kopia), but not enabling not fixed issues.

@hairyhum hairyhum enabled auto-merge (squash) June 21, 2024 16:44
@hairyhum hairyhum disabled auto-merge June 21, 2024 16:45
@hairyhum hairyhum merged commit e6e500b into master Jun 21, 2024
14 of 15 checks passed
@hairyhum hairyhum deleted the image-scanning-split branch June 21, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants