Skip to content

Commit

Permalink
Bug Fix: GitHub Usernames are Case Insensitive (integrations#241)
Browse files Browse the repository at this point in the history
  • Loading branch information
@megan07
megan07 authored Jun 26, 2019
1 parent a129c94 commit 581d4d0
Show file tree
Hide file tree
Showing 8 changed files with 249 additions and 20 deletions.
7 changes: 4 additions & 3 deletions github/resource_github_membership.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ func resourceGithubMembership() *schema.Resource {

Schema: map[string]*schema.Schema{
"username": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: caseInsensitive(),
},
"role": {
Type: schema.TypeString,
Expand Down
61 changes: 57 additions & 4 deletions github/resource_github_membership_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package github

import (
"context"
"errors"
"fmt"
"testing"

Expand All @@ -11,6 +12,10 @@ import (
)

func TestAccGithubMembership_basic(t *testing.T) {
if testCollaborator == "" {
t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set")
}

var membership github.Membership

resource.Test(t, resource.TestCase{
Expand All @@ -19,7 +24,7 @@ func TestAccGithubMembership_basic(t *testing.T) {
CheckDestroy: testAccCheckGithubMembershipDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubMembershipConfig,
Config: testAccGithubMembershipConfig(testCollaborator),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubMembershipExists("github_membership.test_org_membership", &membership),
testAccCheckGithubMembershipRoleState("github_membership.test_org_membership", &membership),
Expand All @@ -29,14 +34,50 @@ func TestAccGithubMembership_basic(t *testing.T) {
})
}

func TestAccGithubMembership_caseInsensitive(t *testing.T) {
if testCollaborator == "" {
t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set")
}

var membership github.Membership
var otherMembership github.Membership

otherCase := flipUsernameCase(testCollaborator)

if testCollaborator == otherCase {
t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` has no letters to flip case")
}

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckGithubMembershipDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubMembershipConfig(testCollaborator),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubMembershipExists("github_membership.test_org_membership", &membership),
),
},
{
Config: testAccGithubMembershipConfig(otherCase),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubMembershipExists("github_membership.test_org_membership", &otherMembership),
testAccGithubMembershipTheSame(&membership, &otherMembership),
),
},
},
})
}

func TestAccGithubMembership_importBasic(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckGithubMembershipDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubMembershipConfig,
Config: testAccGithubMembershipConfig(testCollaborator),
},
{
ResourceName: "github_membership.test_org_membership",
Expand Down Expand Up @@ -134,9 +175,21 @@ func testAccCheckGithubMembershipRoleState(n string, membership *github.Membersh
}
}

var testAccGithubMembershipConfig string = fmt.Sprintf(`
func testAccGithubMembershipConfig(username string) string {
return fmt.Sprintf(`
resource "github_membership" "test_org_membership" {
username = "%s"
role = "member"
}
`, testCollaborator)
`, username)
}

func testAccGithubMembershipTheSame(orig, other *github.Membership) resource.TestCheckFunc {
return func(s *terraform.State) error {
if *orig.URL != *other.URL {
return errors.New("users are different")
}

return nil
}
}
16 changes: 10 additions & 6 deletions github/resource_github_repository_collaborator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log"
"strings"

"github.com/google/go-github/v25/github"
"github.com/hashicorp/terraform/helper/schema"
Expand All @@ -21,9 +22,10 @@ func resourceGithubRepositoryCollaborator() *schema.Resource {
// editing repository collaborators are not supported by github api so forcing new on any changes
Schema: map[string]*schema.Schema{
"username": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: caseInsensitive(),
},
"repository": {
Type: schema.TypeString,
Expand Down Expand Up @@ -87,6 +89,7 @@ func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta inter
if err != nil {
return err
} else if invitation != nil {
username = *invitation.Invitee.Login
permissionName, err := getInvitationPermission(invitation)
if err != nil {
return err
Expand All @@ -112,14 +115,14 @@ func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta inter
}

for _, c := range collaborators {
if *c.Login == username {
if strings.ToLower(*c.Login) == strings.ToLower(username) {
permissionName, err := getRepoPermission(c.Permissions)
if err != nil {
return err
}

d.Set("repository", repoName)
d.Set("username", username)
d.Set("username", c.Login)
d.Set("permission", permissionName)
return nil
}
Expand Down Expand Up @@ -153,6 +156,7 @@ func resourceGithubRepositoryCollaboratorDelete(d *schema.ResourceData, meta int
if err != nil {
return err
} else if invitation != nil {
username = *invitation.Invitee.Login
_, err = client.Repositories.DeleteInvitation(ctx, orgName, repoName, *invitation.ID)
return err
}
Expand All @@ -172,7 +176,7 @@ func findRepoInvitation(client *github.Client, ctx context.Context, owner, repo,
}

for _, i := range invitations {
if *i.Invitee.Login == collaborator {
if strings.ToLower(*i.Invitee.Login) == strings.ToLower(collaborator) {
return i, nil
}
}
Expand Down
99 changes: 95 additions & 4 deletions github/resource_github_repository_collaborator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ package github

import (
"context"
"errors"
"fmt"
"regexp"
"strings"
"testing"

"github.com/google/go-github/v25/github"
"github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
Expand All @@ -14,6 +17,10 @@ import (
const expectedPermission string = "admin"

func TestAccGithubRepositoryCollaborator_basic(t *testing.T) {
if testCollaborator == "" {
t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set")
}

resourceName := "github_repository_collaborator.test_repo_collaborator"
repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5))

Expand All @@ -23,7 +30,7 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) {
CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubRepositoryCollaboratorConfig(repoName),
Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubRepositoryCollaboratorExists(resourceName),
testAccCheckGithubRepositoryCollaboratorPermission(resourceName),
Expand All @@ -35,6 +42,46 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) {
})
}

func TestAccGithubRepositoryCollaborator_caseInsensitive(t *testing.T) {
if testCollaborator == "" {
t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set")
}

resourceName := "github_repository_collaborator.test_repo_collaborator"
repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5))

var origInvitation github.RepositoryInvitation
var otherInvitation github.RepositoryInvitation

otherCase := flipUsernameCase(testCollaborator)

if testCollaborator == otherCase {
t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` has no letters to flip case")
}

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubRepositoryCollaboratorInvited(repoName, testCollaborator, &origInvitation),
),
},
{
Config: testAccGithubRepositoryCollaboratorConfig(repoName, otherCase),
Check: resource.ComposeTestCheckFunc(
testAccCheckGithubRepositoryCollaboratorInvited(repoName, otherCase, &otherInvitation),
resource.TestCheckResourceAttr(resourceName, "username", testCollaborator),
testAccGithubRepositoryCollaboratorTheSame(&origInvitation, &otherInvitation),
),
},
},
})
}

func TestAccGithubRepositoryCollaborator_importBasic(t *testing.T) {
repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5))

Expand All @@ -44,7 +91,7 @@ func TestAccGithubRepositoryCollaborator_importBasic(t *testing.T) {
CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy,
Steps: []resource.TestStep{
{
Config: testAccGithubRepositoryCollaboratorConfig(repoName),
Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator),
},
{
ResourceName: "github_repository_collaborator.test_repo_collaborator",
Expand Down Expand Up @@ -169,7 +216,7 @@ func testAccCheckGithubRepositoryCollaboratorPermission(n string) resource.TestC
}
}

func testAccGithubRepositoryCollaboratorConfig(repoName string) string {
func testAccGithubRepositoryCollaboratorConfig(repoName, username string) string {
return fmt.Sprintf(`
resource "github_repository" "test" {
name = "%s"
Expand All @@ -180,5 +227,49 @@ resource "github_repository_collaborator" "test_repo_collaborator" {
username = "%s"
permission = "%s"
}
`, repoName, testCollaborator, expectedPermission)
`, repoName, username, expectedPermission)
}

func testAccCheckGithubRepositoryCollaboratorInvited(repoName, username string, invitation *github.RepositoryInvitation) resource.TestCheckFunc {
return func(s *terraform.State) error {
opt := &github.ListOptions{PerPage: maxPerPage}

client := testAccProvider.Meta().(*Organization).client
org := testAccProvider.Meta().(*Organization).name

for {
invitations, resp, err := client.Repositories.ListInvitations(context.TODO(), org, repoName, opt)
if err != nil {
return errors.New(err.Error())
}

if len(invitations) > 1 {
return errors.New(fmt.Sprintf("multiple invitations have been sent for repository %s", repoName))
}

for _, i := range invitations {
if strings.ToLower(*i.Invitee.Login) == strings.ToLower(username) {
invitation = i
return nil
}
}

if resp.NextPage == 0 {
break
}
opt.Page = resp.NextPage
}

return errors.New(fmt.Sprintf("no invitation found for %s", username))
}
}

func testAccGithubRepositoryCollaboratorTheSame(orig, other *github.RepositoryInvitation) resource.TestCheckFunc {
return func(s *terraform.State) error {
if orig.ID != other.ID {
return errors.New("collaborators are different")
}

return nil
}
}
7 changes: 4 additions & 3 deletions github/resource_github_team_membership.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ func resourceGithubTeamMembership() *schema.Resource {
ForceNew: true,
},
"username": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: caseInsensitive(),
},
"role": {
Type: schema.TypeString,
Expand Down
Loading

0 comments on commit 581d4d0

Please sign in to comment.