diff --git a/github/resource_github_branch_protection_test.go b/github/resource_github_branch_protection_test.go index a13284d130..d9d7a7bcf3 100644 --- a/github/resource_github_branch_protection_test.go +++ b/github/resource_github_branch_protection_test.go @@ -3,19 +3,18 @@ package github import ( "context" "fmt" - "regexp" "sort" "testing" - "github.com/google/go-github/v31/github" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/kylelemons/godebug/pretty" + "github.com/shurcooL/githubv4" ) func TestAccGithubBranchProtection_basic(t *testing.T) { - var protection github.Protection + var protection BranchProtectionRule rn := "github_branch_protection.master" rString := acctest.RandString(5) @@ -29,149 +28,36 @@ func TestAccGithubBranchProtection_basic(t *testing.T) { { Config: testAccGithubBranchProtectionConfig(repoName), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubProtectedBranchExists(rn, repoName+":master", &protection), - testAccCheckGithubBranchProtectionRequiredStatusChecks(&protection, true, []string{"github/foo"}), - testAccCheckGithubBranchProtectionRestrictions(&protection, []string{testUser}, []string{}), - testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []string{testUser}, []string{}, true), - resource.TestCheckResourceAttr(rn, "repository", repoName), - resource.TestCheckResourceAttr(rn, "branch", "master"), + testAccCheckGithubProtectedBranchExists(rn, &protection), + testAccCheckGithubBranchProtectionRequiredStatusChecks(&protection, true, []githubv4.String{"github/foo"}), + testAccCheckGithubBranchProtectionRestrictions(&protection, []githubv4.String{githubv4.String(testUser)}), + testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []githubv4.String{githubv4.String(testUser)}, true), + resource.TestCheckResourceAttr(rn, "repository_id", repoName), + resource.TestCheckResourceAttr(rn, "pattern", "master"), resource.TestCheckResourceAttr(rn, "enforce_admins", "true"), resource.TestCheckResourceAttr(rn, "require_signed_commits", "true"), resource.TestCheckResourceAttr(rn, "required_status_checks.0.strict", "true"), resource.TestCheckResourceAttr(rn, "required_status_checks.0.contexts.#", "1"), resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismiss_stale_reviews", "true"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_users.#", "1"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_teams.#", "0"), + resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_restrictions.0.actor_ids.#", "1"), resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.require_code_owner_reviews", "true"), - resource.TestCheckResourceAttr(rn, "restrictions.0.users.#", "1"), - resource.TestCheckResourceAttr(rn, "restrictions.0.teams.#", "0"), + resource.TestCheckResourceAttr(rn, "push_restrictions.0.actor_ids.#", "1"), ), }, { Config: testAccGithubBranchProtectionUpdateConfig(repoName), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubProtectedBranchExists(rn, repoName+":master", &protection), - testAccCheckGithubBranchProtectionRequiredStatusChecks(&protection, false, []string{"github/bar"}), + testAccCheckGithubProtectedBranchExists(rn, &protection), + testAccCheckGithubBranchProtectionRequiredStatusChecks(&protection, false, []githubv4.String{"github/bar"}), testAccCheckGithubBranchProtectionNoRestrictionsExist(&protection), testAccCheckGithubBranchProtectionNoPullRequestReviewsExist(&protection), - resource.TestCheckResourceAttr(rn, "repository", repoName), - resource.TestCheckResourceAttr(rn, "branch", "master"), + resource.TestCheckResourceAttr(rn, "repository_id", repoName), + resource.TestCheckResourceAttr(rn, "pattern", "master"), resource.TestCheckResourceAttr(rn, "require_signed_commits", "false"), resource.TestCheckResourceAttr(rn, "required_status_checks.0.strict", "false"), resource.TestCheckResourceAttr(rn, "required_status_checks.0.contexts.#", "1"), resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.#", "0"), - resource.TestCheckResourceAttr(rn, "restrictions.#", "0"), - ), - }, - { - ResourceName: rn, - ImportState: true, - ImportStateVerify: true, - }, - }, - }) -} - -func TestAccGithubBranchProtection_users(t *testing.T) { - rn := "github_branch_protection.master" - rString := acctest.RandString(5) - repoName := fmt.Sprintf("tf-acc-test-branch-prot-%s", rString) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccGithubBranchProtectionDestroy, - Steps: []resource.TestStep{ - { - Config: testAccGithubBranchProtectionConfigUser(repoName, "user_with_underscore"), - ExpectError: regexp.MustCompile("unable to add users in restrictions: user_with_underscore"), - }, - { - Config: testAccGithubBranchProtectionConfigUser(repoName, testUser), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(rn, "repository", repoName), - resource.TestCheckResourceAttr(rn, "branch", "master"), - resource.TestCheckResourceAttr(rn, "enforce_admins", "true"), - resource.TestCheckResourceAttr(rn, "restrictions.0.users.#", "1"), - ), - }, - { - ResourceName: rn, - ImportState: true, - ImportStateVerify: true, - }, - }, - }) -} - -func TestAccGithubBranchProtection_teams(t *testing.T) { - var firstP, secondP, thirdP github.Protection - - rn := "github_branch_protection.master" - rString := acctest.RandString(5) - repoName := fmt.Sprintf("tf-acc-test-branch-prot-%s", rString) - firstTeamName := fmt.Sprintf("team 1 %s", rString) - firstTeamSlug := fmt.Sprintf("team-1-%s", rString) - secondTeamName := fmt.Sprintf("team 2 %s", rString) - secondTeamSlug := fmt.Sprintf("team-2-%s", rString) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccGithubBranchProtectionDestroy, - Steps: []resource.TestStep{ - { - Config: testAccGithubBranchProtectionConfigTeams(repoName, firstTeamName, secondTeamName), - Check: resource.ComposeTestCheckFunc( - testAccCheckGithubProtectedBranchExists(rn, repoName+":master", &firstP), - testAccCheckGithubBranchProtectionRequiredStatusChecks(&firstP, false, []string{}), - testAccCheckGithubBranchProtectionRestrictions(&firstP, []string{}, []string{firstTeamSlug, secondTeamSlug}), - testAccCheckGithubBranchProtectionPullRequestReviews(&firstP, true, []string{}, []string{firstTeamSlug, secondTeamSlug}, false), - resource.TestCheckResourceAttr(rn, "repository", repoName), - resource.TestCheckResourceAttr(rn, "branch", "master"), - resource.TestCheckResourceAttr(rn, "enforce_admins", "true"), - resource.TestCheckResourceAttr(rn, "required_status_checks.0.strict", "false"), - resource.TestCheckResourceAttr(rn, "required_status_checks.0.contexts.#", "0"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismiss_stale_reviews", "true"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_users.#", "0"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_teams.#", "2"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.require_code_owner_reviews", "false"), - resource.TestCheckResourceAttr(rn, "restrictions.0.users.#", "0"), - resource.TestCheckResourceAttr(rn, "restrictions.0.teams.#", "2"), - ), - }, - { - Config: testAccGithubBranchProtectionConfigEmptyItems(repoName), - Check: resource.ComposeTestCheckFunc( - testAccCheckGithubProtectedBranchExists(rn, repoName+":master", &secondP), - testAccCheckGithubBranchProtectionRequiredStatusChecks(&secondP, false, []string{}), - resource.TestCheckResourceAttr(rn, "repository", repoName), - resource.TestCheckResourceAttr(rn, "branch", "master"), - resource.TestCheckResourceAttr(rn, "enforce_admins", "true"), - resource.TestCheckResourceAttr(rn, "require_signed_commits", "false"), - resource.TestCheckResourceAttr(rn, "required_status_checks.#", "1"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.#", "1"), - resource.TestCheckResourceAttr(rn, "restrictions.#", "1"), - ), - }, - { - Config: testAccGithubBranchProtectionConfigTeams(repoName, firstTeamName, secondTeamName), - Check: resource.ComposeTestCheckFunc( - testAccCheckGithubProtectedBranchExists(rn, repoName+":master", &thirdP), - testAccCheckGithubBranchProtectionRequiredStatusChecks(&thirdP, false, []string{}), - testAccCheckGithubBranchProtectionRestrictions(&thirdP, []string{}, []string{firstTeamSlug, secondTeamSlug}), - testAccCheckGithubBranchProtectionPullRequestReviews(&thirdP, true, []string{}, []string{firstTeamSlug, secondTeamSlug}, false), - resource.TestCheckResourceAttr(rn, "repository", repoName), - resource.TestCheckResourceAttr(rn, "branch", "master"), - resource.TestCheckResourceAttr(rn, "enforce_admins", "true"), - resource.TestCheckResourceAttr(rn, "required_status_checks.0.strict", "false"), - resource.TestCheckResourceAttr(rn, "required_status_checks.0.contexts.#", "0"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismiss_stale_reviews", "true"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_users.#", "0"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_teams.#", "2"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.require_code_owner_reviews", "false"), - resource.TestCheckResourceAttr(rn, "restrictions.0.users.#", "0"), - resource.TestCheckResourceAttr(rn, "restrictions.0.teams.#", "2"), + resource.TestCheckResourceAttr(rn, "push_restrictions.#", "0"), ), }, { @@ -185,7 +71,7 @@ func TestAccGithubBranchProtection_teams(t *testing.T) { // See https://github.com/terraform-providers/terraform-provider-github/issues/8 func TestAccGithubBranchProtection_emptyItems(t *testing.T) { - var protection github.Protection + var protection BranchProtectionRule rn := "github_branch_protection.master" rString := acctest.RandString(5) @@ -199,14 +85,14 @@ func TestAccGithubBranchProtection_emptyItems(t *testing.T) { { Config: testAccGithubBranchProtectionConfigEmptyItems(repoName), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &protection), - resource.TestCheckResourceAttr(rn, "repository", repoName), - resource.TestCheckResourceAttr(rn, "branch", "master"), + testAccCheckGithubProtectedBranchExists("github_branch_protection.master", &protection), + resource.TestCheckResourceAttr(rn, "repository_id", repoName), + resource.TestCheckResourceAttr(rn, "pattern", "master"), resource.TestCheckResourceAttr(rn, "enforce_admins", "true"), resource.TestCheckResourceAttr(rn, "require_signed_commits", "false"), resource.TestCheckResourceAttr(rn, "required_status_checks.#", "1"), resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.#", "1"), - resource.TestCheckResourceAttr(rn, "restrictions.#", "1"), + resource.TestCheckResourceAttr(rn, "push_restrictions.#", "1"), ), }, { @@ -219,7 +105,7 @@ func TestAccGithubBranchProtection_emptyItems(t *testing.T) { } func TestAccGithubBranchProtection_emptyDismissalRestrictions(t *testing.T) { - var protection github.Protection + var protection BranchProtectionRule rn := "github_branch_protection.master" repoName := acctest.RandomWithPrefix("tf-acc-test-branch-prot-") @@ -231,13 +117,12 @@ func TestAccGithubBranchProtection_emptyDismissalRestrictions(t *testing.T) { { Config: testAccGithubBranchProtectionEmptyDismissalsConfig(repoName), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &protection), - testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []string{}, []string{}, true), + testAccCheckGithubProtectedBranchExists("github_branch_protection.master", &protection), + testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []githubv4.String{}, true), resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.#", "1"), resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismiss_stale_reviews", "true"), resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.require_code_owner_reviews", "true"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_users.#", "0"), - resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_teams.#", "0"), + resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_restrictions.0.actor_ids.#", "0"), ), }, { @@ -249,46 +134,48 @@ func TestAccGithubBranchProtection_emptyDismissalRestrictions(t *testing.T) { }) } -func testAccCheckGithubProtectedBranchExists(n, id string, protection *github.Protection) resource.TestCheckFunc { +func testAccCheckGithubProtectedBranchExists(n string, protection *BranchProtectionRule) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("Not Found: %s", n) } - if rs.Primary.ID != id { - return fmt.Errorf("Expected ID to be %v, got %v", id, rs.Primary.ID) + if rs.Primary.ID == "" { + return fmt.Errorf("No Branch Protection ID is set") } - conn := testAccProvider.Meta().(*Organization).v3client - o := testAccProvider.Meta().(*Organization).name - r, b, err := parseTwoPartID(rs.Primary.ID, "repository", "branch") - if err != nil { - return err + var query struct { + Node struct { + Node BranchProtectionRule `graphql:"... on BranchProtectionRule"` + } `graphql:"node(id: $id)"` + } + variables := map[string]interface{}{ + "id": rs.Primary.ID, } - githubProtection, _, err := conn.Repositories.GetBranchProtection(context.TODO(), o, r, b) + client := testAccProvider.Meta().(*Organization).v4client + err := client.Query(context.TODO(), &query, variables) if err != nil { return err } - *protection = *githubProtection + *protection = query.Node.Node return nil } } -func testAccCheckGithubBranchProtectionRequiredStatusChecks(protection *github.Protection, expectedStrict bool, expectedContexts []string) resource.TestCheckFunc { +func testAccCheckGithubBranchProtectionRequiredStatusChecks(protection *BranchProtectionRule, expectedStrict githubv4.Boolean, expectedContexts []githubv4.String) resource.TestCheckFunc { return func(s *terraform.State) error { - rsc := protection.GetRequiredStatusChecks() - if rsc == nil { + if protection.RequiredStatusCheckContexts == nil { return fmt.Errorf("Expected RequiredStatusChecks to be present, but was nil") } - if rsc.Strict != expectedStrict { - return fmt.Errorf("Expected RequiredStatusChecks.Strict to be %v, got %v", expectedStrict, rsc.Strict) + if protection.RequiresStrictStatusChecks != expectedStrict { + return fmt.Errorf("Expected RequiredStatusChecks.Strict to be %v, got %v", expectedStrict, bool(protection.RequiresStrictStatusChecks)) } - if diff := pretty.Compare(rsc.Contexts, expectedContexts); diff != "" { + if diff := pretty.Compare(protection.RequiredStatusCheckContexts, expectedContexts); diff != "" { return fmt.Errorf("diff %q: (-got +want)\n%s", "contexts", diff) } @@ -296,9 +183,9 @@ func testAccCheckGithubBranchProtectionRequiredStatusChecks(protection *github.P } } -func testAccCheckGithubBranchProtectionRestrictions(protection *github.Protection, expectedUserLogins []string, expectedTeamNames []string) resource.TestCheckFunc { +func testAccCheckGithubBranchProtectionRestrictions(protection *BranchProtectionRule, expectedActors []githubv4.String) resource.TestCheckFunc { return func(s *terraform.State) error { - restrictions := protection.GetRestrictions() + restrictions := protection.PushAllowances.Nodes if restrictions == nil { return fmt.Errorf("Expected Restrictions to be present, but was nil") } @@ -325,7 +212,7 @@ func testAccCheckGithubBranchProtectionRestrictions(protection *github.Protectio } } -func testAccCheckGithubBranchProtectionPullRequestReviews(protection *github.Protection, expectedStale bool, expectedUsers, expectedTeams []string, expectedCodeOwners bool) resource.TestCheckFunc { +func testAccCheckGithubBranchProtectionPullRequestReviews(protection *BranchProtectionRule, expectedStale bool, expectedActors []githubv4.String, expectedCodeOwners bool) resource.TestCheckFunc { return func(s *terraform.State) error { reviews := protection.GetRequiredPullRequestReviews() if reviews == nil { @@ -368,7 +255,7 @@ func testAccCheckGithubBranchProtectionPullRequestReviews(protection *github.Pro } } -func testAccCheckGithubBranchProtectionNoRestrictionsExist(protection *github.Protection) resource.TestCheckFunc { +func testAccCheckGithubBranchProtectionNoRestrictionsExist(protection *BranchProtectionRule) resource.TestCheckFunc { return func(s *terraform.State) error { if restrictions := protection.GetRestrictions(); restrictions != nil { return fmt.Errorf("Expected Restrictions to be nil, but was %v", restrictions) @@ -379,7 +266,7 @@ func testAccCheckGithubBranchProtectionNoRestrictionsExist(protection *github.Pr } } -func testAccCheckGithubBranchProtectionNoPullRequestReviewsExist(protection *github.Protection) resource.TestCheckFunc { +func testAccCheckGithubBranchProtectionNoPullRequestReviewsExist(protection *BranchProtectionRule) resource.TestCheckFunc { return func(s *terraform.State) error { if requiredPullRequestReviews := protection.GetRequiredPullRequestReviews(); requiredPullRequestReviews != nil { return fmt.Errorf("Expected Pull Request reviews to be nil, but was %v", requiredPullRequestReviews) @@ -427,9 +314,9 @@ resource "github_repository" "test" { } resource "github_branch_protection" "master" { - repository = "${github_repository.test.name}" - branch = "master" - enforce_admins = true + repository_id = "${github_repository.test.node_id}" + pattern = "master" + enforce_admins = true require_signed_commits = true required_status_checks { @@ -439,12 +326,14 @@ resource "github_branch_protection" "master" { required_pull_request_reviews { dismiss_stale_reviews = true - dismissal_users = ["%s"] require_code_owner_reviews = true + dismissal_restrictions { + actor_ids = ["%s"] + } } - restrictions { - users = ["%s"] + push_restrictions { + actor_ids = ["%s"] } } `, repoName, repoName, testUser, testUser) @@ -459,8 +348,8 @@ resource "github_repository" "test" { } resource "github_branch_protection" "master" { - repository = "${github_repository.test.name}" - branch = "master" + repository_id = "${github_repository.test.node_id}" + pattern = "master" required_status_checks { strict = false @@ -479,8 +368,8 @@ resource "github_repository" "test" { } resource "github_branch_protection" "master" { - repository = "${github_repository.test.name}" - branch = "master" + repository_id = "${github_repository.test.node_id}" + pattern = "master" enforce_admins = true required_status_checks { @@ -489,7 +378,7 @@ resource "github_branch_protection" "master" { required_pull_request_reviews { } - restrictions { + push_restrictions { } } `, repoName, repoName) @@ -509,7 +398,7 @@ resource "github_team" "first" { resource "github_team_repository" "first" { team_id = "${github_team.first.id}" - repository = "${github_repository.test.name}" + repository = "${github_repository.test.node_id}" permission = "push" } @@ -525,8 +414,8 @@ resource "github_team_repository" "second" { resource "github_branch_protection" "master" { depends_on = ["github_team_repository.first", "github_team_repository.second"] - repository = "${github_repository.test.name}" - branch = "master" + repository_id = "${github_repository.test.node_id}" + pattern = "master" enforce_admins = true required_status_checks { @@ -534,11 +423,13 @@ resource "github_branch_protection" "master" { required_pull_request_reviews { dismiss_stale_reviews = true - dismissal_teams = ["${github_team.first.slug}", "${github_team.second.slug}"] + dismissal_restrictions { + actor_ids = ["${github_team.first.slug}", "${github_team.second.slug}"] + } } - restrictions { - teams = ["${github_team.first.slug}", "${github_team.second.slug}"] + push_restrictions { + actor_ids = ["${github_team.first.slug}", "${github_team.second.slug}"] } } `, repoName, repoName, firstTeamName, secondTeamName) @@ -553,12 +444,12 @@ resource "github_repository" "test" { } resource "github_branch_protection" "master" { - repository = "${github_repository.test.name}" - branch = "master" + repository_id = "${github_repository.test.name}" + pattern = "master" enforce_admins = true - restrictions { - users = ["%s"] + push_restrictions { + actor_ids = ["%s"] } } `, repoName, repoName, user) @@ -574,12 +465,12 @@ resource "github_repository" "test" { } resource "github_branch_protection" "master" { - repository = "${github_repository.test.name}" - branch = "master" + repository_id = "${github_repository.test.name}" + pattern = "master" enforce_admins = true required_pull_request_reviews { - dismiss_stale_reviews = true + dismiss_stale_reviews = true require_code_owner_reviews = true } }