From 4d2b136a12573f98797d787ef0241294423bff71 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Mon, 6 Feb 2017 14:16:22 -0800 Subject: [PATCH 1/2] providers/google: Fix google_project IAM bug This changes removes read of the deprecated `policy_data` attr in the `google_project` resource. 0.8.5 introduced new behavior that incorrectly read the `policy_data` field during the read lifecycle event. This caused Terraform to assume it owned not just policy defined in the data source, but everything that was associated with the project. Migrating from 0.8.4 to 0.8.5, this would cause the config (partial) to be compared to the state (complete, as it was read from the API) and assume some policies had been explicitly deleted. Terraform would then delete them. Fixes #11556 --- resource_google_project.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/resource_google_project.go b/resource_google_project.go index 4bc26c45..24dc56b8 100644 --- a/resource_google_project.go +++ b/resource_google_project.go @@ -196,20 +196,6 @@ func resourceGoogleProjectRead(d *schema.ResourceData, meta interface{}) error { d.Set("org_id", p.Parent.Id) } - // Read the IAM policy - pol, err := getProjectIamPolicy(pid, config) - if err != nil { - return err - } - - polBytes, err := json.Marshal(pol) - if err != nil { - return err - } - - d.Set("policy_etag", pol.Etag) - d.Set("policy_data", string(polBytes)) - return nil } From 6dfca0f836b04053f03781943c6235ece2609295 Mon Sep 17 00:00:00 2001 From: Paddy Date: Mon, 6 Feb 2017 22:09:53 -0800 Subject: [PATCH 2/2] Add a test that would have caught backwards incompatibility. Add a test that would have caught the backwards incompatibility where project IAM bindings aren't merged, but are overwritten. --- resource_google_project_test.go | 69 +++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/resource_google_project_test.go b/resource_google_project_test.go index aa3c03c5..03bdeee0 100644 --- a/resource_google_project_test.go +++ b/resource_google_project_test.go @@ -48,6 +48,34 @@ func TestAccGoogleProject_create(t *testing.T) { }) } +// Test that a Project resource merges the IAM policies that already +// exist, and won't lock people out. +func TestAccGoogleProject_merge(t *testing.T) { + pid := "terraform-" + acctest.RandString(10) + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + // when policy_data is set, merge + { + Config: testAccGoogleProject_toMerge(pid, pname, org), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleProjectExists("google_project.acceptance", pid), + testAccCheckGoogleProjectHasMoreBindingsThan(pid, 1), + ), + }, + // when policy_data is unset, restore to what it was + { + Config: testAccGoogleProject_mergeEmpty(pid, pname, org), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleProjectExists("google_project.acceptance", pid), + testAccCheckGoogleProjectHasMoreBindingsThan(pid, 0), + ), + }, + }, + }) +} + func testAccCheckGoogleProjectExists(r, pid string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[r] @@ -67,6 +95,19 @@ func testAccCheckGoogleProjectExists(r, pid string) resource.TestCheckFunc { } } +func testAccCheckGoogleProjectHasMoreBindingsThan(pid string, count int) resource.TestCheckFunc { + return func(s *terraform.State) error { + policy, err := getProjectIamPolicy(pid, testAccProvider.Meta().(*Config)) + if err != nil { + return err + } + if len(policy.Bindings) <= count { + return fmt.Errorf("Expected more than %d bindings, got %d: %#v", count, len(policy.Bindings), policy.Bindings) + } + return nil + } +} + func testAccGoogleProjectImportExisting(pid string) string { return fmt.Sprintf(` resource "google_project" "acceptance" { @@ -98,3 +139,31 @@ data "google_iam_policy" "admin" { } }`, pid) } + +func testAccGoogleProject_toMerge(pid, name, org string) string { + return fmt.Sprintf(` +resource "google_project" "acceptance" { + project_id = "%s" + name = "%s" + org_id = "%s" + policy_data = "${data.google_iam_policy.acceptance.policy_data}" +} + +data "google_iam_policy" "acceptance" { + binding { + role = "roles/storage.objectViewer" + members = [ + "user:evanbrown@google.com", + ] + } +}`, pid, name, org) +} + +func testAccGoogleProject_mergeEmpty(pid, name, org string) string { + return fmt.Sprintf(` +resource "google_project" "acceptance" { + project_id = "%s" + name = "%s" + org_id = "%s" +}`, pid, name, org) +}