Skip to content

Commit

Permalink
Merge pull request #136 from terraform-providers/f-team-expose-slug
Browse files Browse the repository at this point in the history 
resource/github_team: Expose slug (and document when to use it)
  • Loading branch information
@radeksimko
radeksimko authored Aug 16, 2018
2 parents 8014e9f + 07158c9 commit f417c82
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 21 deletions.
1 change: 1 addition & 0 deletions github/resource_github_branch_protection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ func resourceGithubBranchProtectionCreate(d *schema.ResourceData, meta interface
if err != nil {
return err
}

d.SetId(buildTwoPartID(&repoName, &branch))

return resourceGithubBranchProtectionRead(d, meta)
Expand Down
144 changes: 135 additions & 9 deletions github/resource_github_branch_protection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package github
import (
"context"
"fmt"
"sort"
"testing"

"github.com/google/go-github/github"
Expand Down Expand Up @@ -62,6 +63,78 @@ func TestAccGithubBranchProtection_basic(t *testing.T) {
})
}

func TestAccGithubBranchProtection_teams(t *testing.T) {
var firstP, secondP, thirdP github.Protection

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.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccGithubBranchProtectionDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubBranchProtectionConfigTeams(repoName, firstTeamName, secondTeamName),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &firstP),
testAccCheckGithubBranchProtectionRequiredStatusChecks(&firstP, false, []string{}),
testAccCheckGithubBranchProtectionRestrictions(&firstP, []string{}, []string{firstTeamSlug, secondTeamSlug}),
testAccCheckGithubBranchProtectionPullRequestReviews(&firstP, true, []string{}, []string{firstTeamSlug, secondTeamSlug}, false),
resource.TestCheckResourceAttr("github_branch_protection.master", "repository", repoName),
resource.TestCheckResourceAttr("github_branch_protection.master", "branch", "master"),
resource.TestCheckResourceAttr("github_branch_protection.master", "enforce_admins", "true"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.strict", "false"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.contexts.#", "0"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismiss_stale_reviews", "true"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_users.#", "0"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_teams.#", "2"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.require_code_owner_reviews", "false"),
resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.users.#", "0"),
resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.teams.#", "2"),
),
},
{
Config: testAccGithubBranchProtectionConfigEmptyItems(repoName),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &secondP),
testAccCheckGithubBranchProtectionRequiredStatusChecks(&secondP, false, []string{}),
resource.TestCheckResourceAttr("github_branch_protection.master", "repository", repoName),
resource.TestCheckResourceAttr("github_branch_protection.master", "branch", "master"),
resource.TestCheckResourceAttr("github_branch_protection.master", "enforce_admins", "true"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.#", "1"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.#", "1"),
resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.#", "1"),
),
},
{
Config: testAccGithubBranchProtectionConfigTeams(repoName, firstTeamName, secondTeamName),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &thirdP),
testAccCheckGithubBranchProtectionRequiredStatusChecks(&thirdP, false, []string{}),
testAccCheckGithubBranchProtectionRestrictions(&thirdP, []string{}, []string{firstTeamSlug, secondTeamSlug}),
testAccCheckGithubBranchProtectionPullRequestReviews(&thirdP, true, []string{}, []string{firstTeamSlug, secondTeamSlug}, false),
resource.TestCheckResourceAttr("github_branch_protection.master", "repository", repoName),
resource.TestCheckResourceAttr("github_branch_protection.master", "branch", "master"),
resource.TestCheckResourceAttr("github_branch_protection.master", "enforce_admins", "true"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.strict", "false"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.contexts.#", "0"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismiss_stale_reviews", "true"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_users.#", "0"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_teams.#", "2"),
resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.require_code_owner_reviews", "false"),
resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.users.#", "0"),
resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.teams.#", "2"),
),
},
},
})
}

// See https://github.com/terraform-providers/terraform-provider-github/issues/8
func TestAccGithubBranchProtection_emptyItems(t *testing.T) {
var protection github.Protection
Expand Down Expand Up @@ -169,15 +242,17 @@ func testAccCheckGithubBranchProtectionRestrictions(protection *github.Protectio
userLogins = append(userLogins, *u.Login)
}
if diff := pretty.Compare(userLogins, expectedUserLogins); diff != "" {
return fmt.Errorf("diff %q: (-got +want)\n%s", "restricted users", diff)
return fmt.Errorf("diff %q: (-got +want)\n%s", "restrictions.users", diff)
}

teamLogins := []string{}
for _, t := range restrictions.Teams {
teamLogins = append(teamLogins, *t.Name)
teamLogins = append(teamLogins, *t.Slug)
}
sort.Strings(teamLogins)
sort.Strings(expectedTeamNames)
if diff := pretty.Compare(teamLogins, expectedTeamNames); diff != "" {
return fmt.Errorf("diff %q: (-got +want)\n%s", "restricted teams", diff)
return fmt.Errorf("diff %q: (-got +want)\n%s", "restrictions.teams", diff)
}

return nil
Expand All @@ -200,15 +275,17 @@ func testAccCheckGithubBranchProtectionPullRequestReviews(protection *github.Pro
users = append(users, *u.Login)
}
if diff := pretty.Compare(users, expectedUsers); diff != "" {
return fmt.Errorf("diff %q: (-got +want)\n%s", "dismissal_users", diff)
return fmt.Errorf("diff %q: (-got +want)\n%s", "required_pull_request_reviews.dismissal_users", diff)
}

teams := []string{}
for _, t := range reviews.DismissalRestrictions.Teams {
teams = append(users, *t.Slug)
teams = append(teams, *t.Slug)
}
sort.Strings(teams)
sort.Strings(expectedTeams)
if diff := pretty.Compare(teams, expectedTeams); diff != "" {
return fmt.Errorf("diff %q: (-got +want)\n%s", "dismissal_teams", diff)
return fmt.Errorf("diff %q: (-got +want)\n%s", "required_pull_request_reviews.dismissal_teams", diff)
}

if reviews.RequireCodeOwnerReviews != expectedCodeOwners {
Expand Down Expand Up @@ -282,7 +359,7 @@ resource "github_branch_protection" "master" {
branch = "master"
enforce_admins = true
required_status_checks = {
required_status_checks {
strict = true
contexts = ["github/foo"]
}
Expand Down Expand Up @@ -312,7 +389,7 @@ resource "github_branch_protection" "master" {
repository = "${github_repository.test.name}"
branch = "master"
required_status_checks = {
required_status_checks {
strict = false
contexts = ["github/bar"]
}
Expand All @@ -333,7 +410,7 @@ resource "github_branch_protection" "master" {
branch = "master"
enforce_admins = true
required_status_checks = {
required_status_checks {
}
required_pull_request_reviews {
Expand All @@ -344,3 +421,52 @@ resource "github_branch_protection" "master" {
}
`, repoName, repoName)
}

func testAccGithubBranchProtectionConfigTeams(repoName, firstTeamName, secondTeamName string) string {
return fmt.Sprintf(`
resource "github_repository" "test" {
name = "%s"
description = "Terraform Acceptance Test %s"
auto_init = true
}
resource "github_team" "first" {
name = "%s"
}
resource "github_team_repository" "first" {
team_id = "${github_team.first.id}"
repository = "${github_repository.test.name}"
permission = "pull"
}
resource "github_team" "second" {
name = "%s"
}
resource "github_team_repository" "second" {
team_id = "${github_team.second.id}"
repository = "${github_repository.test.name}"
permission = "pull"
}
resource "github_branch_protection" "master" {
depends_on = ["github_team_repository.first", "github_team_repository.second"]
repository = "${github_repository.test.name}"
branch = "master"
enforce_admins = true
required_status_checks {
}
required_pull_request_reviews {
dismiss_stale_reviews = true
dismissal_teams = ["${github_team.first.slug}", "${github_team.second.slug}"]
}
restrictions {
teams = ["${github_team.first.slug}", "${github_team.second.slug}"]
}
}
`, repoName, repoName, firstTeamName, secondTeamName)
}
5 changes: 5 additions & 0 deletions github/resource_github_team.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ func resourceGithubTeam() *schema.Resource {
Type: schema.TypeString,
Optional: true,
},
"slug": {
Type: schema.TypeString,
Computed: true,
},
},
}
}
Expand Down Expand Up @@ -98,6 +102,7 @@ func resourceGithubTeamRead(d *schema.ResourceData, meta interface{}) error {
d.Set("parent_team_id", "")
}
d.Set("ldap_dn", team.GetLDAPDN())
d.Set("slug", team.GetSlug())
return nil
}

Expand Down
58 changes: 51 additions & 7 deletions github/resource_github_team_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,67 @@ func TestAccGithubTeam_basic(t *testing.T) {
randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
name := fmt.Sprintf("tf-acc-test-%s", randString)
updatedName := fmt.Sprintf("tf-acc-test-updated-%s", randString)
description := "Terraform acc test group"
updatedDescription := "Terraform acc test group - updated"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckGithubTeamDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubTeamConfig(randString),
Config: testAccGithubTeamConfig(name),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubTeamExists("github_team.foo", &team),
testAccCheckGithubTeamAttributes(&team, name, "Terraform acc test group", nil),
testAccCheckGithubTeamAttributes(&team, name, description, nil),
resource.TestCheckResourceAttr("github_team.foo", "name", name),
resource.TestCheckResourceAttr("github_team.foo", "description", description),
resource.TestCheckResourceAttr("github_team.foo", "privacy", "secret"),
resource.TestCheckNoResourceAttr("github_team.foo", "parent_team_id"),
resource.TestCheckResourceAttr("github_team.foo", "ldap_dn", ""),
resource.TestCheckResourceAttr("github_team.foo", "slug", name),
),
},
{
Config: testAccGithubTeamUpdateConfig(randString),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubTeamExists("github_team.foo", &team),
testAccCheckGithubTeamAttributes(&team, updatedName, "Terraform acc test group - updated", nil),
testAccCheckGithubTeamAttributes(&team, updatedName, updatedDescription, nil),
resource.TestCheckResourceAttr("github_team.foo", "name", updatedName),
resource.TestCheckResourceAttr("github_team.foo", "description", updatedDescription),
resource.TestCheckResourceAttr("github_team.foo", "privacy", "closed"),
resource.TestCheckNoResourceAttr("github_team.foo", "parent_team_id"),
resource.TestCheckResourceAttr("github_team.foo", "ldap_dn", ""),
resource.TestCheckResourceAttr("github_team.foo", "slug", updatedName),
),
},
},
})
}

func TestAccGithubTeam_slug(t *testing.T) {
var team github.Team
randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
name := fmt.Sprintf("TF Acc Test %s", randString)
description := "Terraform acc test group"
expectedSlug := fmt.Sprintf("tf-acc-test-%s", randString)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckGithubTeamDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubTeamConfig(name),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubTeamExists("github_team.foo", &team),
testAccCheckGithubTeamAttributes(&team, name, description, nil),
resource.TestCheckResourceAttr("github_team.foo", "name", name),
resource.TestCheckResourceAttr("github_team.foo", "description", description),
resource.TestCheckResourceAttr("github_team.foo", "privacy", "secret"),
resource.TestCheckNoResourceAttr("github_team.foo", "parent_team_id"),
resource.TestCheckResourceAttr("github_team.foo", "ldap_dn", ""),
resource.TestCheckResourceAttr("github_team.foo", "slug", expectedSlug),
),
},
},
Expand Down Expand Up @@ -67,14 +110,15 @@ func TestAccGithubTeam_hierarchical(t *testing.T) {

func TestAccGithubTeam_importBasic(t *testing.T) {
randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
name := fmt.Sprintf("tf-acc-test-%s", randString)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckGithubTeamDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubTeamConfig(randString),
Config: testAccGithubTeamConfig(name),
},
{
ResourceName: "github_team.foo",
Expand Down Expand Up @@ -161,14 +205,14 @@ func testAccCheckGithubTeamDestroy(s *terraform.State) error {
return nil
}

func testAccGithubTeamConfig(randString string) string {
func testAccGithubTeamConfig(teamName string) string {
return fmt.Sprintf(`
resource "github_team" "foo" {
name = "tf-acc-test-%s"
name = "%s"
description = "Terraform acc test group"
privacy = "secret"
}
`, randString)
`, teamName)
}

func testAccGithubTeamUpdateConfig(randString string) string {
Expand Down
22 changes: 17 additions & 5 deletions website/docs/r/branch_protection.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ This resource allows you to configure branch protection for repositories in your
# Protect the master branch of the foo repository. Additionally, require that
# the "ci/travis" context to be passing and only allow the engineers team merge
# to the branch.
resource "github_branch_protection" "foo_master" {
repository = "foo"
resource "github_branch_protection" "example" {
repository = "${github_repository.example.name}"
branch = "master"
enforce_admins = true
Expand All @@ -31,14 +31,24 @@ resource "github_branch_protection" "foo_master" {
required_pull_request_reviews {
dismiss_stale_reviews = true
dismissal_users = ["foo-user"]
dismissal_teams = ["admins", "engineers"]
dismissal_teams = ["${github_team.example.slug}", "${github_team.second.slug}"]
}
restrictions {
users = ["foo-user"]
teams = ["engineers"]
teams = ["${github_team.example.slug}"]
}
}
resource "github_team" "example" {
name = "Example Name"
}
resource "github_team_repository" "example" {
team_id = "${github_team.example.id}"
repository = "${github_repository.example.name}"
permission = "pull"
}
```

## Argument Reference
Expand All @@ -65,7 +75,8 @@ The following arguments are supported:

* `dismiss_stale_reviews`: (Optional) Dismiss approved reviews automatically when a new commit is pushed. Defaults to `false`.
* `dismissal_users`: (Optional) The list of user logins with dismissal access
* `dismissal_teams`: (Optional) The list of team slugs with dismissal access
* `dismissal_teams`: (Optional) The list of team slugs with dismissal access.
Always use `slug` of the team, **not** its name. Each team already **has** to have access to the repository.
* `require_code_owner_reviews`: (Optional) Require an approved review in pull requests including files with a designated code owner. Defaults to `false`.

### Restrictions
Expand All @@ -74,6 +85,7 @@ The following arguments are supported:

* `users`: (Optional) The list of user logins with push access.
* `teams`: (Optional) The list of team slugs with push access.
Always use `slug` of the team, **not** its name. Each team already **has** to have access to the repository.

`restrictions` is only available for organization-owned repositories.

Expand Down
Loading

0 comments on commit f417c82

Please sign in to comment.