Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow organization policies to be removed/unset from a constraint #3611

Merged
merged 1 commit into from
May 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions google/resource_google_folder_organization_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}
Expand Down
25 changes: 23 additions & 2 deletions google/resource_google_organization_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 10 additions & 2 deletions google/resource_google_project_organization_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}
Expand Down
31 changes: 31 additions & 0 deletions google/resource_google_project_organization_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 6 additions & 3 deletions website/docs/r/google_organization_policy.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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'.

- - -

Expand Down Expand Up @@ -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".

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down