Migrate to for_each. Get rid of *_num options (breaking change). Fix destructiveness of updates. Add new playground test suite for testing updates.

.kitchen.yml

@@ -20,6 +20,11 @@ platforms:
   - name: local
 
 suites:
+  # NOTE: In any test suite you can provide TF_VAR_roles=1 to test
+  #       with only 1 role in bindings. This way you can `converge` and `verify`
+  #       multiple times with roles being 2 and 1 to test how the module
+  #       behaves on configuration updates. This approach is used in the CI
+  #       `/build/int.cloudbuild.yaml`
 
   # Test all IAM modules in `additive` mode
   - name: additive
@@ -48,3 +53,21 @@ suites:
           backend: local
     provisioner:
       name: terraform
+
+  # Test projects IAM on how the module behaves
+  # in static mode (when all resources are statically defined)
+  # and in dynamic mode (when module params are obtained asynchronously
+  # from other resources)
+  # This suite supports specifying 1 to 3 for TF_VAR_roles
+  - name: static-and-dynamic
+    driver:
+      name: terraform
+      command_timeout: 1800
+      root_module_directory: test/fixtures/static-and-dynamic
+    verifier:
+      name: terraform
+      systems:
+        - name: static-and-dynamic
+          backend: local
+    provisioner:
+      name: terraform

CHANGELOG.md

@@ -10,9 +10,15 @@ corresponding pull request appended.
 
 ## [Unreleased]
 
+### Changed
+
+- Remove `*_num` options. [#64]
+- Limit the dynamic configuration usecases. More on this in [caveats][caveats]. [#64]
+
 ### Fixed
 
 - Authoritative bindings are correctly applied. [#61]
+- Migrate to `for_each` which fixes the configuration update issue. [#64]
 
 ## [3.0.0] - 2019-09-26
 
@@ -67,6 +73,7 @@ management.
 [stackdriver-agent-roles-example]: examples/stackdriver_agent_roles
 [subnet-example]: examples/subnet
 [usage-example]: README.md#usage
+[caveats]: README.md#caveats
 
 [Unreleased]: https://github.com/terraform-google-modules/terraform-google-iam/compare/v3.0.0...HEAD
 [3.0.0]: https://github.com/terraform-google-modules/terraform-google-iam/compare/v2.0.0...v3.0.0
@@ -87,3 +94,4 @@ management.
 [#32]: https://github.com/terraform-google-modules/terraform-google-iam/pull/32
 [#43]: https://github.com/terraform-google-modules/terraform-google-iam/pull/43
 [#61]: https://github.com/terraform-google-modules/terraform-google-iam/pull/61
+[#64]: https://github.com/terraform-google-modules/terraform-google-iam/pull/64

Makefile

@@ -18,7 +18,7 @@
 # Make will use bash instead of sh
 SHELL := /usr/bin/env bash
 
-DOCKER_TAG_VERSION_DEVELOPER_TOOLS := 0.1.0
+DOCKER_TAG_VERSION_DEVELOPER_TOOLS := 0.4.1
 DOCKER_IMAGE_DEVELOPER_TOOLS := cft/developer-tools
 REGISTRY_URL := gcr.io/cloud-foundation-cicd
 
@@ -83,4 +83,3 @@ docker_generate_docs:
 # Alias for backwards compatibility
 .PHONY: generate_docs
 generate_docs: docker_generate_docs
-

README.md

@@ -13,6 +13,7 @@ is [1.1.1][v1.1.1].
 
 The following guides are available to assist with upgrades:
 
+- [3.0 -> 4.0](./docs/upgrading_to_iam_4.0.md)
 - [2.0 -> 3.0](./docs/upgrading_to_iam_3.0.md)
 
 ## Usage
@@ -121,55 +122,35 @@ In additive mode, this module leaves existing bindings unaffected. Instead, any
 |------|-------------|:----:|:-----:|:-----:|
 | folders | Folders list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | folders\_bindings | Map of role (key) and list of members (value) to add the Folders IAM policies/bindings | map(list(string)) | n/a | yes |
-| folders\_bindings\_num | Number of Folders bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | folders\_mode | Mode for adding the Folders IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| folders\_num | Number of Folders, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | kms\_crypto\_keys | KMS Crypto Keys list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | kms\_crypto\_keys\_bindings | Map of role (key) and list of members (value) to add the KMS Crypto Keys IAM policies/bindings | map(list(string)) | n/a | yes |
-| kms\_crypto\_keys\_bindings\_num | Number of KMS Crypto Keys bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | kms\_crypto\_keys\_mode | Mode for adding the KMS Crypto Keys IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| kms\_crypto\_keys\_num | Number of KMS Crypto Keys, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | kms\_key\_rings | KMS Key Rings list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | kms\_key\_rings\_bindings | Map of role (key) and list of members (value) to add the KMS Key Rings IAM policies/bindings | map(list(string)) | n/a | yes |
-| kms\_key\_rings\_bindings\_num | Number of KMS Key Rings bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | kms\_key\_rings\_mode | Mode for adding the KMS Key Rings IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| kms\_key\_rings\_num | Number of KMS Key Rings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | organizations | Organizations list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | organizations\_bindings | Map of role (key) and list of members (value) to add the Organizations IAM policies/bindings | map(list(string)) | n/a | yes |
-| organizations\_bindings\_num | Number of Organizations bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | organizations\_mode | Mode for adding the Organizations IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| organizations\_num | Number of Organizations, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | project | Project to add the IAM policies/bindings | string | `""` | no |
 | projects | Projects list to add the IAM policies/bindings | list | `<list>` | no |
 | projects\_bindings | Map of role (key) and list of members (value) to add the Projects IAM policies/bindings | map(list(string)) | n/a | yes |
-| projects\_bindings\_num | Number of Projects bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | projects\_mode | Mode for adding the Projects IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| projects\_num | Number of Projects, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | pubsub\_subscriptions | PubSub Subscriptions list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | pubsub\_subscriptions\_bindings | Map of role (key) and list of members (value) to add the PubSub Subscriptions IAM policies/bindings | map(list(string)) | n/a | yes |
-| pubsub\_subscriptions\_bindings\_num | Number of PubSub Subscriptions bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | pubsub\_subscriptions\_mode | Mode for adding the PubSub Subscriptions IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| pubsub\_subscriptions\_num | Number of PubSub Subscriptions, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | pubsub\_topics | PubSub Topics list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | pubsub\_topics\_bindings | Map of role (key) and list of members (value) to add the PubSub Topics IAM policies/bindings | map(list(string)) | n/a | yes |
-| pubsub\_topics\_bindings\_num | Number of PubSub Topics bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | pubsub\_topics\_mode | Mode for adding the PubSub Topics IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| pubsub\_topics\_num | Number of PubSub Topics, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | service\_accounts | Service Accounts list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | service\_accounts\_bindings | Map of role (key) and list of members (value) to add the Service Accounts IAM policies/bindings | map(list(string)) | n/a | yes |
-| service\_accounts\_bindings\_num | Number of Service Accounts bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | service\_accounts\_mode | Mode for adding the Service Accounts IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| service\_accounts\_num | Number of Service Accounts, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | storage\_buckets | Storage Buckets list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | storage\_buckets\_bindings | Map of role (key) and list of members (value) to add the Storage Buckets IAM policies/bindings | map(list(string)) | n/a | yes |
-| storage\_buckets\_bindings\_num | Number of Storage Buckets bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | storage\_buckets\_mode | Mode for adding the Storage Buckets IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| storage\_buckets\_num | Number of Storage Buckets, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | subnets | Subnets list to add the IAM policies/bindings | list(string) | `<list>` | no |
 | subnets\_bindings | Map of role (key) and list of members (value) to add the Subnets IAM policies/bindings | map(list(string)) | n/a | yes |
-| subnets\_bindings\_num | Number of Subnets bindings, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | subnets\_mode | Mode for adding the Subnets IAM policies/bindings, additive and authoritative | string | `"additive"` | no |
-| subnets\_num | Number of Subnets, in case using dependcies of outher resources's outputs | number | `"0"` | no |
 | subnets\_region | Subnets region | string | n/a | yes |
 
 <!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
@@ -178,107 +159,12 @@ In additive mode, this module leaves existing bindings unaffected. Instead, any
 
 ### Referencing values/attributes from other resources
 
-This Terraform module performs operations over some variables before making any changes on the IAM bindings in GCP.
-
-The following variables have associated `*_num` variables which must be jointly configured with the number of elements:
-
-- `projects`
-- `organizations`
-- `folders`
-- `service_accounts`
-- `subnets`
-- `storage_buckets`
-- `pubsub_topics`
-- `pubsub_subscriptions`
-- `kms_key_rings`
-- `kms_crypto_keys`
-- `projects_bindings`
-- `organizations_bindings`
-- `folders_bindings`
-- `service_accounts_bindings`
-- `subnets_bindings`
-- `storage_buckets_bindings`
-- `pubsub_topics_bindings`
-- `pubsub_subscriptions_bindings`
-- `kms_key_rings_bindings`
-- `kms_crypto_keys_bindings`
-
-* For `authoritative` mode set variable equals to the number of roles applyed
-* For `additive` mode set variable equals to the number of Service Accounts and users and groups applyed
-
-For example, `authoritative` mode:
+This Terraform module performs operations over some variables before making any changes on the IAM bindings in GCP. Because of the limitations of `for_each` ([more info](https://www.terraform.io/docs/configuration/resources.html#using-expressions-in-for_each)), which is widely used in this module, there are certain limitations to what kind of dynamic values you can provide to the module:
 
-```hcl
-resource google_folder "my_new_folder" {
-  display_name = "folder-test"
-  parent       = "76543265432"
-}
-
-resource "google_service_account" "my_service_account" {
-  account_id = "my-new-service-account"
-}
-
-module "folders_iam_bindings" {
-  source  = "terraform-google-modules/iam/google//modules/folders_iam"
-  version = "~> 3.0"
-
-  mode = "authoritative"
-
-  folders     = [google_folder.my_new_folder.id]
-  folders_num = 1
-
-  bindings = {
-    "roles/storage.admin" = [
-      "group:test_sa_group@lnescidev.com",
-      "serviceAccount:${google_service_account.my_service_account.id}",
-    ]
-
-    "roles/compute.networkAdmin" = [
-      "group:test_sa_group@lnescidev.com",
-      "user:someone@google.com",
-    ]
-  }
-
-  bindings_num = 2
-}
-```
-
-`additive` mode:
-
-```hcl
-resource google_folder "my_new_folder" {
-  display_name = "folder-test"
-  parent       = "76543265432"
-}
-
-resource "google_service_account" "my_service_account" {
-  account_id = "my-new-service-account"
-}
-
-module "folders_iam_bindings" {
-  source  = "terraform-google-modules/iam/google//modules/folders_iam"
-  version = "~> 3.0"
-
-  mode = "additive"
-
-  folders      = [google_folder.my_new_folder.id]
-  folders_num  = 1
-
-  bindings = {
-    "roles/storage.admin" = [
-      "group:test_sa_group@lnescidev.com",
-      "serviceAccount:${google_service_account.my_service_account.id}",
-    ]
-
-    "roles/compute.networkAdmin" = [
-      "group:test_sa_group@lnescidev.com",
-      "user:someone@google.com",
-    ]
-  }
-
-  bindings_num = 4
-}
-```
+1. Dynamic entities (for example `projects`) are only allowed for 1 entity.
+2. If you pass 2 or more entities (for example `projects`), the configuration **MUST** be static, meaning that it can't use any of the other resources' fields to get the entity name from (this includes getting the randomly generated hashes through the `random_id` resource).
+3. The role names themselves can never be dynamic.
+4. Members may only be dynamic in `authoritative` mode.
 
 ## IAM Bindings
 

build/int.cloudbuild.yaml

@@ -30,6 +30,63 @@ steps:
 - id: verify
   name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
   args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do verify']
+
+# Test how the module behaves after configuration updates
+# Initial roles amount is 2
+
+# Change to 1 role and test
+- id: converge-1-role
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do converge']
+  env:
+  - 'TF_VAR_roles=1'
+- id: verify-1-role
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do verify']
+
+# Change to 2 roles and test
+- id: converge-2-roles
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do converge']
+  env:
+  - 'TF_VAR_roles=2'
+- id: verify-2-roles
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do verify']
+
+# Do more extensive configuration update testing for the `static-and-dynamic` suite.
+
+# Change static-and-dynamic to 3 roles and test
+- id: converge-static-and-dynamic-3-roles
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do converge static-and-dynamic']
+  env:
+  - 'TF_VAR_roles=3'
+- id: verify-static-and-dynamic-3-roles
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do verify static-and-dynamic']
+
+# Change static-and-dynamic to 1 role and test
+- id: converge-static-and-dynamic-1-role
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do converge static-and-dynamic']
+  env:
+  - 'TF_VAR_roles=1'
+- id: verify-static-and-dynamic-1-role
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do verify static-and-dynamic']
+
+# Change static-and-dynamic to 2 roles and test
+- id: converge-static-and-dynamic-2-roles
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do converge static-and-dynamic']
+  env:
+  - 'TF_VAR_roles=2'
+- id: verify-static-and-dynamic-2-roles
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do verify static-and-dynamic']
+
+# Destroy all suites
 - id: destroy
   name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
   args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && kitchen_do destroy']
@@ -38,4 +95,4 @@ tags:
 - 'integration'
 substitutions:
   _DOCKER_IMAGE_DEVELOPER_TOOLS: 'cft/developer-tools'
-  _DOCKER_TAG_VERSION_DEVELOPER_TOOLS: '0.1.0'
+  _DOCKER_TAG_VERSION_DEVELOPER_TOOLS: '0.4.1'

build/lint.cloudbuild.yaml

@@ -21,4 +21,4 @@ tags:
 - 'lint'
 substitutions:
   _DOCKER_IMAGE_DEVELOPER_TOOLS: 'cft/developer-tools'
-  _DOCKER_TAG_VERSION_DEVELOPER_TOOLS: '0.1.0'
+  _DOCKER_TAG_VERSION_DEVELOPER_TOOLS: '0.4.1'