Skip to content

Commit

Permalink
service_account_key: regression fix for v1.14
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vishen committed Jun 15, 2018
1 parent 43cccf3 commit 5ac6ab0
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 26 deletions.
10 changes: 5 additions & 5 deletions google/data_source_google_service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ func dataSourceGoogleServiceAccountRead(d *schema.ResourceData, meta interface{}
config := meta.(*Config)

// Get the project from the resource or fallback to the project
// in the provider configuration
project, err := getProject(d, config)
// in the provider configuration. If neither exist use an empty project
// when building the service account FQN
project, _ := getProject(d, config)

serviceAccountName, err := serviceAccountFQN(d.Get("service_account_id").(string), project)
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))
Expand Down
10 changes: 5 additions & 5 deletions google/data_source_google_service_account_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ func dataSourceGoogleServiceAccountKeyRead(d *schema.ResourceData, meta interfac
config := meta.(*Config)

// Get the project from the resource or fallback to the project
// in the provider configuration
project, err := getProject(d, config)
// in the provider configuration. If neither exist use an empty project
// when building the service account FQN
project, _ := getProject(d, config)

serviceAccountName, err := serviceAccountFQN(d.Get("service_account_id").(string), project)
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
Expand Down
9 changes: 5 additions & 4 deletions google/resource_google_service_account_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,15 @@ func resourceGoogleServiceAccountKeyCreate(d *schema.ResourceData, meta interfac
config := meta.(*Config)

// Get the project from the resource or fallback to the project
// in the provider configuration
project, err := getProject(d, config)
// in the provider configuration. If neither exist use an empty project
// when building the service account FQN
project, _ := getProject(d, config)

serviceAccountName, err := serviceAccountFQN(d.Get("service_account_id").(string), project)
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),
Expand Down
29 changes: 18 additions & 11 deletions google/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,16 +363,23 @@ 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)
}
// 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
func serviceAccountFQN(serviceAccount, project string) (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
}

// If no project is specified but we are trying to build the fully qaulified
// name return an error
if project == "" {
return "", fmt.Errorf("project: required when using service account id")
}
return serviceAccount
return fmt.Sprintf("projects/-/serviceAccounts/%s@%s.iam.gserviceaccount.com", serviceAccount, project), nil
}
5 changes: 4 additions & 1 deletion google/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,10 @@ func TestServiceAccountFQN(t *testing.T) {
}

for tn, tc := range cases {
serviceAccountName := serviceAccountFQN(tc.serviceAccount, tc.project)
serviceAccountName, err := serviceAccountFQN(tc.serviceAccount, tc.project)
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)
}
Expand Down

0 comments on commit 5ac6ab0

Please sign in to comment.