Add explicit errors when VPC-SC perimeters reference undefined directional policies (#3133)

* Add explicit errors when VPC-SC perimeters reference undefined directional policies

* Fix try calls

* Update defaults

modules/vpc-sc/README.md

@@ -410,7 +410,7 @@ to:
 | [iam_bindings](variables.tf#L149) | Authoritative IAM bindings in {KEY => {role = ROLE, members = [], condition = {}}}. Keys are arbitrary. | <code title="map&#40;object&#40;&#123;&#10;  members &#61; list&#40;string&#41;&#10;  role    &#61; string&#10;  condition &#61; optional&#40;object&#40;&#123;&#10;    expression  &#61; string&#10;    title       &#61; string&#10;    description &#61; optional&#40;string&#41;&#10;  &#125;&#41;&#41;&#10;&#125;&#41;&#41;">map&#40;object&#40;&#123;&#8230;&#125;&#41;&#41;</code> |  | <code>&#123;&#125;</code> |
 | [iam_bindings_additive](variables.tf#L164) | Individual additive IAM bindings. Keys are arbitrary. | <code title="map&#40;object&#40;&#123;&#10;  member &#61; string&#10;  role   &#61; string&#10;  condition &#61; optional&#40;object&#40;&#123;&#10;    expression  &#61; string&#10;    title       &#61; string&#10;    description &#61; optional&#40;string&#41;&#10;  &#125;&#41;&#41;&#10;&#125;&#41;&#41;">map&#40;object&#40;&#123;&#8230;&#125;&#41;&#41;</code> |  | <code>&#123;&#125;</code> |
 | [ingress_policies](variables.tf#L179) | Ingress policy definitions that can be referenced in perimeters. | <code title="map&#40;object&#40;&#123;&#10;  title &#61; optional&#40;string&#41;&#10;  from &#61; object&#40;&#123;&#10;    access_levels &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    identity_type &#61; optional&#40;string&#41;&#10;    identities    &#61; optional&#40;list&#40;string&#41;&#41;&#10;    resources     &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;  &#125;&#41;&#10;  to &#61; object&#40;&#123;&#10;    operations &#61; optional&#40;list&#40;object&#40;&#123;&#10;      method_selectors     &#61; optional&#40;list&#40;string&#41;&#41;&#10;      permission_selectors &#61; optional&#40;list&#40;string&#41;&#41;&#10;      service_name         &#61; string&#10;    &#125;&#41;&#41;, &#91;&#93;&#41;&#10;    resources &#61; optional&#40;list&#40;string&#41;&#41;&#10;    roles     &#61; optional&#40;list&#40;string&#41;&#41;&#10;  &#125;&#41;&#10;&#125;&#41;&#41;">map&#40;object&#40;&#123;&#8230;&#125;&#41;&#41;</code> |  | <code>&#123;&#125;</code> |
-| [perimeters](variables.tf#L221) | Regular service perimeters. | <code title="map&#40;object&#40;&#123;&#10;  description               &#61; optional&#40;string&#41;&#10;  ignore_resource_changes   &#61; optional&#40;bool, false&#41;&#10;  title                     &#61; optional&#40;string&#41;&#10;  use_explicit_dry_run_spec &#61; optional&#40;bool, false&#41;&#10;  spec &#61; optional&#40;object&#40;&#123;&#10;    access_levels       &#61; optional&#40;list&#40;string&#41;&#41;&#10;    egress_policies     &#61; optional&#40;list&#40;string&#41;&#41;&#10;    ingress_policies    &#61; optional&#40;list&#40;string&#41;&#41;&#10;    restricted_services &#61; optional&#40;list&#40;string&#41;&#41;&#10;    resources           &#61; optional&#40;list&#40;string&#41;&#41;&#10;    vpc_accessible_services &#61; optional&#40;object&#40;&#123;&#10;      allowed_services   &#61; list&#40;string&#41;&#10;      enable_restriction &#61; optional&#40;bool, true&#41;&#10;    &#125;&#41;&#41;&#10;  &#125;&#41;&#41;&#10;  status &#61; optional&#40;object&#40;&#123;&#10;    access_levels       &#61; optional&#40;list&#40;string&#41;&#41;&#10;    egress_policies     &#61; optional&#40;list&#40;string&#41;&#41;&#10;    ingress_policies    &#61; optional&#40;list&#40;string&#41;&#41;&#10;    resources           &#61; optional&#40;list&#40;string&#41;&#41;&#10;    restricted_services &#61; optional&#40;list&#40;string&#41;&#41;&#10;    vpc_accessible_services &#61; optional&#40;object&#40;&#123;&#10;      allowed_services   &#61; list&#40;string&#41;&#10;      enable_restriction &#61; optional&#40;bool, true&#41;&#10;    &#125;&#41;&#41;&#10;  &#125;&#41;&#41;&#10;&#125;&#41;&#41;">map&#40;object&#40;&#123;&#8230;&#125;&#41;&#41;</code> |  | <code>&#123;&#125;</code> |
+| [perimeters](variables.tf#L221) | Regular service perimeters. | <code title="map&#40;object&#40;&#123;&#10;  description               &#61; optional&#40;string&#41;&#10;  ignore_resource_changes   &#61; optional&#40;bool, false&#41;&#10;  title                     &#61; optional&#40;string&#41;&#10;  use_explicit_dry_run_spec &#61; optional&#40;bool, false&#41;&#10;  spec &#61; optional&#40;object&#40;&#123;&#10;    access_levels       &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    egress_policies     &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    ingress_policies    &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    restricted_services &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    resources           &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    vpc_accessible_services &#61; optional&#40;object&#40;&#123;&#10;      allowed_services   &#61; list&#40;string&#41;&#10;      enable_restriction &#61; optional&#40;bool, true&#41;&#10;    &#125;&#41;&#41;&#10;  &#125;&#41;&#41;&#10;  status &#61; optional&#40;object&#40;&#123;&#10;    access_levels       &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    egress_policies     &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    ingress_policies    &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    resources           &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    restricted_services &#61; optional&#40;list&#40;string&#41;, &#91;&#93;&#41;&#10;    vpc_accessible_services &#61; optional&#40;object&#40;&#123;&#10;      allowed_services   &#61; list&#40;string&#41;&#10;      enable_restriction &#61; optional&#40;bool, true&#41;&#10;    &#125;&#41;&#41;&#10;  &#125;&#41;&#41;&#10;&#125;&#41;&#41;">map&#40;object&#40;&#123;&#8230;&#125;&#41;&#41;</code> |  | <code>&#123;&#125;</code> |
 | [project_id_search_scope](variables.tf#L255) | Set this to an organization or folder ID to use Cloud Asset Inventory to automatically translate project ids to numbers. | <code>string</code> |  | <code>null</code> |
 
 ## Outputs

modules/vpc-sc/perimeters-additive.tf

@@ -48,8 +48,8 @@ resource "google_access_context_manager_service_perimeter" "additive" {
       ])
 
       dynamic "egress_policies" {
-        for_each = spec.value.egress_policies == null ? [] : [
+        for_each = [
-          for k in spec.value.egress_policies :
+          for k in coalesce(spec.value.egress_policies, []) :
           merge(local.egress_policies[k], { key = k })
         ]
         iterator = policy
@@ -129,8 +129,8 @@ resource "google_access_context_manager_service_perimeter" "additive" {
       }
 
       dynamic "ingress_policies" {
-        for_each = spec.value.ingress_policies == null ? [] : [
+        for_each = [
-          for k in spec.value.ingress_policies :
+          for k in coalesce(spec.value.ingress_policies, []) :
           merge(local.ingress_policies[k], { key = k })
         ]
         iterator = policy
@@ -235,8 +235,8 @@ resource "google_access_context_manager_service_perimeter" "additive" {
       ])
 
       dynamic "egress_policies" {
-        for_each = status.value.egress_policies == null ? [] : [
+        for_each = [
-          for k in status.value.egress_policies :
+          for k in coalesce(status.value.egress_policies, []) :
           merge(local.egress_policies[k], { key = k })
         ]
         iterator = policy
@@ -310,8 +310,8 @@ resource "google_access_context_manager_service_perimeter" "additive" {
       }
 
       dynamic "ingress_policies" {
-        for_each = status.value.ingress_policies == null ? [] : [
+        for_each = [
-          for k in status.value.ingress_policies :
+          for k in coalesce(status.value.ingress_policies, []) :
           merge(local.ingress_policies[k], { key = k })
         ]
         iterator = policy
@@ -397,6 +397,14 @@ resource "google_access_context_manager_service_perimeter" "additive" {
   }
   lifecycle {
     ignore_changes = [spec[0].resources, status[0].resources]
+    precondition {
+      condition     = length(local._undefined_ingress_policies[each.key]) == 0
+      error_message = "Undefined ingress policies: ${join(", ", local._undefined_ingress_policies[each.key])}"
+    }
+    precondition {
+      condition     = length(local._undefined_egress_policies[each.key]) == 0
+      error_message = "Undefined egress policies: ${join(", ", local._undefined_egress_policies[each.key])}"
+    }
   }
   depends_on = [
     google_access_context_manager_access_policy.default,

modules/vpc-sc/perimeters.tf

@@ -20,6 +20,14 @@ locals {
   egress_policies  = merge(local.data.egress_policies, var.egress_policies)
   ingress_policies = merge(local.data.ingress_policies, var.ingress_policies)
   perimeters       = merge(local.data.perimeters, var.perimeters)
+  _undefined_egress_policies = {
+    for k, v in local.perimeters :
+    k => setsubtract(concat(try(v.spec.egress_policies, []), try(v.status.egress_policies, [])), keys(local.egress_policies))
+  }
+  _undefined_ingress_policies = {
+    for k, v in local.perimeters :
+    k => setsubtract(concat(try(v.spec.ingress_policies, []), try(v.status.ingress_policies, [])), keys(local.ingress_policies))
+  }
 }
 
 resource "google_access_context_manager_service_perimeter" "regular" {
@@ -54,8 +62,8 @@ resource "google_access_context_manager_service_perimeter" "regular" {
       ])
 
       dynamic "egress_policies" {
-        for_each = spec.value.egress_policies == null ? [] : [
+        for_each = [
-          for k in spec.value.egress_policies :
+          for k in coalesce(spec.value.egress_policies, []) :
           merge(local.egress_policies[k], { key = k })
         ]
         iterator = policy
@@ -135,8 +143,8 @@ resource "google_access_context_manager_service_perimeter" "regular" {
       }
 
       dynamic "ingress_policies" {
-        for_each = spec.value.ingress_policies == null ? [] : [
+        for_each = [
-          for k in spec.value.ingress_policies :
+          for k in coalesce(spec.value.ingress_policies, []) :
           merge(local.ingress_policies[k], { key = k })
         ]
         iterator = policy
@@ -241,8 +249,8 @@ resource "google_access_context_manager_service_perimeter" "regular" {
       ])
 
       dynamic "egress_policies" {
-        for_each = status.value.egress_policies == null ? [] : [
+        for_each = [
-          for k in status.value.egress_policies :
+          for k in coalesce(status.value.egress_policies, []) :
           merge(local.egress_policies[k], { key = k })
         ]
         iterator = policy
@@ -321,8 +329,8 @@ resource "google_access_context_manager_service_perimeter" "regular" {
       }
 
       dynamic "ingress_policies" {
-        for_each = status.value.ingress_policies == null ? [] : [
+        for_each = [
-          for k in status.value.ingress_policies :
+          for k in coalesce(status.value.ingress_policies, []) :
           merge(local.ingress_policies[k], { key = k })
         ]
         iterator = policy
@@ -406,6 +414,16 @@ resource "google_access_context_manager_service_perimeter" "regular" {
 
     }
   }
+  lifecycle {
+    precondition {
+      condition     = length(local._undefined_ingress_policies[each.key]) == 0
+      error_message = "Undefined ingress policies: ${join(", ", local._undefined_ingress_policies[each.key])}"
+    }
+    precondition {
+      condition     = length(local._undefined_egress_policies[each.key]) == 0
+      error_message = "Undefined egress policies: ${join(", ", local._undefined_egress_policies[each.key])}"
+    }
+  }
   depends_on = [
     google_access_context_manager_access_policy.default,
     google_access_context_manager_access_level.basic

modules/vpc-sc/variables.tf

@@ -226,22 +226,22 @@ variable "perimeters" {
     title                     = optional(string)
     use_explicit_dry_run_spec = optional(bool, false)
     spec = optional(object({
-      access_levels       = optional(list(string))
+      access_levels       = optional(list(string), [])
-      egress_policies     = optional(list(string))
+      egress_policies     = optional(list(string), [])
-      ingress_policies    = optional(list(string))
+      ingress_policies    = optional(list(string), [])
-      restricted_services = optional(list(string))
+      restricted_services = optional(list(string), [])
-      resources           = optional(list(string))
+      resources           = optional(list(string), [])
       vpc_accessible_services = optional(object({
         allowed_services   = list(string)
         enable_restriction = optional(bool, true)
       }))
     }))
     status = optional(object({
-      access_levels       = optional(list(string))
+      access_levels       = optional(list(string), [])
-      egress_policies     = optional(list(string))
+      egress_policies     = optional(list(string), [])
-      ingress_policies    = optional(list(string))
+      ingress_policies    = optional(list(string), [])
-      resources           = optional(list(string))
+      resources           = optional(list(string), [])
-      restricted_services = optional(list(string))
+      restricted_services = optional(list(string), [])
       vpc_accessible_services = optional(object({
         allowed_services   = list(string)
         enable_restriction = optional(bool, true)