Lightly refactor service agents locals in project module (#3380)

* lightly refactor project module service agent locals

* remove debug block

* tfdoc

* revert templatestring to format for universe kms

* reinstate moved block

modules/project/README.md

@@ -1850,7 +1850,7 @@ alerts:
 | [outputs.tf](./outputs.tf) | Module outputs. |  |
 | [quotas.tf](./quotas.tf) | None | <code>google_cloud_quotas_quota_preference</code> |
 | [scc-sha-custom-modules.tf](./scc-sha-custom-modules.tf) | Project-level Custom modules with Security Health Analytics. | <code>google_scc_management_project_security_health_analytics_custom_module</code> |
-| [service-agents.tf](./service-agents.tf) | Service agents supporting resources. | <code>google_project_default_service_accounts</code> · <code>google_project_iam_member</code> · <code>google_project_service_identity</code> · <code>terraform_data</code> |
+| [service-agents.tf](./service-agents.tf) | Service agents supporting resources. | <code>google_project_default_service_accounts</code> · <code>google_project_iam_member</code> · <code>google_project_service_identity</code> |
 | [shared-vpc.tf](./shared-vpc.tf) | Shared VPC project-level configuration. | <code>google_compute_shared_vpc_host_project</code> · <code>google_compute_shared_vpc_service_project</code> · <code>google_compute_subnetwork_iam_member</code> · <code>google_project_iam_member</code> |
 | [tags.tf](./tags.tf) | Manages GCP Secure Tags, keys, values, and IAM. | <code>google_tags_tag_binding</code> · <code>google_tags_tag_key</code> · <code>google_tags_tag_key_iam_binding</code> · <code>google_tags_tag_key_iam_member</code> · <code>google_tags_tag_value</code> · <code>google_tags_tag_value_iam_binding</code> · <code>google_tags_tag_value_iam_member</code> |
 | [variables-iam.tf](./variables-iam.tf) | None |  |

modules/project/cmek.tf

@@ -51,11 +51,15 @@ locals {
   }
   _all_cmek_bindings = flatten([
     for service, keys in var.service_encryption_key_ids : [
-      for dep in try(local._cmek_agents_by_service[service], [for x in local._service_agents_by_api[service] : x.name], [service]) : [
+      for dep in try(
+        local._cmek_agents_by_service[service],
+        [for x in local.service_agents_by_api[service] : x.name],
+        [service]
+        ) : [
         for key in keys : {
           key_id      = key
-          agent_name  = local._aliased_service_agents[dep].name
+          agent_name  = local.aliased_service_agents[dep].name
-          agent_email = local._aliased_service_agents[dep].iam_email
+          agent_email = local.aliased_service_agents[dep].iam_email
         }
       ]
     ]

modules/project/main.tf

@@ -73,8 +73,12 @@ locals {
     ? "${local.prefix}${var.name}"
     : "${local.universe_prefix}${local.prefix}${var.name}"
   )
-  universe_prefix    = var.universe == null ? "" : "${var.universe.prefix}:"
+  universe_prefix = var.universe == null ? "" : "${var.universe.prefix}:"
-  available_services = tolist(setsubtract(var.services, try(var.universe.unavailable_services, [])))
+  # available services are those declared, minus any unsupported by universe
+  available_services = tolist(setsubtract(
+    var.services,
+    try(var.universe.unavailable_services, [])
+  ))
 }
 
 data "google_project" "project" {

modules/project/service-agents.tf

@@ -16,118 +16,117 @@
 
 # tfdoc:file:description Service agents supporting resources.
 
-# debug block
-
-# resource "terraform_data" "precondition" {
-#   lifecycle {
-#     precondition {
-#       condition     = local.primary_service_agents == null
-#       error_message = yamlencode(local.primary_service_agents)
-#     }
-#   }
-# }
-
 locals {
-  services = [
+  _sa_raw = yamldecode(file("${path.module}/service-agents.yaml"))
-    for s in distinct(concat(
+  # initial map of service agents by name, defining agent email
-      local.available_services, try(var.project_reuse.attributes.services_enabled, [])
+  _sa_0 = merge([
-    )) : s if !contains(local._universe_unavailable_si, s)
-  ]
-  _service_agents_data = yamldecode(file("${path.module}/service-agents.yaml"))
-  # map of api => list of agents
-  _service_agents_by_api = {
-    for agent in local._service_agents_data :
-    coalesce(agent.api, "cloudservices") => agent... # cloudservices api is null
-  }
-  _universe_domain = (
-    var.universe == null
-    ? ""
-    : "${var.universe.prefix}-system."
-  )
-  _universe_unavailable_si = try(var.universe.unavailable_service_identities, [])
-  # map of service agent name => agent details for this project
-  _project_service_agents_0 = merge([
     for api in concat(local.services, ["cloudservices"]) : {
-      for agent in lookup(local._service_agents_by_api, api, []) :
+      for agent in lookup(local.service_agents_by_api, api, []) :
       (agent.name) => merge(agent, {
         email = (
-          # If universe variable is set, enfore the use of the service-PROJECT_NUMBER@gcp-sa-ekms.UNIVERSE-system.iam.gserviceaccount.com
+          api == "cloudservices"
-          # instead of service-PROJECT_NUMBER@gcp-sa-kms.UNIVERSE-system.iam.gserviceaccount.com
+          ? format(
-          # as in the TPC universes, the partner KMS is enforced by design
+            "%s@cloudservices.%siam.gserviceaccount.com",
-          var.universe != null && api == "cloudkms.googleapis.com"
+            local.project.number,
-          ? format("service-%s@gcp-sa-ekms.%siam.gserviceaccount.com", local.project.number, local._universe_domain)
+            local._u_domain
+          )
           : (
-            var.universe == null || api != "cloudservices"
+            var.universe == null || !startswith(api, "cloudkms.")
-            ? templatestring(agent.identity, { project_number = local.project.number, universe_domain = local._universe_domain })
+            ? templatestring(agent.identity, {
-            : format("%s@cloudservices.%siam.gserviceaccount.com", local.project.number, local._universe_domain)
+              project_number  = local.project.number
+              universe_domain = local._u_domain
+            })
+            # universe uses partner KMS
+            : format(
+              "service-%s@gcp-sa-ekms.%siam.gserviceaccount.com",
+              local.project.number,
+              local._u_domain
+            )
           )
         )
       })
     }
   ]...)
-  _project_service_agents = {
+  # final map of service agents by name including JIT creation flag
-    for k, v in local._project_service_agents_0 :
+  _sa = {
-    k => merge(v, {
+    for k, v in local._sa_0 : k => merge(v, {
-      iam_email  = "serviceAccount:${v.email}"
+      iam_email = "serviceAccount:${v.email}"
-      create_jit = v.api == null ? false : contains(local.available_services, v.api)
+      create_jit = (
-    }) if !contains(local._universe_unavailable_si, k)
+        v.api == null ? false : contains(local.available_services, v.api)
+      )
+      # skip identities which are unavailable in the defined universe
+    }) if !contains(local._u_unavailable_si, k)
   }
-  # list of APIs with primary agents that should be created for the
+  # map of name => agent for all known aliases
-  # current project, if the user requested it
+  _sa_aliases = merge(local._sa, flatten([
+    for agent_name, agent in local._sa : [
+      for alias in agent.aliases : { (alias) = agent }
+    ]
+  ])...)
+  _u_domain = (
+    var.universe == null ? "" : "${var.universe.prefix}-system."
+  )
+  _u_unavailable_si = try(
+    var.universe.unavailable_service_identities, []
+  )
+  # aliased service agents, with unnecessary fields removed
+  aliased_service_agents = {
+    for k, v in local._sa_aliases : k => {
+      api          = v.api
+      display_name = v.display_name
+      email        = v.email
+      iam_email    = v.iam_email
+      is_primary   = v.is_primary
+      name         = v.name
+      role         = v.role
+    }
+  }
+  # service agents to create in this project
   primary_service_agents = [
-    for agent in local._project_service_agents : agent.api if(
+    for agent in local._sa : agent.api if(
+      # only create if user asked us to (which is the default)
       var.service_agents_config.create_primary_agents && (
-        (agent.is_primary && agent.create_jit) || contains(
+        # only create if agent is primary and JIT flag is true
+        (agent.is_primary && agent.create_jit)
+        ||
+        # or if universe configuration is forcing this agent to be created
+        contains(
           try(var.universe.forced_jit_service_identities, []),
           coalesce(agent.api, "-")
         )
       )
     )
   ]
-  # list of roles that should be granted to service agents for the
+  # group service agents by api, cloudservices api is null
-  # current project, if the user requested it
+  service_agents_by_api = {
+    for v in local._sa_raw : coalesce(v.api, "cloudservices") => v...
+  }
+  # IAM roles for service agents to create in this project
   service_agent_roles = {
-    for agent in local._project_service_agents :
+    for agent in local._sa : (agent.name) => {
-    (agent.name) => {
       role      = agent.role
       iam_email = agent.iam_email
-    }
+      } if alltrue([
-    if alltrue([
+        var.service_agents_config.grant_default_roles,
-      var.service_agents_config.grant_default_roles,
+        agent.role != null,
-      agent.role != null,
+        # TODO: improve the detection below
-      # FIXME: granting roles to the non-primary agents listed below
+        # this skips IAM role grants to the non-primary agents listed below
-      # currently fails, possibly because the agents doesn't exist
+        # as it's failing, possibly because the agents don't exist
-      # after API activation. As a workaround, automatic role
+        # after API activation
-      # assignment for these agents is disabled.
+        !contains([
-      !contains([
+          "apigateway", "apigateway-mgmt", "bigqueryspark", "bigquerytardis",
-        "apigateway", "apigateway-mgmt", "bigqueryspark", "bigquerytardis",
+          "firebase", "krmapihosting", "krmapihosting-dataplane", "logging",
-        "firebase", "krmapihosting", "krmapihosting-dataplane", "logging",
+          "networkactions", "prod-bigqueryomni", "scc-notification",
-        "networkactions", "prod-bigqueryomni", "scc-notification", "securitycenter",
+          "securitycenter"
-      ], agent.name)
+        ], agent.name)
     ])
   }
-  # map of name->agent including all known aliases
+  services = [
-  _aliased_service_agents = merge(
+    for s in distinct(concat(
-    local._project_service_agents,
+      local.available_services,
-    flatten([
+      try(var.project_reuse.attributes.services_enabled, [])
-      for agent_name, agent in local._project_service_agents : [
+    )) : s if !contains(local._u_unavailable_si, s)
-        for alias in agent.aliases :
+  ]
-        { (alias) = agent }
-      ]
-    ])...
-  )
-  # same as _aliased_service_agents with unneeded fields removed
-  aliased_service_agents = {
-    for k, v in local._aliased_service_agents :
-    k => {
-      api          = v.api
-      display_name = v.display_name
-      email        = v.email
-      iam_email    = v.iam_email
-      is_primary   = v.is_primary
-      role         = v.role
-    }
-  }
 }
 
 data "google_storage_project_service_account" "gcs_sa" {
@@ -160,7 +159,6 @@ resource "google_project_service_identity" "default" {
   depends_on = [google_project_service.project_services]
 }
 
-
 moved {
   from = google_project_iam_member.servicenetworking[0]
   to   = google_project_iam_member.service_agents["service-networking"]