From 5af022a3eec8a1fe19e2c07e923f83d69f386e59 Mon Sep 17 00:00:00 2001 From: vanessabodard-voi <63779321+vanessabodard-voi@users.noreply.github.com> Date: Thu, 2 Jul 2020 08:28:26 +0200 Subject: [PATCH] refactor IAM additive bindings variables (#103) * Invert the list for role/member mapping so that member is the key * Add iam_additive_bindings to replace iam_additive_members and iam_additive_roles, change test suite accordingly * attribute 'mode' added as it would error without * Update Readme to reflect the new variable iam_additive_bindings * test branch access * iam_additive_bindings to replace iam_additive roles and iam_additive_members * update foundation samples to new additive bindings format * set bq dataset options in foundation environments to allow destroying * trap exceptions raised during destroy in project module Co-authored-by: Ludovico Magnocavallo --- foundations/business-units/main.tf | 30 ++++++++++-------- foundations/environments/main.tf | 30 ++++++++++-------- foundations/environments/variables.tf | 14 +-------- modules/organization/README.md | 3 +- modules/organization/main.tf | 6 ++-- modules/organization/variables.tf | 10 ++---- modules/project/README.md | 31 +++++++++++++++++-- modules/project/main.tf | 10 +++--- modules/project/outputs.tf | 2 +- modules/project/variables.tf | 11 ++----- tests/modules/organization/fixture/main.tf | 3 +- .../modules/organization/fixture/variables.tf | 6 +--- tests/modules/project/fixture/main.tf | 3 +- tests/modules/project/fixture/variables.tf | 7 +---- 14 files changed, 84 insertions(+), 82 deletions(-) diff --git a/foundations/business-units/main.tf b/foundations/business-units/main.tf index 1a599b8d..b30b5c46 100644 --- a/foundations/business-units/main.tf +++ b/foundations/business-units/main.tf @@ -29,14 +29,15 @@ module "shared-folder" { # Terraform project module "tf-project" { - source = "../../modules/project" - name = "terraform" - parent = module.shared-folder.id - prefix = var.prefix - billing_account = var.billing_account_id - iam_additive_members = { "roles/owner" = var.iam_terraform_owners } - iam_additive_roles = ["roles/owner"] - services = var.project_services + source = "../../modules/project" + name = "terraform" + parent = module.shared-folder.id + prefix = var.prefix + billing_account = var.billing_account_id + iam_additive_bindings = { + for name in var.iam_terraform_owners : (name) => ["roles/owner"] + } + services = var.project_services } # Bootstrap Terraform state GCS bucket @@ -115,6 +116,12 @@ module "audit-dataset" { project_id = module.audit-project.project_id id = "audit_export" friendly_name = "Audit logs export." + # disable delete on destroy for actual use + options = { + default_table_expiration_ms = null + default_partition_expiration_ms = null + delete_contents_on_destroy = true + } } module "audit-log-sinks" { @@ -140,12 +147,9 @@ module "shared-project" { parent = module.shared-folder.id prefix = var.prefix billing_account = var.billing_account_id - iam_additive_members = { - "roles/owner" = var.iam_shared_owners + iam_additive_bindings = { + for name in var.iam_shared_owners : (name) => ["roles/owner"] } - iam_additive_roles = [ - "roles/owner" - ] services = var.project_services } diff --git a/foundations/environments/main.tf b/foundations/environments/main.tf index ee55f161..90b4a8bf 100644 --- a/foundations/environments/main.tf +++ b/foundations/environments/main.tf @@ -19,14 +19,15 @@ # Terraform project module "tf-project" { - source = "../../modules/project" - name = "terraform" - parent = var.root_node - prefix = var.prefix - billing_account = var.billing_account_id - iam_additive_members = { "roles/owner" = var.iam_terraform_owners } - iam_additive_roles = ["roles/owner"] - services = var.project_services + source = "../../modules/project" + name = "terraform" + parent = var.root_node + prefix = var.prefix + billing_account = var.billing_account_id + iam_additive_bindings = { + for name in var.iam_terraform_owners : (name) => ["roles/owner"] + } + services = var.project_services } # per-environment service accounts @@ -130,6 +131,12 @@ module "audit-dataset" { project_id = module.audit-project.project_id id = "audit_export" friendly_name = "Audit logs export." + # disable delete on destroy for actual use + options = { + default_table_expiration_ms = null + default_partition_expiration_ms = null + delete_contents_on_destroy = true + } } module "audit-log-sinks" { @@ -156,12 +163,9 @@ module "sharedsvc-project" { parent = var.root_node prefix = var.prefix billing_account = var.billing_account_id - iam_additive_members = { - "roles/owner" = var.iam_sharedsvc_owners + iam_additive_bindings = { + for name in var.iam_shared_owners : (name) => ["roles/owner"] } - iam_additive_roles = [ - "roles/owner" - ] services = var.project_services } diff --git a/foundations/environments/variables.tf b/foundations/environments/variables.tf index a868140d..5cb74456 100644 --- a/foundations/environments/variables.tf +++ b/foundations/environments/variables.tf @@ -38,18 +38,6 @@ variable "gcs_location" { default = "EU" } -variable "iam_assets_editors" { - description = "Shared assets project editors, in IAM format." - type = list(string) - default = [] -} - -variable "iam_assets_owners" { - description = "Shared assets project owners, in IAM format." - type = list(string) - default = [] -} - variable "iam_audit_viewers" { description = "Audit project viewers, in IAM format." type = list(string) @@ -79,7 +67,7 @@ variable "iam_folder_roles" { ] } -variable "iam_sharedsvc_owners" { +variable "iam_shared_owners" { description = "Shared services project owners, in IAM format." type = list(string) default = [] diff --git a/modules/organization/README.md b/modules/organization/README.md index 53ac2cbb..c95bba8f 100644 --- a/modules/organization/README.md +++ b/modules/organization/README.md @@ -37,8 +37,7 @@ module "org" { |---|---|:---: |:---:|:---:| | org_id | Organization id in nnnnnn format. | number | ✓ | | | *custom_roles* | Map of role name => list of permissions to create in this project. | map(list(string)) | | {} | -| *iam_additive_members* | Map of member lists used to set non authoritative bindings, keyed by role. | map(list(string)) | | {} | -| *iam_additive_roles* | List of roles used to set non authoritative bindings. | list(string) | | [] | +| *iam_additive_bindings* | Map of roles lists used to set non authoritative bindings, keyed by members. | 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))) | | {} | | *iam_members* | Map of member lists used to set authoritative bindings, keyed by role. | map(list(string)) | | {} | | *iam_roles* | List of roles used to set authoritative bindings. | list(string) | | [] | diff --git a/modules/organization/main.tf b/modules/organization/main.tf index 91e01b5f..a96ff141 100644 --- a/modules/organization/main.tf +++ b/modules/organization/main.tf @@ -16,8 +16,8 @@ locals { iam_additive_pairs = flatten([ - for role in var.iam_additive_roles : [ - for member in lookup(var.iam_additive_members, role, []) : + for member, roles in var.iam_additive_bindings : [ + for role in roles : { role = role, member = member } ] ]) @@ -44,7 +44,7 @@ resource "google_organization_iam_binding" "authoritative" { } resource "google_organization_iam_member" "additive" { - for_each = length(var.iam_additive_roles) > 0 ? local.iam_additive : {} + for_each = length(var.iam_additive_bindings) > 0 ? local.iam_additive : {} org_id = var.org_id role = each.value.role member = each.value.member diff --git a/modules/organization/variables.tf b/modules/organization/variables.tf index 05d636bc..240e920f 100644 --- a/modules/organization/variables.tf +++ b/modules/organization/variables.tf @@ -32,18 +32,12 @@ variable "iam_roles" { default = [] } -variable "iam_additive_members" { - description = "Map of member lists used to set non authoritative bindings, keyed by role." +variable "iam_additive_bindings" { + description = "Map of roles lists used to set non authoritative bindings, keyed by members." type = map(list(string)) default = {} } -variable "iam_additive_roles" { - description = "List of roles used to set non authoritative bindings." - type = 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 cd640673..7fd57c01 100644 --- a/modules/project/README.md +++ b/modules/project/README.md @@ -24,6 +24,33 @@ module "project" { } ``` +### Minimal example with IAM additive roles + +```hcl +module "project" { + source = "./modules/project" + name = "project-example" + project_create = false + + iam_additive_bindings = { + "group:usergroup_watermlon_experimentation@lemonadeinc.io" = [ + "roles/viewer", + "roles/storage.objectAdmin" + ], + "group:usergroup_gcp_admin@lemonadeinc.io" = [ + "roles/owner", + ], + "group:usergroup_gcp_privilege_access@lemonadeinc.io" = [ + "roles/editor" + ], + "group:engineering@lemonadeinc.io" = [ + "roles/pubsub.subscriber", + "roles/storage.objectViewer" + ], + } +} +``` + ### Organization policies ```hcl @@ -61,8 +88,7 @@ module "project" { | *auto_create_network* | Whether to create the default network for the project | bool | | false | | *billing_account* | Billing account id. | string | | null | | *custom_roles* | Map of role name => list of permissions to create in this project. | map(list(string)) | | {} | -| *iam_additive_members* | Map of member lists used to set non authoritative bindings, keyed by role. | map(list(string)) | | {} | -| *iam_additive_roles* | List of roles used to set non authoritative bindings. | list(string) | | [] | +| *iam_additive_bindings* | Map of roles list used to set non authoritative bindings, keyed by member. | list(string) | | [] | | *iam_members* | Map of member lists used to set authoritative bindings, keyed by role. | map(list(string)) | | {} | | *iam_roles* | List of roles used to set authoritative bindings. | list(string) | | [] | | *labels* | Resource labels. | map(string) | | {} | @@ -88,3 +114,4 @@ module "project" { | project_id | Project id. | | | service_accounts | Product robot service accounts in project. | | + diff --git a/modules/project/main.tf b/modules/project/main.tf index 637aa8de..a9239970 100644 --- a/modules/project/main.tf +++ b/modules/project/main.tf @@ -16,8 +16,8 @@ locals { iam_additive_pairs = flatten([ - for role in var.iam_additive_roles : [ - for member in lookup(var.iam_additive_members, role, []) : + for member, roles in var.iam_additive_bindings : [ + for role in roles : { role = role, member = member } ] ]) @@ -29,7 +29,9 @@ locals { parent_id = var.parent == null ? null : split("/", var.parent)[1] prefix = var.prefix == null ? "" : "${var.prefix}-" project = ( - var.project_create ? google_project.project.0 : data.google_project.project.0 + var.project_create + ? try(google_project.project.0, null) + : try(data.google_project.project.0, null) ) } @@ -100,7 +102,7 @@ resource "google_project_iam_binding" "authoritative" { } resource "google_project_iam_member" "additive" { - for_each = length(var.iam_additive_roles) > 0 ? local.iam_additive : {} + for_each = length(var.iam_additive_bindings) > 0 ? local.iam_additive : {} project = local.project.project_id role = each.value.role member = each.value.member diff --git a/modules/project/outputs.tf b/modules/project/outputs.tf index 1250f846..2edeecaf 100644 --- a/modules/project/outputs.tf +++ b/modules/project/outputs.tf @@ -16,7 +16,7 @@ output "project_id" { description = "Project id." - value = local.project.project_id + value = try(local.project.project_id, null) depends_on = [ google_project_organization_policy.boolean, google_project_organization_policy.list, diff --git a/modules/project/variables.tf b/modules/project/variables.tf index 6676b76a..44677620 100644 --- a/modules/project/variables.tf +++ b/modules/project/variables.tf @@ -44,18 +44,13 @@ variable "iam_roles" { default = [] } -variable "iam_additive_members" { - description = "Map of member lists used to set non authoritative bindings, keyed by role." + +variable "iam_additive_bindings" { + description = "Map of roles lists used to set non authoritative bindings, keyed by members" type = map(list(string)) default = {} } -variable "iam_additive_roles" { - description = "List of roles used to set non authoritative bindings." - type = 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 20786b5e..63d1f466 100644 --- a/tests/modules/organization/fixture/main.tf +++ b/tests/modules/organization/fixture/main.tf @@ -20,8 +20,7 @@ module "test" { custom_roles = var.custom_roles iam_members = var.iam_members iam_roles = var.iam_roles - iam_additive_members = var.iam_additive_members - iam_additive_roles = var.iam_additive_roles + iam_additive_bindings= var.iam_additive_bindings 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 561b446c..148a43b7 100644 --- a/tests/modules/organization/fixture/variables.tf +++ b/tests/modules/organization/fixture/variables.tf @@ -29,15 +29,11 @@ variable "iam_roles" { default = [] } -variable "iam_additive_members" { +variable "iam_additive_bindings" { type = map(list(string)) default = {} } -variable "iam_additive_roles" { - type = list(string) - default = [] -} variable "iam_audit_config" { type = map(map(list(string))) diff --git a/tests/modules/project/fixture/main.tf b/tests/modules/project/fixture/main.tf index 8d8808c2..924b2648 100644 --- a/tests/modules/project/fixture/main.tf +++ b/tests/modules/project/fixture/main.tf @@ -22,8 +22,7 @@ module "test" { custom_roles = var.custom_roles iam_members = var.iam_members iam_roles = var.iam_roles - iam_additive_members = var.iam_additive_members - iam_additive_roles = var.iam_additive_roles + iam_additive_bindings = var.iam_additive_bindings labels = var.labels lien_reason = var.lien_reason oslogin = var.oslogin diff --git a/tests/modules/project/fixture/variables.tf b/tests/modules/project/fixture/variables.tf index 4b859c92..3b36a5fd 100644 --- a/tests/modules/project/fixture/variables.tf +++ b/tests/modules/project/fixture/variables.tf @@ -34,16 +34,11 @@ variable "iam_roles" { default = [] } -variable "iam_additive_members" { +variable "iam_additive_bindings" { type = map(list(string)) default = {} } -variable "iam_additive_roles" { - type = list(string) - default = [] -} - variable "labels" { type = map(string) default = {}