diff --git a/CHANGELOG.md b/CHANGELOG.md index dd8e34a3b..67b1b7c5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. - **incompatible change** rename prefix for node configuration variables in `gke-nodepool` module [#156] - add support for internally managed service account in `gke-nodepool` module [#156] - made examples in READMEs runnable and testable [#157] +- **incompatible change** `iam_additive` is now keyed by role to be more resilient with dynamic values, a new `iam_additive_members` variable has been added for backwards compatibility. ## [4.0.0] - 2020-11-06 diff --git a/modules/organization/README.md b/modules/organization/README.md index 9d12681c6..9547c67d4 100644 --- a/modules/organization/README.md +++ b/modules/organization/README.md @@ -39,6 +39,7 @@ module "org" { | *custom_roles* | Map of role name => list of permissions to create in this project. | map(list(string)) | | {} | | *iam* | IAM bindings, in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | | *iam_additive* | Non authoritative IAM bindings, in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | +| *iam_additive_members* | IAM additive bindings in {MEMBERS => [ROLE]} format. This might break if members are dynamic values. | map(list(string)) | | {} | | *iam_audit_config* | Service audit logging configuration. Service as key, map of log permission (eg DATA_READ) and excluded members as value for each service. | map(map(list(string))) | | {} | | *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({...})) | | {} | diff --git a/modules/organization/main.tf b/modules/organization/main.tf index 6cf410173..e331d6ae3 100644 --- a/modules/organization/main.tf +++ b/modules/organization/main.tf @@ -16,13 +16,17 @@ locals { iam_additive_pairs = flatten([ - for member, roles in var.iam_additive : [ - for role in roles : - { role = role, member = member } + 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_additive = { - for pair in local.iam_additive_pairs : + for pair in concat(local.iam_additive_pairs, local.iam_additive_member_pairs) : "${pair.role}-${pair.member}" => pair } } @@ -44,10 +48,14 @@ resource "google_organization_iam_binding" "authoritative" { } resource "google_organization_iam_member" "additive" { - for_each = length(var.iam_additive) > 0 ? local.iam_additive : {} - org_id = var.org_id - role = each.value.role - member = each.value.member + for_each = ( + length(var.iam_additive) + length(var.iam_additive_members) > 0 + ? local.iam_additive + : {} + ) + org_id = var.org_id + role = each.value.role + member = each.value.member } resource "google_organization_iam_audit_config" "config" { diff --git a/modules/organization/variables.tf b/modules/organization/variables.tf index 293f01762..8c69fcbb8 100644 --- a/modules/organization/variables.tf +++ b/modules/organization/variables.tf @@ -32,6 +32,12 @@ variable "iam_additive" { default = {} } +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 = {} +} + variable "iam_audit_config" { description = "Service audit logging configuration. Service as key, map of log permission (eg DATA_READ) and excluded members as value for each service." type = map(map(list(string))) diff --git a/modules/project/README.md b/modules/project/README.md index 99755a0e3..e69d3cf87 100644 --- a/modules/project/README.md +++ b/modules/project/README.md @@ -96,6 +96,7 @@ module "project" { | *custom_roles* | Map of role name => list of permissions to create in this project. | map(list(string)) | | {} | | *iam* | IAM bindings in {ROLE => [MEMBERS]} format. | map(set(string)) | | {} | | *iam_additive* | IAM additive bindings in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | +| *iam_additive_members* | IAM additive bindings in {MEMBERS => [ROLE]} format. This might break if members are dynamic values. | map(list(string)) | | {} | | *labels* | Resource labels. | map(string) | | {} | | *lien_reason* | If non-empty, creates a project lien with this description. | string | | | | *oslogin* | Enable OS Login. | bool | | false | diff --git a/modules/project/main.tf b/modules/project/main.tf index f7cf5ddcf..9e5502ae6 100644 --- a/modules/project/main.tf +++ b/modules/project/main.tf @@ -16,13 +16,17 @@ locals { iam_additive_pairs = flatten([ - for member, roles in var.iam_additive : [ - for role in roles : - { role = role, member = member } + 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_additive = { - for pair in local.iam_additive_pairs : + for pair in concat(local.iam_additive_pairs, local.iam_additive_member_pairs) : "${pair.role}-${pair.member}" => pair } parent_type = var.parent == null ? null : split("/", var.parent)[0] @@ -102,10 +106,14 @@ resource "google_project_iam_binding" "authoritative" { } resource "google_project_iam_member" "additive" { - for_each = length(var.iam_additive) > 0 ? local.iam_additive : {} - project = local.project.project_id - role = each.value.role - member = each.value.member + for_each = ( + length(var.iam_additive) + length(var.iam_additive_members) > 0 + ? local.iam_additive + : {} + ) + project = local.project.project_id + role = each.value.role + member = each.value.member depends_on = [ google_project_service.project_services, google_project_iam_custom_role.roles diff --git a/modules/project/variables.tf b/modules/project/variables.tf index 760a91833..feb3b21ae 100644 --- a/modules/project/variables.tf +++ b/modules/project/variables.tf @@ -44,6 +44,12 @@ variable "iam_additive" { default = {} } +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 = {} +} + variable "labels" { description = "Resource labels." type = map(string) diff --git a/tests/modules/organization/fixture/main.tf b/tests/modules/organization/fixture/main.tf index 6c5d0bcae..d1b0dd350 100644 --- a/tests/modules/organization/fixture/main.tf +++ b/tests/modules/organization/fixture/main.tf @@ -15,12 +15,13 @@ */ module "test" { - source = "../../../../modules/organization" - org_id = 1234567890 - custom_roles = var.custom_roles - iam = var.iam - iam_additive = var.iam_additive - iam_audit_config = var.iam_audit_config - policy_boolean = var.policy_boolean - policy_list = var.policy_list + source = "../../../../modules/organization" + org_id = 1234567890 + custom_roles = var.custom_roles + iam = var.iam + iam_additive = var.iam_additive + iam_additive_members = var.iam_additive_members + iam_audit_config = var.iam_audit_config + policy_boolean = var.policy_boolean + policy_list = var.policy_list } diff --git a/tests/modules/organization/fixture/variables.tf b/tests/modules/organization/fixture/variables.tf index 887c33452..7a2ddfdb4 100644 --- a/tests/modules/organization/fixture/variables.tf +++ b/tests/modules/organization/fixture/variables.tf @@ -29,6 +29,11 @@ variable "iam_additive" { default = {} } +variable "iam_additive_members" { + type = map(list(string)) + default = {} +} + variable "iam_audit_config" { type = map(map(list(string))) default = {} diff --git a/tests/modules/organization/test_plan.py b/tests/modules/organization/test_plan.py index 493682d65..8d4152bac 100644 --- a/tests/modules/organization/test_plan.py +++ b/tests/modules/organization/test_plan.py @@ -30,6 +30,22 @@ def test_audit_config(plan_runner): assert log_types == set(['DATA_READ', 'DATA_WRITE']) +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(FIXTURES_DIR, iam_additive_members=iam) + roles = set((r['values']['role'], r['values']['member']) + for r in resources if r['type'] == 'google_organization_iam_member') + assert roles == set([ + ('roles/owner', 'user:one@example.org'), + ('roles/owner', 'user:two@example.org'), + ('roles/editor', 'user:two@example.org') + ]) + + def test_policy_boolean(plan_runner): "Test boolean org policy." policy_boolean = '{policy-a = true, policy-b = false, policy-c = null}' diff --git a/tests/modules/project/fixture/main.tf b/tests/modules/project/fixture/main.tf index e7a9fd0e2..acec2c3f0 100644 --- a/tests/modules/project/fixture/main.tf +++ b/tests/modules/project/fixture/main.tf @@ -15,21 +15,22 @@ */ module "test" { - source = "../../../../modules/project" - name = "my-project" - billing_account = "12345-12345-12345" - auto_create_network = var.auto_create_network - custom_roles = var.custom_roles - iam = var.iam - iam_additive = var.iam_additive - labels = var.labels - lien_reason = var.lien_reason - oslogin = var.oslogin - oslogin_admins = var.oslogin_admins - oslogin_users = var.oslogin_users - parent = var.parent - policy_boolean = var.policy_boolean - policy_list = var.policy_list - prefix = var.prefix - services = var.services + source = "../../../../modules/project" + name = "my-project" + billing_account = "12345-12345-12345" + auto_create_network = var.auto_create_network + custom_roles = var.custom_roles + iam = var.iam + iam_additive = var.iam_additive + iam_additive_members = var.iam_additive_members + labels = var.labels + lien_reason = var.lien_reason + oslogin = var.oslogin + oslogin_admins = var.oslogin_admins + oslogin_users = var.oslogin_users + parent = var.parent + policy_boolean = var.policy_boolean + policy_list = var.policy_list + prefix = var.prefix + services = var.services } diff --git a/tests/modules/project/fixture/variables.tf b/tests/modules/project/fixture/variables.tf index 1a60f8567..3b759ddbf 100644 --- a/tests/modules/project/fixture/variables.tf +++ b/tests/modules/project/fixture/variables.tf @@ -34,6 +34,11 @@ variable "iam_additive" { default = {} } +variable "iam_additive_members" { + type = map(list(string)) + default = {} +} + variable "labels" { type = map(string) default = {} diff --git a/tests/modules/project/test_iam.py b/tests/modules/project/test_iam.py new file mode 100644 index 000000000..02e96faf5 --- /dev/null +++ b/tests/modules/project/test_iam.py @@ -0,0 +1,66 @@ +# 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. + + +import os +import pytest + + +FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture') + + +def test_iam(plan_runner): + "Test IAM bindings." + iam = ( + '{"roles/owner" = ["user:one@example.org"],' + '"roles/viewer" = ["user:two@example.org", "user:three@example.org"]}' + ) + _, resources = plan_runner(FIXTURES_DIR, iam=iam) + roles = dict((r['values']['role'], r['values']['members']) + for r in resources if r['type'] == 'google_project_iam_binding') + assert roles == { + 'roles/owner': ['user:one@example.org'], + 'roles/viewer': ['user:three@example.org', 'user:two@example.org']} + + +def test_iam_additive(plan_runner): + "Test IAM additive bindings." + iam = ( + '{"roles/owner" = ["user:one@example.org"],' + '"roles/viewer" = ["user:two@example.org", "user:three@example.org"]}' + ) + _, resources = plan_runner(FIXTURES_DIR, iam_additive=iam) + roles = set((r['values']['role'], r['values']['member']) + for r in resources if r['type'] == 'google_project_iam_member') + assert roles == set([ + ('roles/owner', 'user:one@example.org'), + ('roles/viewer', 'user:three@example.org'), + ('roles/viewer', 'user:two@example.org') + ]) + + +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(FIXTURES_DIR, iam_additive_members=iam) + roles = set((r['values']['role'], r['values']['member']) + for r in resources if r['type'] == 'google_project_iam_member') + assert roles == set([ + ('roles/owner', 'user:one@example.org'), + ('roles/owner', 'user:two@example.org'), + ('roles/editor', 'user:two@example.org') + ])