From a32fe2d47fe9343c37c89d485de7742cef7804b9 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Thu, 17 Nov 2016 09:49:22 -0800 Subject: [PATCH] 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