From f34848f974e2944409edf2fb315de3b8fa5cc2f2 Mon Sep 17 00:00:00 2001 From: The Magician Date: Thu, 16 May 2019 13:10:44 -0700 Subject: [PATCH] Document that a policy must be defined. (#3611) Signed-off-by: Modular Magician --- ...ource_google_folder_organization_policy.go | 12 +++++-- google/resource_google_organization_policy.go | 25 +++++++++++++-- ...urce_google_project_organization_policy.go | 12 +++++-- ...google_project_organization_policy_test.go | 31 +++++++++++++++++++ ...e_folder_organization_policy.html.markdown | 3 ++ .../google_organization_policy.html.markdown | 9 ++++-- ..._project_organization_policy.html.markdown | 3 ++ 7 files changed, 86 insertions(+), 9 deletions(-) diff --git a/google/resource_google_folder_organization_policy.go b/google/resource_google_folder_organization_policy.go index 8e4b2bea..ebf367fd 100644 --- a/google/resource_google_folder_organization_policy.go +++ b/google/resource_google_folder_organization_policy.go @@ -51,12 +51,16 @@ func resourceFolderOrgPolicyImporter(d *schema.ResourceData, meta interface{}) ( } func resourceGoogleFolderOrganizationPolicyCreate(d *schema.ResourceData, meta interface{}) error { + d.SetId(fmt.Sprintf("%s:%s", d.Get("folder"), d.Get("constraint"))) + + if isOrganizationPolicyUnset(d) { + return resourceGoogleFolderOrganizationPolicyDelete(d, meta) + } + if err := setFolderOrganizationPolicy(d, meta); err != nil { return err } - d.SetId(fmt.Sprintf("%s:%s", d.Get("folder"), d.Get("constraint"))) - return resourceGoogleFolderOrganizationPolicyRead(d, meta) } @@ -84,6 +88,10 @@ func resourceGoogleFolderOrganizationPolicyRead(d *schema.ResourceData, meta int } func resourceGoogleFolderOrganizationPolicyUpdate(d *schema.ResourceData, meta interface{}) error { + if isOrganizationPolicyUnset(d) { + return resourceGoogleFolderOrganizationPolicyDelete(d, meta) + } + if err := setFolderOrganizationPolicy(d, meta); err != nil { return err } diff --git a/google/resource_google_organization_policy.go b/google/resource_google_organization_policy.go index 5922ad3a..2c7f96e1 100644 --- a/google/resource_google_organization_policy.go +++ b/google/resource_google_organization_policy.go @@ -144,12 +144,16 @@ func resourceGoogleOrganizationPolicy() *schema.Resource { } func resourceGoogleOrganizationPolicyCreate(d *schema.ResourceData, meta interface{}) error { + d.SetId(fmt.Sprintf("%s:%s", d.Get("org_id"), d.Get("constraint").(string))) + + if isOrganizationPolicyUnset(d) { + return resourceGoogleOrganizationPolicyDelete(d, meta) + } + if err := setOrganizationPolicy(d, meta); err != nil { return err } - d.SetId(fmt.Sprintf("%s:%s", d.Get("org_id"), d.Get("constraint").(string))) - return resourceGoogleOrganizationPolicyRead(d, meta) } @@ -177,6 +181,10 @@ func resourceGoogleOrganizationPolicyRead(d *schema.ResourceData, meta interface } func resourceGoogleOrganizationPolicyUpdate(d *schema.ResourceData, meta interface{}) error { + if isOrganizationPolicyUnset(d) { + return resourceGoogleOrganizationPolicyDelete(d, meta) + } + if err := setOrganizationPolicy(d, meta); err != nil { return err } @@ -211,6 +219,19 @@ func resourceGoogleOrganizationPolicyImportState(d *schema.ResourceData, meta in return []*schema.ResourceData{d}, nil } +// Organization policies can be "inherited from parent" the UI, and this is the default +// state of the resource without any policy set. In order to revert to this state the current +// resource cannot be updated it must instead be Deleted. This allows Terraform to assert that +// no policy has been set even if previously one had. +// See https://github.com/terraform-providers/terraform-provider-google/issues/3607 +func isOrganizationPolicyUnset(d *schema.ResourceData) bool { + listPolicy := d.Get("list_policy").([]interface{}) + booleanPolicy := d.Get("boolean_policy").([]interface{}) + restorePolicy := d.Get("restore_policy").([]interface{}) + + return len(listPolicy)+len(booleanPolicy)+len(restorePolicy) == 0 +} + func setOrganizationPolicy(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) org := "organizations/" + d.Get("org_id").(string) diff --git a/google/resource_google_project_organization_policy.go b/google/resource_google_project_organization_policy.go index c72c0f74..f509782a 100644 --- a/google/resource_google_project_organization_policy.go +++ b/google/resource_google_project_organization_policy.go @@ -50,12 +50,16 @@ func resourceProjectOrgPolicyImporter(d *schema.ResourceData, meta interface{}) } func resourceGoogleProjectOrganizationPolicyCreate(d *schema.ResourceData, meta interface{}) error { + d.SetId(fmt.Sprintf("%s:%s", d.Get("project"), d.Get("constraint"))) + + if isOrganizationPolicyUnset(d) { + return resourceGoogleProjectOrganizationPolicyDelete(d, meta) + } + if err := setProjectOrganizationPolicy(d, meta); err != nil { return err } - d.SetId(fmt.Sprintf("%s:%s", d.Get("project"), d.Get("constraint"))) - return resourceGoogleProjectOrganizationPolicyRead(d, meta) } @@ -83,6 +87,10 @@ func resourceGoogleProjectOrganizationPolicyRead(d *schema.ResourceData, meta in } func resourceGoogleProjectOrganizationPolicyUpdate(d *schema.ResourceData, meta interface{}) error { + if isOrganizationPolicyUnset(d) { + return resourceGoogleProjectOrganizationPolicyDelete(d, meta) + } + if err := setProjectOrganizationPolicy(d, meta); err != nil { return err } diff --git a/google/resource_google_project_organization_policy_test.go b/google/resource_google_project_organization_policy_test.go index 319b92d1..415dfcb5 100644 --- a/google/resource_google_project_organization_policy_test.go +++ b/google/resource_google_project_organization_policy_test.go @@ -22,6 +22,7 @@ func TestAccProjectOrganizationPolicy(t *testing.T) { "list_denySome": testAccProjectOrganizationPolicy_list_denySome, "list_update": testAccProjectOrganizationPolicy_list_update, "restore_policy": testAccProjectOrganizationPolicy_restore_defaultTrue, + "empty_policy": testAccProjectOrganizationPolicy_none, } for name, tc := range testCases { @@ -179,6 +180,27 @@ func testAccProjectOrganizationPolicy_restore_defaultTrue(t *testing.T) { }) } +func testAccProjectOrganizationPolicy_none(t *testing.T) { + projectId := getTestProjectFromEnv() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckGoogleProjectOrganizationPolicyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccProjectOrganizationPolicyConfig_none(projectId), + Check: testAccCheckGoogleProjectOrganizationPolicyDestroy, + }, + { + ResourceName: "google_project_organization_policy.none", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func testAccCheckGoogleProjectOrganizationPolicyDestroy(s *terraform.State) error { config := testAccProvider.Meta().(*Config) @@ -387,6 +409,15 @@ resource "google_project_organization_policy" "restore" { `, pid) } +func testAccProjectOrganizationPolicyConfig_none(pid string) string { + return fmt.Sprintf(` +resource "google_project_organization_policy" "none" { + project = "%s" + constraint = "constraints/serviceuser.services" +} +`, pid) +} + func canonicalProjectId(project string) string { if strings.HasPrefix(project, "projects/") { return project diff --git a/website/docs/r/google_folder_organization_policy.html.markdown b/website/docs/r/google_folder_organization_policy.html.markdown index 3ada600a..99461b41 100644 --- a/website/docs/r/google_folder_organization_policy.html.markdown +++ b/website/docs/r/google_folder_organization_policy.html.markdown @@ -94,6 +94,9 @@ can also be used to allow or deny all values. Structure is documented below. * `restore_policy` - (Optional) A restore policy is a constraint to restore the default policy. Structure is documented below. +~> **Note:** If none of [`boolean_policy`, `list_policy`, `restore_policy`] are defined the policy for a given constraint will +effectively be unset. This is represented in the UI as the constraint being 'Inherited'. + - - - The `boolean_policy` block supports: diff --git a/website/docs/r/google_organization_policy.html.markdown b/website/docs/r/google_organization_policy.html.markdown index 2e939fd4..424c094a 100644 --- a/website/docs/r/google_organization_policy.html.markdown +++ b/website/docs/r/google_organization_policy.html.markdown @@ -86,11 +86,14 @@ The following arguments are supported: * `version` - (Optional) Version of the Policy. Default version is 0. -* `boolean_policy` - (Optional) A boolean policy is a constraint that is either enforced or not. Structure is documented below. +* `boolean_policy` - (Optional) A boolean policy is a constraint that is either enforced or not. Structure is documented below. * `list_policy` - (Optional) A policy that can define specific values that are allowed or denied for the given constraint. It can also be used to allow or deny all values. Structure is documented below. -* `restore_policy` - (Optional) A restore policy is a constraint to restore the default policy. Structure is documented below. +* `restore_policy` - (Optional) A restore policy is a constraint to restore the default policy. Structure is documented below. + +~> **Note:** If none of [`boolean_policy`, `list_policy`, `restore_policy`] are defined the policy for a given constraint will +effectively be unset. This is represented in the UI as the constraint being 'Inherited'. - - - @@ -122,7 +125,7 @@ The `restore_policy` block supports: In addition to the arguments listed above, the following computed attributes are exported: -* `etag` - (Computed) The etag of the organization policy. `etag` is used for optimistic concurrency control as a way to help prevent simultaneous updates of a policy from overwriting each other. +* `etag` - (Computed) The etag of the organization policy. `etag` is used for optimistic concurrency control as a way to help prevent simultaneous updates of a policy from overwriting each other. * `update_time` - (Computed) The timestamp in RFC3339 UTC "Zulu" format, accurate to nanoseconds, representing when the variable was last updated. Example: "2016-10-09T12:33:37.578138407Z". diff --git a/website/docs/r/google_project_organization_policy.html.markdown b/website/docs/r/google_project_organization_policy.html.markdown index 07c266f8..7c45da31 100644 --- a/website/docs/r/google_project_organization_policy.html.markdown +++ b/website/docs/r/google_project_organization_policy.html.markdown @@ -93,6 +93,9 @@ The following arguments are supported: * `restore_policy` - (Optional) A restore policy is a constraint to restore the default policy. Structure is documented below. +~> **Note:** If none of [`boolean_policy`, `list_policy`, `restore_policy`] are defined the policy for a given constraint will +effectively be unset. This is represented in the UI as the constraint being 'Inherited'. + - - - The `boolean_policy` block supports: