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 <ludomagno@google.com>

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"

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
 }

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 = []

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