Revert iam_additive behaviour (#160)

* revert iam_additive format, add iam_additive_members

* revert iam_additive format, add iam_additive_members

* update CHANGELOG

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

modules/organization/README.md

@@ -39,6 +39,7 @@ module "org" {
 | *custom_roles* | Map of role name => list of permissions to create in this project. | <code title="map&#40;list&#40;string&#41;&#41;">map(list(string))</code> |  | <code title="">{}</code> |
 | *iam* | IAM bindings, in {ROLE => [MEMBERS]} format. | <code title="map&#40;list&#40;string&#41;&#41;">map(list(string))</code> |  | <code title="">{}</code> |
 | *iam_additive* | Non authoritative IAM bindings, in {ROLE => [MEMBERS]} format. | <code title="map&#40;list&#40;string&#41;&#41;">map(list(string))</code> |  | <code title="">{}</code> |
+| *iam_additive_members* | IAM additive bindings in {MEMBERS => [ROLE]} format. This might break if members are dynamic values. | <code title="map&#40;list&#40;string&#41;&#41;">map(list(string))</code> |  | <code title="">{}</code> |
 | *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. | <code title="map&#40;map&#40;list&#40;string&#41;&#41;&#41;">map(map(list(string)))</code> |  | <code title="">{}</code> |
 | *policy_boolean* | Map of boolean org policies and enforcement value, set value to null for policy restore. | <code title="map&#40;bool&#41;">map(bool)</code> |  | <code title="">{}</code> |
 | *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. | <code title="map&#40;object&#40;&#123;&#10;inherit_from_parent &#61; bool&#10;suggested_value     &#61; string&#10;status              &#61; bool&#10;values              &#61; list&#40;string&#41;&#10;&#125;&#41;&#41;">map(object({...}))</code> |  | <code title="">{}</code> |

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" {

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)))

modules/project/README.md

@@ -96,6 +96,7 @@ module "project" {
 | *custom_roles* | Map of role name => list of permissions to create in this project. | <code title="map&#40;list&#40;string&#41;&#41;">map(list(string))</code> |  | <code title="">{}</code> |
 | *iam* | IAM bindings in {ROLE => [MEMBERS]} format. | <code title="map&#40;set&#40;string&#41;&#41;">map(set(string))</code> |  | <code title="">{}</code> |
 | *iam_additive* | IAM additive bindings in {ROLE => [MEMBERS]} format. | <code title="map&#40;list&#40;string&#41;&#41;">map(list(string))</code> |  | <code title="">{}</code> |
+| *iam_additive_members* | IAM additive bindings in {MEMBERS => [ROLE]} format. This might break if members are dynamic values. | <code title="map&#40;list&#40;string&#41;&#41;">map(list(string))</code> |  | <code title="">{}</code> |
 | *labels* | Resource labels. | <code title="map&#40;string&#41;">map(string)</code> |  | <code title="">{}</code> |
 | *lien_reason* | If non-empty, creates a project lien with this description. | <code title="">string</code> |  | <code title=""></code> |
 | *oslogin* | Enable OS Login. | <code title="">bool</code> |  | <code title="">false</code> |

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

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)

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
 }

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 = {}

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}'

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
 }

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 = {}

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')
+  ])