From 5423349b681173729e64e284ece80548698cb29b Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Mon, 7 Nov 2016 23:27:32 -0800 Subject: [PATCH 1/6] providers/google: Create and delete Service Accounts --- builtin/providers/google/config.go | 13 +- builtin/providers/google/provider.go | 1 + .../google/resource_google_project_test.go | 1 - .../google/resource_google_service_account.go | 321 ++++++++++++++++++ .../resource_google_service_account_test.go | 138 ++++++++ .../google/r/google_project.html.markdown | 4 +- .../r/google_service_account.html.markdown | 73 ++++ 7 files changed, 546 insertions(+), 5 deletions(-) create mode 100644 builtin/providers/google/resource_google_service_account.go create mode 100644 builtin/providers/google/resource_google_service_account_test.go create mode 100644 website/source/docs/providers/google/r/google_service_account.html.markdown diff --git a/builtin/providers/google/config.go b/builtin/providers/google/config.go index 063c93792..09cd750b7 100644 --- a/builtin/providers/google/config.go +++ b/builtin/providers/google/config.go @@ -17,6 +17,7 @@ import ( "google.golang.org/api/compute/v1" "google.golang.org/api/container/v1" "google.golang.org/api/dns/v1" + "google.golang.org/api/iam/v1" "google.golang.org/api/pubsub/v1" "google.golang.org/api/sqladmin/v1beta4" "google.golang.org/api/storage/v1" @@ -36,6 +37,7 @@ type Config struct { clientResourceManager *cloudresourcemanager.Service clientStorage *storage.Service clientSqlAdmin *sqladmin.Service + clientIAM *iam.Service } func (c *Config) loadAndValidate() error { @@ -135,12 +137,19 @@ func (c *Config) loadAndValidate() error { } c.clientPubsub.UserAgent = userAgent - log.Printf("[INFO] Instatiating Google CloudResourceManager Client...") + log.Printf("[INFO] Instatiating Google Cloud ResourceManager Client...") c.clientResourceManager, err = cloudresourcemanager.New(client) if err != nil { return err } - c.clientPubsub.UserAgent = userAgent + c.clientResourceManager.UserAgent = userAgent + + log.Printf("[INFO] Instatiating Google Cloud IAM Client...") + c.clientIAM, err = iam.New(client) + if err != nil { + return err + } + c.clientIAM.UserAgent = userAgent return nil } diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index b439f5a2d..e126d7562 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -96,6 +96,7 @@ func Provider() terraform.ResourceProvider { "google_project": resourceGoogleProject(), "google_pubsub_topic": resourcePubsubTopic(), "google_pubsub_subscription": resourcePubsubSubscription(), + "google_service_account": resourceGoogleServiceAccount(), "google_storage_bucket": resourceStorageBucket(), "google_storage_bucket_acl": resourceStorageBucketAcl(), "google_storage_bucket_object": resourceStorageBucketObject(), diff --git a/builtin/providers/google/resource_google_project_test.go b/builtin/providers/google/resource_google_project_test.go index f9208e11e..161b6b4eb 100644 --- a/builtin/providers/google/resource_google_project_test.go +++ b/builtin/providers/google/resource_google_project_test.go @@ -468,5 +468,4 @@ data "google_iam_policy" "admin" { "user:evandbrown@gmail.com", ] } - }` diff --git a/builtin/providers/google/resource_google_service_account.go b/builtin/providers/google/resource_google_service_account.go new file mode 100644 index 000000000..e0385f5a8 --- /dev/null +++ b/builtin/providers/google/resource_google_service_account.go @@ -0,0 +1,321 @@ +package google + +import ( + "encoding/json" + "fmt" + "log" + + "github.com/hashicorp/terraform/helper/schema" + "google.golang.org/api/googleapi" + "google.golang.org/api/iam/v1" +) + +func resourceGoogleServiceAccount() *schema.Resource { + return &schema.Resource{ + Create: resourceGoogleServiceAccountCreate, + Read: resourceGoogleServiceAccountRead, + Delete: resourceGoogleServiceAccountDelete, + Update: resourceGoogleServiceAccountUpdate, + Schema: map[string]*schema.Schema{ + "email": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + "unique_id": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + "name": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + "account_id": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "display_name": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "project": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + "policy_data": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + }, + } +} + +func resourceGoogleServiceAccountCreate(d *schema.ResourceData, meta interface{}) error { + config := meta.(*Config) + project, err := getProject(d, config) + if err != nil { + return err + } + aid := d.Get("account_id").(string) + displayName := d.Get("display_name").(string) + + sa := &iam.ServiceAccount{ + DisplayName: displayName, + } + + r := &iam.CreateServiceAccountRequest{ + AccountId: aid, + ServiceAccount: sa, + } + + sa, err = config.clientIAM.Projects.ServiceAccounts.Create("projects/"+project, r).Do() + if err != nil { + return fmt.Errorf("Error creating service account: %s", err) + } + + d.SetId(sa.Name) + + // Apply the IAM policy if it is set + if pString, ok := d.GetOk("policy_data"); ok { + // The policy string is just a marshaled cloudresourcemanager.Policy. + // Unmarshal it to a struct. + var policy iam.Policy + if err = json.Unmarshal([]byte(pString.(string)), &policy); err != nil { + return err + } + + // Retrieve existing IAM policy from project. This will be merged + // with the policy defined here. + // TODO(evanbrown): Add an 'authoritative' flag that allows policy + // in manifest to overwrite existing policy. + p, err := getServiceAccountIamPolicy(sa.Name, config) + if err != nil { + return fmt.Errorf("Could not find service account %q when applying IAM policy: %s", sa.Name, err) + } + log.Printf("[DEBUG] Got existing bindings for service account: %#v", p.Bindings) + + // Merge the existing policy bindings with those defined in this manifest. + p.Bindings = saMergeBindings(append(p.Bindings, policy.Bindings...)) + + // Apply the merged policy + log.Printf("[DEBUG] Setting new policy for service account: %#v", p) + _, err = config.clientIAM.Projects.ServiceAccounts.SetIamPolicy(sa.Name, + &iam.SetIamPolicyRequest{Policy: p}).Do() + + if err != nil { + return fmt.Errorf("Error applying IAM policy for service account %q: %s", sa.Name, err) + } + } + return resourceGoogleServiceAccountRead(d, meta) +} + +func resourceGoogleServiceAccountRead(d *schema.ResourceData, meta interface{}) error { + config := meta.(*Config) + + // Confirm the service account exists + sa, err := config.clientIAM.Projects.ServiceAccounts.Get(d.Id()).Do() + if err != nil { + if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 { + log.Printf("[WARN] Removing reference to service account %q because it no longer exists", d.Id()) + + return fmt.Errorf("Error getting service account with name %q: %s", d.Id(), err) + // The resource doesn't exist anymore + d.SetId("") + } + return fmt.Errorf("Error reading service account %q: %q", d.Id(), err) + } + + d.Set("email", sa.Email) + d.Set("unique_id", sa.UniqueId) + d.Set("name", sa.Name) + d.Set("display_name", sa.DisplayName) + return nil +} + +func resourceGoogleServiceAccountDelete(d *schema.ResourceData, meta interface{}) error { + config := meta.(*Config) + name := d.Id() + _, err := config.clientIAM.Projects.ServiceAccounts.Delete(name).Do() + if err != nil { + return err + } + d.SetId("") + return nil +} + +func resourceGoogleServiceAccountUpdate(d *schema.ResourceData, meta interface{}) error { + config := meta.(*Config) + var err error + if ok := d.HasChange("display_name"); ok { + 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), + Etag: sa.Etag, + }).Do() + if err != nil { + return fmt.Errorf("Error updating service account %q: %s", d.Id(), err) + } + } + + if ok := d.HasChange("policy_data"); ok { + // The policy string is just a marshaled cloudresourcemanager.Policy. + // Unmarshal it to a struct that contains the old and new policies + oldP, newP := d.GetChange("policy_data") + oldPString := oldP.(string) + newPString := newP.(string) + + // JSON Unmarshaling would fail + if oldPString == "" { + oldPString = "{}" + } + if newPString == "" { + newPString = "{}" + } + + oldPStringf, _ := json.MarshalIndent(oldPString, "", " ") + newPStringf, _ := json.MarshalIndent(newPString, "", " ") + log.Printf("[DEBUG]: Old policy: %v\nNew policy: %v", string(oldPStringf), string(newPStringf)) + + var oldPolicy, newPolicy iam.Policy + if err = json.Unmarshal([]byte(newPString), &newPolicy); err != nil { + return err + } + if err = json.Unmarshal([]byte(oldPString), &oldPolicy); err != nil { + return err + } + + // Find any Roles and Members that were removed (i.e., those that are present + // in the old but absent in the new + oldMap := saRolesToMembersMap(oldPolicy.Bindings) + newMap := saRolesToMembersMap(newPolicy.Bindings) + deleted := make(map[string]map[string]bool) + + // Get each role and its associated members in the old state + for role, members := range oldMap { + // Initialize map for role + if _, ok := deleted[role]; !ok { + deleted[role] = make(map[string]bool) + } + // The role exists in the new state + if _, ok := newMap[role]; ok { + // Check each memeber + for member, _ := range members { + // Member does not exist in new state, so it was deleted + if _, ok = newMap[role][member]; !ok { + deleted[role][member] = true + } + } + } else { + // This indicates an entire role was deleted. Mark all members + // for delete. + for member, _ := range members { + deleted[role][member] = true + } + } + } + log.Printf("[DEBUG] Roles and Members to be deleted: %#v", deleted) + + // Retrieve existing IAM policy from project. This will be merged + // with the policy in the current state + // TODO(evanbrown): Add an 'authoritative' flag that allows policy + // in manifest to overwrite existing policy. + p, err := getServiceAccountIamPolicy(d.Id(), config) + if err != nil { + return err + } + log.Printf("[DEBUG] Got existing bindings from service account %q: %#v", d.Id(), p.Bindings) + + // Merge existing policy with policy in the current state + log.Printf("[DEBUG] Merging new bindings from service account %q: %#v", d.Id(), newPolicy.Bindings) + mergedBindings := saMergeBindings(append(p.Bindings, newPolicy.Bindings...)) + + // Remove any roles and members that were explicitly deleted + mergedBindingsMap := saRolesToMembersMap(mergedBindings) + for role, members := range deleted { + for member, _ := range members { + delete(mergedBindingsMap[role], member) + } + } + + p.Bindings = saRolesToMembersBinding(mergedBindingsMap) + log.Printf("[DEBUG] Setting new policy for project: %#v", p) + + dump, _ := json.MarshalIndent(p.Bindings, " ", " ") + log.Printf(string(dump)) + _, err = config.clientIAM.Projects.ServiceAccounts.SetIamPolicy(d.Id(), + &iam.SetIamPolicyRequest{Policy: p}).Do() + + if err != nil { + return fmt.Errorf("Error applying IAM policy for service account %q: %s", d.Id(), err) + } + } + return nil +} + +// Retrieve the existing IAM Policy for a service account +func getServiceAccountIamPolicy(sa string, config *Config) (*iam.Policy, error) { + p, err := config.clientIAM.Projects.ServiceAccounts.GetIamPolicy(sa).Do() + + if err != nil { + return nil, fmt.Errorf("Error retrieving IAM policy for service account %q: %s", sa, err) + } + return p, nil +} + +// Convert a map of roles->members to a list of Binding +func saRolesToMembersBinding(m map[string]map[string]bool) []*iam.Binding { + bindings := make([]*iam.Binding, 0) + for role, members := range m { + b := iam.Binding{ + Role: role, + Members: make([]string, 0), + } + for m, _ := range members { + b.Members = append(b.Members, m) + } + bindings = append(bindings, &b) + } + return bindings +} + +// Map a role to a map of members, allowing easy merging of multiple bindings. +func saRolesToMembersMap(bindings []*iam.Binding) map[string]map[string]bool { + bm := make(map[string]map[string]bool) + // Get each binding + for _, b := range bindings { + // Initialize members map + if _, ok := bm[b.Role]; !ok { + bm[b.Role] = make(map[string]bool) + } + // Get each member (user/principal) for the binding + for _, m := range b.Members { + // Add the member + bm[b.Role][m] = true + } + } + return bm +} + +// Merge multiple Bindings such that Bindings with the same Role result in +// a single Binding with combined Members +func saMergeBindings(bindings []*iam.Binding) []*iam.Binding { + bm := saRolesToMembersMap(bindings) + rb := make([]*iam.Binding, 0) + + for role, members := range bm { + var b iam.Binding + b.Role = role + b.Members = make([]string, 0) + for m, _ := range members { + b.Members = append(b.Members, m) + } + rb = append(rb, &b) + } + + return rb +} diff --git a/builtin/providers/google/resource_google_service_account_test.go b/builtin/providers/google/resource_google_service_account_test.go new file mode 100644 index 000000000..398f73c20 --- /dev/null +++ b/builtin/providers/google/resource_google_service_account_test.go @@ -0,0 +1,138 @@ +package google + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" +) + +var ( + accountId = "tf-test" + accountId2 = "tf-test-2" + displayName = "Terraform Test" + displayName2 = "Terraform Test Update" +) + +// Test that a service account resource can be created, updated, and destroyed +func TestAccGoogleServiceAccount_basic(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + // The first step creates a basic service account + resource.TestStep{ + Config: fmt.Sprintf(testAccGoogleServiceAccount_basic, accountId, displayName), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleServiceAccountExists("google_service_account.acceptance"), + ), + }, + // The second step updates the service account + resource.TestStep{ + Config: fmt.Sprintf(testAccGoogleServiceAccount_basic, accountId, displayName2), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleServiceAccountNameModified("google_service_account.acceptance"), + ), + }, + }, + }) +} + +// Test that a service account resource can be created with a policy, updated, +// and destroyed. +func TestAccGoogleServiceAccount_createPolicy(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + // The first step creates a basic service account with an IAM policy + resource.TestStep{ + Config: fmt.Sprintf(testAccGoogleServiceAccount_policy, accountId2, displayName, projectId), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 1), + ), + }, + // The second step updates the service account with no IAM policy + resource.TestStep{ + Config: fmt.Sprintf(testAccGoogleServiceAccount_basic, accountId2, displayName), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 0), + ), + }, + // The final step re-applies the IAM policy + resource.TestStep{ + Config: fmt.Sprintf(testAccGoogleServiceAccount_policy, accountId2, displayName, projectId), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 1), + ), + }, + }, + }) +} + +func testAccCheckGoogleServiceAccountPolicyCount(r string, n int) resource.TestCheckFunc { + return func(s *terraform.State) error { + c := testAccProvider.Meta().(*Config) + p, err := getServiceAccountIamPolicy(s.RootModule().Resources[r].Primary.ID, c) + if err != nil { + return fmt.Errorf("Failed to retrieve IAM Policy for service account: %s", err) + } + if len(p.Bindings) != n { + return fmt.Errorf("The service account has %v bindings but %v were expected", len(p.Bindings), n) + } + return nil + } +} + +func testAccCheckGoogleServiceAccountExists(r string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[r] + if !ok { + return fmt.Errorf("Not found: %s", r) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No ID is set") + } + + return nil + } +} + +func testAccCheckGoogleServiceAccountNameModified(r string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[r] + if !ok { + return fmt.Errorf("Not found: %s", r) + } + + if rs.Primary.Attributes["display_name"] != displayName2 { + return fmt.Errorf("display_name is %q expected %q", rs.Primary.Attributes["display_name"], displayName2) + } + + return nil + } +} + +var testAccGoogleServiceAccount_basic = ` +resource "google_service_account" "acceptance" { + account_id = "%v" + display_name = "%v" +}` + +var testAccGoogleServiceAccount_policy = ` +resource "google_service_account" "acceptance" { + account_id = "%v" + display_name = "%v" + policy_data = "${data.google_iam_policy.service_account.policy_data}" +} + +data "google_iam_policy" "service_account" { + binding { + role = "roles/iam.serviceAccountActor" + members = [ + "serviceAccount:tf-test-2@%v.iam.gserviceaccount.com", + ] + } +}` diff --git a/website/source/docs/providers/google/r/google_project.html.markdown b/website/source/docs/providers/google/r/google_project.html.markdown index 72a34c5ca..e0c5ae4f5 100644 --- a/website/source/docs/providers/google/r/google_project.html.markdown +++ b/website/source/docs/providers/google/r/google_project.html.markdown @@ -19,9 +19,9 @@ project's existing policy. The policy is always specified in a ## Example Usage ```js -resource "google_project" "my-project" { +resource "google_project" "my_project" { id = "your-project-id" - policy_data = "${data.google_iam_policy.admin.policy}" + policy_data = "${data.google_iam_policy.admin.policy_data}" } data "google_iam_policy" "admin" { diff --git a/website/source/docs/providers/google/r/google_service_account.html.markdown b/website/source/docs/providers/google/r/google_service_account.html.markdown new file mode 100644 index 000000000..4e72c068e --- /dev/null +++ b/website/source/docs/providers/google/r/google_service_account.html.markdown @@ -0,0 +1,73 @@ +--- +layout: "google" +page_title: "Google: google_service_account" +sidebar_current: "docs-google-service-account" +description: |- + Allows management of a Google Cloud Platform service account. +--- + +# google\_service\_account + +Allows management of a Google Cloud Platform service account. + +## Example Usage + +This snippet creates a service account, then gives it objectViewer +permission in a project. + +```js +resource "google_service_account" "object_viewer" { + account_id = "object-viewer" + display_name = "Object viewer" +} + +resource "google_project" "my_project" { + id = "your-project-id" + policy_data = "${data.google_iam_policy.admin.policy_data}" +} + +data "google_iam_policy" "admin" { + binding { + role = "roles/storage.objectViewer" + members = [ + "serviceAccount:${google_service_account.object_viewer.email}", + ] + } +} +``` + +## Argument Reference + +The following arguments are supported: + +* `account_id` - (Required) The service account ID. + Changing this forces a new service account to be created. + +* `display_name` - (Optional) The display name for the service account. + Can be updated without creating a new resource. + +* `project` - (Optional) The project that the service account will be created in. + Defaults to the provider project configuration. + +* `policy_data` - (Optional) The `google_iam_policy` data source that represents + the IAM policy that will be applied to the service account. The policy will be + merged with any existing policy. + + Changing this updates the policy. + + Deleting this removes the policy, but leaves the original policy + intact. If there are overlapping `binding` entries between the original + policy and the data source policy, they will be removed. + +## Attributes Reference + +In addition to the arguments listed above, the following computed attributes are +exported: + +* `email` - The e-mail address of the service account. This value + should be referenced from any `google_iam_policy` data sources + that would grant the service account privileges. + +* `name` - The fully-qualified name of the service account. + +* `unique_id` - The unique id of the service account. From 9a188049a708aae50643ccb0e9a574a3e5b57b3c Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Mon, 14 Nov 2016 09:42:11 -0800 Subject: [PATCH 2/6] providers/google: random resource names in SA test --- .../google/resource_google_service_account_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/builtin/providers/google/resource_google_service_account_test.go b/builtin/providers/google/resource_google_service_account_test.go index 398f73c20..03d5ee6ef 100644 --- a/builtin/providers/google/resource_google_service_account_test.go +++ b/builtin/providers/google/resource_google_service_account_test.go @@ -4,19 +4,19 @@ import ( "fmt" "testing" + "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" ) var ( - accountId = "tf-test" - accountId2 = "tf-test-2" displayName = "Terraform Test" displayName2 = "Terraform Test Update" ) // Test that a service account resource can be created, updated, and destroyed func TestAccGoogleServiceAccount_basic(t *testing.T) { + accountId := "a" + acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -42,27 +42,28 @@ func TestAccGoogleServiceAccount_basic(t *testing.T) { // Test that a service account resource can be created with a policy, updated, // and destroyed. func TestAccGoogleServiceAccount_createPolicy(t *testing.T) { + accountId := "a" + acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ // The first step creates a basic service account with an IAM policy resource.TestStep{ - Config: fmt.Sprintf(testAccGoogleServiceAccount_policy, accountId2, displayName, projectId), + Config: fmt.Sprintf(testAccGoogleServiceAccount_policy, accountId, displayName, accountId, projectId), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 1), ), }, // The second step updates the service account with no IAM policy resource.TestStep{ - Config: fmt.Sprintf(testAccGoogleServiceAccount_basic, accountId2, displayName), + Config: fmt.Sprintf(testAccGoogleServiceAccount_basic, accountId, displayName), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 0), ), }, // The final step re-applies the IAM policy resource.TestStep{ - Config: fmt.Sprintf(testAccGoogleServiceAccount_policy, accountId2, displayName, projectId), + Config: fmt.Sprintf(testAccGoogleServiceAccount_policy, accountId, displayName, accountId, projectId), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 1), ), @@ -132,7 +133,7 @@ data "google_iam_policy" "service_account" { binding { role = "roles/iam.serviceAccountActor" members = [ - "serviceAccount:tf-test-2@%v.iam.gserviceaccount.com", + "serviceAccount:%v@%v.iam.gserviceaccount.com", ] } }` From 08dba7eb66d8dfed6629136da26282ee64179eb1 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Mon, 14 Nov 2016 09:51:07 -0800 Subject: [PATCH 3/6] docs: Add google_service_account to sidebar --- website/source/layouts/google.erb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/source/layouts/google.erb b/website/source/layouts/google.erb index f35178715..98d9c8543 100644 --- a/website/source/layouts/google.erb +++ b/website/source/layouts/google.erb @@ -24,6 +24,9 @@ From 1612d6a78f0d7aeedfea0927bb4525d8d6118a95 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Mon, 14 Nov 2016 09:59:44 -0800 Subject: [PATCH 4/6] Fix go vet issue --- builtin/providers/google/resource_google_service_account.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/providers/google/resource_google_service_account.go b/builtin/providers/google/resource_google_service_account.go index e0385f5a8..6eb45fa26 100644 --- a/builtin/providers/google/resource_google_service_account.go +++ b/builtin/providers/google/resource_google_service_account.go @@ -118,10 +118,11 @@ func resourceGoogleServiceAccountRead(d *schema.ResourceData, meta interface{}) if err != nil { if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 { log.Printf("[WARN] Removing reference to service account %q because it no longer exists", d.Id()) - - return fmt.Errorf("Error getting service account with name %q: %s", d.Id(), err) + saName := d.Id() // The resource doesn't exist anymore d.SetId("") + + return fmt.Errorf("Error getting service account with name %q: %s", saName, err) } return fmt.Errorf("Error reading service account %q: %q", d.Id(), err) } From 4bdf65c36f7dd2092692d6e311845050b08add99 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Mon, 14 Nov 2016 11:12:55 -0800 Subject: [PATCH 5/6] Fix spacing inconsistencies --- .../providers/google/resource_google_service_account_test.go | 2 +- .../providers/google/r/google_service_account.html.markdown | 4 ++-- website/source/layouts/google.erb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/providers/google/resource_google_service_account_test.go b/builtin/providers/google/resource_google_service_account_test.go index 03d5ee6ef..a2577b7de 100644 --- a/builtin/providers/google/resource_google_service_account_test.go +++ b/builtin/providers/google/resource_google_service_account_test.go @@ -125,7 +125,7 @@ resource "google_service_account" "acceptance" { var testAccGoogleServiceAccount_policy = ` resource "google_service_account" "acceptance" { account_id = "%v" - display_name = "%v" + display_name = "%v" policy_data = "${data.google_iam_policy.service_account.policy_data}" } diff --git a/website/source/docs/providers/google/r/google_service_account.html.markdown b/website/source/docs/providers/google/r/google_service_account.html.markdown index 4e72c068e..c54594ef1 100644 --- a/website/source/docs/providers/google/r/google_service_account.html.markdown +++ b/website/source/docs/providers/google/r/google_service_account.html.markdown @@ -18,7 +18,7 @@ permission in a project. ```js resource "google_service_account" "object_viewer" { account_id = "object-viewer" - display_name = "Object viewer" + display_name = "Object viewer" } resource "google_project" "my_project" { @@ -55,7 +55,7 @@ The following arguments are supported: Changing this updates the policy. - Deleting this removes the policy, but leaves the original policy + Deleting this removes the policy, but leaves the original policy intact. If there are overlapping `binding` entries between the original policy and the data source policy, they will be removed. diff --git a/website/source/layouts/google.erb b/website/source/layouts/google.erb index 98d9c8543..419b0dbc6 100644 --- a/website/source/layouts/google.erb +++ b/website/source/layouts/google.erb @@ -26,8 +26,8 @@ google_project > - google_service_account - + google_service_account + From a32fe2d47fe9343c37c89d485de7742cef7804b9 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Thu, 17 Nov 2016 09:49:22 -0800 Subject: [PATCH 6/6] Resolve review feedback --- .../google/resource_google_service_account.go | 4 +- .../resource_google_service_account_test.go | 42 ++++++++++--------- .../r/google_service_account.html.markdown | 7 ++-- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/builtin/providers/google/resource_google_service_account.go b/builtin/providers/google/resource_google_service_account.go index 6eb45fa26..b97e602c7 100644 --- a/builtin/providers/google/resource_google_service_account.go +++ b/builtin/providers/google/resource_google_service_account.go @@ -178,9 +178,7 @@ func resourceGoogleServiceAccountUpdate(d *schema.ResourceData, meta interface{} newPString = "{}" } - oldPStringf, _ := json.MarshalIndent(oldPString, "", " ") - newPStringf, _ := json.MarshalIndent(newPString, "", " ") - log.Printf("[DEBUG]: Old policy: %v\nNew policy: %v", string(oldPStringf), string(newPStringf)) + log.Printf("[DEBUG]: Old policy: %q\nNew policy: %q", string(oldPString), string(newPString)) var oldPolicy, newPolicy iam.Policy if err = json.Unmarshal([]byte(newPString), &newPolicy); err != nil { diff --git a/builtin/providers/google/resource_google_service_account_test.go b/builtin/providers/google/resource_google_service_account_test.go index a2577b7de..ecf014808 100644 --- a/builtin/providers/google/resource_google_service_account_test.go +++ b/builtin/providers/google/resource_google_service_account_test.go @@ -9,30 +9,27 @@ import ( "github.com/hashicorp/terraform/terraform" ) -var ( - displayName = "Terraform Test" - displayName2 = "Terraform Test Update" -) - // Test that a service account resource can be created, updated, and destroyed func TestAccGoogleServiceAccount_basic(t *testing.T) { accountId := "a" + acctest.RandString(10) + displayName := "Terraform Test" + displayName2 := "Terraform Test Update" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ // The first step creates a basic service account resource.TestStep{ - Config: fmt.Sprintf(testAccGoogleServiceAccount_basic, accountId, displayName), + Config: testAccGoogleServiceAccountBasic(accountId, displayName), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleServiceAccountExists("google_service_account.acceptance"), ), }, // The second step updates the service account resource.TestStep{ - Config: fmt.Sprintf(testAccGoogleServiceAccount_basic, accountId, displayName2), + Config: testAccGoogleServiceAccountBasic(accountId, displayName2), Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleServiceAccountNameModified("google_service_account.acceptance"), + testAccCheckGoogleServiceAccountNameModified("google_service_account.acceptance", displayName2), ), }, }, @@ -43,27 +40,28 @@ func TestAccGoogleServiceAccount_basic(t *testing.T) { // and destroyed. func TestAccGoogleServiceAccount_createPolicy(t *testing.T) { accountId := "a" + acctest.RandString(10) + displayName := "Terraform Test" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ // The first step creates a basic service account with an IAM policy resource.TestStep{ - Config: fmt.Sprintf(testAccGoogleServiceAccount_policy, accountId, displayName, accountId, projectId), + Config: testAccGoogleServiceAccountPolicy(accountId, projectId), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 1), ), }, // The second step updates the service account with no IAM policy resource.TestStep{ - Config: fmt.Sprintf(testAccGoogleServiceAccount_basic, accountId, displayName), + Config: testAccGoogleServiceAccountBasic(accountId, displayName), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 0), ), }, // The final step re-applies the IAM policy resource.TestStep{ - Config: fmt.Sprintf(testAccGoogleServiceAccount_policy, accountId, displayName, accountId, projectId), + Config: testAccGoogleServiceAccountPolicy(accountId, projectId), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 1), ), @@ -101,29 +99,32 @@ func testAccCheckGoogleServiceAccountExists(r string) resource.TestCheckFunc { } } -func testAccCheckGoogleServiceAccountNameModified(r string) resource.TestCheckFunc { +func testAccCheckGoogleServiceAccountNameModified(r, n string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[r] if !ok { return fmt.Errorf("Not found: %s", r) } - if rs.Primary.Attributes["display_name"] != displayName2 { - return fmt.Errorf("display_name is %q expected %q", rs.Primary.Attributes["display_name"], displayName2) + if rs.Primary.Attributes["display_name"] != n { + return fmt.Errorf("display_name is %q expected %q", rs.Primary.Attributes["display_name"], n) } return nil } } -var testAccGoogleServiceAccount_basic = ` -resource "google_service_account" "acceptance" { +func testAccGoogleServiceAccountBasic(account, name string) string { + t := `resource "google_service_account" "acceptance" { account_id = "%v" display_name = "%v" -}` + }` + return fmt.Sprintf(t, account, name) +} -var testAccGoogleServiceAccount_policy = ` -resource "google_service_account" "acceptance" { +func testAccGoogleServiceAccountPolicy(account, name string) string { + + t := `resource "google_service_account" "acceptance" { account_id = "%v" display_name = "%v" policy_data = "${data.google_iam_policy.service_account.policy_data}" @@ -137,3 +138,6 @@ data "google_iam_policy" "service_account" { ] } }` + + return fmt.Sprintf(t, account, name, account, projectId) +} diff --git a/website/source/docs/providers/google/r/google_service_account.html.markdown b/website/source/docs/providers/google/r/google_service_account.html.markdown index c54594ef1..a1840c1f5 100644 --- a/website/source/docs/providers/google/r/google_service_account.html.markdown +++ b/website/source/docs/providers/google/r/google_service_account.html.markdown @@ -8,7 +8,7 @@ description: |- # google\_service\_account -Allows management of a Google Cloud Platform service account. +Allows management of a [Google Cloud Platform service account](https://cloud.google.com/compute/docs/access/service-accounts) ## Example Usage @@ -55,9 +55,8 @@ The following arguments are supported: Changing this updates the policy. - Deleting this removes the policy, but leaves the original policy - intact. If there are overlapping `binding` entries between the original - policy and the data source policy, they will be removed. + Deleting this removes the policy declared in Terraform. Any policy bindings + associated with the project before Terraform was used are not deleted. ## Attributes Reference