From a97c606253cc84f5a1fe05086c2b3c805adc92c1 Mon Sep 17 00:00:00 2001 From: lcaggio Date: Sat, 22 Jan 2022 08:32:13 +0100 Subject: [PATCH] Support service dependencies for crypto key bindings in project module (#443) * Support services that require crypto/decrypt role on robot service accounts * delete test and upload refactored implementation * fix duplicate key on dependent services, add tests Co-authored-by: Ludovico Magnocavallo --- modules/project/main.tf | 35 ++++++++++++------ tests/modules/project/fixture/main.tf | 41 +++++++++++----------- tests/modules/project/fixture/variables.tf | 5 +++ tests/modules/project/test_plan.py | 29 +++++++++++++++ 4 files changed, 79 insertions(+), 31 deletions(-) diff --git a/modules/project/main.tf b/modules/project/main.tf index 7594c07a..f86c04dd 100644 --- a/modules/project/main.tf +++ b/modules/project/main.tf @@ -15,8 +15,18 @@ */ locals { - descriptive_name = var.descriptive_name != null ? var.descriptive_name : "${local.prefix}${var.name}" - group_iam_roles = distinct(flatten(values(var.group_iam))) + # service dependendencies for encryption keys + _service_encryption_key_dependent_services = { + "composer" : [ + "composer", + "artifactregistry", "container-engine", "compute", "pubsub", "storage" + ] + "dataflow" : ["dataflow", "compute"] + } + descriptive_name = ( + var.descriptive_name != null ? var.descriptive_name : "${local.prefix}${var.name}" + ) + 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 @@ -74,17 +84,18 @@ locals { if sink.iam && sink.type == type } } - service_encryption_key_ids = flatten([ - for service in keys(var.service_encryption_key_ids) : [ - for key in var.service_encryption_key_ids[service] : { - service = service - key = key - } if key != null + service_encryption_key_ids = distinct(flatten([ + for s in keys(var.service_encryption_key_ids) : [ + for ss in try(local._service_encryption_key_dependent_services[s], [s]) : [ + for key in var.service_encryption_key_ids[s] : { + service = ss + key = key + } if key != null + ] ] - ]) + ])) } - data "google_project" "project" { count = var.project_create ? 0 : 1 project_id = "${local.prefix}${var.name}" @@ -383,7 +394,9 @@ resource "google_access_context_manager_service_perimeter_resource" "service-per resource "google_kms_crypto_key_iam_member" "crypto_key" { for_each = { - for service_key in local.service_encryption_key_ids : "${service_key.service}.${service_key.key}" => service_key if service_key != service_key.key + for service_key in local.service_encryption_key_ids : + "${service_key.service}.${service_key.key}" => service_key + if service_key != service_key.key } crypto_key_id = each.value.key role = "roles/cloudkms.cryptoKeyEncrypterDecrypter" diff --git a/tests/modules/project/fixture/main.tf b/tests/modules/project/fixture/main.tf index 2d2e53da..6d3f55ea 100644 --- a/tests/modules/project/fixture/main.tf +++ b/tests/modules/project/fixture/main.tf @@ -15,24 +15,25 @@ */ 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 - 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 - logging_sinks = var.logging_sinks - logging_exclusions = var.logging_exclusions + 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 + service_encryption_key_ids = var.service_encryption_key_ids + services = var.services + logging_sinks = var.logging_sinks + logging_exclusions = var.logging_exclusions } diff --git a/tests/modules/project/fixture/variables.tf b/tests/modules/project/fixture/variables.tf index 4af03a1e..c2b8d0ca 100644 --- a/tests/modules/project/fixture/variables.tf +++ b/tests/modules/project/fixture/variables.tf @@ -89,6 +89,11 @@ variable "prefix" { default = null } +variable "service_encryption_key_ids" { + type = map(list(string)) + default = {} +} + variable "services" { type = list(string) default = [] diff --git a/tests/modules/project/test_plan.py b/tests/modules/project/test_plan.py index cb1b2f47..21870f55 100644 --- a/tests/modules/project/test_plan.py +++ b/tests/modules/project/test_plan.py @@ -40,3 +40,32 @@ def test_no_parent(plan_runner): assert len(resources) == 1 assert resources[0]['values'].get('folder_id') == None assert resources[0]['values'].get('org_id') == None + + +def test_service_encryption_keys(plan_runner): + "Test service encryption keys with no dependencies." + _, resources = plan_runner(service_encryption_key_ids=( + '{compute=["key1"], storage=["key1", "key2"]}' + )) + key_bindings = [ + r['index'] for r in resources + if r['type'] == 'google_kms_crypto_key_iam_member' + ] + assert len(key_bindings), 3 + assert key_bindings == ['compute.key1', 'storage.key1', 'storage.key2'] + + +def test_service_encryption_key_dependencies(plan_runner): + "Test service encryption keys with dependencies." + _, resources = plan_runner(service_encryption_key_ids=( + '{compute=["key1"], dataflow=["key1", "key2"]}' + )) + key_bindings = [ + r['index'] for r in resources + if r['type'] == 'google_kms_crypto_key_iam_member' + ] + assert len(key_bindings), 3 + # compute.key1 cannot repeat or we'll get a duplicate key error in for_each + assert key_bindings == [ + 'compute.key1', 'compute.key2', 'dataflow.key1', 'dataflow.key2' + ]