Closed celanthe closed 2 years ago
celanthe
commented
2 years ago Hi @Langleu! Interestingly enough, it seems that the PR you linked had some movement as of earlier today! :) It looks like adding the Trivy scans on lines 68-78 might be possible to do now, if I'm reading this GitHub Actions documentation render correctly?
I welcome your thoughts!
Langleu
commented
2 years ago @celanthe , great news! I didn't expect them to merge that quickly as it was stalling for a while. Sure, go ahead and give it a try!
celanthe
commented
2 years ago Awesome @Langleu! That's great news! I ran the YAML through one last check and it's all valid! :)
Unfortunately, I'm running into an issue with the pre-commit check. All my tests come back 'passed' except for 'prettier'. Which is still failing, causing the pre-commit check to fail. It looks like the workflows and actions will work, however!
Validate GitHub Workflows................................................Passed Validate GitHub Actions..................................................Passed Validate Example Workflows...............................................Passed Shellcheck Bash Linter...................................................Passed
The message in the logs says, "pre-commit hook(s) made changes.
If you are seeing this message in CI, reproduce locally with: pre-commit run --all-files.
To run pre-commit as part of git workflow, use pre-commit install.
All changes made by hooks:
diff --git a/action.yml b/action.yml
index c3e13d6..4255359 100644"
I've dug through google and the logs and this seems to be intentional in that prettier hooks will fail if a hook modifies a commit. It seems like I have a few options here:
git addI'm not quite sure how to reproduce locally and tell prettier that everything is okay. I welcome your thoughts here!
Langleu
commented
2 years ago Hey @celanthe, you can fix those errors by running pre-commit locally and push the changes.
When cloning the repo and developing locally, a pre-commit hook will be triggered with every commit you try to add, this will lint and format your code changes so you won't be able to push anything out of the line.
In this case you'll have to run the suggested pre-commit run --all-files as your changes are already commited.
For more information on pre-commit and how to install it, have a look at their website https://pre-commit.com/ .
Generally, the PR was just an ADR (Architecture Decision Record), so till the actual implementation is done it can still take quite some time.
As mentioned last time, line 68-78 would have to be deleted anyway as you can't define a GitHub Action within another.
So for now the only possible way to integrate it into this composite action would be to manually retrieve the trivy binary and then execute the steps like that via bash.
celanthe
commented
2 years ago Hi @Langleu! Awesome, I fixed this up! :)
I added a bash script that should download and install Trivy, and deleted lines 68-78! Fingers crossed that should get us a bit closer to this working! And all the tests are passing now. I got pre-check working and all sorted! :D
I was wondering if it's possible to make this optional? I suppose the best way to do that would be to release a new version as we'd discussed with this new code included, and people can upgrade if they so choose? Or would another path forward be potentially breaking out lines 68-98 into their own action? Or perhaps running a [dispatch action](https://github.com/marketplace/actions/dispatch-action] where this would be step one, and if an exit-code: 0 is returned it would send a message to kick off the community-action-maven-release workflow?
Just thinking out loud! :)
Langleu
commented
2 years ago Hey @celanthe,
one could introduce an additional input called e.g. trivy or activate-trivy enable-vuln-scan, something that fits. Set it to default "false" or "0". In the bash script, one could then only run the trivy stuff if the input is set to either true or 1.
Or would another path forward be potentially breaking out lines 68-98 into their own action?
Yes, this would definitely be an option. The only downside is that it will require more "effort" from the user base to include all of it or exclude it if a different solution is introduced in the future and is, therefore, more error-prone. Your idea would then be to trigger the release action via Trivy, which should certainly be possible as well. As I said it will require a lot of changes from the implementation side and also from the user base for a potentially temporary implementation.
One way to test if your action is working would be to use our demo repository - https://github.com/camunda/infra-github-demo .
E.g. create an extra branch, and change in there the following line to https://github.com/camunda/infra-github-demo/blob/a96978ad29f91e4a8dbdda195eaf80ff3d0482d8/.github/workflows/deploy.yml#L32 e.g. camunda-community-hub/community-action-maven-release@celanthe-patch-2 and then create a release based on the branch, which should, in turn, run the branch you referenced of the release action. In case it is successful, don't forget to delete/ drop without releasing the repository in Sonatype as we don't want to publish it to maven central.
Let me know what you think and whether we should have a synchronous call on the topic to iterate over it / pair programming?
celanthe
commented
2 years ago HI @Langleu! I like the idea of introducing an additional input called enable-trivy-vuln-scan and setting it to default "false" or "0", and as you'd mentioned n the bash script, one could then only run the Trivy piece if the input is set to either true or 1.
I would love to pair program on this part, for sure! I unfortunately am getting a 404 error when I try to access the infra-github-demo you linked. Is that a GitHub permissions issue? Would I need to open a ticket with IT to get access to that repo for testing purposes?
Thank you so much! I look forward to pairing, and will try to find a time on our calendars over the next few weeks to make it work!
celanthe
commented
2 years ago Hi @Langleu! I got access to the infra-demo repository and set up a new branch/test workflow and changed L32 as you'd suggested. I can get the workflow from this PR to build successfully 🎊 , but it's failing on the deploy step. Both your run https://github.com/camunda/infra-github-demo/actions/runs/1077574813 and mine at https://github.com/camunda/infra-github-demo/actions/runs/1091353319 have the same four errors, too, which is interesting. I've done some digging and it seems to maybe be something related to templating in GitHub Actions...
Not sure if that's what's causing our issue here, but it might be a good place to start? :) I look forward to connecting!
Description: This pull request introduces two Trivy security scans running in repo mode on both
PUSHandPULLrequests.The first scan checks for unknown, low, and medium security vulnerabilities, returns an exit-code: 0, and formats the scan results into a template.
The second scan runs in repo mode and checks for high and critical vulnerabilities, fails with exit-code: 1 if vulnerabilities are found, and formats the scan results into a template.