ci: add token for cron-update-rust.yml #1580
Conversation
| - uses: tibdex/github-app-token@v1 | ||
| id: generate-token | ||
| with: | ||
| app_id: ${{ secrets.APP_ID }} | ||
| private_key: ${{ secrets.APP_PRIVATE_KEY }} |
There was a problem hiding this comment.
Do we need this new GH Action ?
Although it's suggested in the peter-evans/create-pull-request, can't we just follow the same approach as rust-bitcoin (here) and use a new token ?
| - uses: crazy-max/ghaction-import-gpg@v5 | ||
| with: | ||
| gpg_private_key: ${{ secrets.GPG_PRIVATE_KEY }} | ||
| git_user_signingkey: true | ||
| git_commit_gpgsign: true |
There was a problem hiding this comment.
I'm unsure how this action impact security-wise, but it doesn't seem very used/maintained :think:. We can always sign off the commits if we opt to not use this action.
There was a problem hiding this comment.
It looks like the rust-bitcoin repo doesn't enforce that the commits be signed. With our current repo rules we can't merge unsigned commits. https://github.com/rust-bitcoin/rust-bitcoin/pull/3267/commits
So we'd need to take off this protection, or figure out a way to get the github action to create a properly signed commit.
There was a problem hiding this comment.
This looks like it will work, but I want to make sure we've considered the alternatives and that these new actions won't be a hassle to maintain for someone like @notmandatory
Options:
- for security, fork or pin third-party actions to a commit SHA (requires code review)
- drop requirement to sign commits (not ideal)
- allow actions to create PR but don't merge it (not ideal)
- just pin clippy to stable and forgo the extra automation
There was a problem hiding this comment.
I'm OK with these automations since they will prevent us from getting unexpected breaks when stable updates behind the scenes. My main concern is I don't want to use my (or anyone else's) github or gpg credentials to create and sign the PR commit.
I think the github app credentials for PR creation as I have it now are a fine way to go. If the GPG plugin I added doesn't work then we could just let the action create the PR and who ever merges it will need to first rebase-resign the commit manually. It shouldn't happen too often so I'm not worried about the extra step.
There was a problem hiding this comment.
BTW I think I can test this in my personal repo, will post here to confirm if it works as expected.
Also I'm not so worried about the plugin versions being pinned since this can only create PRs it can't commit them.
There was a problem hiding this comment.
I tested on my local repo and it works, see: https://github.com/notmandatory/bdk/pull/9/commits
I had to create a new GPG key for the action, add the corresponding GPG pubkey to my account, and finally verify the email for this GPG key with my account. But after all that github created the PR, automatically runs the CI, and is able to verify the commit so the PR can be merged. 🎉
There was a problem hiding this comment.
I tested on my local repo and it works, see: https://github.com/notmandatory/bdk/pull/9/commits
Do you happen to know why the ci ran 41 checks on that PR?
There was a problem hiding this comment.
I guess it's because it ran on pull_request and push dispatches 🤔
Description
Add organization app token and GPG signing key for cron-update-rust.yml.
Notes to the reviewers
I went with the organization github app token option mentioned here:
https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#triggering-further-workflow-runs
I added gpg commit signing with below instructions. The instructions say to use PAT for signing but the plugin doesn't mention it's needed so I want to try it with only the github app token.
https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#gpg-commit-signature-verification