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

Provide "require signed commits" on github_branch_protection #212

Closed
wants to merge 10 commits into from

Conversation

bltavares
Copy link
Contributor

This commit provides changing the toggle to require signed commits on
branch protection.

The API for signed commits
is separate endpoint from the branch protection url, and does not come on the same payload.
This means that we need to make an extra request to read and modify it.

This changes requires go-github at least v19.

Needs:

Closes https://github.com/terraform-providers/terraform-provider-github/issues/87

mattburgess and others added 10 commits April 23, 2019 10:29
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Update imports as go-github converted to using modules starting with
v18.

Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
This commit includes the `required_approving_review_count` field on the
`github_branch_protection` resource, allowing to manage the miminum
amount of reviewers to allow a merge to happen.

This field must be between 1 - 6, according to the docs, and must be
valid if present.

Bumping the `go-github` to `v24` made it default to `0` when not
present, causing the followin error

<detail>
<summary>GitHub API error when count is 0</summary>

```
422 Invalid request.

No subschema in "anyOf" matched.
0 must be greater than or equal to 1.
Not all subschemas of "allOf" matched.
For 'anyOf/1', {"dismissal_restrictions"=>{"users"=>[], "teams"=>[]}, "dismiss_stale_reviews"=>false, "require_code_owner_reviews"=>true, "required_approving_review_count"=>0} is not a null. []

Payload:
{
 "required_status_checks": {
  "strict": true,
  "contexts": [
   "lint",
   "test"
  ]
 },
 "required_pull_request_reviews": {
  "dismissal_restrictions": {
   "users": [],
   "teams": []
  },
  "dismiss_stale_reviews": false,
  "require_code_owner_reviews": true,
  "required_approving_review_count": 0
 },
 "enforce_admins": true,
 "restrictions": null
}
```
</detail>

This PR is important when upgrading `go-github`.

Built on top of: https://github.com/terraform-providers/terraform-provider-github/pull/207
This commit provides changing the toggle to require signed commits on
branch protection.

The [API for signed commits](https://developer.github.com/v3/repos/branches/#get-required-signatures-of-protected-branch)
is separate endpoint from the branch protection url, and does not come on the same payload.
This means that we need to make an extra request to read and modify it.

This changes requires `go-github` at least v19.

Need:
 - [ ] Update `go-github` version: https://github.com/terraform-providers/terraform-provider-github/pull/207
 - [ ] Provide default required reviewers count to avoid GitHub update error: https://github.com/terraform-providers/terraform-provider-github/pull/211
Closes https://github.com/terraform-providers/terraform-provider-github/issues/87
@ghost ghost added the size/XXL label Apr 23, 2019
@bltavares
Copy link
Contributor Author

I'll gladly rebase or pick the last commit apart if necessary, but updating the version of go-github is important for it. After #207 lands, I'll gladly update the PR.

Note: precompiled plugin to validate the changes here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] support requiring "signed commits" on github_branch_protection
2 participants