Fix IAM tests (#1337)

google/iam_kms_crypto_key.go

@@ -2,6 +2,7 @@ package google
 
 import (
 	"fmt"
+
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/helper/schema"
 	"google.golang.org/api/cloudkms/v1"
@@ -35,8 +36,13 @@ func NewKmsCryptoKeyIamUpdater(d *schema.ResourceData, config *Config) (Resource
 	}, nil
 }
 
-func CryptoIdParseFunc(d *schema.ResourceData, _ *Config) error {
+func CryptoIdParseFunc(d *schema.ResourceData, config *Config) error {
-	d.Set("crypto_key_id", d.Id())
+	cryptoKeyId, err := parseKmsCryptoKeyId(d.Id(), config)
+	if err != nil {
+		return err
+	}
+	d.Set("crypto_key_id", cryptoKeyId.cryptoKeyId())
+	d.SetId(cryptoKeyId.cryptoKeyId())
 	return nil
 }
 

google/iam_kms_key_ring.go

@@ -2,6 +2,7 @@ package google
 
 import (
 	"fmt"
+
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/helper/schema"
 	"google.golang.org/api/cloudkms/v1"
@@ -35,8 +36,14 @@ func NewKmsKeyRingIamUpdater(d *schema.ResourceData, config *Config) (ResourceIa
 	}, nil
 }
 
-func KeyRingIdParseFunc(d *schema.ResourceData, _ *Config) error {
+func KeyRingIdParseFunc(d *schema.ResourceData, config *Config) error {
-	d.Set("key_ring_id", d.Id())
+	keyRingId, err := parseKmsKeyRingId(d.Id(), config)
+	if err != nil {
+		return err
+	}
+
+	d.Set("key_ring_id", keyRingId.keyRingId())
+	d.SetId(keyRingId.keyRingId())
 	return nil
 }
 

google/iam_pubsub_topic.go

@@ -2,6 +2,7 @@ package google
 
 import (
 	"fmt"
+
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/helper/schema"
 	"google.golang.org/api/cloudresourcemanager/v1"

google/resource_kms_crypto_key.go

@@ -28,9 +28,10 @@ func resourceKmsCryptoKey() *schema.Resource {
 				ForceNew: true,
 			},
 			"key_ring": &schema.Schema{
-				Type:     schema.TypeString,
+				Type:             schema.TypeString,
-				Required: true,
+				Required:         true,
-				ForceNew: true,
+				ForceNew:         true,
+				DiffSuppressFunc: kmsCryptoKeyRingsEquivalent,
 			},
 			"rotation_period": &schema.Schema{
 				Type:         schema.TypeString,
@@ -42,6 +43,16 @@ func resourceKmsCryptoKey() *schema.Resource {
 	}
 }
 
+func kmsCryptoKeyRingsEquivalent(k, old, new string, d *schema.ResourceData) bool {
+	keyRingIdWithSpecifiersRegex := regexp.MustCompile("^projects/([a-z0-9-]+)/locations/([a-z0-9-])+/keyRings/([a-zA-Z0-9_-]{1,63})$")
+	normalizedKeyRingIdRegex := regexp.MustCompile("^([a-z0-9-]+)/([a-z0-9-])+/([a-zA-Z0-9_-]{1,63})$")
+	if matches := keyRingIdWithSpecifiersRegex.FindStringSubmatch(new); matches != nil {
+		normMatches := normalizedKeyRingIdRegex.FindStringSubmatch(old)
+		return normMatches != nil && normMatches[1] == matches[1] && normMatches[2] == matches[2] && normMatches[3] == matches[3]
+	}
+	return false
+}
+
 type kmsCryptoKeyId struct {
 	KeyRingId kmsKeyRingId
 	Name      string
@@ -95,7 +106,7 @@ func resourceKmsCryptoKeyCreate(d *schema.ResourceData, meta interface{}) error
 
 	log.Printf("[DEBUG] Created CryptoKey %s", cryptoKey.Name)
 
-	d.SetId(cryptoKeyId.terraformId())
+	d.SetId(cryptoKeyId.cryptoKeyId())
 
 	return resourceKmsCryptoKeyRead(d, meta)
 }
@@ -118,7 +129,7 @@ func resourceKmsCryptoKeyRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("name", cryptoKeyId.Name)
 	d.Set("rotation_period", cryptoKey.RotationPeriod)
 
-	d.SetId(cryptoKeyId.terraformId())
+	d.SetId(cryptoKeyId.cryptoKeyId())
 
 	return nil
 }
@@ -220,6 +231,7 @@ func parseKmsCryptoKeyId(id string, config *Config) (*kmsCryptoKeyId, error) {
 
 	cryptoKeyIdRegex := regexp.MustCompile("^([a-z0-9-]+)/([a-z0-9-])+/([a-zA-Z0-9_-]{1,63})/([a-zA-Z0-9_-]{1,63})$")
 	cryptoKeyIdWithoutProjectRegex := regexp.MustCompile("^([a-z0-9-])+/([a-zA-Z0-9_-]{1,63})/([a-zA-Z0-9_-]{1,63})$")
+	cryptoKeyRelativeLinkRegex := regexp.MustCompile("^projects/([a-z0-9-]+)/locations/([a-z0-9-]+)/keyRings/([a-zA-Z0-9_-]{1,63})/cryptoKeys/([a-zA-Z0-9_-]{1,63})$")
 
 	if cryptoKeyIdRegex.MatchString(id) {
 		return &kmsCryptoKeyId{
@@ -247,5 +259,15 @@ func parseKmsCryptoKeyId(id string, config *Config) (*kmsCryptoKeyId, error) {
 		}, nil
 	}
 
+	if parts := cryptoKeyRelativeLinkRegex.FindStringSubmatch(id); parts != nil {
+		return &kmsCryptoKeyId{
+			KeyRingId: kmsKeyRingId{
+				Project:  parts[1],
+				Location: parts[2],
+				Name:     parts[3],
+			},
+			Name: parts[4],
+		}, nil
+	}
 	return nil, fmt.Errorf("Invalid CryptoKey id format, expecting `{projectId}/{locationId}/{KeyringName}/{cryptoKeyName}` or `{locationId}/{keyRingName}/{cryptoKeyName}.`")
 }

google/resource_kms_key_ring.go

@@ -2,11 +2,12 @@ package google
 
 import (
 	"fmt"
-	"github.com/hashicorp/terraform/helper/schema"
-	"google.golang.org/api/cloudkms/v1"
 	"log"
 	"regexp"
 	"strings"
+
+	"github.com/hashicorp/terraform/helper/schema"
+	"google.golang.org/api/cloudkms/v1"
 )
 
 func resourceKmsKeyRing() *schema.Resource {
@@ -79,7 +80,7 @@ func resourceKmsKeyRingCreate(d *schema.ResourceData, meta interface{}) error {
 
 	log.Printf("[DEBUG] Created KeyRing %s", keyRing.Name)
 
-	d.SetId(keyRingId.terraformId())
+	d.SetId(keyRingId.keyRingId())
 
 	return resourceKmsKeyRingRead(d, meta)
 }
@@ -135,6 +136,7 @@ func parseKmsKeyRingId(id string, config *Config) (*kmsKeyRingId, error) {
 
 	keyRingIdRegex := regexp.MustCompile("^([a-z0-9-]+)/([a-z0-9-])+/([a-zA-Z0-9_-]{1,63})$")
 	keyRingIdWithoutProjectRegex := regexp.MustCompile("^([a-z0-9-])+/([a-zA-Z0-9_-]{1,63})$")
+	keyRingRelativeLinkRegex := regexp.MustCompile("^projects/([a-z0-9-]+)/locations/([a-z0-9-]+)/keyRings/([a-zA-Z0-9_-]{1,63})$")
 
 	if keyRingIdRegex.MatchString(id) {
 		return &kmsKeyRingId{
@@ -156,6 +158,13 @@ func parseKmsKeyRingId(id string, config *Config) (*kmsKeyRingId, error) {
 		}, nil
 	}
 
+	if parts := keyRingRelativeLinkRegex.FindStringSubmatch(id); parts != nil {
+		return &kmsKeyRingId{
+			Project:  parts[1],
+			Location: parts[2],
+			Name:     parts[3],
+		}, nil
+	}
 	return nil, fmt.Errorf("Invalid KeyRing id format, expecting `{projectId}/{locationId}/{keyRingName}` or `{locationId}/{keyRingName}.`")
 }
 
@@ -174,7 +183,7 @@ func resourceKmsKeyRingImportState(d *schema.ResourceData, meta interface{}) ([]
 		d.Set("project", keyRingId.Project)
 	}
 
-	d.SetId(keyRingId.terraformId())
+	d.SetId(keyRingId.keyRingId())
 
 	return []*schema.ResourceData{d}, nil
 }

google/resource_pubsub_topic_iam_test.go

@@ -28,6 +28,12 @@ func TestAccPubsubTopicIamBinding(t *testing.T) {
 					fmt.Sprintf("serviceAccount:%s-1@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()),
 				}),
 			},
+			{
+				ResourceName:      "google_pubsub_topic_iam_binding.foo",
+				ImportStateId:     fmt.Sprintf("%s roles/pubsub.publisher", getComputedTopicName(getTestProjectFromEnv(), topic)),
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
 			{
 				// Test IAM Binding update
 				Config: testAccPubsubTopicIamBinding_update(topic, account),
@@ -46,6 +52,28 @@ func TestAccPubsubTopicIamBinding(t *testing.T) {
 	})
 }
 
+func TestAccPubsubTopicIamBinding_topicName(t *testing.T) {
+	t.Parallel()
+
+	topic := "test-topic-iam-" + acctest.RandString(10)
+	account := "test-topic-iam-" + acctest.RandString(10)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				// Test IAM Binding creation
+				Config: testAccPubsubTopicIamBinding_topicName(topic, account),
+				Check: testAccCheckPubsubTopicIam(topic, "roles/pubsub.publisher", []string{
+					fmt.Sprintf("serviceAccount:%s-1@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()),
+				}),
+			},
+			// No import step- imports want the resource to be defined using the full id as the topic
+		},
+	})
+}
+
 func TestAccPubsubTopicIamMember(t *testing.T) {
 	t.Parallel()
 
@@ -131,7 +159,7 @@ func testAccCheckPubsubTopicIam(topic, role string, members []string) resource.T
 	}
 }
 
-func testAccPubsubTopicIamBinding_basic(topic, account string) string {
+func testAccPubsubTopicIamBinding_topicName(topic, account string) string {
 	return fmt.Sprintf(`
 resource "google_pubsub_topic" "topic" {
   name = "%s"
@@ -153,6 +181,28 @@ resource "google_pubsub_topic_iam_binding" "foo" {
 `, topic, account, getTestProjectFromEnv())
 }
 
+func testAccPubsubTopicIamBinding_basic(topic, account string) string {
+	return fmt.Sprintf(`
+resource "google_pubsub_topic" "topic" {
+  name = "%s"
+}
+
+resource "google_service_account" "test-account-1" {
+  account_id   = "%s-1"
+  display_name = "Iam Testing Account"
+}
+
+resource "google_pubsub_topic_iam_binding" "foo" {
+  # use the id instead of the name because it's more compatible with import
+  topic   = "${google_pubsub_topic.topic.id}"
+  role    = "roles/pubsub.publisher"
+  members = [
+    "serviceAccount:${google_service_account.test-account-1.email}",
+  ]
+}
+`, topic, account)
+}
+
 func testAccPubsubTopicIamBinding_update(topic, account string) string {
 	return fmt.Sprintf(`
 resource "google_pubsub_topic" "topic" {
@@ -170,7 +220,8 @@ resource "google_service_account" "test-account-2" {
 }
 
 resource "google_pubsub_topic_iam_binding" "foo" {
-  topic   = "${google_pubsub_topic.topic.name}"
+  # use the id instead of the name because it's more compatible with import
+  topic   = "${google_pubsub_topic.topic.id}"
   role    = "roles/pubsub.publisher"
   members = [
     "serviceAccount:${google_service_account.test-account-1.email}",