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

feat: automatically comment on PR for govulncheck #623

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MDr164
Copy link
Contributor

@MDr164 MDr164 commented Mar 8, 2024

No description provided.

@MDr164
Copy link
Contributor Author

MDr164 commented Mar 8, 2024

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: The nested job 'govulncheck_job' is requesting 'pull-requests: write', but is only allowed 'pull-requests: none'.

@@ -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
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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)

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@MDr164 MDr164 marked this pull request as ready for review March 11, 2024 11:05
Signed-off-by: Marvin Drees <marvin.drees@9elements.com>
@MDr164
Copy link
Contributor Author

MDr164 commented Mar 11, 2024

It would still require write permissions to actually allow commenting. That's why most jobs don't run right now.

@rdimitrov
Copy link
Contributor

Looking good 👍 Is there anything else left to do or we can merge it?

@MDr164
Copy link
Contributor Author

MDr164 commented Mar 12, 2024

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...

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.

None yet

2 participants