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

feat: adds new branch protection options for last reviewer and locking branch #1407

Merged
merged 3 commits into from
Dec 5, 2022

Conversation

wwsean08
Copy link
Contributor

@wwsean08 wwsean08 commented Dec 2, 2022

Resolves #1352 (at least for the v4 version of branch protection)
Resolves #1341 (at least for the v4 version of branch protection)

Behavior

This enables two new options introduced in October around branch protections, specifically, the ability to require the last approval be from someone besides the last person to push to the branch (for the case that multiple people are working on the same branch), as well as the ability to lock branches.

The new option did require upgrading shurcooL/githubv4.

Before the change?

  • This can't be changed via terraform

After the change?

  • These options are available for managing the state of these branch protection options

Other information

  • N/A

Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

Type: Feature


Manual Testing:

Overrode terraform to use by locally built version via dev.tfrc, and these were the results (with repo/org names redacted):

main.tf:

terraform {
  required_providers {
    github = {
      source  = "integrations/github"
      version = "~> 5.0"
    }
  }
}

data "github_repository" "test" {
  full_name = "REDACTED/REDACTED"
}

resource "github_branch_protection" "example" {
  repository_id = data.github_repository.test.node_id
  pattern = "test"

  required_pull_request_reviews {
    require_last_push_approval = true
  }

  lock_branch = true
}

Screenshot 2022-12-02 at 1 35 34 PM

Branch protection rules created (in two screenshots because of the size of the page)
Screenshot 2022-12-02 at 1 35 55 PM
Screenshot 2022-12-02 at 1 36 01 PM

@wwsean08 wwsean08 marked this pull request as ready for review December 2, 2022 21:39
@wwsean08
Copy link
Contributor Author

wwsean08 commented Dec 2, 2022

Note I would have added the feature label, but don't have the ability to

@kfcampbell kfcampbell added the Type: Feature New feature or request label Dec 2, 2022
Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution and thoughtful manual testing description!

Integration tests here are broken on main and the changes in this PR do not cause any new breakages.

@kfcampbell kfcampbell merged commit 710c65a into integrations:main Dec 5, 2022
@wwsean08 wwsean08 deleted the feat/1352 branch December 5, 2022 23:15
TheQueenIsDead pushed a commit to TheQueenIsDead/terraform-provider-github that referenced this pull request Dec 12, 2022
kazaker pushed a commit to auto1-oss/terraform-provider-github that referenced this pull request Dec 28, 2022
kfcampbell added a commit that referenced this pull request Jan 20, 2023
* feat: add new schema for check block resource under required_status_checks

* feat: iterate provided `check` blocks and build array of RequiredStatusCheck

* feat: set default app_id to -1

* feat: implement checks flattening for required status checks

* Add resource github_app_installation_repositories (#1376)

* Add resource github_app_installation_repositories

This resource allows multiple repositories to be passed in; which greatly improves the performance of the resource compared to the single repository version
when needing to control state of multiple app installations with multiple repos, required in larger organisations.

The optimisation occurs as only a single call to get the list of repos is required per installation per read, regardless of the number of respositories being added.

- Add resource_github_app_installation_repositories
- Add tests

* Update docs and link

Co-authored-by: Keegan Campbell <[email protected]>

* feat: adds new branch protection options for last reviewer and locking branch (#1407)

Co-authored-by: Keegan Campbell <[email protected]>

* feat(github_release): adding github_release resource and tests (#1122)

* feat(github_release): adding github_release resource and tests

* feat(docs) adding github_release page to website docs

* chore: update changelog with this pr's new resource

* fix: adding node_id and release_id to resource attributes

* Update CHANGELOG.md

* Fix broken merge/build

Co-authored-by: Keegan Campbell <[email protected]>

* 🚧 Workflows have changed

Workflow changes have been made in the Octokit org repo. This PR is propagating those changes.

* Issue template tweak (#1422)

* Don't link to a real PR

* Wording tweak

* feat: allow branch protection check app_id to be null

* chore: change branch protection flatten function to use GetAppID sdk method

* feat: change branch protection v3 utils to flatten and expand contexts into checks

* feat: change checks from it's own resource to a list of strings

* chore: resolve incorrect merge of main

* chore: update deprecation notice on contexts array

* chore(docs): Update branch_protection_v3 docs to mention the new `checks` functionality

* fix: Initialise literal empty slice of RequiredStatusCheck to mitigate errors when passing nil to the sdk

* chore(lint): resolve gosimple S1082 violation (errors.New => fmt.Errorf)

* chore: remove unused code comment

Co-authored-by: David Bain <[email protected]>
Co-authored-by: Keegan Campbell <[email protected]>
Co-authored-by: Sean Smith <[email protected]>
Co-authored-by: Trent Millar <[email protected]>
Co-authored-by: Nick Floyd <[email protected]>
avidspartan1 pushed a commit to avidspartan1/terraform-provider-github that referenced this pull request Feb 5, 2024
avidspartan1 pushed a commit to avidspartan1/terraform-provider-github that referenced this pull request Feb 5, 2024
* feat: add new schema for check block resource under required_status_checks

* feat: iterate provided `check` blocks and build array of RequiredStatusCheck

* feat: set default app_id to -1

* feat: implement checks flattening for required status checks

* Add resource github_app_installation_repositories (integrations#1376)

* Add resource github_app_installation_repositories

This resource allows multiple repositories to be passed in; which greatly improves the performance of the resource compared to the single repository version
when needing to control state of multiple app installations with multiple repos, required in larger organisations.

The optimisation occurs as only a single call to get the list of repos is required per installation per read, regardless of the number of respositories being added.

- Add resource_github_app_installation_repositories
- Add tests

* Update docs and link

Co-authored-by: Keegan Campbell <[email protected]>

* feat: adds new branch protection options for last reviewer and locking branch (integrations#1407)

Co-authored-by: Keegan Campbell <[email protected]>

* feat(github_release): adding github_release resource and tests (integrations#1122)

* feat(github_release): adding github_release resource and tests

* feat(docs) adding github_release page to website docs

* chore: update changelog with this pr's new resource

* fix: adding node_id and release_id to resource attributes

* Update CHANGELOG.md

* Fix broken merge/build

Co-authored-by: Keegan Campbell <[email protected]>

* 🚧 Workflows have changed

Workflow changes have been made in the Octokit org repo. This PR is propagating those changes.

* Issue template tweak (integrations#1422)

* Don't link to a real PR

* Wording tweak

* feat: allow branch protection check app_id to be null

* chore: change branch protection flatten function to use GetAppID sdk method

* feat: change branch protection v3 utils to flatten and expand contexts into checks

* feat: change checks from it's own resource to a list of strings

* chore: resolve incorrect merge of main

* chore: update deprecation notice on contexts array

* chore(docs): Update branch_protection_v3 docs to mention the new `checks` functionality

* fix: Initialise literal empty slice of RequiredStatusCheck to mitigate errors when passing nil to the sdk

* chore(lint): resolve gosimple S1082 violation (errors.New => fmt.Errorf)

* chore: remove unused code comment

Co-authored-by: David Bain <[email protected]>
Co-authored-by: Keegan Campbell <[email protected]>
Co-authored-by: Sean Smith <[email protected]>
Co-authored-by: Trent Millar <[email protected]>
Co-authored-by: Nick Floyd <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
2 participants