From 38b28a273a0ac15f6b28b4d2d84f03b7fc30ba3d Mon Sep 17 00:00:00 2001 From: Jonathan Pentecost Date: Fri, 15 Jun 2018 18:24:53 +0100 Subject: [PATCH] service_account_key: regression fix for v1.14 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 --- google/data_source_google_service_account.go | 7 +--- .../data_source_google_service_account_key.go | 7 +--- google/resource_google_service_account_key.go | 6 +--- google/utils.go | 34 ++++++++++++------- google/utils_test.go | 7 +++- 5 files changed, 31 insertions(+), 30 deletions(-) diff --git a/google/data_source_google_service_account.go b/google/data_source_google_service_account.go index 6c75da0eaba..e2a1f3cd66d 100644 --- a/google/data_source_google_service_account.go +++ b/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)) diff --git a/google/data_source_google_service_account_key.go b/google/data_source_google_service_account_key.go index 81d3050f459..e876836cadd 100644 --- a/google/data_source_google_service_account_key.go +++ b/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 diff --git a/google/resource_google_service_account_key.go b/google/resource_google_service_account_key.go index e107b66b00d..315d0a8ac0c 100644 --- a/google/resource_google_service_account_key.go +++ b/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), diff --git a/google/utils.go b/google/utils.go index edca2317ac1..ba6a14e932e 100644 --- a/google/utils.go +++ b/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/(-|)/serviceAccounts/@.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/(-|)/serviceAccounts/@.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 } diff --git a/google/utils_test.go b/google/utils_test.go index d987b43c3ee..82118fd4b63 100644 --- a/google/utils_test.go +++ b/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) }