-
Notifications
You must be signed in to change notification settings - Fork 107
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
feat: automatically comment on PR for govulncheck #623
Conversation
708c064
to
36f6adf
Compare
Hm, looks like adding the write permission to that one job borks the CI so that no other jobs run. Without that permission on the govulncheck job we can't post a comment using the token generated during execution. EDIT: This is the error: |
.github/workflows/linting.yml
Outdated
@@ -28,6 +30,14 @@ jobs: | |||
with: | |||
go-version-file: 'go.mod' | |||
go-package: ./... | |||
- name: Comment on PR | |||
if: steps.govulncheck.outcome == 'failure' | |||
uses: thollander/actions-comment-pull-request@fabd468d3a1a0b97feee5f6b9e499eab0dd903f6 # v2.5.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it difficult if we use the gh api instead of another action?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an approach that uses curl and the API and one that uses the script action to allow calling gh-action functions. I just briefly looked into it though. I suspect they'd also need write rights in order to post the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- name: Comment to PR
env:
URL: ${{ github.event.pull_request.comments_url }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
curl \
-X POST \
$URL \
-H "Content-Type: application/json" \
-H "Authorization: token $GITHUB_TOKEN" \
--data '{ "body": "Place comment here" }'
This would be the curl approach but I think this will only create, not edit any comments. So for every PR update it will post a comment. I did not verify that though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually referring to the gh
client as it's already available in the action but I haven't played around with it that much to tell if it will actually work properly for our use case (comment once -> edit afterwards)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like that might also be feasible but I actually discovered an even better way using a rather underdocumented feature: The env var $GITHUB_STEP_SUMMARY
which you can just echo into and it displays this as a comment on the discussions tab. I'll try this out and update this draft.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, it displays on the summary and not discussion tab. I added a way that uses the gh client as you noted now.
36f6adf
to
c4177f4
Compare
c4177f4
to
87b5094
Compare
It would still require write permissions to actually allow commenting. That's why most jobs don't run right now. |
Looking good 👍 Is there anything else left to do or we can merge it? |
Please don't merge it yet. Code-wise there is nothing needed to do but as of right now it will disable the majority of the workflows due to a violation of the permissions. The whole org forbids workflows right now that have write permissions and given that we try to claim write rights on that one job it disables all associated other workflows. I think I'm not in the position to decide a change as it will affect every repo in the org... |
Signed-off-by: Marvin Drees <[email protected]>
Signed-off-by: Marvin Drees <[email protected]>
87b5094
to
261d36e
Compare
I'm revisiting this PR since govulncheck 1.1.x is out but it does not seem like there is a fix for for the false positive reporting for the Go stdlib issue it will mark each run creating a lot of noise. It also doesn't seem like this behavior is configurable. Therefore I'm closing this PR given that elevating the rights for actions does not seem like a good idea and having a red-cross on every run also doesn't seem ideal. |
No description provided.