Organization module end-to-end tests (#1860)

* added tag serial to mark tests to be run serially * always run tests using loadgroup distribution to make use of serial tag * added end-to-end tests for organization, not adding to custom constraints as the name has to be unique * fixed granting custom roles created in the same module call
GoogleCloudPlatform · Nov 14, 2023 · 03bf0b1 · 03bf0b1
1 parent 1c2f1c7
commit 03bf0b1
Show file tree

Hide file tree

Showing 10 changed files with 66 additions and 81 deletions.
diff --git a/modules/organization/README.md b/modules/organization/README.md
@@ -35,16 +35,16 @@ To manage organization policies, the `orgpolicy.googleapis.com` service should b
 ```hcl
 module "org" {
   source          = "./fabric/modules/organization"
-  organization_id = "organizations/1234567890"
+  organization_id = var.organization_id
   group_iam = {
-    "[email protected]" = ["roles/owner", "roles/projectCreator"]
+    (var.group_email) = ["roles/owner"]
   }
   iam = {
-    "roles/resourcemanager.projectCreator" = ["group:[email protected]"]
+    "roles/resourcemanager.projectCreator" = ["group:${var.group_email}"]
   }
   iam_bindings_additive = {
     am1-storage-admin = {
-      member = "user:[email protected]"
+      member = "group:${var.group_email}"
       role   = "roles/storage.admin"
     }
   }
@@ -57,9 +57,6 @@ module "org" {
     }
   }
   org_policies = {
-    "custom.gkeEnableAutoUpgrade" = {
-      rules = [{ enforce = true }]
-    }
     "compute.disableGuestAttributesAccess" = {
       rules = [{ enforce = true }]
     }
@@ -118,7 +115,7 @@ module "org" {
     }
   }
 }
-# tftest modules=1 resources=15 inventory=basic.yaml
+# tftest modules=1 resources=13 inventory=basic.yaml e2e serial
 ```
 
 ## IAM
@@ -262,7 +259,7 @@ module "org" {
     policy = module.firewall-policy.id
   }
 }
-# tftest modules=2 resources=2
+# tftest modules=2 resources=2 e2e serial
 ```
 
 ## Log Sinks
@@ -273,6 +270,7 @@ The following example shows how to define organization-level log sinks:
 module "gcs" {
   source        = "./fabric/modules/gcs"
   project_id    = var.project_id
+  prefix        = var.prefix
   name          = "gcs_sink"
   force_destroy = true
 }
@@ -292,7 +290,7 @@ module "pubsub" {
 module "bucket" {
   source      = "./fabric/modules/logging-bucket"
   parent_type = "project"
-  parent      = "my-project"
+  parent      = var.project_id
   id          = "bucket"
 }
 
@@ -330,7 +328,7 @@ module "org" {
     no-gce-instances = "resource.type=gce_instance"
   }
 }
-# tftest modules=5 resources=13 inventory=logging.yaml
+# tftest modules=5 resources=13 inventory=logging.yaml e2e serial
 ```
 
 ## Data Access Logs
@@ -344,20 +342,20 @@ module "org" {
   logging_data_access = {
     allServices = {
       # logs for principals listed here will be excluded
-      ADMIN_READ = ["group:[email protected]"]
+      ADMIN_READ = ["group:${var.group_email}"]
     }
     "storage.googleapis.com" = {
       DATA_READ  = []
       DATA_WRITE = []
     }
   }
 }
-# tftest modules=1 resources=2 inventory=logging-data-access.yaml
+# tftest modules=1 resources=2 inventory=logging-data-access.yaml e2e serial
 ```
 
 ## Custom Roles
 
-Custom roles can be defined via the `custom_roles` variable, and referenced via the `custom_role_id` output:
+Custom roles can be defined via the `custom_roles` variable, and referenced via the `custom_role_id` output (this also provides explicit dependency on the custom role):
 
 ```hcl
 module "org" {
@@ -369,10 +367,10 @@ module "org" {
     ]
   }
   iam = {
-    (module.org.custom_role_id.myRole) = ["user:[email protected]"]
+    (module.org.custom_role_id.myRole) = ["group:${var.group_email}"]
   }
 }
-# tftest modules=1 resources=2 inventory=roles.yaml
+# tftest modules=1 resources=2 inventory=roles.yaml e2e serial
 ```
 
 ## Tags
@@ -387,25 +385,24 @@ module "org" {
     environment = {
       description = "Environment specification."
       iam = {
-        "roles/resourcemanager.tagAdmin" = ["group:[email protected]"]
+        "roles/resourcemanager.tagAdmin" = ["group:${var.group_email}"]
       }
       values = {
         dev = {}
         prod = {
           description = "Environment: production."
           iam = {
-            "roles/resourcemanager.tagViewer" = ["user:[email protected]"]
+            "roles/resourcemanager.tagViewer" = ["group:${var.group_email}"]
           }
         }
       }
     }
   }
   tag_bindings = {
     env-prod = module.org.tag_values["environment/prod"].id
-    foo      = "tagValues/12345678"
   }
 }
-# tftest modules=1 resources=7 inventory=tags.yaml
+# tftest modules=1 resources=6 inventory=tags.yaml e2e serial
 ```
 
 You can also define network tags, through a dedicated variable *network_tags*:
@@ -417,23 +414,23 @@ module "org" {
   network_tags = {
     net-environment = {
       description = "This is a network tag."
-      network     = "my_project/my_vpc"
+      network     = "${var.project_id}/${var.vpc.name}"
       iam = {
-        "roles/resourcemanager.tagAdmin" = ["group:[email protected]"]
+        "roles/resourcemanager.tagAdmin" = ["group:${var.group_email}"]
       }
       values = {
         dev = null
         prod = {
           description = "Environment: production."
           iam = {
-            "roles/resourcemanager.tagUser" = ["user:[email protected]"]
+            "roles/resourcemanager.tagUser" = ["group:${var.group_email}"]
           }
         }
       }
     }
   }
 }
-# tftest modules=1 resources=5 inventory=network-tags.yaml
+# tftest modules=1 resources=5 inventory=network-tags.yaml e2e serial
 ```
 
 <!-- TFDOC OPTS files:1 -->

diff --git a/modules/organization/iam.tf b/modules/organization/iam.tf
@@ -46,6 +46,7 @@ resource "google_organization_iam_binding" "authoritative" {
   org_id   = local.organization_id_numeric
   role     = each.key
   members  = each.value
+  # ensuring that custom role exists is left to the caller, by leveraging custom_role_id output
 }
 
 resource "google_organization_iam_binding" "bindings" {
@@ -61,6 +62,7 @@ resource "google_organization_iam_binding" "bindings" {
       description = each.value.condition.description
     }
   }
+  # ensuring that custom role exists is left to the caller, by leveraging custom_role_id output
 }
 
 resource "google_organization_iam_member" "bindings" {
@@ -76,4 +78,5 @@ resource "google_organization_iam_member" "bindings" {
       description = each.value.condition.description
     }
   }
+  # ensuring that custom role exists is left to the caller, by leveraging custom_role_id output
 }
diff --git a/tests/examples/conftest.py b/tests/examples/conftest.py
@@ -18,6 +18,7 @@
 from pathlib import Path
 
 import marko
+import pytest
 
 FABRIC_ROOT = Path(__file__).parents[2]
 
@@ -78,7 +79,12 @@ def pytest_generate_tests(metafunc, test_group='example',
             if index > 1:
               name += f' {index}'
             ids.append(f'{path}:{last_header}:{index}')
-            examples.append(Example(name, code, path, files[last_header]))
+            # if test is marked with 'serial' in tftest line then add them to this xdist group
+            # this, together with `--dist loadgroup` will ensure that those tests will be run one after another
+            # even if multiple workers are used
+            # see: https://pytest-xdist.readthedocs.io/en/latest/distribution.html
+            marks = [pytest.mark.xdist_group("serial")] if 'serial' in tftest_tag else []
+            examples.append(pytest.param(Example(name, code, path, files[last_header]), marks=marks))
         elif isinstance(child, marko.block.Heading):
           last_header = child.children[0].children
           index = 0

diff --git a/tests/examples_e2e/setup_module/e2e_tests.tfvars.tftpl b/tests/examples_e2e/setup_module/e2e_tests.tfvars.tftpl
@@ -18,7 +18,7 @@ kms_key = {
     id = "${kms_key_id}"
 }
 group_email = "${group_email}"
-organization_id = "${organization_id}"
+organization_id = "organizations/${organization_id}"
 folder_id = "folders/${folder_id}"
 project_id = "${project_id}"
 region = "${region}"

diff --git a/tests/modules/organization/examples/basic.yaml b/tests/modules/organization/examples/basic.yaml
@@ -14,8 +14,8 @@
 
 values:
   module.org.google_org_policy_policy.default["compute.disableGuestAttributesAccess"]:
-    name: organizations/1234567890/policies/compute.disableGuestAttributesAccess
-    parent: organizations/1234567890
+    name: organizations/1122334455/policies/compute.disableGuestAttributesAccess
+    parent: organizations/1122334455
     spec:
     - inherit_from_parent: null
       reset: null
@@ -27,8 +27,8 @@ values:
         values: []
     timeouts: null
   module.org.google_org_policy_policy.default["compute.skipDefaultNetworkCreation"]:
-    name: organizations/1234567890/policies/compute.skipDefaultNetworkCreation
-    parent: organizations/1234567890
+    name: organizations/1122334455/policies/compute.skipDefaultNetworkCreation
+    parent: organizations/1122334455
     spec:
     - inherit_from_parent: null
       reset: null
@@ -40,8 +40,8 @@ values:
         values: []
     timeouts: null
   module.org.google_org_policy_policy.default["compute.trustedImageProjects"]:
-    name: organizations/1234567890/policies/compute.trustedImageProjects
-    parent: organizations/1234567890
+    name: organizations/1122334455/policies/compute.trustedImageProjects
+    parent: organizations/1122334455
     spec:
     - inherit_from_parent: null
       reset: null
@@ -56,8 +56,8 @@ values:
           denied_values: null
     timeouts: null
   module.org.google_org_policy_policy.default["compute.vmExternalIpAccess"]:
-    name: organizations/1234567890/policies/compute.vmExternalIpAccess
-    parent: organizations/1234567890
+    name: organizations/1122334455/policies/compute.vmExternalIpAccess
+    parent: organizations/1122334455
     spec:
     - inherit_from_parent: null
       reset: null
@@ -68,22 +68,9 @@ values:
         enforce: null
         values: []
     timeouts: null
-  module.org.google_org_policy_policy.default["custom.gkeEnableAutoUpgrade"]:
-    name: organizations/1234567890/policies/custom.gkeEnableAutoUpgrade
-    parent: organizations/1234567890
-    spec:
-    - inherit_from_parent: null
-      reset: null
-      rules:
-      - allow_all: null
-        condition: []
-        deny_all: null
-        enforce: 'TRUE'
-        values: []
-    timeouts: null
   module.org.google_org_policy_policy.default["iam.allowedPolicyMemberDomains"]:
-    name: organizations/1234567890/policies/iam.allowedPolicyMemberDomains
-    parent: organizations/1234567890
+    name: organizations/1122334455/policies/iam.allowedPolicyMemberDomains
+    parent: organizations/1122334455
     spec:
     - inherit_from_parent: null
       reset: null
@@ -114,8 +101,8 @@ values:
           denied_values: null
     timeouts: null
   module.org.google_org_policy_policy.default["iam.disableServiceAccountKeyCreation"]:
-    name: organizations/1234567890/policies/iam.disableServiceAccountKeyCreation
-    parent: organizations/1234567890
+    name: organizations/1122334455/policies/iam.disableServiceAccountKeyCreation
+    parent: organizations/1122334455
     spec:
     - inherit_from_parent: null
       reset: null
@@ -127,8 +114,8 @@ values:
         values: []
     timeouts: null
   module.org.google_org_policy_policy.default["iam.disableServiceAccountKeyUpload"]:
-    name: organizations/1234567890/policies/iam.disableServiceAccountKeyUpload
-    parent: organizations/1234567890
+    name: organizations/1122334455/policies/iam.disableServiceAccountKeyUpload
+    parent: organizations/1122334455
     spec:
     - inherit_from_parent: null
       reset: null
@@ -151,29 +138,23 @@ values:
   module.org.google_organization_iam_binding.authoritative["roles/owner"]:
     condition: []
     members:
-    - group:cloud-owners@example.org
-    org_id: '1234567890'
+    - group:organization-admins@example.org
+    org_id: '1122334455'
     role: roles/owner
-  module.org.google_organization_iam_binding.authoritative["roles/projectCreator"]:
-    condition: []
-    members:
-    - group:[email protected]
-    org_id: '1234567890'
-    role: roles/projectCreator
   module.org.google_organization_iam_binding.authoritative["roles/resourcemanager.projectCreator"]:
     condition: []
     members:
-    - group:cloud[email protected]
-    org_id: '1234567890'
+    - group:organization[email protected]
+    org_id: '1122334455'
     role: roles/resourcemanager.projectCreator
   module.org.google_organization_iam_member.bindings["am1-storage-admin"]:
     condition: []
-    member: user:am1@example.org
-    org_id: '1234567890'
+    member: group:organization-admins@example.org
+    org_id: '1122334455'
     role: roles/storage.admin
   module.org.google_tags_tag_key.default["allowexternal"]:
     description: Allow external identities.
-    parent: organizations/1234567890
+    parent: organizations/1122334455
     purpose: null
     purpose_data: null
     short_name: allowexternal
@@ -188,12 +169,12 @@ values:
     timeouts: null
 
 counts:
-  google_org_policy_policy: 8
-  google_organization_iam_binding: 3
+  google_org_policy_policy: 7
+  google_organization_iam_binding: 2
   google_organization_iam_member: 1
   google_tags_tag_key: 1
   google_tags_tag_value: 2
   modules: 1
-  resources: 15
+  resources: 13
 
 outputs: {}
diff --git a/tests/modules/organization/examples/logging.yaml b/tests/modules/organization/examples/logging.yaml
@@ -47,7 +47,7 @@ values:
     name: notice
     org_id: '1122334455'
   module.org.google_logging_organization_sink.sink["warnings"]:
-    destination: storage.googleapis.com/gcs_sink
+    destination: storage.googleapis.com/test-gcs_sink
     disabled: false
     exclusions: []
     filter: severity=WARNING

diff --git a/tests/modules/organization/examples/network-tags.yaml b/tests/modules/organization/examples/network-tags.yaml
@@ -18,13 +18,13 @@ values:
     parent: organizations/1122334455
     purpose: GCE_FIREWALL
     purpose_data:
-      network: my_project/my_vpc
+      network: project-id/vpc_name
     short_name: net-environment
     timeouts: null
   module.org.google_tags_tag_key_iam_binding.default["net-environment:roles/resourcemanager.tagAdmin"]:
     condition: []
     members:
-    - group:admins@example.com
+    - group:organization-admins@example.org
     role: roles/resourcemanager.tagAdmin
   module.org.google_tags_tag_value.default["net-environment/dev"]:
     description: Managed by the Terraform organization module.
@@ -37,7 +37,7 @@ values:
   module.org.google_tags_tag_value_iam_binding.default["net-environment/prod:roles/resourcemanager.tagUser"]:
     condition: []
     members:
-    - user:user1@example.com
+    - group:organization-admins@example.org
     role: roles/resourcemanager.tagUser
 
 counts: