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

Support commit signing for homebrew taps (and maybe other platforms too) #4616

Open
2 of 3 tasks
vedantmgoyal9 opened this issue Feb 8, 2024 · 9 comments
Open
2 of 3 tasks
Assignees
Labels
enhancement New feature or request waiting waiting on some condition

Comments

@vedantmgoyal9
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Support GPG and SSH commit signing where we push to Git repos, like homebrew taps/winget-pkgs.

Pushing commit fails, or PR is not able to merge, where "Require signed commits" is enabled in branch protection rules (see option 9 - https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/managing-a-branch-protection-rule#creating-a-branch-protection-rule)

Describe the solution you'd like

GitHub (google/go-github) SDK provides a way to sign commits.

https://github.com/google/go-github/blob/master/github/git_commits.go

Previous issue:

That's a fair assessment. I assume the best way forward would be for GitHub to bring that functionality (signing and deployment keys) into the API/SDK.

Originally posted by @radeksimko in #1774 (comment)

Describe alternatives you've considered

N/A

Search

  • I did search for other open and closed issues before opening this

Supporter

Code of Conduct

  • I agree to follow this project's Code of Conduct

Additional context

No response

@vedantmgoyal9 vedantmgoyal9 added enhancement New feature or request triage Issue pending triage by one of the maintainers labels Feb 8, 2024
@caarlos0
Copy link
Member

caarlos0 commented Feb 8, 2024

still don't know if I want to do this, for the same reasons laid of in the original issue.

that said, if you use the git option and appropriately setup git to sign (with git config) it might work already...

@caarlos0
Copy link
Member

closing as not possible with github api.

@caarlos0 caarlos0 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 15, 2024
@caarlos0 caarlos0 added wontfix This will not be worked on and removed enhancement New feature or request triage Issue pending triage by one of the maintainers labels Feb 15, 2024
@vedantmgoyal9
Copy link
Contributor Author

Commits made by GitHub GraphQL API are automatically signed.

@caarlos0
Copy link
Member

thats a very big change, not sure if its worth the risk

@vedantmgoyal9
Copy link
Contributor Author

Maybe you can re-open and add a label for community vote? If it gets enough +1s, then it can be implemented if it gets high demand.

@caarlos0 caarlos0 reopened this Feb 15, 2024
@caarlos0
Copy link
Member

if someone PRs it, I'm willing to accept.

Probably would have to be opt-in for now at least, as there's a too many details already taken care of in the current implementation... I wouldn't feel confident to just switch it, I think.

@caarlos0 caarlos0 added enhancement New feature or request waiting waiting on some condition and removed wontfix This will not be worked on labels Feb 15, 2024
@vedantmgoyal9
Copy link
Contributor Author

vedantmgoyal9 commented Feb 15, 2024

I think we don't have change the whole implementation. We can just update the following part to use GraphQL API. Rest other things will remain same as they are now - use REST API.

if defBranch != branch && branch != "" {
_, res, err := c.client.Repositories.GetBranch(ctx, repo.Owner, repo.Name, branch, 100)
if err != nil && (res == nil || res.StatusCode != http.StatusNotFound) {
return fmt.Errorf("could not get branch %q: %w", branch, err)
}
if res.StatusCode == http.StatusNotFound {
defRef, _, err := c.client.Git.GetRef(ctx, repo.Owner, repo.Name, "refs/heads/"+defBranch)
if err != nil {
return fmt.Errorf("could not get ref %q: %w", "refs/heads/"+defBranch, err)
}
if _, _, err := c.client.Git.CreateRef(ctx, repo.Owner, repo.Name, &github.Reference{
Ref: github.String("refs/heads/" + branch),
Object: &github.GitObject{
SHA: defRef.Object.SHA,
},
}); err != nil {
rerr := new(github.ErrorResponse)
if !errors.As(err, &rerr) || rerr.Message != "Reference already exists" {
return fmt.Errorf("could not create ref %q from %q: %w", "refs/heads/"+branch, defRef.Object.GetSHA(), err)
}
}
}
}
file, _, res, err := c.client.Repositories.GetContents(
ctx,
repo.Owner,
repo.Name,
path,
&github.RepositoryContentGetOptions{
Ref: branch,
},
)
if err != nil && (res == nil || res.StatusCode != http.StatusNotFound) {
return fmt.Errorf("could not get %q: %w", path, err)
}
options.SHA = github.String(file.GetSHA())
if _, _, err := c.client.Repositories.UpdateFile(
ctx,
repo.Owner,
repo.Name,
path,
options,
); err != nil {
return fmt.Errorf("could not update %q: %w", path, err)
}
return nil

Edit: GraphQL API doesn't support custom commit author name and email, so we can make it optional with a config parameter like:

# .goreleaser.yaml
github:
  sign_commits: true # boolean

and based on that use GraphQL API. If custom commit author and email is set along with sign_commits parameter, just give a warning that they will be ignored.

@caarlos0
Copy link
Member

I would rather make it more explicit, something like:

github:
  use_graphql_api: true

and we can start with the commit only, but eventually I'd say to reimplement the whole client

@vedantmgoyal9
Copy link
Contributor Author

That can also be done. We can implement things one-by-one and switch to GraphQL API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request waiting waiting on some condition
Projects
None yet
Development

No branches or pull requests

2 participants