Update GitHub Actions and container scanning - #298
Conversation
achaikou
left a comment
achaikou
left a comment
There was a problem hiding this comment.
Looks nice!
Secrets are probably not scanned because there are no secrets in the images.
I assume you saw how trivy failed during development, so I assume it does the job 👍
ajaust
left a comment
ajaust
left a comment
There was a problem hiding this comment.
Thanks for pointing to the example by the AppSec team. I completely missed that. Now I am insecure how to proceed. Drop my approach and go for the one recommended by the AppSec team or not?
I tend to lean towards the AppSec approach because I assume they have tested it a bit and thus have experience that it works well. Running the tests only on main will also prevent PRs from breaking due to (unrelated) flaws in the dependencies.
What do you think?
|
I just tried to understand the AppSec approach a bit more. I have to think a bit longer if I understand it correctly. I am currently not sure if they scan the build Docker image, only the Dockerfile or if the image is build from the Dockerfile. I will do some additional experiments. |
There was a problem hiding this comment.
I actually have not considered the implications 😄
I like your approach because it is simple 😄 , but maybe their approach makes more sense.
- Some issues would appear on people's PRs while not being caused by their PRs.
- I am not sure if it is always possible to fix vulnerabilities the moment they occur.
- From the other side causing PR to fail gives incentive to fix the problem now and not let it be forgotten in the security tab.
So probably apsec approach is less frustrating in the long run. Especially if it is not run on PRs or main, but only on schedule. UPD: or it would not matter much if exit code is set to 0 anyway.
8fcaaac to
a195a26
Compare
achaikou
left a comment
achaikou
left a comment
There was a problem hiding this comment.
Seems reasonable 👍
Don't know if we can check everything works fine with upload to Security tab yet, but lets deal with it after merge.
ajaust
left a comment
ajaust
left a comment
There was a problem hiding this comment.
Thanks for the review. I added two fixups
- Address the issues you raised.
- Full autoformat of the YAML files using Red Hat's YAML extension for VS Code. Does this improve the YAML file or should we rather drop this fixup?
I would like to get your opinion on some additional point for which I added a comment. This affects our scheduled runs if we do not push the change of this PR to the radix branch after merging.
ajaust
left a comment
ajaust
left a comment
There was a problem hiding this comment.
I added a fixup to drop the scheduled runs. I also ran the actions on my branch again to check that they still work: https://github.com/ajaust/oneseismic-api/actions/runs/19758495193/job/56614772320
achaikou
left a comment
achaikou
left a comment
There was a problem hiding this comment.
All resolved, no more questions.
Go rebase, maybe change the commit message to remove info about schedule and merge 👍
|
Just added an amend commit to verify on GitHub that the commit message looks good. I will rebase, squash and merge now. 🙂 |
e7ed56f to
54e256a
Compare
Trivy [1] is the currently recommended tool for container scanning since Snyk is being phased out. We pin the version of the Trivy Action [2] to its most recent release as recommended for third party Actions [3]. This is recommended to safeguard against supply chain attacks. We mimic both, Snyk's scanning of configuration files for infrastructure as code (radixconfig and Docker) and scanning of Docker images. The Docker image generated on the `radix` branch is also scanned. Reporting of the results depends on which branch is scanned. If the scan is run on the `main` or `radix` branch, the reports are uploaded to GitHub and presented in the "Security" tab of the repository. In all other cases, the results are reported within the Actions output and the action fails if an error is found. The container images are only build and scanned for the `main` and `radix` branch to save resources and to avoid unnecessary noise inside pull requests. [1]: https://github.com/aquasecurity/trivy [2]: https://github.com/aquasecurity/trivy-action [3]: https://docs.github.com/en/actions/reference/security/secure-use#using-third-party-actions
2 participants
This PR aims to replace Snyk's container scanning by Trivy and reduce the permissions provided to the GitHub token in GitHub ACtions.