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

Switch to using the GitHub API to commit changes, for GPG #391

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

s0
Copy link
Contributor

@s0 s0 commented Aug 25, 2024

fixes #392

Turns out that there wasn't any good TypeScript libraries for making modifications to files directly, let alone one that looks at the files changed locally to determine what needs to be pushed to GitHub, so I went ahead and created one here: https://github.com/s0/ghcommit

I then used this repo as a test-case for the updated actions to see if it all works, and so far so good.

Would you like this functionality? And if so, do we want to just switch to this behavior by default + have a major version bump? or do we want to hide it behind an action input / argument for now and have it as opt-in?

My thinking is that just switching to this behavior overall makes the most sense, as most people probably want to attribute the commits to the owner of the GITHUB_TOKEN, and more and more people are going to require that commits are signed as the industry takes supply-chain-security more and more seriously.

s0 added 6 commits August 25, 2024 14:39
To allow for signed tags to be created,
rather than use the git CLI to push tags,
manually push each tag using the GitHub API,
which will sign the tag using the built-in GitHub GPG key.
To allow for all commits to be signed,
use the GitHub API to push changes.
Copy link

changeset-bot bot commented Aug 25, 2024

🦋 Changeset detected

Latest commit: a369cc5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@changesets/action Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@s0 s0 changed the title Switch to using the GitHub API to commit changes, so all commits and tags are signed Switch to using the GitHub API to commit changes, for GPG Aug 25, 2024
@Andarist
Copy link
Member

This is a clever solution but tbh... I'm not sure if I like it for this project. It feels less maintainable - with git itself we have certain freedom in how we interact with it. I'm also worried that this would result in hitting rate limits more often (which already is an issue that this action hits from time to time).

@s0
Copy link
Contributor Author

s0 commented Aug 25, 2024

Hmm rate limits is a fair concern. Would you support having this as an opt-in feature then rather than replacing the existing git functionality for those of us that require signed commits? 🤔

@Andarist
Copy link
Member

I don't like having options like this but then I also see the value in this approach 😅 cc @emmatown thoughts?

@s0
Copy link
Contributor Author

s0 commented Aug 26, 2024

I was looking into the rate-limit issue again, and I realize I actually experienced it here: https://github.com/s0/ghcommit/actions/runs/10548632928/job/29222763569 (and can see from #192 that it's the same rate-limit as I faced).

My guess is that we can probably improve things a lot by avoiding the search APIs, which typically have higher rate-limit costs (at least 5x according to https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#calculating-points-for-the-secondary-rate-limit). But beyond that, primary rate limits for the search API are tracked separately to other REST endpoints, so it wouldn't surprise me if there's some conflict / interference with other tokens / repos based on e.g. which runners these are executed on? Given that, at least with the primary rate limits, the GitHub Actions GITHUB_TOKEN has a separate rate-limit per repo.

I imagine that we'll be able to avoid hitting the limits at all if we switch from the search API to the pull request list API, which provides filters for everything you use the search API for: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#list-pull-requests

I could open a separate PR for this change if you like?

@s0
Copy link
Contributor Author

s0 commented Aug 26, 2024

PR opened :) #393

@kazuki-hanai
Copy link

Hi @dalmena
I want to use this signed commits feature. When will this PR be merged?

@graffhyrum
Copy link

+1

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.

Allow for GPG signing without manually configuring keys
5 participants