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())
+				}
+			}
+		})
+	}
+}