Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add description to service account. #2513

Merged
merged 5 commits into from
Oct 25, 2019
Merged

Add description to service account. #2513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2ec765c
Add description to service account.
nat-henderson Oct 22, 2019
8aed702
Include docs for the field.
nat-henderson Oct 22, 2019
27ab7e2
Add comment explaining sleeps.
nat-henderson Oct 25, 2019
dea8f18
Update resource_google_service_account.go
nat-henderson Oct 25, 2019
21e5f14
Update tracked submodules -> HEAD on Fri Oct 25 22:03:36 UTC 2019
modular-magician Oct 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/terraform
Open in desktop
Submodule terraform updated from 77891f to 887d1b
2 changes: 1 addition & 1 deletion build/terraform-beta
Open in desktop
Submodule terraform-beta updated from 01c066 to d52448
17 changes: 16 additions & 1 deletion third_party/terraform/resources/resource_google_service_account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package google
import (
"fmt"
"strings"
"time"

"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"google.golang.org/api/iam/v1"
Expand Down Expand Up @@ -40,6 +41,10 @@ func resourceGoogleServiceAccount() *schema.Resource {
Type: schema.TypeString,
Optional: true,
},
"description": {
Type: schema.TypeString,
Optional: true,
},
"project": {
Type: schema.TypeString,
Computed: true,
Expand All @@ -63,9 +68,11 @@ func resourceGoogleServiceAccountCreate(d *schema.ResourceData, meta interface{}
}
aid := d.Get("account_id").(string)
displayName := d.Get("display_name").(string)
description := d.Get("description").(string)

sa := &iam.ServiceAccount{
DisplayName: displayName,
Description: description,
}

r := &iam.CreateServiceAccountRequest{
Expand All @@ -79,6 +86,10 @@ func resourceGoogleServiceAccountCreate(d *schema.ResourceData, meta interface{}
}

d.SetId(sa.Name)
// This API is meant to be synchronous, but in practice it shows the old value for
// a few milliseconds after the update goes through. A second is more than enough
// time to ensure following reads are correct.
time.Sleep(time.Second)

return resourceGoogleServiceAccountRead(d, meta)
}
Expand All @@ -98,6 +109,7 @@ func resourceGoogleServiceAccountRead(d *schema.ResourceData, meta interface{})
d.Set("account_id", strings.Split(sa.Email, "@")[0])
d.Set("name", sa.Name)
d.Set("display_name", sa.DisplayName)
d.Set("description", sa.Description)
return nil
}

Expand All @@ -114,19 +126,22 @@ func resourceGoogleServiceAccountDelete(d *schema.ResourceData, meta interface{}

func resourceGoogleServiceAccountUpdate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)
if ok := d.HasChange("display_name"); ok {
if d.HasChange("display_name") || d.HasChange("description") {
sa, err := config.clientIAM.Projects.ServiceAccounts.Get(d.Id()).Do()
if err != nil {
return fmt.Errorf("Error retrieving service account %q: %s", d.Id(), err)
}
_, err = config.clientIAM.Projects.ServiceAccounts.Update(d.Id(),
&iam.ServiceAccount{
DisplayName: d.Get("display_name").(string),
Description: d.Get("description").(string),
Etag: sa.Etag,
}).Do()
if err != nil {
return fmt.Errorf("Error updating service account %q: %s", d.Id(), err)
}
// See comment in Create.
time.Sleep(time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing sleep's in a test makes me uncomfortable, sleep's in production code much more so. Is this only to make the test pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, the test passes about 70% of the time without it. It's just that this api is only mostly synchronous. This is more than enough time to get a consistent result, which is good for users and tests. Costs very little, even though it's terrible practice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh dear. Can you add a comment inline otherwise without context I can see this being removed by a well meaning future developer.

}

return nil
Expand Down
2 changes: 2 additions & 0 deletions third_party/terraform/tests/resource_google_service_account_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func testAccServiceAccountBasic(account, name string) string {
resource "google_service_account" "acceptance" {
account_id = "%v"
display_name = "%v"
description = "foo"
}
`, account, name)
}
Expand All @@ -105,6 +106,7 @@ resource "google_service_account" "acceptance" {
project = "%v"
account_id = "%v"
display_name = "%v"
description = "foo"
}
`, project, account, name)
}
2 changes: 2 additions & 0 deletions third_party/terraform/website/docs/r/google_service_account.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ The following arguments are supported:
* `display_name` - (Optional) The display name for the service account.
Can be updated without creating a new resource.

* `description` - (Optional) A text description 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.

Expand Down