From f8a3335bf9f318cc1384147de7fb4556501bc0b3 Mon Sep 17 00:00:00 2001 From: Jonathan Pentecost Date: Mon, 18 Jun 2018 21:37:41 +0100 Subject: [PATCH] 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 --- 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 6c75da0e..e2a1f3cd 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 81d3050f..e876836c 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 e107b66b..315d0a8a 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 edca2317..ba6a14e9 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 } - return serviceAccount + + // If the service account id is an email + if strings.Contains(serviceAccount, "@") { + return "projects/-/serviceAccounts/" + serviceAccount, nil + } + + // 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 d987b43c..82118fd4 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) }