diff --git a/google/resource_google_service_account.go b/google/resource_google_service_account.go index 146ffa08..14cb347a 100644 --- a/google/resource_google_service_account.go +++ b/google/resource_google_service_account.go @@ -1,9 +1,7 @@ package google import ( - "encoding/json" "fmt" - "log" "strings" "github.com/hashicorp/terraform/helper/schema" @@ -49,9 +47,9 @@ func resourceGoogleServiceAccount() *schema.Resource { ForceNew: true, }, "policy_data": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - Deprecated: "Use the 'google_service_account_iam_policy' resource to define policies for a service account", + Type: schema.TypeString, + Optional: true, + Removed: "Use the 'google_service_account_iam_policy' resource to define policies for a service account", }, }, } @@ -82,36 +80,6 @@ func resourceGoogleServiceAccountCreate(d *schema.ResourceData, meta interface{} 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: overwrite existing policy, instead of merging it - 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) } @@ -146,7 +114,6 @@ func resourceGoogleServiceAccountDelete(d *schema.ResourceData, meta interface{} 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 { @@ -162,95 +129,6 @@ func resourceGoogleServiceAccountUpdate(d *schema.ResourceData, meta interface{} } } - 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 = "{}" - } - - 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 { - 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: overwrite existing policy instead of merging it - 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 } diff --git a/google/resource_google_service_account_test.go b/google/resource_google_service_account_test.go index 6e2f5009..d1279d58 100644 --- a/google/resource_google_service_account_test.go +++ b/google/resource_google_service_account_test.go @@ -69,61 +69,6 @@ func TestAccServiceAccount_basic(t *testing.T) { }) } -// Test that a service account resource can be created with a policy, updated, -// and destroyed. -func TestAccServiceAccount_createPolicy(t *testing.T) { - t.Parallel() - - 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: testAccServiceAccountPolicy(accountId, getTestProjectFromEnv()), - Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 1), - ), - }, - resource.TestStep{ - ResourceName: "google_service_account.acceptance", - ImportState: true, - ImportStateVerify: true, - // policy_data isn't a field on the service account object, and so isn't set in state. - ImportStateVerifyIgnore: []string{"policy_data"}, - }, - // The second step updates the service account with no IAM policy - resource.TestStep{ - Config: testAccServiceAccountBasic(accountId, displayName), - Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 0), - ), - }, - resource.TestStep{ - ResourceName: "google_service_account.acceptance", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"policy_data"}, - }, - // The final step re-applies the IAM policy - resource.TestStep{ - Config: testAccServiceAccountPolicy(accountId, getTestProjectFromEnv()), - Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleServiceAccountPolicyCount("google_service_account.acceptance", 1), - ), - }, - resource.TestStep{ - ResourceName: "google_service_account.acceptance", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"policy_data"}, - }, - }, - }) -} - func testAccStoreServiceAccountUniqueId(uniqueId *string) resource.TestCheckFunc { return func(s *terraform.State) error { *uniqueId = s.RootModule().Resources["google_service_account.acceptance"].Primary.Attributes["unique_id"] @@ -131,20 +76,6 @@ func testAccStoreServiceAccountUniqueId(uniqueId *string) resource.TestCheckFunc } } -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 testAccServiceAccountBasic(account, name string) string { return fmt.Sprintf(` resource "google_service_account" "acceptance" { @@ -169,7 +100,6 @@ func testAccServiceAccountPolicy(account, project string) string { 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" { diff --git a/website/docs/r/google_service_account.html.markdown b/website/docs/r/google_service_account.html.markdown index ed6af779..5e6bf41c 100644 --- a/website/docs/r/google_service_account.html.markdown +++ b/website/docs/r/google_service_account.html.markdown @@ -35,15 +35,6 @@ The following arguments are supported: * `project` - (Optional) The ID of the project that the service account will be created in. Defaults to the provider project configuration. -* `policy_data` - (DEPRECATED, 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. - - This attribute has been deprecated. Use the [google_service_account_iam_* resources](google_service_account_iam.html) instead. - - Deleting this removes the policy declared in Terraform. Any policy bindings - associated with the project before Terraform was used are not deleted. - ## Attributes Reference In addition to the arguments listed above, the following computed attributes are