Fix service account key data source name (#1932)

* fix service account key data source name

* switch id to name

* update docs

* doc format

* fixes for validation and tests

* last fixes for service account key data source

google/data_source_google_service_account_key.go

@@ -5,32 +5,30 @@ import (
 
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/helper/validation"
+	"regexp"
 )
 
 func dataSourceGoogleServiceAccountKey() *schema.Resource {
 	return &schema.Resource{
 		Read: dataSourceGoogleServiceAccountKeyRead,
 
 		Schema: map[string]*schema.Schema{
-			"service_account_id": &schema.Schema{
-				Type:     schema.TypeString,
-				Required: true,
+			"name": {
+				Type:         schema.TypeString,
+				Optional:     true,
+				Computed:     true,
+				ValidateFunc: validateRegexp(ServiceAccountKeyNameRegex),
 			},
-			"public_key_type": &schema.Schema{
+			"public_key_type": {
 				Type:         schema.TypeString,
 				Default:      "TYPE_X509_PEM_FILE",
 				Optional:     true,
 				ValidateFunc: validation.StringInSlice([]string{"TYPE_NONE", "TYPE_X509_PEM_FILE", "TYPE_RAW_PUBLIC_KEY"}, false),
 			},
-			"project": &schema.Schema{
+			"project": {
 				Type:     schema.TypeString,
 				Optional: true,
 			},
-			// Computed
-			"name": &schema.Schema{
-				Type:     schema.TypeString,
-				Computed: true,
-			},
 			"key_algorithm": {
 				Type:     schema.TypeString,
 				Computed: true,
@@ -39,24 +37,30 @@ func dataSourceGoogleServiceAccountKey() *schema.Resource {
 				Type:     schema.TypeString,
 				Computed: true,
 			},
+			"service_account_id": {
+				Type:          schema.TypeString,
+				Optional:      true,
+				ConflictsWith: []string{"name"},
+				Deprecated:    "Please use name to specify full service account key path projects/{project}/serviceAccounts/{serviceAccount}/keys/{keyId}",
+			},
 		},
 	}
 }
 
 func dataSourceGoogleServiceAccountKeyRead(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
-	serviceAccountName, err := serviceAccountFQN(d.Get("service_account_id").(string), d, config)
+	keyName, err := getDataSourceServiceAccountKeyName(d)
 	if err != nil {
 		return err
 	}
 
 	publicKeyType := d.Get("public_key_type").(string)
 
 	// Confirm the service account key exists
-	sak, err := config.clientIAM.Projects.ServiceAccounts.Keys.Get(serviceAccountName).PublicKeyType(publicKeyType).Do()
+	sak, err := config.clientIAM.Projects.ServiceAccounts.Keys.Get(keyName).PublicKeyType(publicKeyType).Do()
 	if err != nil {
-		return handleNotFoundError(err, d, fmt.Sprintf("Service Account Key %q", serviceAccountName))
+		return handleNotFoundError(err, d, fmt.Sprintf("Service Account Key %q", keyName))
 	}
 
 	d.SetId(sak.Name)
@@ -67,3 +71,28 @@ func dataSourceGoogleServiceAccountKeyRead(d *schema.ResourceData, meta interfac
 
 	return nil
 }
+
+func getDataSourceServiceAccountKeyName(d *schema.ResourceData) (string, error) {
+	keyName := d.Get("name").(string)
+	keyFromSAId := d.Get("service_account_id").(string)
+
+	// Neither name nor service_account_id specified
+	if keyName == "" && keyFromSAId == "" {
+		return "", fmt.Errorf("please use name to specify service account key being added as this data source")
+	}
+
+	fullKeyName := keyName
+	if fullKeyName == "" {
+		// Key name specified as incorrectly named, deprecated service account ID field
+		fullKeyName = keyFromSAId
+	}
+
+	// Validate name since interpolated values (i.e from a key or service
+	// account resource) will not get validated at plan time.
+	r := regexp.MustCompile(ServiceAccountKeyNameRegex)
+	if r.MatchString(fullKeyName) {
+		return fullKeyName, nil
+	}
+
+	return "", fmt.Errorf("invalid key name %q does not match regexp %q", fullKeyName, ServiceAccountKeyNameRegex)
+}

google/data_source_google_service_account_key_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/hashicorp/terraform/helper/acctest"
 	"github.com/hashicorp/terraform/helper/resource"
+	"strings"
 )
 
 func TestAccDatasourceGoogleServiceAccountKey_basic(t *testing.T) {
@@ -25,7 +26,7 @@ func TestAccDatasourceGoogleServiceAccountKey_basic(t *testing.T) {
 		PreCheck:  func() { testAccPreCheck(t) },
 		Providers: testAccProviders,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccDatasourceGoogleServiceAccountKey(account),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckGoogleServiceAccountKeyExists(resourceName),
@@ -35,6 +36,49 @@ func TestAccDatasourceGoogleServiceAccountKey_basic(t *testing.T) {
 					resource.TestCheckResourceAttrSet(resourceName, "public_key"),
 				),
 			},
+			{
+				Config: testAccDatasourceGoogleServiceAccountKey_deprecated(account),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGoogleServiceAccountKeyExists(resourceName),
+					// Check that the 'name' starts with the service account name
+					resource.TestMatchResourceAttr(resourceName, "name", regexp.MustCompile(serviceAccountName)),
+					resource.TestCheckResourceAttrSet(resourceName, "key_algorithm"),
+					resource.TestCheckResourceAttrSet(resourceName, "public_key"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccDatasourceGoogleServiceAccountKey_errors(t *testing.T) {
+	t.Parallel()
+
+	account := acctest.RandomWithPrefix("tf-test")
+	serviceAccountName := fmt.Sprintf(
+		"projects/%s/serviceAccounts/%s@%s.iam.gserviceaccount.com",
+		getTestProjectFromEnv(),
+		account,
+		getTestProjectFromEnv(),
+	)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccDatasourceGoogleServiceAccountKey_error(
+					account,
+					`name = "${google_service_account.acceptance.name}"`),
+				ExpectError: regexp.MustCompile(
+					fmt.Sprintf("invalid key name %q", serviceAccountName)),
+			},
+			{
+				Config: testAccDatasourceGoogleServiceAccountKey_error(
+					account,
+					`service_account_id = "${google_service_account.acceptance.id}"`),
+				ExpectError: regexp.MustCompile(
+					fmt.Sprintf("invalid key name %q", serviceAccountName)),
+			},
 		},
 	})
 }
@@ -51,6 +95,38 @@ resource "google_service_account_key" "acceptance" {
 }
 
 data "google_service_account_key" "acceptance" {
-	service_account_id = "${google_service_account_key.acceptance.id}"
+	name = "${google_service_account_key.acceptance.name}"
 }`, account)
 }
+
+func testAccDatasourceGoogleServiceAccountKey_deprecated(account string) string {
+	return fmt.Sprintf(`
+resource "google_service_account" "acceptance" {
+	account_id = "%s"
+}
+
+resource "google_service_account_key" "acceptance" {
+	service_account_id = "${google_service_account.acceptance.name}"
+	public_key_type = "TYPE_X509_PEM_FILE"
+}
+
+data "google_service_account_key" "acceptance" {
+	service_account_id = "${google_service_account_key.acceptance.name}"
+}`, account)
+}
+
+func testAccDatasourceGoogleServiceAccountKey_error(account string, incorrectDataFields ...string) string {
+	return fmt.Sprintf(`
+resource "google_service_account" "acceptance" {
+	account_id = "%s"
+}
+
+resource "google_service_account_key" "acceptance" {
+	service_account_id = "${google_service_account.acceptance.name}"
+	public_key_type = "TYPE_X509_PEM_FILE"
+}
+
+data "google_service_account_key" "acceptance" {
+%s
+}`, account, strings.Join(incorrectDataFields, "\n\t"))
+}

google/validation.go

@@ -42,6 +42,8 @@ var (
 	}
 	ServiceAccountLinkRegex = ServiceAccountLinkRegexPrefix + "(" + strings.Join(PossibleServiceAccountNames, "|") + ")"
 
+	ServiceAccountKeyNameRegex = ServiceAccountLinkRegexPrefix + "(.+)/keys/(.+)"
+
 	// Format of service accounts created through the API
 	CreatedServiceAccountNameRegex = fmt.Sprintf(RFC1035NameTemplate, 4, 28) + "@" + ProjectNameInDNSFormRegex + "\\.iam\\.gserviceaccount\\.com$"
 	ProjectNameInDNSFormRegex      = "[-a-z0-9\\.]{1,63}"

website/docs/d/datasource_google_service_account_key.html.markdown

@@ -14,27 +14,27 @@ Get service account public key. For more information, see [the official document
 ## Example Usage
 
 ```hcl
-data "google_service_account" "myaccount" {
-  account_id = "myaccount"
+resource "google_service_account" "myaccount" {
+  account_id = "dev-foo-account"
 }
 
-data "google_service_account_key" "mykey" {
-  service_account_id = "${data.google_service_account.myaccount.name}"
-  public_key_type = "TYPE_X509_PEM_FILE"
+resource "google_service_account_key" "mykey" {
+  service_account_id = "${google_service_account.myaccount.name}"
 }
 
-output "mykey_public_key" {
-  value = "${data.google_service_account_key.mykey.public_key}"
+data "google_service_account_key" "mykey" {
+  name = "${google_service_account_key.mykey.name}"
+  public_key_type = "TYPE_X509_PEM_FILE"
 }
 ```
 
 ## Argument Reference
 
 The following arguments are supported:
 
-* `service_account_id` - (Required) The Service account id of the Key Pair. This can be a string in the format
-`{ACCOUNT}` or `projects/{PROJECT_ID}/serviceAccounts/{ACCOUNT}`, where `{ACCOUNT}` is the email address or
-unique id of the service account. If the `{ACCOUNT}` syntax is used, the project will be inferred from the account.
+* `name` - (Required) The name of the service account key. This must have format
+    `projects/{PROJECT_ID}/serviceAccounts/{ACCOUNT}/keys/{KEYID}`, where `{ACCOUNT}`
+    is the email address or unique id of the service account.
 
 * `project` - (Optional) The ID of the project that the service account will be created in.
     Defaults to the provider project configuration.
@@ -45,6 +45,4 @@ unique id of the service account. If the `{ACCOUNT}` syntax is used, the project
 
 The following attributes are exported in addition to the arguments listed above:
 
-* `name` - The name used for this key pair
-
 * `public_key` - The public key, base64 encoded