Merge pull request #108 from terraform-providers/b-prevent-id-crash

resource/github_*: Prevent crashing on invalid ID format
integrations · Aug 3, 2018 · 06b9134 · 06b9134
2 parents a4ace96 + 7bcea9e
commit 06b9134
Show file tree

Hide file tree

Showing 16 changed files with 129 additions and 97 deletions.
diff --git a/github/resource_github_branch_protection.go b/github/resource_github_branch_protection.go
@@ -147,7 +147,10 @@ func resourceGithubBranchProtectionCreate(d *schema.ResourceData, meta interface
 
 func resourceGithubBranchProtectionRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Organization).client
-	r, b := parseTwoPartID(d.Id())
+	r, b, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	githubProtection, _, err := client.Repositories.GetBranchProtection(context.TODO(), meta.(*Organization).name, r, b)
 	if err != nil {
@@ -180,7 +183,10 @@ func resourceGithubBranchProtectionRead(d *schema.ResourceData, meta interface{}
 
 func resourceGithubBranchProtectionUpdate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Organization).client
-	r, b := parseTwoPartID(d.Id())
+	r, b, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	protectionRequest, err := buildProtectionRequest(d)
 	if err != nil {
@@ -206,9 +212,12 @@ func resourceGithubBranchProtectionUpdate(d *schema.ResourceData, meta interface
 
 func resourceGithubBranchProtectionDelete(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Organization).client
-	r, b := parseTwoPartID(d.Id())
+	r, b, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
-	_, err := client.Repositories.RemoveBranchProtection(context.TODO(), meta.(*Organization).name, r, b)
+	_, err = client.Repositories.RemoveBranchProtection(context.TODO(), meta.(*Organization).name, r, b)
 	return err
 }
 

diff --git a/github/resource_github_branch_protection_test.go b/github/resource_github_branch_protection_test.go
@@ -123,7 +123,10 @@ func testAccCheckGithubProtectedBranchExists(n, id string, protection *github.Pr
 
 		conn := testAccProvider.Meta().(*Organization).client
 		o := testAccProvider.Meta().(*Organization).name
-		r, b := parseTwoPartID(rs.Primary.ID)
+		r, b, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		githubProtection, _, err := conn.Repositories.GetBranchProtection(context.TODO(), o, r, b)
 		if err != nil {
@@ -246,7 +249,11 @@ func testAccGithubBranchProtectionDestroy(s *terraform.State) error {
 		}
 
 		o := testAccProvider.Meta().(*Organization).name
-		r, b := parseTwoPartID(rs.Primary.ID)
+		r, b, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
+
 		protection, res, err := conn.Repositories.GetBranchProtection(context.TODO(), o, r, b)
 
 		if err == nil {

diff --git a/github/resource_github_issue_label.go b/github/resource_github_issue_label.go
@@ -74,7 +74,11 @@ func resourceGithubIssueLabelCreateOrUpdate(d *schema.ResourceData, meta interfa
 		if d.Id() == "" {
 			oname = n
 		} else {
-			_, oname = parseTwoPartID(d.Id())
+			var err error
+			_, oname, err = parseTwoPartID(d.Id())
+			if err != nil {
+				return err
+			}
 		}
 
 		_, _, err := client.Issues.EditLabel(context.TODO(), o, r, oname, label)
@@ -99,7 +103,10 @@ func resourceGithubIssueLabelCreateOrUpdate(d *schema.ResourceData, meta interfa
 
 func resourceGithubIssueLabelRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Organization).client
-	r, n := parseTwoPartID(d.Id())
+	r, n, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	log.Printf("[DEBUG] Reading label: %s/%s", r, n)
 	githubLabel, _, err := client.Issues.GetLabel(context.TODO(), meta.(*Organization).name, r, n)

diff --git a/github/resource_github_issue_label_test.go b/github/resource_github_issue_label_test.go
@@ -96,7 +96,10 @@ func testAccCheckGithubIssueLabelExists(n string, label *github.Label) resource.
 
 		conn := testAccProvider.Meta().(*Organization).client
 		o := testAccProvider.Meta().(*Organization).name
-		r, n := parseTwoPartID(rs.Primary.ID)
+		r, n, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		githubLabel, _, err := conn.Issues.GetLabel(context.TODO(), o, r, n)
 		if err != nil {
@@ -131,7 +134,11 @@ func testAccGithubIssueLabelDestroy(s *terraform.State) error {
 		}
 
 		o := testAccProvider.Meta().(*Organization).name
-		r, n := parseTwoPartID(rs.Primary.ID)
+		r, n, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
+
 		label, res, err := conn.Issues.GetLabel(context.TODO(), o, r, n)
 
 		if err == nil {

diff --git a/github/resource_github_membership.go b/github/resource_github_membership.go
@@ -15,7 +15,7 @@ func resourceGithubMembership() *schema.Resource {
 		Update: resourceGithubMembershipUpdate,
 		Delete: resourceGithubMembershipDelete,
 		Importer: &schema.ResourceImporter{
-			State: resourceGithubMembershipImport,
+			State: schema.ImportStatePassthrough,
 		},
 
 		Schema: map[string]*schema.Schema{
@@ -52,7 +52,10 @@ func resourceGithubMembershipCreate(d *schema.ResourceData, meta interface{}) er
 
 func resourceGithubMembershipRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Organization).client
-	_, n := parseTwoPartID(d.Id())
+	_, n, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	membership, _, err := client.Organizations.GetOrgMembership(context.TODO(), n, meta.(*Organization).name)
 	if err != nil {
@@ -89,14 +92,3 @@ func resourceGithubMembershipDelete(d *schema.ResourceData, meta interface{}) er
 
 	return err
 }
-
-func resourceGithubMembershipImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
-	// All we do here is validate that the import string is in a correct enough
-	// format to be parsed.  parseTwoPartID will panic if it's missing elements,
-	// and is used otherwise in places where that should never happen, so we want
-	// to keep it that way.
-	if err := validateTwoPartID(d.Id()); err != nil {
-		return nil, err
-	}
-	return []*schema.ResourceData{d}, nil
-}
diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go
@@ -54,7 +54,10 @@ func testAccCheckGithubMembershipDestroy(s *terraform.State) error {
 		if rs.Type != "github_membership" {
 			continue
 		}
-		o, u := parseTwoPartID(rs.Primary.ID)
+		o, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		membership, resp, err := conn.Organizations.GetOrgMembership(context.TODO(), u, o)
 
@@ -84,7 +87,10 @@ func testAccCheckGithubMembershipExists(n string, membership *github.Membership)
 		}
 
 		conn := testAccProvider.Meta().(*Organization).client
-		o, u := parseTwoPartID(rs.Primary.ID)
+		o, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		githubMembership, _, err := conn.Organizations.GetOrgMembership(context.TODO(), u, o)
 		if err != nil {
@@ -107,7 +113,10 @@ func testAccCheckGithubMembershipRoleState(n string, membership *github.Membersh
 		}
 
 		conn := testAccProvider.Meta().(*Organization).client
-		o, u := parseTwoPartID(rs.Primary.ID)
+		o, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		githubMembership, _, err := conn.Organizations.GetOrgMembership(context.TODO(), u, o)
 		if err != nil {

diff --git a/github/resource_github_repository_collaborator.go b/github/resource_github_repository_collaborator.go
@@ -59,7 +59,10 @@ func resourceGithubRepositoryCollaboratorCreate(d *schema.ResourceData, meta int
 
 func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Organization).client
-	r, u := parseTwoPartID(d.Id())
+	r, u, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	// First, check if the user has been invited but has not yet accepted
 	invitation, err := findRepoInvitation(client, meta.(*Organization).name, r, u)

diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go
@@ -60,7 +60,11 @@ func testAccCheckGithubRepositoryCollaboratorDestroy(s *terraform.State) error {
 		}
 
 		o := testAccProvider.Meta().(*Organization).name
-		r, u := parseTwoPartID(rs.Primary.ID)
+		r, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
+
 		isCollaborator, _, err := conn.Repositories.IsCollaborator(context.TODO(), o, r, u)
 
 		if err != nil {
@@ -90,7 +94,10 @@ func testAccCheckGithubRepositoryCollaboratorExists(n string) resource.TestCheck
 
 		conn := testAccProvider.Meta().(*Organization).client
 		o := testAccProvider.Meta().(*Organization).name
-		r, u := parseTwoPartID(rs.Primary.ID)
+		r, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		invitations, _, err := conn.Repositories.ListInvitations(context.TODO(), o, r, nil)
 		if err != nil {
@@ -126,7 +133,10 @@ func testAccCheckGithubRepositoryCollaboratorPermission(n string) resource.TestC
 
 		conn := testAccProvider.Meta().(*Organization).client
 		o := testAccProvider.Meta().(*Organization).name
-		r, u := parseTwoPartID(rs.Primary.ID)
+		r, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		invitations, _, err := conn.Repositories.ListInvitations(context.TODO(), o, r, nil)
 		if err != nil {

diff --git a/github/resource_github_repository_deploy_key.go b/github/resource_github_repository_deploy_key.go
@@ -77,7 +77,10 @@ func resourceGithubRepositoryDeployKeyRead(d *schema.ResourceData, meta interfac
 	client := meta.(*Organization).client
 
 	owner := meta.(*Organization).name
-	repo, id := parseTwoPartID(d.Id())
+	repo, id, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	i, err := strconv.ParseInt(id, 10, 64)
 	if err != nil {
@@ -101,7 +104,10 @@ func resourceGithubRepositoryDeployKeyDelete(d *schema.ResourceData, meta interf
 	client := meta.(*Organization).client
 
 	owner := meta.(*Organization).name
-	repo, id := parseTwoPartID(d.Id())
+	repo, id, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	i, err := strconv.ParseInt(id, 10, 64)
 	if err != nil {

diff --git a/github/resource_github_repository_deploy_key_test.go b/github/resource_github_repository_deploy_key_test.go
@@ -62,7 +62,11 @@ func testAccCheckGithubRepositoryDeployKeyDestroy(s *terraform.State) error {
 		}
 
 		o := testAccProvider.Meta().(*Organization).name
-		r, i := parseTwoPartID(rs.Primary.ID)
+		r, i, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
+
 		id, err := strconv.ParseInt(i, 10, 64)
 		if err != nil {
 			return err
@@ -92,7 +96,11 @@ func testAccCheckGithubRepositoryDeployKeyExists(n string) resource.TestCheckFun
 
 		conn := testAccProvider.Meta().(*Organization).client
 		o := testAccProvider.Meta().(*Organization).name
-		r, i := parseTwoPartID(rs.Primary.ID)
+		r, i, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
+
 		id, err := strconv.ParseInt(i, 10, 64)
 		if err != nil {
 			return err

diff --git a/github/resource_github_team_membership.go b/github/resource_github_team_membership.go
@@ -61,7 +61,10 @@ func resourceGithubTeamMembershipCreate(d *schema.ResourceData, meta interface{}
 
 func resourceGithubTeamMembershipRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Organization).client
-	t, n := parseTwoPartID(d.Id())
+	t, n, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	membership, _, err := client.Organizations.GetTeamMembership(context.TODO(), toGithubID(t), n)
 

diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go
@@ -84,7 +84,11 @@ func testAccCheckGithubTeamMembershipDestroy(s *terraform.State) error {
 			continue
 		}
 
-		t, u := parseTwoPartID(rs.Primary.ID)
+		t, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
+
 		membership, resp, err := conn.Organizations.GetTeamMembership(context.TODO(), toGithubID(t), u)
 		if err == nil {
 			if membership != nil {
@@ -111,7 +115,10 @@ func testAccCheckGithubTeamMembershipExists(n string, membership *github.Members
 		}
 
 		conn := testAccProvider.Meta().(*Organization).client
-		t, u := parseTwoPartID(rs.Primary.ID)
+		t, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		teamMembership, _, err := conn.Organizations.GetTeamMembership(context.TODO(), toGithubID(t), u)
 
@@ -135,7 +142,10 @@ func testAccCheckGithubTeamMembershipRoleState(n, expected string, membership *g
 		}
 
 		conn := testAccProvider.Meta().(*Organization).client
-		t, u := parseTwoPartID(rs.Primary.ID)
+		t, u, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		teamMembership, _, err := conn.Organizations.GetTeamMembership(context.TODO(), toGithubID(t), u)
 		if err != nil {

diff --git a/github/resource_github_team_repository.go b/github/resource_github_team_repository.go
@@ -58,7 +58,10 @@ func resourceGithubTeamRepositoryCreate(d *schema.ResourceData, meta interface{}
 
 func resourceGithubTeamRepositoryRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Organization).client
-	t, r := parseTwoPartID(d.Id())
+	t, r, err := parseTwoPartID(d.Id())
+	if err != nil {
+		return err
+	}
 
 	repo, _, repoErr := client.Organizations.IsTeamRepo(context.TODO(), toGithubID(t), meta.(*Organization).name, r)
 

diff --git a/github/resource_github_team_repository_test.go b/github/resource_github_team_repository_test.go
@@ -114,7 +114,10 @@ func testAccCheckGithubTeamRepositoryExists(n string, repository *github.Reposit
 		}
 
 		conn := testAccProvider.Meta().(*Organization).client
-		t, r := parseTwoPartID(rs.Primary.ID)
+		t, r, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		repo, _, err := conn.Organizations.IsTeamRepo(context.TODO(),
 			toGithubID(t),
@@ -135,7 +138,10 @@ func testAccCheckGithubTeamRepositoryDestroy(s *terraform.State) error {
 		if rs.Type != "github_team_repository" {
 			continue
 		}
-		t, r := parseTwoPartID(rs.Primary.ID)
+		t, r, err := parseTwoPartID(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
 		repo, resp, err := conn.Organizations.IsTeamRepo(context.TODO(),
 			toGithubID(t),