service_account_key: regression fix for v1.14 (#1664)

Commit 8f31fec introduced a bug for the 'service_account_key' resource
where it required a project be set either in the provider or in the
resource for 'service_account_key', but a project isn't required if the
service account is a service account fully qualified name or a service
account email.

This PR relaxes the requirement that a project needs to be set for the
'service_account_key' resource, 'service_account' datasource and
'service_account_key' datasource, but will error if we try to build a
fully qualified name from a service account id when no project can be
found.

This also cleans up 'serviceAccountFQN' so it is slightly easier to
follow and return an error if there is no project but we need one to
build the service account fully qualified name.

Fixes: #1655

Author: vishen

google/data_source_google_service_account.go

@@ -43,16 +43,11 @@ func dataSourceGoogleServiceAccount() *schema.Resource {
 func dataSourceGoogleServiceAccountRead(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
-	// Get the project from the resource or fallback to the project
-	// in the provider configuration
-	project, err := getProject(d, config)
+	serviceAccountName, err := serviceAccountFQN(d.Get("account_id").(string), d, config)
 	if err != nil {
 		return err
 	}
 
-	// Get the service account as a fully qualified name
-	serviceAccountName := serviceAccountFQN(d.Get("account_id").(string), project)
-
 	sa, err := config.clientIAM.Projects.ServiceAccounts.Get(serviceAccountName).Do()
 	if err != nil {
 		return handleNotFoundError(err, d, fmt.Sprintf("Service Account %q", serviceAccountName))

google/data_source_google_service_account_key.go

@@ -46,16 +46,11 @@ func dataSourceGoogleServiceAccountKey() *schema.Resource {
 func dataSourceGoogleServiceAccountKeyRead(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
-	// Get the project from the resource or fallback to the project
-	// in the provider configuration
-	project, err := getProject(d, config)
+	serviceAccountName, err := serviceAccountFQN(d.Get("service_account_id").(string), d, config)
 	if err != nil {
 		return err
 	}
 
-	// Get the service account as the fully qualified name
-	serviceAccountName := serviceAccountFQN(d.Get("service_account_id").(string), project)
-
 	publicKeyType := d.Get("public_key_type").(string)
 
 	// Confirm the service account key exists

google/resource_google_service_account_key.go

@@ -87,15 +87,11 @@ func resourceGoogleServiceAccountKey() *schema.Resource {
 func resourceGoogleServiceAccountKeyCreate(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
-	// Get the project from the resource or fallback to the project
-	// in the provider configuration
-	project, err := getProject(d, config)
+	serviceAccountName, err := serviceAccountFQN(d.Get("service_account_id").(string), d, config)
 	if err != nil {
 		return err
 	}
 
-	serviceAccountName := serviceAccountFQN(d.Get("service_account_id").(string), project)
-
 	r := &iam.CreateServiceAccountKeyRequest{
 		KeyAlgorithm:   d.Get("key_algorithm").(string),
 		PrivateKeyType: d.Get("private_key_type").(string),

google/utils.go

@@ -362,17 +362,27 @@ func lockedCall(lockKey string, f func() error) error {
 }
 
 // serviceAccountFQN will attempt to generate the fully qualified name in the format of:
-// "projects/(-|<project_name>)/serviceAccounts/<service_account_id>@<project_name>.iam.gserviceaccount.com"
-func serviceAccountFQN(serviceAccount, project string) string {
-	// If the service account id isn't already the fully qualified name
-	if !strings.HasPrefix(serviceAccount, "projects/") {
-		// If the service account id is an email
-		if strings.Contains(serviceAccount, "@") {
-			serviceAccount = "projects/-/serviceAccounts/" + serviceAccount
-		} else {
-			// If the service account id doesn't contain the email, build it
-			serviceAccount = fmt.Sprintf("projects/-/serviceAccounts/%s@%s.iam.gserviceaccount.com", serviceAccount, project)
-		}
+// "projects/(-|<project>)/serviceAccounts/<service_account_id>@<project>.iam.gserviceaccount.com"
+// A project is required if we are trying to build the FQN from a service account id and
+// and error will be returned in this case if no project is set in the resource or the
+// provider-level config
+func serviceAccountFQN(serviceAccount string, d TerraformResourceData, config *Config) (string, error) {
+	// If the service account id is already the fully qualified name
+	if strings.HasPrefix(serviceAccount, "projects/") {
+		return serviceAccount, nil
+	}
+
+	// If the service account id is an email
+	if strings.Contains(serviceAccount, "@") {
+		return "projects/-/serviceAccounts/" + serviceAccount, nil
 	}
-	return serviceAccount
+
+	// Get the project from the resource or fallback to the project
+	// in the provider configuration
+	project, err := getProject(d, config)
+	if err != nil {
+		return "", err
+	}
+
+	return fmt.Sprintf("projects/-/serviceAccounts/%s@%s.iam.gserviceaccount.com", serviceAccount, project), nil
 }

google/utils_test.go

@@ -465,7 +465,12 @@ func TestServiceAccountFQN(t *testing.T) {
 	}
 
 	for tn, tc := range cases {
-		serviceAccountName := serviceAccountFQN(tc.serviceAccount, tc.project)
+		config := &Config{Project: tc.project}
+		d := &schema.ResourceData{}
+		serviceAccountName, err := serviceAccountFQN(tc.serviceAccount, d, config)
+		if err != nil {
+			t.Fatalf("unexpected error for service account FQN: %s", err)
+		}
 		if serviceAccountName != serviceAccountExpected {
 			t.Errorf("bad: %s, expected '%s' but returned '%s", tn, serviceAccountExpected, serviceAccountName)
 		}