Merge pull request #940 from GoogleCloudPlatform/jccb/test-org-polici…

…es-implementation Ensures the implementation of org policies is consistent
GoogleCloudPlatform · Nov 2, 2022 · 48b1053 · 48b1053
2 parents edc9fa7 + 38e761c
commit 48b1053
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 7 deletions.
diff --git a/modules/folder/organization-policies.tf b/modules/folder/organization-policies.tf
@@ -20,6 +20,9 @@ locals {
   org_policies = {
     for k, v in var.org_policies :
     k => merge(v, {
+      name   = "${local.folder.name}/policies/${k}"
+      parent = local.folder.name
+
       is_boolean_policy = v.allow == null && v.deny == null
       has_values = (
         length(coalesce(try(v.allow.values, []), [])) > 0 ||
@@ -40,8 +43,8 @@ locals {
 
 resource "google_org_policy_policy" "default" {
   for_each = local.org_policies
-  name     = "${local.folder.name}/policies/${each.key}"
-  parent   = local.folder.name
+  name     = each.value.name
+  parent   = each.value.parent
 
   spec {
     inherit_from_parent = each.value.inherit_from_parent

diff --git a/modules/organization/organization-policies.tf b/modules/organization/organization-policies.tf
@@ -20,6 +20,9 @@ locals {
   org_policies = {
     for k, v in var.org_policies :
     k => merge(v, {
+      name   = "${var.organization_id}/policies/${k}"
+      parent = var.organization_id
+
       is_boolean_policy = v.allow == null && v.deny == null
       has_values = (
         length(coalesce(try(v.allow.values, []), [])) > 0 ||
@@ -40,8 +43,8 @@ locals {
 
 resource "google_org_policy_policy" "default" {
   for_each = local.org_policies
-  name     = "${var.organization_id}/policies/${each.key}"
-  parent   = var.organization_id
+  name     = each.value.name
+  parent   = each.value.parent
 
   spec {
     inherit_from_parent = each.value.inherit_from_parent
@@ -99,5 +102,4 @@ resource "google_org_policy_policy" "default" {
     google_organization_iam_member.additive,
     google_organization_iam_policy.authoritative,
   ]
-
 }
diff --git a/modules/project/organization-policies.tf b/modules/project/organization-policies.tf
@@ -20,6 +20,9 @@ locals {
   org_policies = {
     for k, v in var.org_policies :
     k => merge(v, {
+      name   = "projects/${local.project.project_id}/policies/${k}"
+      parent = "projects/${local.project.project_id}"
+
       is_boolean_policy = v.allow == null && v.deny == null
       has_values = (
         length(coalesce(try(v.allow.values, []), [])) > 0 ||
@@ -40,8 +43,8 @@ locals {
 
 resource "google_org_policy_policy" "default" {
   for_each = local.org_policies
-  name     = "projects/${local.project.project_id}/policies/${each.key}"
-  parent   = "projects/${local.project.project_id}"
+  name     = each.value.name
+  parent   = each.value.parent
 
   spec {
     inherit_from_parent = each.value.inherit_from_parent

diff --git a/tests/modules/organization/test_plan_org_policies.py b/tests/modules/organization/test_plan_org_policies.py
@@ -12,6 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import difflib
+from pathlib import Path
+
 
 def test_policy_boolean(plan_runner):
   "Test boolean org policy."
@@ -225,3 +228,75 @@ def test_policy_list(plan_runner):
       'enforce': None,
       'values': []
   }
+
+
+def test_policy_implementation(plan_runner):
+  '''Verify org policy implementation is the same (except minor
+  differences) in the organization, folder and project modules.'''
+
+  modules_path = Path(__file__).parents[3] / 'modules'
+  lines = {}
+  for module in ['project', 'folder', 'organization']:
+    path = modules_path / module / 'organization-policies.tf'
+    lines[module] = path.open().readlines()
+
+  diff1 = difflib.unified_diff(lines['project'], lines['folder'])
+  assert list(diff1) == [
+      '--- \n',
+      '+++ \n',
+      '@@ -14,14 +14,14 @@\n',
+      '  * limitations under the License.\n',
+      '  */\n',
+      ' \n',
+      '-# tfdoc:file:description Project-level organization policies.\n',
+      '+# tfdoc:file:description Folder-level organization policies.\n',
+      ' \n',
+      ' locals {\n',
+      '   org_policies = {\n',
+      '     for k, v in var.org_policies :\n',
+      '     k => merge(v, {\n',
+      '-      name   = "projects/${local.project.project_id}/policies/${k}"\n',
+      '-      parent = "projects/${local.project.project_id}"\n',
+      '+      name   = "${local.folder.name}/policies/${k}"\n',
+      '+      parent = local.folder.name\n',
+      ' \n',
+      '       is_boolean_policy = v.allow == null && v.deny == null\n',
+      '       has_values = (\n',
+  ]
+
+  diff2 = difflib.unified_diff(lines['folder'], lines['organization'])
+  assert list(diff2) == [
+      '--- \n',
+      '+++ \n',
+      '@@ -14,14 +14,14 @@\n',
+      '  * limitations under the License.\n',
+      '  */\n',
+      ' \n',
+      '-# tfdoc:file:description Folder-level organization policies.\n',
+      '+# tfdoc:file:description Organization-level organization policies.\n',
+      ' \n',
+      ' locals {\n',
+      '   org_policies = {\n',
+      '     for k, v in var.org_policies :\n',
+      '     k => merge(v, {\n',
+      '-      name   = "${local.folder.name}/policies/${k}"\n',
+      '-      parent = local.folder.name\n',
+      '+      name   = "${var.organization_id}/policies/${k}"\n',
+      '+      parent = var.organization_id\n',
+      ' \n',
+      '       is_boolean_policy = v.allow == null && v.deny == null\n',
+      '       has_values = (\n',
+      '@@ -94,4 +94,12 @@\n',
+      '       }\n',
+      '     }\n',
+      '   }\n',
+      '+\n',
+      '+  depends_on = [\n',
+      '+    google_organization_iam_audit_config.config,\n',
+      '+    google_organization_iam_binding.authoritative,\n',
+      '+    google_organization_iam_custom_role.roles,\n',
+      '+    google_organization_iam_member.additive,\n',
+      '+    google_organization_iam_policy.authoritative,\n',
+      '+  ]\n',
+      ' }\n',
+  ]