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.

Sign up for GitHub

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

resource/github_team: Expose slug (and document when to use it) #136

Merged
merged 3 commits into from
Aug 16, 2018
Merged

resource/github_team: Expose slug (and document when to use it) #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
05e063b
github_team: Expose slug
radeksimko Aug 15, 2018
2517e19
github_branch_protection: Add test w/ teams and user
radeksimko Aug 15, 2018
07158c9
docs/github_branch_protection: Document team slugs must be used
radeksimko Aug 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions github/resource_github_branch_protection.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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