Merge pull request #72 from terraform-providers/b-membership-import-c…

…rash

r/membership: Fix import crash on incorrect ID

github/resource_github_membership.go

@@ -15,7 +15,7 @@ func resourceGithubMembership() *schema.Resource {
  Update: resourceGithubMembershipUpdate,
  Delete: resourceGithubMembershipDelete,
  Importer: &schema.ResourceImporter{
- State: schema.ImportStatePassthrough,
+ State: resourceGithubMembershipImport,
  },
 
  Schema: map[string]*schema.Schema{
@@ -89,3 +89,14 @@ 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
+}

github/util.go

@@ -1,6 +1,7 @@
 package github
 
 import (
+ "errors"
  "fmt"
  "strconv"
  "strings"
@@ -46,6 +47,19 @@ func parseTwoPartID(id string) (string, string) {
  return parts[0], parts[1]
 }
 
+// validateTwoPartID performs a quick validation of a two-part ID, designed for
+// use when validation has not been previously possible, such as importing.
+func validateTwoPartID(id string) error {
+ if id == "" {
+ return errors.New("no ID supplied. Please supply an ID format matching organization:username")
+ }
+ parts := strings.Split(id, ":")
+ if len(parts) != 2 {
+ return fmt.Errorf("incorrectly formatted ID %q. Please supply an ID format matching organization:username", id)
+ }
+ return nil
+}
+
 // format the strings into an id `a:b`
 func buildTwoPartID(a, b *string) string {
  return fmt.Sprintf("%s:%s", *a, *b)

github/util_test.go

@@ -53,3 +53,44 @@ func TestAccGithubUtilTwoPartID(t *testing.T) {
  t.Fatalf("Expected parsed part two bar, actual: %s", parsedPartTwo)
  }
 }
+
+func TestAccValidateTwoPartID(t *testing.T) {
+ cases := []struct {
+ name string
+ id string
+ expectedErr string
+ }{
+ {
+ name: "valid",
+ id: "foo:bar",
+ },
+ {
+ name: "blank ID",
+ id: "",
+ expectedErr: "no ID supplied. Please supply an ID format matching organization:username",
+ },
+ {
+ name: "not enough parts",
+ id: "foo",
+ expectedErr: "incorrectly formatted ID \"foo\". Please supply an ID format matching organization:username",
+ },
+ {
+ name: "too many parts",
+ id: "foo:bar:baz",
+ expectedErr: "incorrectly formatted ID \"foo:bar:baz\". Please supply an ID format matching organization:username",
+ },
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ err := validateTwoPartID(tc.id)
+ switch {
+ case err != nil && tc.expectedErr == "":
+ t.Fatalf("expected no error, got %q", err)
+ case err != nil && tc.expectedErr != "":
+ if err.Error() != tc.expectedErr {
+ t.Fatalf("expected error to be %q, got %q", tc.expectedErr, err.Error())
+ }
+ }
+ })
+ }
+}