From 630b245770c302b4d196a1d17d0a0235380fa6bc Mon Sep 17 00:00:00 2001 From: Paddy Date: Mon, 13 Mar 2017 16:19:53 -0700 Subject: [PATCH 1/2] provider/google: Remove deprecated project fields. Remove the deprecated fields from google_project, and drop all the logic that went into supporting them. Tests still pass after one minor change. --- .../google/resource_google_project.go | 195 ++---------------- .../google/resource_google_project_test.go | 27 +-- 2 files changed, 20 insertions(+), 202 deletions(-) diff --git a/builtin/providers/google/resource_google_project.go b/builtin/providers/google/resource_google_project.go index b4bcb9c4f..9b947a66c 100644 --- a/builtin/providers/google/resource_google_project.go +++ b/builtin/providers/google/resource_google_project.go @@ -1,7 +1,6 @@ package google import ( - "encoding/json" "fmt" "log" "net/http" @@ -16,13 +15,6 @@ import ( // resourceGoogleProject returns a *schema.Resource that allows a customer // to declare a Google Cloud Project resource. -// -// This example shows a project with a policy declared in config: -// -// resource "google_project" "my-project" { -// project = "a-project-id" -// policy = "${data.google_iam_policy.admin.policy}" -// } func resourceGoogleProject() *schema.Resource { return &schema.Resource{ SchemaVersion: 1, @@ -39,22 +31,15 @@ func resourceGoogleProject() *schema.Resource { Schema: map[string]*schema.Schema{ "id": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - Computed: true, - Deprecated: "The id field has unexpected behaviour and probably doesn't do what you expect. See https://www.terraform.io/docs/providers/google/r/google_project.html#id-field for more information. Please use project_id instead; future versions of Terraform will remove the id field.", + Type: schema.TypeString, + Optional: true, + Computed: true, + Removed: "The id field has been removed. Use project_id instead.", }, "project_id": &schema.Schema{ Type: schema.TypeString, - Optional: true, + Required: true, ForceNew: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // This suppresses the diff if project_id is not set - if new == "" { - return true - } - return false - }, }, "skip_delete": &schema.Schema{ Type: schema.TypeBool, @@ -63,26 +48,23 @@ func resourceGoogleProject() *schema.Resource { }, "name": &schema.Schema{ Type: schema.TypeString, - Optional: true, - Computed: true, + Required: true, }, "org_id": &schema.Schema{ Type: schema.TypeString, - Optional: true, - Computed: true, + Required: true, ForceNew: true, }, "policy_data": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - Computed: true, - Deprecated: "Use the 'google_project_iam_policy' resource to define policies for a Google Project", - DiffSuppressFunc: jsonPolicyDiffSuppress, + Type: schema.TypeString, + Optional: true, + Computed: true, + Removed: "Use the 'google_project_iam_policy' resource to define policies for a Google Project", }, "policy_etag": &schema.Schema{ - Type: schema.TypeString, - Computed: true, - Deprecated: "Use the the 'google_project_iam_policy' resource to define policies for a Google Project", + Type: schema.TypeString, + Computed: true, + Removed: "Use the the 'google_project_iam_policy' resource to define policies for a Google Project", }, "number": &schema.Schema{ Type: schema.TypeString, @@ -102,27 +84,6 @@ func resourceGoogleProjectCreate(d *schema.ResourceData, meta interface{}) error var pid string var err error pid = d.Get("project_id").(string) - if pid == "" { - pid, err = getProject(d, config) - if err != nil { - return fmt.Errorf("Error getting project ID: %v", err) - } - if pid == "" { - return fmt.Errorf("'project_id' must be set in the config") - } - } - - // we need to check if name and org_id are set, and throw an error if they aren't - // we can't just set these as required on the object, however, as that would break - // all configs that used previous iterations of the resource. - // TODO(paddy): remove this for 0.9 and set these attributes as required. - name, org_id := d.Get("name").(string), d.Get("org_id").(string) - if name == "" { - return fmt.Errorf("`name` must be set in the config if you're creating a project.") - } - if org_id == "" { - return fmt.Errorf("`org_id` must be set in the config if you're creating a project.") - } log.Printf("[DEBUG]: Creating new project %q", pid) project := &cloudresourcemanager.Project{ @@ -147,37 +108,6 @@ func resourceGoogleProjectCreate(d *schema.ResourceData, meta interface{}) error return waitErr } - // 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 cloudresourcemanager.Policy - if err := json.Unmarshal([]byte(pString.(string)), &policy); err != nil { - return err - } - log.Printf("[DEBUG] Got policy from config: %#v", policy.Bindings) - - // Retrieve existing IAM policy from project. This will be merged - // with the policy defined here. - p, err := getProjectIamPolicy(pid, config) - if err != nil { - return err - } - log.Printf("[DEBUG] Got existing bindings from project: %#v", p.Bindings) - - // Merge the existing policy bindings with those defined in this manifest. - p.Bindings = mergeBindings(append(p.Bindings, policy.Bindings...)) - - // Apply the merged policy - log.Printf("[DEBUG] Setting new policy for project: %#v", p) - _, err = config.clientResourceManager.Projects.SetIamPolicy(pid, - &cloudresourcemanager.SetIamPolicyRequest{Policy: p}).Do() - - if err != nil { - return fmt.Errorf("Error applying IAM policy for project %q: %s", pid, err) - } - } - // Set the billing account if v, ok := d.GetOk("billing_account"); ok { name := v.(string) @@ -242,6 +172,7 @@ func resourceGoogleProjectRead(d *schema.ResourceData, meta interface{}) error { func prefixedProject(pid string) string { return "projects/" + pid } + func resourceGoogleProjectUpdate(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) pid := d.Id() @@ -282,7 +213,7 @@ func resourceGoogleProjectUpdate(d *schema.ResourceData, meta interface{}) error return fmt.Errorf("Error updating billing account %q for project %q: %v", name, prefixedProject(pid), err) } } - return updateProjectIamPolicy(d, config, pid) + return nil } func resourceGoogleProjectDelete(d *schema.ResourceData, meta interface{}) error { @@ -298,97 +229,3 @@ func resourceGoogleProjectDelete(d *schema.ResourceData, meta interface{}) error d.SetId("") return nil } - -func updateProjectIamPolicy(d *schema.ResourceData, config *Config, pid string) error { - // Policy has changed - 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", oldPString, newPString) - - var oldPolicy, newPolicy cloudresourcemanager.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 := rolesToMembersMap(oldPolicy.Bindings) - newMap := rolesToMembersMap(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 := getProjectIamPolicy(pid, config) - if err != nil { - return err - } - log.Printf("[DEBUG] Got existing bindings from project: %#v", p.Bindings) - - // Merge existing policy with policy in the current state - log.Printf("[DEBUG] Merging new bindings from project: %#v", newPolicy.Bindings) - mergedBindings := mergeBindings(append(p.Bindings, newPolicy.Bindings...)) - - // Remove any roles and members that were explicitly deleted - mergedBindingsMap := rolesToMembersMap(mergedBindings) - for role, members := range deleted { - for member, _ := range members { - delete(mergedBindingsMap[role], member) - } - } - - p.Bindings = rolesToMembersBinding(mergedBindingsMap) - dump, _ := json.MarshalIndent(p.Bindings, " ", " ") - log.Printf("[DEBUG] Setting new policy for project: %#v:\n%s", p, string(dump)) - - _, err = config.clientResourceManager.Projects.SetIamPolicy(pid, - &cloudresourcemanager.SetIamPolicyRequest{Policy: p}).Do() - - if err != nil { - return fmt.Errorf("Error applying IAM policy for project %q: %s", pid, err) - } - } - return nil -} diff --git a/builtin/providers/google/resource_google_project_test.go b/builtin/providers/google/resource_google_project_test.go index 8381cb334..351a468fd 100644 --- a/builtin/providers/google/resource_google_project_test.go +++ b/builtin/providers/google/resource_google_project_test.go @@ -214,35 +214,16 @@ resource "google_project" "acceptance" { `, pid) } -func testAccGoogleProjectImportExistingWithIam(pid string) string { - return fmt.Sprintf(` -resource "google_project" "acceptance" { - project_id = "%v" - policy_data = "${data.google_iam_policy.admin.policy_data}" -} -data "google_iam_policy" "admin" { - binding { - role = "roles/storage.objectViewer" - members = [ - "user:evanbrown@google.com", - ] - } - binding { - role = "roles/compute.instanceAdmin" - members = [ - "user:evanbrown@google.com", - "user:evandbrown@gmail.com", - ] - } -}`, pid) -} - func testAccGoogleProject_toMerge(pid, name, org string) string { return fmt.Sprintf(` resource "google_project" "acceptance" { project_id = "%s" name = "%s" org_id = "%s" +} + +resource "google_project_iam_policy" "acceptance" { + project = "${google_project.acceptance.project_id}" policy_data = "${data.google_iam_policy.acceptance.policy_data}" } From c14fc480ba9ed087a91014ecf6a041e43fd73231 Mon Sep 17 00:00:00 2001 From: Paddy Date: Tue, 14 Mar 2017 16:38:40 -0700 Subject: [PATCH 2/2] Prune dead function. This function isn't called anymore, so let's get rid of it. --- builtin/providers/google/resource_google_project_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/builtin/providers/google/resource_google_project_test.go b/builtin/providers/google/resource_google_project_test.go index 351a468fd..fea4c7465 100644 --- a/builtin/providers/google/resource_google_project_test.go +++ b/builtin/providers/google/resource_google_project_test.go @@ -205,15 +205,6 @@ func testAccCheckGoogleProjectHasMoreBindingsThan(pid string, count int) resourc } } -func testAccGoogleProjectImportExisting(pid string) string { - return fmt.Sprintf(` -resource "google_project" "acceptance" { - project_id = "%s" - -} -`, pid) -} - func testAccGoogleProject_toMerge(pid, name, org string) string { return fmt.Sprintf(` resource "google_project" "acceptance" {