From ecadebe90b040601b360264b11c8b1b45fa04fd7 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Fri, 11 Mar 2022 09:46:32 +0100 Subject: [PATCH] Add support for IAM additive to folder module (#580) --- modules/folder/README.md | 20 ++++---- modules/folder/iam.tf | 35 ++++++++++++-- modules/folder/variables.tf | 14 ++++++ tests/modules/folder/fixture/main.tf | 3 ++ tests/modules/folder/fixture/variables.tf | 51 +++++++++----------- tests/modules/folder/test_plan.py | 58 ++++++++++++++++------- 6 files changed, 121 insertions(+), 60 deletions(-) diff --git a/modules/folder/README.md b/modules/folder/README.md index 4f6898b9..2e5b8b5a 100644 --- a/modules/folder/README.md +++ b/modules/folder/README.md @@ -256,7 +256,7 @@ module "folder" { | name | description | resources | |---|---|---| | [firewall-policies.tf](./firewall-policies.tf) | None | google_compute_firewall_policy · google_compute_firewall_policy_association · google_compute_firewall_policy_rule | -| [iam.tf](./iam.tf) | IAM bindings, roles and audit logging resources. | google_folder_iam_binding | +| [iam.tf](./iam.tf) | IAM bindings, roles and audit logging resources. | google_folder_iam_binding · google_folder_iam_member | | [logging.tf](./logging.tf) | Log sinks and supporting resources. | google_bigquery_dataset_iam_member · google_logging_folder_exclusion · google_logging_folder_sink · google_project_iam_member · google_pubsub_topic_iam_member · google_storage_bucket_iam_member | | [main.tf](./main.tf) | Module-level locals and resources. | google_essential_contacts_contact · google_folder | | [organization-policies.tf](./organization-policies.tf) | Folder-level organization policies. | google_folder_organization_policy | @@ -276,14 +276,16 @@ module "folder" { | [folder_create](variables.tf#L58) | Create folder. When set to false, uses id to reference an existing folder. | bool | | true | | [group_iam](variables.tf#L64) | Authoritative IAM binding for organization groups, in {GROUP_EMAIL => [ROLES]} format. Group emails need to be static. Can be used in combination with the `iam` variable. | map(list(string)) | | {} | | [iam](variables.tf#L71) | IAM bindings in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | -| [id](variables.tf#L78) | Folder ID in case you use folder_create=false. | string | | null | -| [logging_exclusions](variables.tf#L84) | Logging exclusions for this folder in the form {NAME -> FILTER}. | map(string) | | {} | -| [logging_sinks](variables.tf#L91) | Logging sinks to create for this folder. | map(object({…})) | | {} | -| [name](variables.tf#L112) | Folder name. | string | | null | -| [parent](variables.tf#L118) | Parent in folders/folder_id or organizations/org_id format. | string | | null | -| [policy_boolean](variables.tf#L128) | Map of boolean org policies and enforcement value, set value to null for policy restore. | map(bool) | | {} | -| [policy_list](variables.tf#L135) | 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({…})) | | {} | -| [tag_bindings](variables.tf#L147) | Tag bindings for this folder, in key => tag value id format. | map(string) | | null | +| [iam_additive](variables.tf#L78) | Non authoritative IAM bindings, in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | +| [iam_additive_members](variables.tf#L85) | IAM additive bindings in {MEMBERS => [ROLE]} format. This might break if members are dynamic values. | map(list(string)) | | {} | +| [id](variables.tf#L92) | Folder ID in case you use folder_create=false. | string | | null | +| [logging_exclusions](variables.tf#L98) | Logging exclusions for this folder in the form {NAME -> FILTER}. | map(string) | | {} | +| [logging_sinks](variables.tf#L105) | Logging sinks to create for this folder. | map(object({…})) | | {} | +| [name](variables.tf#L126) | Folder name. | string | | null | +| [parent](variables.tf#L132) | Parent in folders/folder_id or organizations/org_id format. | string | | null | +| [policy_boolean](variables.tf#L142) | Map of boolean org policies and enforcement value, set value to null for policy restore. | map(bool) | | {} | +| [policy_list](variables.tf#L149) | 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({…})) | | {} | +| [tag_bindings](variables.tf#L161) | Tag bindings for this folder, in key => tag value id format. | map(string) | | null | ## Outputs diff --git a/modules/folder/iam.tf b/modules/folder/iam.tf index 52886bad..995dbdab 100644 --- a/modules/folder/iam.tf +++ b/modules/folder/iam.tf @@ -17,19 +17,33 @@ # tfdoc:file:description IAM bindings, roles and audit logging resources. locals { - group_iam_roles = distinct(flatten(values(var.group_iam))) - group_iam = { - for r in local.group_iam_roles : r => [ + _group_iam_roles = distinct(flatten(values(var.group_iam))) + _group_iam = { + for r in local._group_iam_roles : r => [ for k, v in var.group_iam : "group:${k}" if try(index(v, r), null) != null ] } + _iam_additive_pairs = flatten([ + for role, members in var.iam_additive : [ + for member in members : { role = role, member = member } + ] + ]) + _iam_additive_member_pairs = flatten([ + for member, roles in var.iam_additive_members : [ + for role in roles : { role = role, member = member } + ] + ]) iam = { - for role in distinct(concat(keys(var.iam), keys(local.group_iam))) : + for role in distinct(concat(keys(var.iam), keys(local._group_iam))) : role => concat( try(var.iam[role], []), - try(local.group_iam[role], []) + try(local._group_iam[role], []) ) } + iam_additive = { + for pair in concat(local._iam_additive_pairs, local._iam_additive_member_pairs) : + "${pair.role}-${pair.member}" => pair + } } resource "google_folder_iam_binding" "authoritative" { @@ -38,3 +52,14 @@ resource "google_folder_iam_binding" "authoritative" { role = each.key members = each.value } + +resource "google_folder_iam_member" "additive" { + for_each = ( + length(var.iam_additive) + length(var.iam_additive_members) > 0 + ? local.iam_additive + : {} + ) + folder = local.folder.name + role = each.value.role + member = each.value.member +} diff --git a/modules/folder/variables.tf b/modules/folder/variables.tf index a3f32e37..19ed18f3 100644 --- a/modules/folder/variables.tf +++ b/modules/folder/variables.tf @@ -75,6 +75,20 @@ variable "iam" { nullable = false } +variable "iam_additive" { + description = "Non authoritative IAM bindings, in {ROLE => [MEMBERS]} format." + type = map(list(string)) + default = {} + nullable = false +} + +variable "iam_additive_members" { + description = "IAM additive bindings in {MEMBERS => [ROLE]} format. This might break if members are dynamic values." + type = map(list(string)) + default = {} + nullable = false +} + variable "id" { description = "Folder ID in case you use folder_create=false." type = string diff --git a/tests/modules/folder/fixture/main.tf b/tests/modules/folder/fixture/main.tf index 05fe8105..2fa1b4fd 100644 --- a/tests/modules/folder/fixture/main.tf +++ b/tests/modules/folder/fixture/main.tf @@ -18,7 +18,10 @@ module "test" { source = "../../../../modules/folder" parent = "organizations/12345678" name = "folder-a" + group_iam = var.group_iam iam = var.iam + iam_additive = var.iam_additive + iam_additive_members = var.iam_additive_members policy_boolean = var.policy_boolean policy_list = var.policy_list firewall_policies = var.firewall_policies diff --git a/tests/modules/folder/fixture/variables.tf b/tests/modules/folder/fixture/variables.tf index 5917d7dc..da676deb 100644 --- a/tests/modules/folder/fixture/variables.tf +++ b/tests/modules/folder/fixture/variables.tf @@ -14,59 +14,52 @@ * limitations under the License. */ +variable "group_iam" { + type = any + default = {} +} + variable "iam" { - type = map(list(string)) + type = any + default = {} +} + +variable "iam_additive" { + type = any + default = {} +} + +variable "iam_additive_members" { + type = any default = {} } variable "policy_boolean" { - type = map(bool) + type = any default = {} } variable "policy_list" { - type = map(object({ - inherit_from_parent = bool - suggested_value = string - status = bool - values = list(string) - })) + type = any default = {} } variable "firewall_policies" { - type = map(map(object({ - description = string - direction = string - action = string - priority = number - ranges = list(string) - ports = map(list(string)) - target_service_accounts = list(string) - target_resources = list(string) - logging = bool - }))) + type = any default = {} } variable "firewall_policy_association" { - type = map(string) + type = any default = {} } variable "logging_sinks" { - type = map(object({ - destination = string - type = string - filter = string - iam = bool - include_children = bool - exclusions = map(string) - })) + type = any default = {} } variable "logging_exclusions" { - type = map(string) + type = any default = {} } diff --git a/tests/modules/folder/test_plan.py b/tests/modules/folder/test_plan.py index 31e082ca..0ce1ae4a 100644 --- a/tests/modules/folder/test_plan.py +++ b/tests/modules/folder/test_plan.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. + def test_folder(plan_runner): "Test folder resources." _, resources = plan_runner() @@ -23,26 +24,49 @@ def test_folder(plan_runner): def test_iam(plan_runner): - "Test folder resources with iam roles and members." - iam = '{"roles/owner" = ["user:a@b.com"] }' - _, resources = plan_runner(iam=iam) - assert len(resources) == 2 - - -def test_iam_multiple_members(plan_runner): - "Test folder resources with multiple iam members." - iam = '{"roles/owner" = ["user:a@b.com", "user:c@d.com"] }' - _, resources = plan_runner(iam=iam) - assert len(resources) == 2 + "Test IAM." + group_iam = ( + '{' + '"owners@example.org" = ["roles/owner", "roles/resourcemanager.folderAdmin"],' + '"viewers@example.org" = ["roles/viewer"]' + '}') + iam = ('{' + '"roles/owner" = ["user:one@example.org", "user:two@example.org"],' + '"roles/browser" = ["domain:example.org"]' + '}') + _, resources = plan_runner(group_iam=group_iam, iam=iam) + roles = sorted([(r['values']['role'], sorted(r['values']['members'])) + for r in resources + if r['type'] == 'google_folder_iam_binding']) + assert roles == [ + ('roles/browser', ['domain:example.org']), + ('roles/owner', [ + 'group:owners@example.org', 'user:one@example.org', + 'user:two@example.org' + ]), + ('roles/resourcemanager.folderAdmin', ['group:owners@example.org']), + ('roles/viewer', ['group:viewers@example.org']), + ] def test_iam_multiple_roles(plan_runner): "Test folder resources with multiple iam roles." - iam = ( - '{ ' - '"roles/owner" = ["user:a@b.com"], ' - '"roles/viewer" = ["user:c@d.com"] ' - '} ' - ) + iam = ('{ ' + '"roles/owner" = ["user:a@b.com"], ' + '"roles/viewer" = ["user:c@d.com"] ' + '} ') _, resources = plan_runner(iam=iam) assert len(resources) == 3 + + +def test_iam_additive_members(plan_runner): + "Test IAM additive members." + iam = ('{"user:one@example.org" = ["roles/owner"],' + '"user:two@example.org" = ["roles/owner", "roles/editor"]}') + _, resources = plan_runner(iam_additive_members=iam) + roles = set((r['values']['role'], r['values']['member']) + for r in resources + if r['type'] == 'google_folder_iam_member') + assert roles == set([('roles/owner', 'user:one@example.org'), + ('roles/owner', 'user:two@example.org'), + ('roles/editor', 'user:two@example.org')])