-
Notifications
You must be signed in to change notification settings - Fork 569
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
Use new GitHub app in workflows instead of token #10650
Conversation
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.
the backport and helm-weekly-release-pr workflows are also using GH_BOT_ACCESS_TOKEN
- name: Download yq | ||
uses: dsaltares/[email protected] | ||
with: | ||
repo: 'mikefarah/yq' | ||
version: 'tags/v4.30.6' | ||
file: 'yq_linux_amd64' | ||
target: 'bin/yq' | ||
token: ${{ secrets.GITHUB_TOKEN }} |
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.
For this one, and other similar cases below, do we have any problems with GITHUB_TOKEN
, actually? This one is a short-lived token, auto generated by GitHub, only for the life-time of the action's execution (ref GitHub docs).
That is, GH_BOT_ACCESS_TOKEN
is the PAT, that we SHOULD to get rid of. The GITHUB_TOKEN
IS fine, as it's managed by GitHub, isn't it?
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.
You're right, we should keep using a GitHub short-lived token in this workflow.
gh pr review $PR_URL \ | ||
--approve -b "**I'm approving** this pull request, since it is a helm release." | ||
env: | ||
PR_URL: ${{github.event.pull_request.html_url}} | ||
GITHUB_TOKEN: ${{secrets.GH_BOT_ACCESS_TOKEN}} |
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.
BTW, today this doesn't work, because grafanabot
(the principal of GH_BOT_ACCESS_TOKEN
) is trying to approve its own PR 🤖
Do you happen to know if, after we update to a GH app, this will use different principals for opening a PR, and an auto-approval?
cbc928a
to
12a4edb
Compare
Dimitar mentioned above, that the backport and helm-weekly-release-pr workflows also rely on the |
|
||
# Use the secrets, they will be obfuscated but that way we'll know they're accessible | ||
- name: Echo secrets | ||
run: | | ||
echo "${{ env.APP_ID }}" | ||
echo "${{ env.PRIVATE_KEY }}" |
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'm not sure about this debug. Below (and in other workflows in this PR) we don't use these envs but rely on the step's output. GitHub should obfuscate them, but leaving that feels a little bit dangerous.
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.
Yes this was only for testing that the value could be fetched, but now doc-validator is just not running.
This reverts commit 5d97440.
7a19969
to
f4cc9fc
Compare
This uses the same GitHub app as the one we use in backend-enterprise. This should allow us to stop using the GitHub token we were previously using.