diff --git a/foundations/environments/main.tf b/foundations/environments/main.tf index 701bca93..34ad8d08 100644 --- a/foundations/environments/main.tf +++ b/foundations/environments/main.tf @@ -85,17 +85,14 @@ module "tf-gcs-environments" { ############################################################################### module "environment-folders" { - source = "../../modules/folders" - parent = var.root_node - names = var.environments - iam_roles = { - for name in var.environments : (name) => local.folder_roles - } + source = "../../modules/folder" + for_each = var.environments + parent = var.root_node + name = each.value + iam_roles = local.folder_roles iam_members = { - for name in var.environments : (name) => { - for role in local.folder_roles : - (role) => [module.tf-service-accounts.iam_emails[name]] - } + for role in local.folder_roles : + (role) => [module.tf-service-accounts.iam_emails[each.value]] } } diff --git a/foundations/environments/outputs.tf b/foundations/environments/outputs.tf index 495de1f7..fee9506f 100644 --- a/foundations/environments/outputs.tf +++ b/foundations/environments/outputs.tf @@ -24,7 +24,7 @@ output "bootstrap_tf_gcs_bucket" { output "environment_folders" { description = "Top-level environment folders." - value = module.environment-folders.ids + value = { for folder in module.environment-folders : folder.name => folder.id } } output "environment_tf_gcs_buckets" { diff --git a/foundations/environments/variables.tf b/foundations/environments/variables.tf index 436d8557..7fa2e500 100644 --- a/foundations/environments/variables.tf +++ b/foundations/environments/variables.tf @@ -29,7 +29,7 @@ variable "billing_account_id" { variable "environments" { description = "Environment short names." - type = list(string) + type = set(string) } variable "gcs_location" { diff --git a/modules/folders/README.md b/modules/folder/README.md similarity index 53% rename from modules/folders/README.md rename to modules/folder/README.md index 7efb4b68..c9d84cc0 100644 --- a/modules/folders/README.md +++ b/modules/folder/README.md @@ -1,6 +1,6 @@ # Google Cloud Folder Module -This module allow creation and management of sets of folders sharing a common parent, and their individual IAM bindings. It also allows setting a common set of organization policies on all folders. +This module allows the creation and management of folders together with their individual IAM bindings and organization policies. ## Examples @@ -8,17 +8,13 @@ This module allow creation and management of sets of folders sharing a common pa ```hcl module "folder" { - source = "./modules/folders" + source = "./modules/folder" parent = "organizations/1234567890" - names = ["Folder one", "Folder two"] + name = "Folder name" iam_members = { - "Folder one" = { - "roles/owner" = ["group:users@example.com"] - } - } - iam_roles = { - "Folder one" = ["roles/owner"] + "roles/owner" = ["group:users@example.com"] } + iam_roles = ["roles/owner"] } ``` @@ -26,9 +22,9 @@ module "folder" { ```hcl module "folder" { - source = "./modules/folders" + source = "./modules/folder" parent = "organizations/1234567890" - names = ["Folder one", "Folder two"] + name = "Folder name" policy_boolean = { "constraints/compute.disableGuestAttributesAccess" = true "constraints/compute.skipDefaultNetworkCreation" = true @@ -49,10 +45,10 @@ module "folder" { | name | description | type | required | default | |---|---|:---: |:---:|:---:| -| parent | Parent in folders/folder_id or organizations/org_id format. | string | ✓ | | -| *iam_members* | List of IAM members keyed by folder name and role. | map(map(list(string))) | | null | -| *iam_roles* | List of IAM roles keyed by folder name. | map(list(string)) | | null | -| *names* | Folder names. | list(string) | | [] | +| name | Folder name. | string | ✓ | | +| parent | Parent in folders/folder_id or organizations/org_id format. | string | ✓ | | +| *iam_members* | List of IAM members keyed by role. | map(set(string)) | | null | +| *iam_roles* | List of IAM roles. | set(string) | | null | | *policy_boolean* | Map of boolean org policies and enforcement value, set value to null for policy restore. | map(bool) | | {} | | *policy_list* | Map of list org policies, status is true for allow, false for deny, null for restore. Values can only be used for allow or deny. | map(object({...})) | | {} | @@ -60,12 +56,7 @@ module "folder" { | name | description | sensitive | |---|---|:---:| -| folder | Folder resource (for single use). | | -| folders | Folder resources. | | -| id | Folder id (for single use). | | -| ids | Folder ids. | | -| ids_list | List of folder ids. | | -| name | Folder name (for single use). | | -| names | Folder names. | | -| names_list | List of folder names. | | +| folder | Folder resource. | | +| id | Folder id. | | +| name | Folder name. | | diff --git a/modules/folders/main.tf b/modules/folder/main.tf similarity index 51% rename from modules/folders/main.tf rename to modules/folder/main.tf index e144726b..ce183910 100644 --- a/modules/folders/main.tf +++ b/modules/folder/main.tf @@ -1,5 +1,5 @@ /** - * Copyright 2018 Google LLC + * Copyright 2020 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,63 +14,26 @@ * limitations under the License. */ -locals { - folders = ( - local.has_folders - ? [for name in var.names : google_folder.folders[name]] - : [] - ) - # needed when destroying - has_folders = length(google_folder.folders) > 0 - iam_pairs = var.iam_roles == null ? [] : flatten([ - for name, roles in var.iam_roles : - [for role in roles : { name = name, role = role }] - ]) - iam_keypairs = { - for pair in local.iam_pairs : - "${pair.name}-${pair.role}" => pair - } - iam_members = var.iam_members == null ? {} : var.iam_members - policy_boolean_pairs = { - for pair in setproduct(var.names, keys(var.policy_boolean)) : - "${pair.0}-${pair.1}" => { - folder = pair.0, - policy = pair.1, - policy_data = var.policy_boolean[pair.1] - } - } - policy_list_pairs = { - for pair in setproduct(var.names, keys(var.policy_list)) : - "${pair.0}-${pair.1}" => { - folder = pair.0, - policy = pair.1, - policy_data = var.policy_list[pair.1] - } - } -} -resource "google_folder" "folders" { - for_each = toset(var.names) - display_name = each.value +resource "google_folder" "folder" { + display_name = var.name parent = var.parent } resource "google_folder_iam_binding" "authoritative" { - for_each = local.iam_keypairs - folder = google_folder.folders[each.value.name].name - role = each.value.role - members = lookup( - lookup(local.iam_members, each.value.name, {}), each.value.role, [] - ) + for_each = var.iam_roles + folder = google_folder.folder.name + role = each.key + members = lookup(var.iam_members, each.key, []) } resource "google_folder_organization_policy" "boolean" { - for_each = local.policy_boolean_pairs - folder = google_folder.folders[each.value.folder].id - constraint = each.value.policy + for_each = var.policy_boolean + folder = google_folder.folder.name + constraint = each.key dynamic boolean_policy { - for_each = each.value.policy_data == null ? [] : [each.value.policy_data] + for_each = each.value == null ? [] : [each.value] iterator = policy content { enforced = policy.value @@ -78,7 +41,7 @@ resource "google_folder_organization_policy" "boolean" { } dynamic restore_policy { - for_each = each.value.policy_data == null ? [""] : [] + for_each = each.value == null ? [""] : [] content { default = true } @@ -86,12 +49,12 @@ resource "google_folder_organization_policy" "boolean" { } resource "google_folder_organization_policy" "list" { - for_each = local.policy_list_pairs - folder = google_folder.folders[each.value.folder].id - constraint = each.value.policy + for_each = var.policy_list + folder = google_folder.folder.name + constraint = each.key dynamic list_policy { - for_each = each.value.policy_data.status == null ? [] : [each.value.policy_data] + for_each = each.value.status == null ? [] : [each.value] iterator = policy content { inherit_from_parent = policy.value.inherit_from_parent @@ -130,7 +93,7 @@ resource "google_folder_organization_policy" "list" { } dynamic restore_policy { - for_each = each.value.policy_data.status == null ? [true] : [] + for_each = each.value.status == null ? [true] : [] content { default = true } diff --git a/modules/folder/outputs.tf b/modules/folder/outputs.tf new file mode 100644 index 00000000..4be12eb1 --- /dev/null +++ b/modules/folder/outputs.tf @@ -0,0 +1,35 @@ +/** + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +output "folder" { + description = "Folder resource." + value = google_folder.folder +} + +output "id" { + description = "Folder id." + value = google_folder.folder.name + depends_on = [ + google_folder_iam_binding.authoritative, + google_folder_organization_policy.boolean, + google_folder_organization_policy.list + ] +} + +output "name" { + description = "Folder name." + value = google_folder.folder.display_name +} diff --git a/modules/folders/variables.tf b/modules/folder/variables.tf similarity index 74% rename from modules/folders/variables.tf rename to modules/folder/variables.tf index 3dbf502b..47c2843b 100644 --- a/modules/folders/variables.tf +++ b/modules/folder/variables.tf @@ -1,5 +1,5 @@ /** - * Copyright 2018 Google LLC + * Copyright 2020 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,26 +15,29 @@ */ variable "iam_members" { - description = "List of IAM members keyed by folder name and role." - type = map(map(list(string))) + description = "List of IAM members keyed by role." + type = map(set(string)) default = null } variable "iam_roles" { - description = "List of IAM roles keyed by folder name." - type = map(list(string)) + description = "List of IAM roles." + type = set(string) default = null } -variable "names" { - description = "Folder names." - type = list(string) - default = [] +variable "name" { + description = "Folder name." + type = string } variable "parent" { description = "Parent in folders/folder_id or organizations/org_id format." type = string + validation { + condition = can(regex("(organizations|folders)/[0-9]+", var.parent)) + error_message = "Parent must be of the form folders/folder_id or organizations/organization_id." + } } variable "policy_boolean" { diff --git a/modules/folders/versions.tf b/modules/folder/versions.tf similarity index 94% rename from modules/folders/versions.tf rename to modules/folder/versions.tf index bc4c2a9d..2a088552 100644 --- a/modules/folders/versions.tf +++ b/modules/folder/versions.tf @@ -15,5 +15,5 @@ */ terraform { - required_version = ">= 0.12.6" + required_version = ">= 0.13.0" } diff --git a/modules/folders/outputs.tf b/modules/folders/outputs.tf deleted file mode 100644 index cf7b54ca..00000000 --- a/modules/folders/outputs.tf +++ /dev/null @@ -1,78 +0,0 @@ -/** - * Copyright 2018 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -output "folder" { - description = "Folder resource (for single use)." - value = local.has_folders ? local.folders[0] : null -} - -output "id" { - description = "Folder id (for single use)." - value = local.has_folders ? local.folders[0].name : null - depends_on = [ - google_folder_iam_binding.authoritative, - google_folder_organization_policy.boolean, - google_folder_organization_policy.list - ] -} - -output "name" { - description = "Folder name (for single use)." - value = local.has_folders ? local.folders[0].display_name : null -} - -output "folders" { - description = "Folder resources." - value = local.folders -} - -output "ids" { - description = "Folder ids." - value = ( - local.has_folders - ? zipmap(var.names, [for f in local.folders : f.name]) - : {} - ) - depends_on = [ - google_folder_iam_binding.authoritative, - google_folder_organization_policy.boolean, - google_folder_organization_policy.list - ] -} - -output "names" { - description = "Folder names." - value = ( - local.has_folders - ? zipmap(var.names, [for f in local.folders : f.display_name]) - : {} - ) -} - -output "ids_list" { - description = "List of folder ids." - value = [for f in local.folders : f.name] - depends_on = [ - google_folder_iam_binding.authoritative, - google_folder_organization_policy.boolean, - google_folder_organization_policy.list - ] -} - -output "names_list" { - description = "List of folder names." - value = [for f in local.folders : f.display_name] -} diff --git a/tests/foundations/environments/test_plan.py b/tests/foundations/environments/test_plan.py index 96fd124e..5bdc18b1 100644 --- a/tests/foundations/environments/test_plan.py +++ b/tests/foundations/environments/test_plan.py @@ -23,16 +23,16 @@ FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture') def test_folder_roles(plan_runner): "Test folder roles." _, modules = plan_runner(FIXTURES_DIR, is_module=False) - resources = modules['module.test.module.environment-folders'] - folders = [r for r in resources if r['type'] == 'google_folder'] - assert len(folders) == 2 - assert set(r['values']['display_name'] - for r in folders) == set(['prod', 'test']) - bindings = [r['index'].split('-') - for r in resources if r['type'] == 'google_folder_iam_binding'] - assert len(bindings) == 10 - assert set(b[0] for b in bindings) == set(['prod', 'test']) - assert len(set(b[1] for b in bindings)) == 5 + for env in ["test", "prod"]: + resources = modules[f'module.test.module.environment-folders["{env}"]'] + folders = [r for r in resources if r['type'] == 'google_folder'] + assert len(folders) == 1 + folder = folders[0] + assert folder['values']['display_name'] == env + + bindings = [r['index'] + for r in resources if r['type'] == 'google_folder_iam_binding'] + assert len(bindings) == 5 def test_org_roles(plan_runner): @@ -42,12 +42,13 @@ def test_org_roles(plan_runner): 'iam_xpn_config': '{grant = true, target_org = true}' } _, modules = plan_runner(FIXTURES_DIR, is_module=False, **vars) - resources = modules['module.test.module.environment-folders'] - folder_bindings = [r['index'].split('-') + resources = (modules['module.test.module.environment-folders["test"]'] + + modules['module.test.module.environment-folders["prod"]']) + folder_bindings = [r['index'] for r in resources if r['type'] == 'google_folder_iam_binding'] assert len(folder_bindings) == 8 resources = modules['module.test.module.tf-service-accounts'] org_bindings = [r['index'].split('-') for r in resources if r['type'] == 'google_organization_iam_member'] assert len(org_bindings) == 4 - assert set(b[0] for b in org_bindings) == set(['prod', 'test']) + assert {b[0] for b in org_bindings} == {'prod', 'test'} diff --git a/tests/modules/folders/fixture/main.tf b/tests/modules/folders/fixture/main.tf index 71d13cd2..5eef7c12 100644 --- a/tests/modules/folders/fixture/main.tf +++ b/tests/modules/folders/fixture/main.tf @@ -15,9 +15,9 @@ */ module "test" { - source = "../../../../modules/folders" + source = "../../../../modules/folder" parent = "organizations/12345678" - names = ["folder-a", "folder-b"] + name = "folder-a" iam_members = var.iam_members iam_roles = var.iam_roles policy_boolean = var.policy_boolean diff --git a/tests/modules/folders/fixture/variables.tf b/tests/modules/folders/fixture/variables.tf index d8a71531..9b91f610 100644 --- a/tests/modules/folders/fixture/variables.tf +++ b/tests/modules/folders/fixture/variables.tf @@ -15,13 +15,13 @@ */ variable "iam_members" { - type = map(map(list(string))) - default = null + type = map(list(string)) + default = {} } variable "iam_roles" { - type = map(list(string)) - default = null + type = list(string) + default = [] } variable "policy_boolean" { diff --git a/tests/modules/folders/test_plan.py b/tests/modules/folders/test_plan.py index fcb8fa64..d8d100de 100644 --- a/tests/modules/folders/test_plan.py +++ b/tests/modules/folders/test_plan.py @@ -23,27 +23,48 @@ FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture') def test_folder(plan_runner): "Test folder resources." _, resources = plan_runner(FIXTURES_DIR) - assert len(resources) == 2 - assert set(r['type'] for r in resources) == set(['google_folder']) - assert set(r['values']['display_name'] for r in resources) == set([ - 'folder-a', 'folder-b' - ]) - assert set(r['values']['parent'] for r in resources) == set([ - 'organizations/12345678' - ]) + assert len(resources) == 1 + resource = resources[0] + assert resource['type'] == 'google_folder' + assert resource['values']['display_name'] == 'folder-a' + assert resource['values']['parent'] == 'organizations/12345678' def test_iam_roles_only(plan_runner): "Test folder resources with only iam roles passed." - _, resources = plan_runner( - FIXTURES_DIR, iam_roles='{folder-a = [ "roles/owner"]}') - assert len(resources) == 3 + _, resources = plan_runner(FIXTURES_DIR, + iam_roles='["roles/owner"]') + assert len(resources) == 2 def test_iam(plan_runner): "Test folder resources with iam roles and members." - iam_roles = '{folder-a = ["roles/owner"], folder-b = ["roles/viewer"]}' - iam_members = '{folder-a = { "roles/owner" = ["user:a@b.com"] }}' - _, resources = plan_runner( - FIXTURES_DIR, iam_roles=iam_roles, iam_members=iam_members) - assert len(resources) == 4 + iam_roles = '["roles/owner"]' + iam_members = '{"roles/owner" = ["user:a@b.com"] }' + _, resources = plan_runner(FIXTURES_DIR, + iam_roles=iam_roles, + iam_members=iam_members) + assert len(resources) == 2 + +def test_iam_multiple_members(plan_runner): + "Test folder resources with multiple iam members." + iam_roles = '["roles/owner"]' + iam_members = '{"roles/owner" = ["user:a@b.com", "user:c@d.com"] }' + _, resources = plan_runner(FIXTURES_DIR, + iam_roles=iam_roles, + iam_members=iam_members) + assert len(resources) == 2 + +def test_iam_multiple_roles(plan_runner): + "Test folder resources with multiple iam roles." + iam_roles = '["roles/owner", "roles/viewer"]' + iam_members = ( + '{ ' + '"roles/owner" = ["user:a@b.com"], ' + '"roles/viewer" = ["user:c@d.com"] ' + '} ' + ) + _, resources = plan_runner(FIXTURES_DIR, + iam_roles=iam_roles, + iam_members=iam_members) + assert len(resources) == 3 diff --git a/tests/modules/folders/test_plan_org_policies.py b/tests/modules/folders/test_plan_org_policies.py index dde2b30c..09d83cf5 100644 --- a/tests/modules/folders/test_plan_org_policies.py +++ b/tests/modules/folders/test_plan_org_policies.py @@ -24,16 +24,14 @@ def test_policy_boolean(plan_runner): "Test boolean folder policy." policy_boolean = '{policy-a = true, policy-b = false, policy-c = null}' _, resources = plan_runner(FIXTURES_DIR, policy_boolean=policy_boolean) - assert len(resources) == 8 + + assert len(resources) == 4 resources = [r for r in resources if r['type'] == 'google_folder_organization_policy'] assert sorted([r['index'] for r in resources]) == [ - 'folder-a-policy-a', - 'folder-a-policy-b', - 'folder-a-policy-c', - 'folder-b-policy-a', - 'folder-b-policy-b', - 'folder-b-policy-c' + 'policy-a', + 'policy-b', + 'policy-c', ] policy_values = [] for resource in resources: @@ -42,12 +40,9 @@ def test_policy_boolean(plan_runner): if value: policy_values.append((resource['index'], policy,) + value[0].popitem()) assert sorted(policy_values) == [ - ('folder-a-policy-a', 'boolean_policy', 'enforced', True), - ('folder-a-policy-b', 'boolean_policy', 'enforced', False), - ('folder-a-policy-c', 'restore_policy', 'default', True), - ('folder-b-policy-a', 'boolean_policy', 'enforced', True), - ('folder-b-policy-b', 'boolean_policy', 'enforced', False), - ('folder-b-policy-c', 'restore_policy', 'default', True) + ('policy-a', 'boolean_policy', 'enforced', True), + ('policy-b', 'boolean_policy', 'enforced', False), + ('policy-c', 'restore_policy', 'default', True), ] @@ -61,26 +56,20 @@ def test_policy_list(plan_runner): '}' ) _, resources = plan_runner(FIXTURES_DIR, policy_list=policy_list) - assert len(resources) == 8 + assert len(resources) == 4 resources = [r for r in resources if r['type'] == 'google_folder_organization_policy'] assert sorted([r['index'] for r in resources]) == [ - 'folder-a-policy-a', - 'folder-a-policy-b', - 'folder-a-policy-c', - 'folder-b-policy-a', - 'folder-b-policy-b', - 'folder-b-policy-c' + 'policy-a', + 'policy-b', + 'policy-c', ] values = [r['values'] for r in resources] assert [r['constraint'] for r in values] == [ - 'policy-a', 'policy-b', 'policy-c', 'policy-a', 'policy-b', 'policy-c' + 'policy-a', 'policy-b', 'policy-c' ] - for i in (0, 3): - assert values[i]['list_policy'][0]['allow'] == [ - {'all': True, 'values': None}] - for i in (1, 4): - assert values[i]['list_policy'][0]['deny'] == [ - {'all': False, 'values': ["bar"]}] - for i in (2, 5): - assert values[i]['restore_policy'] == [{'default': True}] + assert values[0]['list_policy'][0]['allow'] == [ + {'all': True, 'values': None}] + assert values[1]['list_policy'][0]['deny'] == [ + {'all': False, 'values': ["bar"]}] + assert values[2]['restore_policy'] == [{'default': True}]