Add support for org policies at the organization level #523

rosbo · 2017-10-04T00:39:59Z

Fixes #507

rosbo · 2017-10-04T00:40:32Z

TF_ACC=1 go test ./google -v -run TestAccGoogleOrganizationPolicy_ -timeout 120m
=== RUN   TestAccGoogleOrganizationPolicy_import
--- PASS: TestAccGoogleOrganizationPolicy_import (2.38s)
=== RUN   TestAccGoogleOrganizationPolicy_boolean_enforced
--- PASS: TestAccGoogleOrganizationPolicy_boolean_enforced (1.70s)
=== RUN   TestAccGoogleOrganizationPolicy_boolean_notEnforced
--- PASS: TestAccGoogleOrganizationPolicy_boolean_notEnforced (1.80s)
=== RUN   TestAccGoogleOrganizationPolicy_boolean_update
--- PASS: TestAccGoogleOrganizationPolicy_boolean_update (4.30s)
=== RUN   TestAccGoogleOrganizationPolicy_list_allowAll
--- PASS: TestAccGoogleOrganizationPolicy_list_allowAll (2.20s)
=== RUN   TestAccGoogleOrganizationPolicy_list_allowSome
--- PASS: TestAccGoogleOrganizationPolicy_list_allowSome (1.80s)
=== RUN   TestAccGoogleOrganizationPolicy_list_denyAll
--- PASS: TestAccGoogleOrganizationPolicy_list_denyAll (1.80s)
=== RUN   TestAccGoogleOrganizationPolicy_list_denySome
--- PASS: TestAccGoogleOrganizationPolicy_list_denySome (1.90s)
=== RUN   TestAccGoogleOrganizationPolicy_list_update
--- PASS: TestAccGoogleOrganizationPolicy_list_update (4.84s)
PASS

TF_ACC=1 go test ./google -v -run TestAccGoogleFolder_rename -timeout 120m
=== RUN   TestAccGoogleFolder_rename
--- PASS: TestAccGoogleFolder_rename (16.88s)
PASS

rosbo · 2017-10-04T00:44:29Z

I wrote the preliminary documentation but wanted to first get an agreement on the schema structure before spending more time finishing it.

Specifically for List Policy. There is many ways we could structure the schema. The API structure is not user-friendly so I would rather not follow it.

Option A (the one I implemented)

... nested
 list_policy {
  allow {
    values = [
      "cloudresourcemanager.googleapis.com",
      "compute.googleapis.com",
    ]
  }
}

list_policy {
  deny {
    all = true
  }
}
...

Option B

... nested
list_policy {
  allow_values = [
      "cloudresourcemanager.googleapis.com",
      "compute.googleapis.com",
    ]
  }
}

list_policy {
  deny_all = true
}
...

rosbo · 2017-10-04T21:37:28Z

google/resource_google_organization_policy_test.go

+	})
+}
+
+func TestAccGoogleOrganizationPolicy_list_allowSome(t *testing.T) {


This test and the deny tests could have side effects with other tests running in parallel.

We have different options:

Run this test serially (can we annotate a test to force the test runner to run it serially?)

Find harmless constraint. For instance, allowing all services won't cause side effect both denying all potentially could.

Don't run on the CI server. Only run locally.

I will try 2) but I am not convinced I can find harmless cases for the 4 different cases (allow some, allow all, deny some, deny all)

I have been able to do 2). However, I haven't found a way to test the denyAll without side-effects but knowing the implementation, if the allowAll works, it is very likely that the denyAll works also, it is not ideal but good enough.

Also, the same methods (expandListOrganizationPolicy and flattenListOrganizationPolicy) will be used for org policies on folders. Since I can create a new folder easily, I will be able to test the denyAll case without side effects on other tests running in parallel.

danawillow · 2017-10-04T21:50:56Z

google/resource_google_organization_policy.go

+							MaxItems: 1,
+							Elem: &schema.Resource{
+								Schema: map[string]*schema.Schema{
+									"all": {


can you have this conflict with values? (should be possible with list_policy.0.allow.0.values)

Right, I thought ConflictsWith didn't work with list but when there is only one item in the list, it does. I will fix this.

danawillow · 2017-10-04T21:58:05Z

google/resource_google_organization_policy.go

+	deny := listPolicyMap["deny"].([]interface{})
+
+	if len(allow) > 0 && len(deny) > 0 {
+		return nil, fmt.Errorf("You cannot specified an `allow` and a `deny` block")


nit: specify

Code has been removed

danawillow · 2017-10-04T21:58:53Z

google/resource_google_organization_policy.go

+	allow := listPolicyMap["allow"].([]interface{})
+	deny := listPolicyMap["deny"].([]interface{})
+
+	if len(allow) > 0 && len(deny) > 0 {


can we do this check with conflictswith?

danawillow · 2017-10-04T22:01:34Z

google/resource_google_organization_policy.go

+	}, nil
+}
+
+func canonicalOrgPolicyConstraint(constraint string) string {


if we allow people to set a non-canonical version, wouldn't that lead to diffs next plan once the constraint comes back from the api with a canonical version? Should be an easy enough fix with a diffsuppress

There is already a DiffSuppress func set in the schema.

danawillow · 2017-10-04T22:04:06Z

google/resource_google_organization_policy_test.go

+			Constraint: constraint,
+		}).Do()
+
+		if err != nil {


wait, shouldn't this be the opposite? I'd expect a nil error to mean it exists.

Good catch. I fixed it. It basically always returns an org policy. If it has been cleared, both the boolean and list policy field are nil.

danawillow

looks good pending docs update

danawillow · 2017-10-04T23:39:36Z

website/docs/r/google_organization_policy.html.markdown

+
+* `version` - (Optional) Version of the Policy. Default version is 0.
+
+TODO(rosbo): Add boolean and list policy block once I get agreement on the schema format.


* Fetch latest resource manager client * Add new resource to manage Org Policy at the organization level. * Update documentation

ghost · 2020-03-30T14:33:17Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

rosbo added 3 commits October 2, 2017 15:59

Fetch latest resource manager client

ab4f96a

Add new resource to manage Org Policy at the organization level.

cc9226d

Add preliminary documentation

0f19b67

rosbo requested a review from danawillow October 4, 2017 00:39

rosbo commented Oct 4, 2017

View reviewed changes

danawillow suggested changes Oct 4, 2017

View reviewed changes

Addressed Dana's comment

7dd8440

danawillow approved these changes Oct 4, 2017

View reviewed changes

Update documentation

bf2eff1

rosbo merged commit 87f3a58 into hashicorp:master Oct 5, 2017

rosbo deleted the org-policies branch October 5, 2017 20:20

rosbo mentioned this pull request Oct 6, 2017

Configure constraints on a GCP folder #414

Closed

c8w mentioned this pull request Oct 24, 2017

Add support for yaml file to organization policy #627

Closed

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for org policies at the organization level #523

Add support for org policies at the organization level #523

rosbo commented Oct 4, 2017 •

edited

Loading

rosbo commented Oct 4, 2017

rosbo commented Oct 4, 2017

rosbo Oct 4, 2017 •

edited

Loading

rosbo Oct 4, 2017

danawillow Oct 4, 2017

rosbo Oct 4, 2017

rosbo Oct 4, 2017

danawillow Oct 4, 2017

rosbo Oct 4, 2017

danawillow Oct 4, 2017

rosbo Oct 4, 2017

danawillow Oct 4, 2017

rosbo Oct 4, 2017

danawillow Oct 4, 2017

rosbo Oct 4, 2017

danawillow left a comment

danawillow Oct 4, 2017

rosbo Oct 5, 2017

ghost commented Mar 30, 2020


		* `version` - (Optional) Version of the Policy. Default version is 0.

		TODO(rosbo): Add boolean and list policy block once I get agreement on the schema format.

Add support for org policies at the organization level #523

Add support for org policies at the organization level #523

Conversation

rosbo commented Oct 4, 2017 • edited Loading

rosbo commented Oct 4, 2017

rosbo commented Oct 4, 2017

rosbo Oct 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danawillow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Mar 30, 2020

rosbo commented Oct 4, 2017 •

edited

Loading

rosbo Oct 4, 2017 •

edited

Loading