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

GetSignaturesProtectedBranch Does Not Return ErrBranchNotProtected When it Could #2821

Open
cedricvanrompay-datadog opened this issue Jun 28, 2023 · 1 comment
Assignees

Comments

@cedricvanrompay-datadog

When calling the GitHub API endpoint https://api.github.com/repos/OWNER/REPO/branches/BRANCH/protection/required_signatures (documentation), if the branch does not have any protection rules, the GitHub API will reply with a HTTP 404 Not Found code and the following body:

{
  "message": "Branch not protected",
  "documentation_url": "https://docs.github.com/rest/branches/branch-protection#get-commit-signature-protection"
}

Other functions in go-github that may receive this kind of response usually manage it the following way:

go-github/github/repos.go

Lines 1347 to 1349 in 96726d8

if isBranchNotProtected(err) {
err = ErrBranchNotProtected
}

However, GetSignaturesProtectedBranch does not:

go-github/github/repos.go

Lines 1405 to 1409 in 96726d8

resp, err := s.client.Do(ctx, req, p)
if err != nil {
return nil, resp, err
}

I think this function should have the same behaviour, allowing callers to handle this case the same way as when calling say GetBranchProtection. Note that function isBranchNotProtected sounds like it would work without changes:

go-github/github/repos.go

Lines 2033 to 2036 in 96726d8

func isBranchNotProtected(err error) bool {
errorResponse, ok := err.(*ErrorResponse)
return ok && errorResponse.Message == githubBranchNotProtected
}

const githubBranchNotProtected string = "Branch not protected"

I am happy to provide a Pull Request (should be trivial) for this.

@gmlewis
Copy link
Collaborator

gmlewis commented Jun 28, 2023

@cedricvanrompay-datadog - may I first say that this is one of the best issue messages I have ever read?
It is clear, to the point, and provides all the details needed with examples and a pointer to the problem.
Well done, and thank you very much!

Absolutely, I agree, and the issue is yours. Thanks again!

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

No branches or pull requests

2 participants