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 scheduled Snyk check for Docker image #995

Merged
merged 3 commits into from
Feb 12, 2025
Merged

Conversation

keyvaann
Copy link
Contributor

This PR adds a weekly scan by Snyk to check the Docker image and will suggest fixes if it fails.

@mpgxvii
Copy link
Member

mpgxvii commented Feb 11, 2025

@keyvaann I think the main test workflow is failing because of the deprecated action upload-artifact. We can just update to v4 and the tests will run again. I've updated here: 6c775c6

mpgxvii
mpgxvii previously approved these changes Feb 11, 2025
Copy link
Member

@mpgxvii mpgxvii left a comment

Choose a reason for hiding this comment

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

LGTM! Just minor comment.

@keyvaann
Copy link
Contributor Author

Thanks @mpgxvii, I applied the fix.

@mpgxvii mpgxvii self-requested a review February 12, 2025 09:30
mpgxvii
mpgxvii previously approved these changes Feb 12, 2025
Copy link
Member

@mpgxvii mpgxvii left a comment

Choose a reason for hiding this comment

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

Thanks! Also, are we merging to master directly?

@keyvaann
Copy link
Contributor Author

@mpgxvii I don't know where is the best place to merge it. Maybe @pvannierop has suggestions?

@pvannierop
Copy link
Collaborator

@mpgxvii @keyvaann

Thanks! Also, are we merging to master directly?

Depends on the state of the dev branch. We need to make a periodic release that fixes vulnerabilities. I rather not have to deal with problems that arise from features that are in development. I, therefore, would like a PR directly to master.

Copy link
Collaborator

@pvannierop pvannierop left a comment

Choose a reason for hiding this comment

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

I have two issues that I would like to raise:

  • Should the scheduled action only trigger on master branch?
  • Should the scheduled docker action replace the current Snyk scan that is performed for each commit to a PR branch?

@keyvaann
Copy link
Contributor Author

I have two issues that I would like to raise:

  • Should the scheduled action only trigger on master branch?
  • Should the scheduled docker action replace the current Snyk scan that is performed for each commit to a PR branch?

It's probably better to run it only on master branch since this is used to fix security vulnerabilities.
I think having a snyk scan that directly checks the codebase is better than the docker image scan so I think the current snyk scan in each commit is good, but I don't know how much attention is actually being paid to it.

@keyvaann keyvaann merged commit 0c9cb40 into master Feb 12, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants