From 340fd531eb848b0cef8d9347fe2817576eae72e1 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Thu, 10 Nov 2022 09:49:19 +0100 Subject: [PATCH 1/7] wip --- modules/vpc-sc/access-levels.tf | 96 ++++++++-------- modules/vpc-sc/variables.tf | 188 +++++++++++++++++--------------- 2 files changed, 147 insertions(+), 137 deletions(-) diff --git a/modules/vpc-sc/access-levels.tf b/modules/vpc-sc/access-levels.tf index 9aeb232be8..d824916780 100644 --- a/modules/vpc-sc/access-levels.tf +++ b/modules/vpc-sc/access-levels.tf @@ -21,59 +21,59 @@ # google_access_context_manager_access_levels resource resource "google_access_context_manager_access_level" "basic" { - for_each = var.access_levels == null ? {} : var.access_levels - parent = "accessPolicies/${local.access_policy}" - name = "accessPolicies/${local.access_policy}/accessLevels/${each.key}" - title = each.key + for_each = var.access_levels + parent = "accessPolicies/${local.access_policy}" + name = "accessPolicies/${local.access_policy}/accessLevels/${each.key}" + title = each.key + description = each.value.description + basic { combining_function = each.value.combining_function + dynamic "conditions" { - for_each = toset( - each.value.conditions == null ? [] : each.value.conditions - ) - iterator = condition + for_each = toset(each.value.conditions) + iterator = c content { - # uncomment here and in the variable type to enable - # dynamic "device_policy" { - # for_each = toset( - # condition.key.device_policy == null ? [] : [condition.key.device_policy] - # ) - # iterator = device_policy - # content { - # dynamic "os_constraints" { - # for_each = toset( - # device_policy.key.os_constraints == null ? [] : device_policy.key.os_constraints - # ) - # iterator = os_constraint - # content { - # minimum_version = os_constraint.key.minimum_version - # os_type = os_constraint.key.os_type - # require_verified_chrome_os = os_constraint.key.require_verified_chrome_os - # } - # } - # allowed_encryption_statuses = device_policy.key.allowed_encryption_statuses - # allowed_device_management_levels = device_policy.key.allowed_device_management_levels - # require_admin_approval = device_policy.key.require_admin_approval - # require_corp_owned = device_policy.key.require_corp_owned - # require_screen_lock = device_policy.key.require_screen_lock - # } - # } - ip_subnetworks = ( - condition.key.ip_subnetworks == null ? [] : condition.key.ip_subnetworks - ) - members = ( - condition.key.members == null ? [] : condition.key.members - ) - negate = condition.key.negate - regions = ( - condition.key.regions == null ? [] : condition.key.regions - ) - required_access_levels = ( - condition.key.required_access_levels == null - ? [] - : condition.key.required_access_levels - ) + ip_subnetworks = c.value.ip_subnetworks + members = c.value.members + negate = c.value.negate + regions = c.value.regions + required_access_levels = c.value.required_access_levels == null + + dynamic "device_policy" { + for_each = c.value.device_policy == null ? [] : [c.value.device_policy] + iterator = dp + content { + + allowed_device_management_levels = ( + dp.value.allowed_device_management_levels + ) + allowed_encryption_statuses = ( + dp.value.allowed_encryption_statuses + ) + require_admin_approval = dp.value.key.require_admin_approval + require_corp_owned = dp.value.require_corp_owned + require_screen_lock = dp.value.require_screen_lock + + dynamic "os_constraints" { + for_each = toset( + dp.value.os_constraints == null + ? [] + : dp.value.os_constraints + ) + iterator = oc + content { + minimum_version = oc.value.minimum_version + os_type = oc.value.os_type + require_verified_chrome_os = oc.value.require_verified_chrome_os + } + } + + } + } + } } + } } diff --git a/modules/vpc-sc/variables.tf b/modules/vpc-sc/variables.tf index e7318de710..464e5bee6d 100644 --- a/modules/vpc-sc/variables.tf +++ b/modules/vpc-sc/variables.tf @@ -17,29 +17,30 @@ variable "access_levels" { description = "Map of access levels in name => [conditions] format." type = map(object({ - combining_function = string + combining_function = optional(string) conditions = list(object({ - # disabled to reduce var surface, uncomment here and in resource to enable - # device_policy = object({ - # require_screen_lock = bool - # allowed_encryption_statuses = list(string) - # allowed_device_management_levels = list(string) - # os_constraints = list(object({ - # minimum_version = string - # os_type = string - # require_verified_chrome_os = bool - # })) - # require_admin_approval = bool - # require_corp_owned = bool - # }) - ip_subnetworks = list(string) - members = list(string) - negate = bool - regions = list(string) - required_access_levels = list(string) - })) + device_policy = optional(object({ + allowed_device_management_levels = optional(list(string)) + allowed_encryption_statuses = optional(list(string)) + require_admin_approval = bool + require_corp_owned = bool + require_screen_lock = optional(bool) + os_constraints = optional(list(object({ + os_type = string + minimum_version = optional(string) + require_verified_chrome_os = optional(bool) + }))) + })) + ip_subnetworks = optional(list(string), []) + members = optional(list(string), []) + negate = optional(bool) + regions = optional(list(string), []) + required_access_levels = optional(list(string), []) + }), []) + description = optional(string) })) - default = {} + default = {} + nullable = false validation { condition = alltrue([ for k, v in var.access_levels : ( @@ -66,6 +67,66 @@ variable "access_policy_create" { default = null } +variable "egress_policies" { + description = "Egress policy definitions that can be referenced in perimeters." + type = map(object({ + from = object({ + identity_type = optional(string, "ANY_IDENTITY") + identities = optional(list(string)) + }) + to = object({ + operations = optional(list(object({ + method_selectors = list(string) + service_name = string + }))) + resources = optional(list(string)) + resource_type_external = optional(bool, false) + }) + })) + default = {} + nullable = false + validation { + condition = alltrue([ + for k, v in var.egress_policies : contains([ + "IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", + "ANY_USER", "ANY_SERVICE_ACCOUNT" + ], v.from.identity_type) + ]) + error_message = "Invalid `from.identity_type` value in eress policy." + } +} + +variable "ingress_policies" { + description = "Ingress policy definitions that can be referenced in perimeters." + type = map(object({ + from = object({ + access_levels = optional(list(string)) + identity_type = optional(string) + identities = optional(list(string)) + resources = optional(list(string)) + }) + to = object({ + operations = list(object({ + method_selectors = list(string) + service_name = string + })) + resources = list(string) + }) + })) + default = {} + nullable = false + validation { + condition = alltrue([ + for k, v in var.ingress_policies : + v.from.identity_type == null || contains([ + "IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", + "ANY_USER", "ANY_SERVICE_ACCOUNT" + ], v.from.identity_type) + ]) + error_message = "Invalid `from.identity_type` value in eress policy." + } +} + variable "service_perimeters_bridge" { description = "Bridge service perimeters." type = map(object({ @@ -80,80 +141,29 @@ variable "service_perimeters_regular" { description = "Regular service perimeters." type = map(object({ spec = object({ - access_levels = list(string) - resources = list(string) - restricted_services = list(string) - egress_policies = list(object({ - egress_from = object({ - identity_type = string - identities = list(string) - }) - egress_to = object({ - operations = list(object({ - method_selectors = list(string) - service_name = string - })) - resources = list(string) - }) - })) - ingress_policies = list(object({ - ingress_from = object({ - identity_type = string - identities = list(string) - source_access_levels = list(string) - source_resources = list(string) - }) - ingress_to = object({ - operations = list(object({ - method_selectors = list(string) - service_name = string - })) - resources = list(string) - }) - })) - vpc_accessible_services = object({ + access_levels = optional(list(string)) + resources = optional(list(string)) + restricted_services = optional(list(string)) + egress_policies = optional(list(string)) + ingress_policies = optional(list(string)) + vpc_accessible_services = optional(object({ allowed_services = list(string) enable_restriction = bool - }) + })) }) status = object({ - access_levels = list(string) - resources = list(string) - restricted_services = list(string) - egress_policies = list(object({ - egress_from = object({ - identity_type = string - identities = list(string) - }) - egress_to = object({ - operations = list(object({ - method_selectors = list(string) - service_name = string - })) - resources = list(string) - }) - })) - ingress_policies = list(object({ - ingress_from = object({ - identity_type = string - identities = list(string) - source_access_levels = list(string) - source_resources = list(string) - }) - ingress_to = object({ - operations = list(object({ - method_selectors = list(string) - service_name = string - })) - resources = list(string) - }) - })) - vpc_accessible_services = object({ + access_levels = optional(list(string)) + resources = optional(list(string)) + restricted_services = optional(list(string)) + egress_policies = optional(list(string)) + ingress_policies = optional(list(string)) + vpc_accessible_services = optional(object({ allowed_services = list(string) enable_restriction = bool - }) + })) }) - use_explicit_dry_run_spec = bool + use_explicit_dry_run_spec = optional(bool, false) })) - default = {} + default = {} + nullable = false } From af04e33f48a0bd9512a89e03fb547cf5de890283 Mon Sep 17 00:00:00 2001 From: Ludo Date: Thu, 10 Nov 2022 11:22:24 +0100 Subject: [PATCH 2/7] example tests --- modules/vpc-sc/README.md | 135 ++++++------ modules/vpc-sc/access-levels.tf | 4 +- modules/vpc-sc/service-perimeters-bridge.tf | 2 + modules/vpc-sc/service-perimeters-regular.tf | 208 +++++++------------ modules/vpc-sc/variables.tf | 36 ++-- 5 files changed, 163 insertions(+), 222 deletions(-) diff --git a/modules/vpc-sc/README.md b/modules/vpc-sc/README.md index 06407eeba4..a0854f4819 100644 --- a/modules/vpc-sc/README.md +++ b/modules/vpc-sc/README.md @@ -2,7 +2,7 @@ This module offers a unified interface to manage VPC Service Controls [Access Policy](https://cloud.google.com/access-context-manager/docs/create-access-policy), [Access Levels](https://cloud.google.com/access-context-manager/docs/manage-access-levels), and [Service Perimeters](https://cloud.google.com/vpc-service-controls/docs/service-perimeters). -Given the complexity of the underlying resources, the module intentionally mimics their interfaces to make it easier to map their documentation onto its variables, and reduce the internal complexity. The tradeoff is some verbosity, and a very complex type for the `service_perimeters_regular` variable (while [optional type attributes](https://www.terraform.io/language/expressions/type-constraints#experimental-optional-object-type-attributes) are still an experiment). +Given the complexity of the underlying resources, the module intentionally mimics their interfaces to make it easier to map their documentation onto its variables, and reduce the internal complexity. If you are using [Application Default Credentials](https://cloud.google.com/sdk/gcloud/reference/auth/application-default) with Terraform and run into permissions issues, make sure to check out the recommended provider configuration in the [VPC SC resources documentation](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/access_context_manager_access_level). @@ -44,21 +44,16 @@ module "test" { access_policy = "12345678" access_levels = { a1 = { - combining_function = null - conditions = [{ - members = ["user:user1@example.com"], ip_subnetworks = null, - negate = null, regions = null, required_access_levels = null - }] + conditions = [ + { members = ["user:user1@example.com"] } + ] } a2 = { combining_function = "OR" - conditions = [{ - regions = ["IT", "FR"], ip_subnetworks = null, - members = null, negate = null, required_access_levels = null - },{ - ip_subnetworks = ["101.101.101.0/24"], members = null, - negate = null, regions = null, required_access_levels = null - }] + conditions = [ + { regions = ["IT", "FR"] }, + { ip_subnetworks = ["101.101.101.0/24"] } + ] } } } @@ -85,12 +80,9 @@ module "test" { access_policy = "12345678" service_perimeters_bridge = { b1 = { - status_resources = ["projects/111110", "projects/111111"] - spec_resources = null - use_explicit_dry_run_spec = false + status_resources = ["projects/111110", "projects/111111"] } b2 = { - status_resources = null spec_resources = ["projects/222220", "projects/222221"] use_explicit_dry_run_spec = true } @@ -107,59 +99,56 @@ module "test" { access_policy = "12345678" access_levels = { a1 = { - combining_function = null - conditions = [{ - members = ["user:user1@example.com"], ip_subnetworks = null, - negate = null, regions = null, required_access_levels = null - }] + conditions = [ + { members = ["user:user1@example.com"] } + ] } a2 = { - combining_function = null - conditions = [{ - members = ["user:user2@example.com"], ip_subnetworks = null, - negate = null, regions = null, required_access_levels = null - }] + conditions = [ + { members = ["user:user2@example.com"] } + ] + } + } + egress_policies = { + # allow writing to external GCS bucket from a specific SA + gcs-sa-foo = { + from = { + identities = [ + "serviceAccount:foo@myproject.iam.gserviceaccount.com" + ] + } + to = { + operations = [{ + method_selectors = ["*"] + service_name = "storage.googleapis.com" + }] + resources = ["projects/123456789"] + } + } + } + ingress_policies = { + # allow management from external automation SA + sa-tf-test = { + from = { + identities = [ + "serviceAccount:test-tf@myproject.iam.gserviceaccount.com", + ], + source_access_levels = ["*"] + } + to = { + operations = [{ service_name = "*" }] + resources = ["*"] + } } } service_perimeters_regular = { r1 = { - spec = null status = { access_levels = [module.test.access_level_names["a1"], "a2"] resources = ["projects/11111", "projects/111111"] restricted_services = ["storage.googleapis.com"] - # example: allow writing to external GCS bucket - egress_policies = [ - { - egress_from = { - identity_type = null - identities = [ - "serviceAccount:foo@myproject.iam.gserviceaccount.com" - ] - } - egress_to = { - operations = [{ - method_selectors = ["*"], service_name = "storage.googleapis.com" - }] - resources = ["projects/123456789"] - } - } - ] - # example: allow management from external automation SA - ingress_policies = [ - { - ingress_from = { - identities = [ - "serviceAccount:test-tf@myproject.iam.gserviceaccount.com", - ], - source_access_levels = ["*"], identity_type = null, source_resources = null - } - ingress_to = { - operations = [{ method_selectors = [], service_name = "*" }] - resources = ["*"] - } - } - ] + egress_policies = ["gcs-sa-foo"] + ingress_policies = ["sa-tf-test"] vpc_accessible_services = { allowed_services = ["storage.googleapis.com"] enable_restriction = true @@ -179,17 +168,33 @@ module "test" { ## TODO - [ ] implement support for the `google_access_context_manager_gcp_user_access_binding` resource + + +## Files + +| name | description | resources | +|---|---|---| +| [access-levels.tf](./access-levels.tf) | Access level resources. | google_access_context_manager_access_level | +| [main.tf](./main.tf) | Module-level locals and resources. | google_access_context_manager_access_policy | +| [outputs.tf](./outputs.tf) | Module outputs. | | +| [service-perimeters-bridge.tf](./service-perimeters-bridge.tf) | Bridge service perimeter resources. | google_access_context_manager_service_perimeter | +| [service-perimeters-regular.tf](./service-perimeters-regular.tf) | Regular service perimeter resources. | google_access_context_manager_service_perimeter | +| [variables.tf](./variables.tf) | Module variables. | | +| [versions.tf](./versions.tf) | Version pins. | | + ## Variables | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [access_policy](variables.tf#L55) | Access Policy name, leave null to use auto-created one. | string | ✓ | | -| [access_levels](variables.tf#L17) | Map of access levels in name => [conditions] format. | map(object({…})) | | {} | -| [access_policy_create](variables.tf#L60) | Access Policy configuration, fill in to create. Parent is in 'organizations/123456' format. | object({…}) | | null | -| [service_perimeters_bridge](variables.tf#L69) | Bridge service perimeters. | map(object({…})) | | {} | -| [service_perimeters_regular](variables.tf#L79) | Regular service perimeters. | map(object({…})) | | {} | +| [access_policy](variables.tf#L56) | Access Policy name, leave null to use auto-created one. | string | ✓ | | +| [access_levels](variables.tf#L17) | Map of access levels in name => [conditions] format. | map(object({…})) | | {} | +| [access_policy_create](variables.tf#L61) | Access Policy configuration, fill in to create. Parent is in 'organizations/123456' format. | object({…}) | | null | +| [egress_policies](variables.tf#L70) | Egress policy definitions that can be referenced in perimeters. | map(object({…})) | | {} | +| [ingress_policies](variables.tf#L99) | Ingress policy definitions that can be referenced in perimeters. | map(object({…})) | | {} | +| [service_perimeters_bridge](variables.tf#L130) | Bridge service perimeters. | map(object({…})) | | {} | +| [service_perimeters_regular](variables.tf#L140) | Regular service perimeters. | map(object({…})) | | {} | ## Outputs diff --git a/modules/vpc-sc/access-levels.tf b/modules/vpc-sc/access-levels.tf index d824916780..1eb85343f2 100644 --- a/modules/vpc-sc/access-levels.tf +++ b/modules/vpc-sc/access-levels.tf @@ -14,7 +14,7 @@ * limitations under the License. */ -# TODO(ludomagno): add a second variable and resource for custom access levels +# tfdoc:file:description Access level resources. # this code implements "additive" access levels, if "authoritative" # access levels are needed, switch to the @@ -38,7 +38,7 @@ resource "google_access_context_manager_access_level" "basic" { members = c.value.members negate = c.value.negate regions = c.value.regions - required_access_levels = c.value.required_access_levels == null + required_access_levels = coalesce(c.value.required_access_levels, []) dynamic "device_policy" { for_each = c.value.device_policy == null ? [] : [c.value.device_policy] diff --git a/modules/vpc-sc/service-perimeters-bridge.tf b/modules/vpc-sc/service-perimeters-bridge.tf index f50d6d5762..e3233082c0 100644 --- a/modules/vpc-sc/service-perimeters-bridge.tf +++ b/modules/vpc-sc/service-perimeters-bridge.tf @@ -14,6 +14,8 @@ * limitations under the License. */ +# tfdoc:file:description Bridge service perimeter resources. + # this code implements "additive" service perimeters, if "authoritative" # service perimeters are needed, switch to the # google_access_context_manager_service_perimeters resource diff --git a/modules/vpc-sc/service-perimeters-regular.tf b/modules/vpc-sc/service-perimeters-regular.tf index a834ce7580..6553c29b9f 100644 --- a/modules/vpc-sc/service-perimeters-regular.tf +++ b/modules/vpc-sc/service-perimeters-regular.tf @@ -14,6 +14,8 @@ * limitations under the License. */ +# tfdoc:file:description Regular service perimeter resources. + # this code implements "additive" service perimeters, if "authoritative" # service perimeters are needed, switch to the # google_access_context_manager_service_perimeters resource @@ -26,7 +28,7 @@ resource "google_access_context_manager_service_perimeter" "regular" { perimeter_type = "PERIMETER_TYPE_REGULAR" use_explicit_dry_run_spec = each.value.use_explicit_dry_run_spec dynamic "spec" { - for_each = each.value.spec == null ? {} : { 1 = 1 } + for_each = each.value.spec == null ? [] : [""] content { access_levels = ( each.value.spec.access_levels == null ? null : [ @@ -36,43 +38,33 @@ resource "google_access_context_manager_service_perimeter" "regular" { ) resources = each.value.spec.resources restricted_services = each.value.spec.restricted_services - # begin egress_policies + dynamic "egress_policies" { - for_each = toset( - each.value.spec.egress_policies == null - ? [] - : each.value.spec.egress_policies - ) + for_each = each.value.spec.egress_policies == null ? {} : { + for k in each.value.spec.egress_policies : + k => lookup(var.egress_policies, k, null) + if contains(keys(var.egress_policies), k) + } iterator = policy content { - # begin egress_from dynamic "egress_from" { - for_each = policy.key.egress_from == null ? {} : { 1 = 1 } + for_each = policy.value.from == null ? [] : [""] content { - identity_type = policy.key.egress_from.identity_type - identities = policy.key.egress_from.identities + identity_type = policy.value.from.identity_type + identities = policy.value.from.identities } } - # end egress_from - # begin egress_to dynamic "egress_to" { - for_each = policy.key.egress_to == null ? {} : { 1 = 1 } + for_each = policy.value.to == null ? [] : [""] content { - resources = policy.key.egress_to.resources + resources = policy.value.to.resources dynamic "operations" { - for_each = toset( - policy.key.egress_to.operations == null - ? [] - : policy.key.egress_to.operations - ) + for_each = toset(policy.value.to.operations) + iterator = o content { - service_name = operations.key.service_name + service_name = o.value.service_name dynamic "method_selectors" { - for_each = toset( - operations.key.method_selectors == null - ? [] - : operations.key.method_selectors - ) + for_each = toset(coalesce(o.value.method_selectors, [])) content { method = method_selectors.key } @@ -81,69 +73,47 @@ resource "google_access_context_manager_service_perimeter" "regular" { } } } - # end egress_to } } - # end egress_policies - # begin ingress_policies + dynamic "ingress_policies" { - for_each = toset( - each.value.spec.ingress_policies == null - ? [] - : each.value.spec.ingress_policies - ) + for_each = each.value.spec.ingress_policies == null ? {} : { + for k in each.value.spec.ingress_policies : + k => lookup(var.ingress_policies, k, null) + if contains(keys(var.ingress_policies), k) + } iterator = policy content { - # begin ingress_from dynamic "ingress_from" { - for_each = policy.key.ingress_from == null ? {} : { 1 = 1 } + for_each = policy.value.from == null ? [] : [""] content { - identity_type = policy.key.ingress_from.identity_type - identities = policy.key.ingress_from.identities - # begin sources + identity_type = policy.value.from.identity_type + identities = policy.value.from.identities dynamic "sources" { - for_each = toset( - policy.key.ingress_from.source_access_levels == null - ? [] - : policy.key.ingress_from.source_access_levels - ) + for_each = toset(policy.value.from.access_levels) content { access_level = sources.key } } dynamic "sources" { - for_each = toset( - policy.key.ingress_from.source_resources == null - ? [] - : policy.key.ingress_from.source_resources - ) + for_each = toset(policy.value.from.resources) content { resource = sources.key } } - # end sources } } - # end ingress_from - # begin ingress_to dynamic "ingress_to" { - for_each = policy.key.ingress_to == null ? {} : { 1 = 1 } + for_each = policy.key.ingress_to == null ? [] : [""] content { - resources = policy.key.ingress_to.resources + resources = policy.value.to.resources dynamic "operations" { - for_each = toset( - policy.key.ingress_to.operations == null - ? [] - : policy.key.ingress_to.operations - ) + for_each = toset(policy.value.to.operations) + iterator = o content { - service_name = operations.key.service_name + service_name = o.value.service_name dynamic "method_selectors" { - for_each = toset( - operations.key.method_selectors == null - ? [] - : operations.key.method_selectors - ) + for_each = toset(coalesce(o.value.method_selectors, [])) content { method = method_selectors.key } @@ -152,11 +122,9 @@ resource "google_access_context_manager_service_perimeter" "regular" { } } } - # end ingress_to } } - # end ingress_policies - # begin vpc_accessible_services + dynamic "vpc_accessible_services" { for_each = each.value.spec.vpc_accessible_services == null ? {} : { 1 = 1 } content { @@ -164,7 +132,7 @@ resource "google_access_context_manager_service_perimeter" "regular" { enable_restriction = each.value.spec.vpc_accessible_services.enable_restriction } } - # end vpc_accessible_services + } } dynamic "status" { @@ -178,43 +146,33 @@ resource "google_access_context_manager_service_perimeter" "regular" { ) resources = each.value.status.resources restricted_services = each.value.status.restricted_services - # begin egress_policies + dynamic "egress_policies" { - for_each = toset( - each.value.status.egress_policies == null - ? [] - : each.value.status.egress_policies - ) + for_each = each.value.spec.egress_policies == null ? {} : { + for k in each.value.spec.egress_policies : + k => lookup(var.egress_policies, k, null) + if contains(keys(var.egress_policies), k) + } iterator = policy content { - # begin egress_from dynamic "egress_from" { - for_each = policy.key.egress_from == null ? {} : { 1 = 1 } + for_each = policy.value.from == null ? [] : [""] content { - identity_type = policy.key.egress_from.identity_type - identities = policy.key.egress_from.identities + identity_type = policy.value.from.identity_type + identities = policy.value.from.identities } } - # end egress_from - # begin egress_to dynamic "egress_to" { - for_each = policy.key.egress_to == null ? {} : { 1 = 1 } + for_each = policy.value.to == null ? [] : [""] content { - resources = policy.key.egress_to.resources + resources = policy.value.to.resources dynamic "operations" { - for_each = toset( - policy.key.egress_to.operations == null - ? [] - : policy.key.egress_to.operations - ) + for_each = toset(policy.value.to.operations) + iterator = o content { - service_name = operations.key.service_name + service_name = o.value.service_name dynamic "method_selectors" { - for_each = toset( - operations.key.method_selectors == null - ? [] - : operations.key.method_selectors - ) + for_each = toset(coalesce(o.value.method_selectors, [])) content { method = method_selectors.key } @@ -223,69 +181,47 @@ resource "google_access_context_manager_service_perimeter" "regular" { } } } - # end egress_to } } - # end egress_policies - # begin ingress_policies + dynamic "ingress_policies" { - for_each = toset( - each.value.status.ingress_policies == null - ? [] - : each.value.status.ingress_policies - ) + for_each = each.value.spec.ingress_policies == null ? {} : { + for k in each.value.spec.ingress_policies : + k => lookup(var.ingress_policies, k, null) + if contains(keys(var.ingress_policies), k) + } iterator = policy content { - # begin ingress_from dynamic "ingress_from" { - for_each = policy.key.ingress_from == null ? {} : { 1 = 1 } + for_each = policy.value.from == null ? [] : [""] content { - identity_type = policy.key.ingress_from.identity_type - identities = policy.key.ingress_from.identities - # begin sources + identity_type = policy.value.from.identity_type + identities = policy.value.from.identities dynamic "sources" { - for_each = toset( - policy.key.ingress_from.source_access_levels == null - ? [] - : policy.key.ingress_from.source_access_levels - ) + for_each = toset(policy.value.from.access_levels) content { access_level = sources.key } } dynamic "sources" { - for_each = toset( - policy.key.ingress_from.source_resources == null - ? [] - : policy.key.ingress_from.source_resources - ) + for_each = toset(policy.value.from.resources) content { resource = sources.key } } - # end sources } } - # end ingress_from - # begin ingress_to dynamic "ingress_to" { - for_each = policy.key.ingress_to == null ? {} : { 1 = 1 } + for_each = policy.key.ingress_to == null ? [] : [""] content { - resources = policy.key.ingress_to.resources + resources = policy.value.to.resources dynamic "operations" { - for_each = toset( - policy.key.ingress_to.operations == null - ? [] - : policy.key.ingress_to.operations - ) + for_each = toset(policy.value.to.operations) + iterator = o content { - service_name = operations.key.service_name + service_name = o.value.service_name dynamic "method_selectors" { - for_each = toset( - operations.key.method_selectors == null - ? [] - : operations.key.method_selectors - ) + for_each = toset(coalesce(o.value.method_selectors, [])) content { method = method_selectors.key } @@ -294,11 +230,9 @@ resource "google_access_context_manager_service_perimeter" "regular" { } } } - # end ingress_to } } - # end ingress_policies - # begin vpc_accessible_services + dynamic "vpc_accessible_services" { for_each = each.value.status.vpc_accessible_services == null ? {} : { 1 = 1 } content { @@ -306,7 +240,7 @@ resource "google_access_context_manager_service_perimeter" "regular" { enable_restriction = each.value.status.vpc_accessible_services.enable_restriction } } - # end vpc_accessible_services + } } # lifecycle { diff --git a/modules/vpc-sc/variables.tf b/modules/vpc-sc/variables.tf index 464e5bee6d..cfcbe34ee0 100644 --- a/modules/vpc-sc/variables.tf +++ b/modules/vpc-sc/variables.tf @@ -18,7 +18,7 @@ variable "access_levels" { description = "Map of access levels in name => [conditions] format." type = map(object({ combining_function = optional(string) - conditions = list(object({ + conditions = optional(list(object({ device_policy = optional(object({ allowed_device_management_levels = optional(list(string)) allowed_encryption_statuses = optional(list(string)) @@ -36,7 +36,7 @@ variable "access_levels" { negate = optional(bool) regions = optional(list(string), []) required_access_levels = optional(list(string), []) - }), []) + })), []) description = optional(string) })) default = {} @@ -76,9 +76,9 @@ variable "egress_policies" { }) to = object({ operations = optional(list(object({ - method_selectors = list(string) + method_selectors = optional(list(string)) service_name = string - }))) + })), []) resources = optional(list(string)) resource_type_external = optional(bool, false) }) @@ -100,17 +100,17 @@ variable "ingress_policies" { description = "Ingress policy definitions that can be referenced in perimeters." type = map(object({ from = object({ - access_levels = optional(list(string)) + access_levels = optional(list(string), []) identity_type = optional(string) identities = optional(list(string)) - resources = optional(list(string)) + resources = optional(list(string), []) }) to = object({ - operations = list(object({ - method_selectors = list(string) + operations = optional(list(object({ + method_selectors = optional(list(string)) service_name = string - })) - resources = list(string) + })), []) + resources = optional(list(string)) }) })) default = {} @@ -121,7 +121,7 @@ variable "ingress_policies" { v.from.identity_type == null || contains([ "IDENTITY_TYPE_UNSPECIFIED", "ANY_IDENTITY", "ANY_USER", "ANY_SERVICE_ACCOUNT" - ], v.from.identity_type) + ], coalesce(v.from.identity_type, "-")) ]) error_message = "Invalid `from.identity_type` value in eress policy." } @@ -130,9 +130,9 @@ variable "ingress_policies" { variable "service_perimeters_bridge" { description = "Bridge service perimeters." type = map(object({ - spec_resources = list(string) - status_resources = list(string) - use_explicit_dry_run_spec = bool + spec_resources = optional(list(string)) + status_resources = optional(list(string)) + use_explicit_dry_run_spec = optional(bool, false) })) default = {} } @@ -140,7 +140,7 @@ variable "service_perimeters_bridge" { variable "service_perimeters_regular" { description = "Regular service perimeters." type = map(object({ - spec = object({ + spec = optional(object({ access_levels = optional(list(string)) resources = optional(list(string)) restricted_services = optional(list(string)) @@ -150,8 +150,8 @@ variable "service_perimeters_regular" { allowed_services = list(string) enable_restriction = bool })) - }) - status = object({ + }), {}) + status = optional(object({ access_levels = optional(list(string)) resources = optional(list(string)) restricted_services = optional(list(string)) @@ -161,7 +161,7 @@ variable "service_perimeters_regular" { allowed_services = list(string) enable_restriction = bool })) - }) + }), {}) use_explicit_dry_run_spec = optional(bool, false) })) default = {} From 44373a4d59605a7de32d6a2746e4ac099dc0cfa1 Mon Sep 17 00:00:00 2001 From: Ludo Date: Thu, 10 Nov 2022 11:52:12 +0100 Subject: [PATCH 3/7] module tests --- modules/vpc-sc/README.md | 2 +- modules/vpc-sc/service-perimeters-regular.tf | 5 +- modules/vpc-sc/variables.tf | 2 +- tests/modules/vpc_sc/fixture/main.tf | 136 ++---------------- .../vpc_sc/fixture/test.regular.tfvars | 96 +++++++++++++ tests/modules/vpc_sc/fixture/variables.tf | 53 +++++++ tests/modules/vpc_sc/test_plan.py | 11 +- 7 files changed, 172 insertions(+), 133 deletions(-) create mode 100644 tests/modules/vpc_sc/fixture/test.regular.tfvars create mode 100644 tests/modules/vpc_sc/fixture/variables.tf diff --git a/modules/vpc-sc/README.md b/modules/vpc-sc/README.md index a0854f4819..a9ab496513 100644 --- a/modules/vpc-sc/README.md +++ b/modules/vpc-sc/README.md @@ -188,7 +188,7 @@ module "test" { | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [access_policy](variables.tf#L56) | Access Policy name, leave null to use auto-created one. | string | ✓ | | +| [access_policy](variables.tf#L56) | Access Policy name, set to null if creating one. | string | ✓ | | | [access_levels](variables.tf#L17) | Map of access levels in name => [conditions] format. | map(object({…})) | | {} | | [access_policy_create](variables.tf#L61) | Access Policy configuration, fill in to create. Parent is in 'organizations/123456' format. | object({…}) | | null | | [egress_policies](variables.tf#L70) | Egress policy definitions that can be referenced in perimeters. | map(object({…})) | | {} | diff --git a/modules/vpc-sc/service-perimeters-regular.tf b/modules/vpc-sc/service-perimeters-regular.tf index 6553c29b9f..a401b322a7 100644 --- a/modules/vpc-sc/service-perimeters-regular.tf +++ b/modules/vpc-sc/service-perimeters-regular.tf @@ -92,7 +92,10 @@ resource "google_access_context_manager_service_perimeter" "regular" { dynamic "sources" { for_each = toset(policy.value.from.access_levels) content { - access_level = sources.key + access_level = try( + google_access_context_manager_access_level.basic[k].id, k + ) + } } dynamic "sources" { diff --git a/modules/vpc-sc/variables.tf b/modules/vpc-sc/variables.tf index cfcbe34ee0..1b75e90308 100644 --- a/modules/vpc-sc/variables.tf +++ b/modules/vpc-sc/variables.tf @@ -54,7 +54,7 @@ variable "access_levels" { } variable "access_policy" { - description = "Access Policy name, leave null to use auto-created one." + description = "Access Policy name, set to null if creating one." type = string } diff --git a/tests/modules/vpc_sc/fixture/main.tf b/tests/modules/vpc_sc/fixture/main.tf index 29cc672bc3..b6a951e506 100644 --- a/tests/modules/vpc_sc/fixture/main.tf +++ b/tests/modules/vpc_sc/fixture/main.tf @@ -14,133 +14,13 @@ * limitations under the License. */ -variable "access_policy" { - description = "Access Policy name, leave null to use auto-created one." - type = string - default = null -} - -variable "access_policy_create" { - description = "Access Policy configuration, fill in to create. Parent is in 'organizations/123456' format." - type = object({ - parent = string - title = string - }) - default = { - parent = "organizations/123456" - title = "vpcsc-policy" - } -} - module "test" { - source = "../../../../modules/vpc-sc" - access_policy = var.access_policy - access_policy_create = var.access_policy_create - access_levels = { - a1 = { - combining_function = null - conditions = [ - { - device_policy = null - ip_subnetworks = null - members = ["user:ludomagno@google.com"] - negate = null - regions = null - required_access_levels = null - } - ] - } - a2 = { - combining_function = "OR" - conditions = [ - { - device_policy = null - ip_subnetworks = null - members = null - negate = null - regions = ["IT", "FR"] - required_access_levels = null - }, - { - device_policy = null - ip_subnetworks = null - members = null - negate = null - regions = ["US"] - required_access_levels = null - } - ] - } - } - service_perimeters_bridge = { - b1 = { - status_resources = ["projects/111110", "projects/111111"] - spec_resources = null - use_explicit_dry_run_spec = false - } - b2 = { - status_resources = ["projects/111110", "projects/222220"] - spec_resources = ["projects/111110", "projects/222220"] - use_explicit_dry_run_spec = true - } - } - service_perimeters_regular = { - r1 = { - spec = null - status = { - access_levels = [module.test.access_level_names["a1"]] - resources = ["projects/11111", "projects/111111"] - restricted_services = ["storage.googleapis.com"] - egress_policies = null - ingress_policies = null - vpc_accessible_services = { - allowed_services = ["compute.googleapis.com"] - enable_restriction = true - } - } - use_explicit_dry_run_spec = false - } - r2 = { - spec = null - status = { - access_levels = [module.test.access_level_names["a1"], "a2"] - resources = ["projects/222220", "projects/222221"] - restricted_services = ["storage.googleapis.com"] - egress_policies = [ - { - egress_from = { - identity_type = null - identities = ["user:foo@example.com"] - } - egress_to = { - operations = null - resources = ["projects/333330"] - } - } - ] - ingress_policies = [ - { - ingress_from = { - identity_type = null - identities = null - source_access_levels = [module.test.access_level_names["a2"]] - source_resources = ["projects/333330"] - } - ingress_to = { - operations = [{ - method_selectors = null - service_name = "compute.googleapis.com" - }] - resources = ["projects/222220"] - } - } - ] - vpc_accessible_services = { - allowed_services = ["compute.googleapis.com"] - enable_restriction = true - } - } - use_explicit_dry_run_spec = false - } - } + source = "../../../../modules/vpc-sc" + access_policy = var.access_policy + access_policy_create = var.access_policy_create + access_levels = var.access_levels + egress_policies = var.egress_policies + ingress_policies = var.ingress_policies + service_perimeters_bridge = var.service_perimeters_bridge + service_perimeters_regular = var.service_perimeters_regular } diff --git a/tests/modules/vpc_sc/fixture/test.regular.tfvars b/tests/modules/vpc_sc/fixture/test.regular.tfvars new file mode 100644 index 0000000000..012e16fb89 --- /dev/null +++ b/tests/modules/vpc_sc/fixture/test.regular.tfvars @@ -0,0 +1,96 @@ +access_levels = { + a1 = { + combining_function = null + conditions = [ + { + device_policy = null + ip_subnetworks = null + members = ["user:ludomagno@google.com"] + negate = null + regions = null + required_access_levels = null + } + ] + } + a2 = { + combining_function = "OR" + conditions = [ + { + device_policy = null + ip_subnetworks = null + members = null + negate = null + regions = ["IT", "FR"] + required_access_levels = null + }, + { + device_policy = null + ip_subnetworks = null + members = null + negate = null + regions = ["US"] + required_access_levels = null + } + ] + } +} +egress_policies = { + foo = { + from = { + identities = ["user:foo@example.com"] + } + to = { + resources = ["projects/333330"] + } + } +} +ingress_policies = { + foo = { + from = { + source_access_levels = ["a2"] + source_resources = ["projects/333330"] + } + to = { + operations = [{ + service_name = "compute.googleapis.com" + }] + resources = ["projects/222220"] + } + } +} +service_perimeters_bridge = { + b1 = { + status_resources = ["projects/111110", "projects/111111"] + } + b2 = { + status_resources = ["projects/111110", "projects/222220"] + spec_resources = ["projects/111110", "projects/222220"] + use_explicit_dry_run_spec = true + } +} +service_perimeters_regular = { + r1 = { + status = { + access_levels = ["a1"] + resources = ["projects/11111", "projects/111111"] + restricted_services = ["storage.googleapis.com"] + vpc_accessible_services = { + allowed_services = ["compute.googleapis.com"] + enable_restriction = true + } + } + } + r2 = { + status = { + access_levels = ["a1", "a2"] + resources = ["projects/222220", "projects/222221"] + restricted_services = ["storage.googleapis.com"] + egress_policies = ["foo"] + ingress_policies = ["foo"] + vpc_accessible_services = { + allowed_services = ["compute.googleapis.com"] + enable_restriction = true + } + } + } +} diff --git a/tests/modules/vpc_sc/fixture/variables.tf b/tests/modules/vpc_sc/fixture/variables.tf new file mode 100644 index 0000000000..1b7457f480 --- /dev/null +++ b/tests/modules/vpc_sc/fixture/variables.tf @@ -0,0 +1,53 @@ +/** + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +variable "access_levels" { + type = any + default = {} + nullable = false +} + +variable "access_policy" { + type = string +} + +variable "access_policy_create" { + type = any + default = null +} + +variable "egress_policies" { + type = any + default = {} + nullable = false +} + +variable "ingress_policies" { + type = any + default = {} + nullable = false +} + +variable "service_perimeters_bridge" { + type = any + default = {} +} + +variable "service_perimeters_regular" { + type = any + default = {} + nullable = false +} diff --git a/tests/modules/vpc_sc/test_plan.py b/tests/modules/vpc_sc/test_plan.py index d8cae72ee5..326d1e86d4 100644 --- a/tests/modules/vpc_sc/test_plan.py +++ b/tests/modules/vpc_sc/test_plan.py @@ -12,9 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. + def test_create_policy(plan_runner): "Test with auto-created policy." - _, resources = plan_runner() + access_policy_create = '''{ + parent = "organizations/123456" + title = "vpcsc-policy" + }''' + _, resources = plan_runner(tf_var_file='test.regular.tfvars', + access_policy='null', + access_policy_create=access_policy_create) counts = {} for r in resources: n = f'{r["type"]}.{r["name"]}' @@ -29,7 +36,7 @@ def test_create_policy(plan_runner): def test_use_policy(plan_runner): "Test with existing policy." - _, resources = plan_runner(access_policy_create="null", + _, resources = plan_runner(tf_var_file='test.regular.tfvars', access_policy="accessPolicies/foobar") counts = {} for r in resources: From 30c9d513e59a1f0cb56d0ec2b578a2a0b2396d24 Mon Sep 17 00:00:00 2001 From: Ludo Date: Thu, 10 Nov 2022 11:57:47 +0100 Subject: [PATCH 4/7] streamline example --- modules/vpc-sc/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/vpc-sc/README.md b/modules/vpc-sc/README.md index a9ab496513..71b86c4259 100644 --- a/modules/vpc-sc/README.md +++ b/modules/vpc-sc/README.md @@ -144,7 +144,7 @@ module "test" { service_perimeters_regular = { r1 = { status = { - access_levels = [module.test.access_level_names["a1"], "a2"] + access_levels = ["a1", "a2"] resources = ["projects/11111", "projects/111111"] restricted_services = ["storage.googleapis.com"] egress_policies = ["gcs-sa-foo"] @@ -154,7 +154,6 @@ module "test" { enable_restriction = true } } - use_explicit_dry_run_spec = false } } } From 25acb0fe262b5a302eaba12e9f5dc22d3fcac20a Mon Sep 17 00:00:00 2001 From: Ludo Date: Thu, 10 Nov 2022 15:16:07 +0100 Subject: [PATCH 5/7] fast --- fast/stages/02-security/README.md | 116 +++++------ fast/stages/02-security/variables.tf | 133 ++++++------- fast/stages/02-security/vpc-sc.tf | 181 +++++++----------- modules/vpc-sc/service-perimeters-regular.tf | 17 +- modules/vpc-sc/variables.tf | 2 +- .../fast/stages/s02_security/fixture/main.tf | 89 ++++----- 6 files changed, 234 insertions(+), 304 deletions(-) diff --git a/fast/stages/02-security/README.md b/fast/stages/02-security/README.md index e61918bb1c..a23af3f7b2 100644 --- a/fast/stages/02-security/README.md +++ b/fast/stages/02-security/README.md @@ -156,109 +156,85 @@ This allows configuring VPC SC in a fairly flexible and concise way, without rep #### Dry-run vs. enforced -The VPC SC configuration is set up by default in dry-run mode to allow easy experimentation, and detecting violations before enforcement. Once everything is set up correctly, switching to enforced mode needs to be done in code, by swapping the contents of the `spec` and `status` attributes for perimeters in the `vpc-sc.tf` file. The effort involved is minimal (2 lines of code per perimeter), and comments help identify the correct lines. - -#### Perimeter resources - -Projects are added to perimeters via the `vpc_sc_perimeter_projects`, and that's currently the only way of doing it without generating permadiffs or conflicts, because of the way the Terraform provider is implemented. - -Once the Google Terraform Provider [implements support for dry-run mode in the additive resource](https://github.com/hashicorp/terraform-provider-google/issues/7270), it will be possible to concurrently manage perimeter resources both here and in subsequent Terraform configurations, for example to allow the Project Factory to add a project to a perimeter during the creation process. - -Bridge perimeters are auto-populated with all projects configured for the connected regular perimeters. - -An example of adding projects to perimeters using project numbers: - -```hcl -# terraform.tfvars - -vpc_sc_perimeter_projects = { - dev = ["projects/12345678", "projects/12345679"] - landing = ["projects/12345670"] - prod = ["projects/12345674", "projects/12345675"] -} -``` +The VPC SC configuration is set up by default in dry-run mode to allow easy experimentation, and detecting violations before enforcement. Once everything is set up correctly, switching to enforced mode needs to be done in code, by changing the `vpc_sc_explicit_dry_run_spec` local. #### Access levels -Below an example for an access level that allows unconditional ingress from a set of IP CIDR ranges can be configured once, and enabled on selected perimeters: +Access levels are defined via the `vpc_sc_access_levels` variable, and referenced by key in perimeter definitions: ```hcl -# terraform.tfvars - vpc_sc_access_levels = { onprem = { - combining_function = null, conditions = [{ - ip_subnetworks = ["101.101.101.0/24"], - members = null, negate = null, - regions = null, required_access_levels = null + ip_subnetworks = ["101.101.101.0/24"] }] } } -vpc_sc_perimeter_access_levels = { - dev = null - landing = ["onprem"] - prod = ["onprem"] -} ``` #### Ingress and Egress policies -The same applies to Ingress and Egress policies, as shown in the examples below referencing the automation service account for this stage. - -Below you can find an ingress policy configuration that allows applying Terraform from outside the perimeter, useful when bringing up this stage to avoid generating violations: +Ingress and egress policy are defined via the `vpc_sc_egress_policies` and `vpc_sc_ingress_policies`, and referenced by key in perimeter definitions: ```hcl -# terraform.tfvars - +vpc_sc_egress_policies = { + iac-gcs = { + from = { + identities = [ + "serviceAccount:xxx-prod-resman-security-0@xxx-prod-iac-core-0.iam.gserviceaccount.com" + ] + } + to = { + operations = [{ + method_selectors = ["*"] + service_name = "storage.googleapis.com" + }] + resources = ["projects/123456782"] + } + } +} vpc_sc_ingress_policies = { iac = { - ingress_from = { + from = { identities = [ "serviceAccount:xxx-prod-resman-security-0@xxx-prod-iac-core-0.iam.gserviceaccount.com" ] - source_access_levels = ["*"] - identity_type = null - source_resources = null + access_levels = ["*"] } - ingress_to = { + to = { operations = [{ method_selectors = [], service_name = "*" }] resources = ["*"] } } } -vpc_sc_perimeter_ingress_policies = { - dev = ["iac"] - landing = ["iac"] - prod = ["iac"] -} ``` -Below you can find an egress policy that allows writing Terraform state to the automation bucket, useful once Terraform starts running inside the perimeter in a pipeline: +#### Perimeters -```hcl -# terraform.tfvars +Regular perimeters are defined via the the `vpc_sc_perimeters` variable, and bridge perimeters are automatically populated from that variable. -vpc_sc_egress_policies = { - iac-gcs = { - egress_from = { - identity_type = null - identities = [ - "serviceAccount:xxx-prod-resman-security-0@xxx-prod-iac-core-0.iam.gserviceaccount.com" - ] - } - egress_to = { - operations = [{ - method_selectors = ["*"], service_name = "storage.googleapis.com" - }] - resources = ["projects/123456782"] - } +Support for independently adding projects to perimeters outside of this Terraform setup is pending resolution of [this Google Terraform Provider issue](https://github.com/hashicorp/terraform-provider-google/issues/7270), which implements support for dry-run mode in the additive resource. + +Access levels and egress/ingress policies are referenced in perimeters via keys. + +```hcl +vpc_sc_perimeters = { + dev = { + egress_policies = ["iac-gcs"] + ingress_policies = ["iac"] + resources = ["projects/1111111111"] + } + dev = { + egress_policies = ["iac-gcs"] + ingress_policies = ["iac"] + resources = ["projects/0000000000"] + } + dev = { + access_levels = ["onprem"] + egress_policies = ["iac-gcs"] + ingress_policies = ["iac"] + resources = ["projects/2222222222"] } -} -vpc_sc_perimeter_ingress_policies = { - dev = ["iac-gcs"] - landing = ["iac-gcs"] - prod = ["iac-gcs"] } ``` diff --git a/fast/stages/02-security/variables.tf b/fast/stages/02-security/variables.tf index ff0edc7757..a28ed56ae4 100644 --- a/fast/stages/02-security/variables.tf +++ b/fast/stages/02-security/variables.tf @@ -118,92 +118,95 @@ variable "prefix" { variable "vpc_sc_access_levels" { description = "VPC SC access level definitions." type = map(object({ - combining_function = string - conditions = list(object({ - ip_subnetworks = list(string) - members = list(string) - negate = bool - regions = list(string) - required_access_levels = list(string) - })) + combining_function = optional(string) + conditions = optional(list(object({ + device_policy = optional(object({ + allowed_device_management_levels = optional(list(string)) + allowed_encryption_statuses = optional(list(string)) + require_admin_approval = bool + require_corp_owned = bool + require_screen_lock = optional(bool) + os_constraints = optional(list(object({ + os_type = string + minimum_version = optional(string) + require_verified_chrome_os = optional(bool) + }))) + })) + ip_subnetworks = optional(list(string), []) + members = optional(list(string), []) + negate = optional(bool) + regions = optional(list(string), []) + required_access_levels = optional(list(string), []) + })), []) + description = optional(string) })) - default = {} + default = {} + nullable = false } variable "vpc_sc_egress_policies" { description = "VPC SC egress policy defnitions." type = map(object({ - egress_from = object({ - identity_type = string - identities = list(string) + from = object({ + identity_type = optional(string, "ANY_IDENTITY") + identities = optional(list(string)) }) - egress_to = object({ - operations = list(object({ - method_selectors = list(string) + to = object({ + operations = optional(list(object({ + method_selectors = optional(list(string)) service_name = string - })) - resources = list(string) + })), []) + resources = optional(list(string)) + resource_type_external = optional(bool, false) }) })) - default = {} + default = {} + nullable = false } variable "vpc_sc_ingress_policies" { description = "VPC SC ingress policy defnitions." type = map(object({ - ingress_from = object({ - identity_type = string - identities = list(string) - source_access_levels = list(string) - source_resources = list(string) + from = object({ + access_levels = optional(list(string), []) + identity_type = optional(string) + identities = optional(list(string)) + resources = optional(list(string), []) }) - ingress_to = object({ - operations = list(object({ - method_selectors = list(string) + to = object({ + operations = optional(list(object({ + method_selectors = optional(list(string)) service_name = string - })) - resources = list(string) + })), []) + resources = optional(list(string)) }) })) - default = {} -} - -variable "vpc_sc_perimeter_access_levels" { - description = "VPC SC perimeter access_levels." - type = object({ - dev = list(string) - landing = list(string) - prod = list(string) - }) - default = null -} - -variable "vpc_sc_perimeter_egress_policies" { - description = "VPC SC egress policies per perimeter, values reference keys defined in the `vpc_sc_ingress_policies` variable." - type = object({ - dev = list(string) - landing = list(string) - prod = list(string) - }) - default = null -} - -variable "vpc_sc_perimeter_ingress_policies" { - description = "VPC SC ingress policies per perimeter, values reference keys defined in the `vpc_sc_ingress_policies` variable." - type = object({ - dev = list(string) - landing = list(string) - prod = list(string) - }) - default = null + default = {} + nullable = false } -variable "vpc_sc_perimeter_projects" { - description = "VPC SC perimeter resources." +variable "vpc_sc_perimeters" { + description = "VPC SC regular perimeter definitions." type = object({ - dev = list(string) - landing = list(string) - prod = list(string) + dev = optional(object({ + access_levels = optional(list(string), []) + egress_policies = optional(list(string), []) + ingress_policies = optional(list(string), []) + resources = optional(list(string), []) + }), {}) + landing = optional(object({ + access_levels = optional(list(string), []) + egress_policies = optional(list(string), []) + ingress_policies = optional(list(string), []) + resources = optional(list(string), []) + }), {}) + prod = optional(object({ + access_levels = optional(list(string), []) + egress_policies = optional(list(string), []) + ingress_policies = optional(list(string), []) + resources = optional(list(string), []) + }), {}) }) - default = null + default = {} + nullable = false } diff --git a/fast/stages/02-security/vpc-sc.tf b/fast/stages/02-security/vpc-sc.tf index e9887d2053..60767fe5ff 100644 --- a/fast/stages/02-security/vpc-sc.tf +++ b/fast/stages/02-security/vpc-sc.tf @@ -15,118 +15,34 @@ */ locals { - _perimeter_names = ["dev", "landing", "prod"] - # dereference perimeter egress policy names to the actual objects - _vpc_sc_perimeter_egress_policies = { - for k, v in coalesce(var.vpc_sc_perimeter_egress_policies, {}) : - k => [ - for i in coalesce(v, []) : var.vpc_sc_egress_policies[i] - if lookup(var.vpc_sc_egress_policies, i, null) != null - ] - } - # dereference perimeter ingress policy names to the actual objects - _vpc_sc_perimeter_ingress_policies = { - for k, v in coalesce(var.vpc_sc_perimeter_ingress_policies, {}) : - k => [ - for i in coalesce(v, []) : var.vpc_sc_ingress_policies[i] - if lookup(var.vpc_sc_ingress_policies, i, null) != null - ] - } + _vpc_sc_vpc_accessible_services = null + _vpc_sc_restricted_services = yamldecode( + file("${path.module}/vpc-sc-restricted-services.yaml") + ) # compute the number of projects in each perimeter to detect which to create vpc_sc_counts = { - for k in local._perimeter_names : k => length( - local.vpc_sc_perimeter_projects[k] - ) + for k, v in var.vpc_sc_perimeters : k => length(v.resources) } # define dry run spec at file level for convenience vpc_sc_explicit_dry_run_spec = true # compute perimeter bridge resources (projects) - vpc_sc_p_bridge_resources = { + vpc_sc_bridge_resources = { landing_to_dev = concat( - local.vpc_sc_perimeter_projects.landing, - local.vpc_sc_perimeter_projects.dev + var.vpc_sc_perimeters.landing.resources, + var.vpc_sc_perimeters.dev.resources ) landing_to_prod = concat( - local.vpc_sc_perimeter_projects.landing, - local.vpc_sc_perimeter_projects.prod + var.vpc_sc_perimeters.landing.resources, + var.vpc_sc_perimeters.prod.resources ) } - # computer perimeter regular specs / status - vpc_sc_p_regular_specs = { - dev = { - access_levels = coalesce( - try(var.vpc_sc_perimeter_access_levels.dev, null), [] - ) - resources = local.vpc_sc_perimeter_projects.dev - restricted_services = local.vpc_sc_restricted_services - egress_policies = try( - local._vpc_sc_perimeter_egress_policies.dev, null - ) - ingress_policies = try( - local._vpc_sc_perimeter_ingress_policies.dev, null - ) - vpc_accessible_services = null - # vpc_accessible_services = { - # allowed_services = ["RESTRICTED-SERVICES"] - # enable_restriction = true - # } - } - landing = { - access_levels = coalesce( - try(var.vpc_sc_perimeter_access_levels.landing, null), [] - ) - resources = local.vpc_sc_perimeter_projects.landing - restricted_services = local.vpc_sc_restricted_services - egress_policies = try( - local._vpc_sc_perimeter_egress_policies.landing, null - ) - ingress_policies = try( - local._vpc_sc_perimeter_ingress_policies.landing, null - ) - vpc_accessible_services = null - # vpc_accessible_services = { - # allowed_services = ["RESTRICTED-SERVICES"] - # enable_restriction = true - # } - } - prod = { - access_levels = coalesce( - try(var.vpc_sc_perimeter_access_levels.prod, null), [] - ) - # combine the security project, and any specified in the variable - resources = local.vpc_sc_perimeter_projects.prod - restricted_services = local.vpc_sc_restricted_services - egress_policies = try( - local._vpc_sc_perimeter_egress_policies.prod, null - ) - ingress_policies = try( - local._vpc_sc_perimeter_ingress_policies.prod, null - ) - vpc_accessible_services = null - # vpc_accessible_services = { - # allowed_services = ["RESTRICTED-SERVICES"] - # enable_restriction = true - # } - } + # compute spec/status for each perimeter + vpc_sc_perimeters = { + dev = merge(var.vpc_sc_perimeters.dev, { + restricted_services = local._vpc_sc_restricted_services + vpc_accessible_services = local._vpc_sc_vpc_accessible_services + }) } - # account for null values in variable - vpc_sc_perimeter_projects = ( - var.vpc_sc_perimeter_projects == null ? - { - for k in local._perimeter_names : k => [] - } - : { - for k in local._perimeter_names : k => ( - var.vpc_sc_perimeter_projects[k] == null - ? [] - : var.vpc_sc_perimeter_projects[k] - ) - } - ) - # get the list of restricted services from the yaml file - vpc_sc_restricted_services = yamldecode( - file("${path.module}/vpc-sc-restricted-services.yaml") - ) } module "vpc-sc" { @@ -138,22 +54,39 @@ module "vpc-sc" { parent = "organizations/${var.organization.id}" title = "default" } - access_levels = coalesce(try(var.vpc_sc_access_levels, null), {}) - # bridge type perimeters + access_levels = var.vpc_sc_access_levels + egress_policies = var.vpc_sc_egress_policies + ingress_policies = var.vpc_sc_ingress_policies service_perimeters_bridge = merge( # landing to dev, only we have projects in landing and dev perimeters local.vpc_sc_counts.landing * local.vpc_sc_counts.dev == 0 ? {} : { landing_to_dev = { - spec_resources = local.vpc_sc_p_bridge_resources.landing_to_dev - status_resources = null + spec_resources = ( + local.vpc_sc_explicit_dry_run_spec + ? local.vpc_sc_bridge_resources.landing_to_dev + : null + ) + status_resources = ( + local.vpc_sc_explicit_dry_run_spec + ? null + : local.vpc_sc_bridge_resources.landing_to_dev + ) use_explicit_dry_run_spec = local.vpc_sc_explicit_dry_run_spec } }, # landing to prod, only we have projects in landing and prod perimeters local.vpc_sc_counts.landing * local.vpc_sc_counts.prod == 0 ? {} : { landing_to_prod = { - spec_resources = local.vpc_sc_p_bridge_resources.landing_to_prod - status_resources = null + spec_resources = ( + local.vpc_sc_explicit_dry_run_spec + ? local.vpc_sc_bridge_resources.landing_to_prod + : null + ) + status_resources = ( + local.vpc_sc_explicit_dry_run_spec + ? null + : local.vpc_sc_bridge_resources.landing_to_prod + ) use_explicit_dry_run_spec = local.vpc_sc_explicit_dry_run_spec } } @@ -163,24 +96,48 @@ module "vpc-sc" { # dev if we have projects in var.vpc_sc_perimeter_projects.dev local.vpc_sc_counts.dev == 0 ? {} : { dev = { - spec = local.vpc_sc_p_regular_specs.dev - status = null + spec = ( + local.vpc_sc_explicit_dry_run_spec + ? var.vpc_sc_perimeters.dev + : null + ) + status = ( + local.vpc_sc_explicit_dry_run_spec + ? null + : var.vpc_sc_perimeters.dev + ) use_explicit_dry_run_spec = local.vpc_sc_explicit_dry_run_spec } }, # landing if we have projects in var.vpc_sc_perimeter_projects.landing local.vpc_sc_counts.landing == 0 ? {} : { landing = { - spec = local.vpc_sc_p_regular_specs.landing - status = null + spec = ( + local.vpc_sc_explicit_dry_run_spec + ? var.vpc_sc_perimeters.landing + : null + ) + status = ( + local.vpc_sc_explicit_dry_run_spec + ? null + : var.vpc_sc_perimeters.landing + ) use_explicit_dry_run_spec = local.vpc_sc_explicit_dry_run_spec } }, # prod if we have projects in var.vpc_sc_perimeter_projects.prod local.vpc_sc_counts.prod == 0 ? {} : { prod = { - spec = local.vpc_sc_p_regular_specs.prod - status = null + spec = ( + local.vpc_sc_explicit_dry_run_spec + ? var.vpc_sc_perimeters.prod + : null + ) + status = ( + local.vpc_sc_explicit_dry_run_spec + ? null + : var.vpc_sc_perimeters.prod + ) use_explicit_dry_run_spec = local.vpc_sc_explicit_dry_run_spec } }, diff --git a/modules/vpc-sc/service-perimeters-regular.tf b/modules/vpc-sc/service-perimeters-regular.tf index a401b322a7..5ef41c321b 100644 --- a/modules/vpc-sc/service-perimeters-regular.tf +++ b/modules/vpc-sc/service-perimeters-regular.tf @@ -91,11 +91,11 @@ resource "google_access_context_manager_service_perimeter" "regular" { identities = policy.value.from.identities dynamic "sources" { for_each = toset(policy.value.from.access_levels) + iterator = s content { access_level = try( - google_access_context_manager_access_level.basic[k].id, k + google_access_context_manager_access_level.basic[s.value].id, s.value ) - } } dynamic "sources" { @@ -107,7 +107,7 @@ resource "google_access_context_manager_service_perimeter" "regular" { } } dynamic "ingress_to" { - for_each = policy.key.ingress_to == null ? [] : [""] + for_each = policy.value.to == null ? [] : [""] content { resources = policy.value.to.resources dynamic "operations" { @@ -118,7 +118,7 @@ resource "google_access_context_manager_service_perimeter" "regular" { dynamic "method_selectors" { for_each = toset(coalesce(o.value.method_selectors, [])) content { - method = method_selectors.key + method = method_selectors.value } } } @@ -202,8 +202,11 @@ resource "google_access_context_manager_service_perimeter" "regular" { identities = policy.value.from.identities dynamic "sources" { for_each = toset(policy.value.from.access_levels) + iterator = s content { - access_level = sources.key + access_level = try( + google_access_context_manager_access_level.basic[s.value].id, s.value + ) } } dynamic "sources" { @@ -215,7 +218,7 @@ resource "google_access_context_manager_service_perimeter" "regular" { } } dynamic "ingress_to" { - for_each = policy.key.ingress_to == null ? [] : [""] + for_each = policy.value.to == null ? [] : [""] content { resources = policy.value.to.resources dynamic "operations" { @@ -226,7 +229,7 @@ resource "google_access_context_manager_service_perimeter" "regular" { dynamic "method_selectors" { for_each = toset(coalesce(o.value.method_selectors, [])) content { - method = method_selectors.key + method = method_selectors.value } } } diff --git a/modules/vpc-sc/variables.tf b/modules/vpc-sc/variables.tf index 1b75e90308..1be23ae3dc 100644 --- a/modules/vpc-sc/variables.tf +++ b/modules/vpc-sc/variables.tf @@ -15,7 +15,7 @@ */ variable "access_levels" { - description = "Map of access levels in name => [conditions] format." + description = "Access level definitions." type = map(object({ combining_function = optional(string) conditions = optional(list(object({ diff --git a/tests/fast/stages/s02_security/fixture/main.tf b/tests/fast/stages/s02_security/fixture/main.tf index 67173eb8c5..d65805f9ea 100644 --- a/tests/fast/stages/s02_security/fixture/main.tf +++ b/tests/fast/stages/s02_security/fixture/main.tf @@ -49,68 +49,59 @@ module "stage" { project-factory-dev = "foobar@iam.gserviceaccount.com" project-factory-prod = "foobar@iam.gserviceaccount.com" } - vpc_sc_ingress_policies = { - iac = { - ingress_from = { - identities = [ - "serviceAccount:fast-prod-resman-security-0@fast-prod-iac-core-0.iam.gserviceaccount.com" - ], - source_access_levels = ["*"], identity_type = null, source_resources = null - } - ingress_to = { - operations = [{ method_selectors = [], service_name = "*" }] - resources = ["*"] - } - } - } - vpc_sc_perimeter_ingress_policies = { - dev = ["iac"] - landing = null - prod = ["iac"] - } - vpc_sc_perimeter_projects = { - dev = [ - "projects/345678912", # ludo-dev-sec-core-0 - ] - landing = [] - prod = [ - "projects/234567891", # ludo-prod-sec-core-0 - ] - } - vpc_sc_access_levels = { - all = { - combining_function = null + onprem = { conditions = [{ - members = [ - "serviceAccount:quota-monitor@foobar.iam.gserviceaccount.com", - ], - ip_subnetworks = null, negate = null, regions = null, - required_access_levels = null + ip_subnetworks = ["101.101.101.0/24"] }] } } - - vpc_sc_perimeter_access_levels = { - dev = ["all"] - landing = null - prod = ["all"] - } - vpc_sc_egress_policies = { iac-gcs = { - egress_from = { - identity_type = null + from = { identities = [ - "serviceAccount:fast-prod-resman-security-0@fast-prod-iac-core-0.iam.gserviceaccount.com" + "serviceAccount:xxx-prod-resman-security-0@xxx-prod-iac-core-0.iam.gserviceaccount.com" ] } - egress_to = { + to = { operations = [{ - method_selectors = ["*"], service_name = "storage.googleapis.com" + method_selectors = ["*"] + service_name = "storage.googleapis.com" }] - resources = ["projects/123456789"] + resources = ["projects/123456782"] } } } + vpc_sc_ingress_policies = { + iac = { + from = { + identities = [ + "serviceAccount:xxx-prod-resman-security-0@xxx-prod-iac-core-0.iam.gserviceaccount.com" + ] + access_levels = ["*"] + } + to = { + operations = [{ method_selectors = [], service_name = "*" }] + resources = ["*"] + } + } + } + vpc_sc_perimeters = { + dev = { + egress_policies = ["iac-gcs"] + ingress_policies = ["iac"] + resources = ["projects/1111111111"] + } + dev = { + egress_policies = ["iac-gcs"] + ingress_policies = ["iac"] + resources = ["projects/0000000000"] + } + dev = { + access_levels = ["onprem"] + egress_policies = ["iac-gcs"] + ingress_policies = ["iac"] + resources = ["projects/2222222222"] + } + } } From 05f617054dfbf5ffdb045dbe246926dc7d04d9f0 Mon Sep 17 00:00:00 2001 From: Ludo Date: Thu, 10 Nov 2022 15:16:51 +0100 Subject: [PATCH 6/7] tfdoc --- fast/stages/02-security/README.md | 11 ++++------- modules/vpc-sc/README.md | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/fast/stages/02-security/README.md b/fast/stages/02-security/README.md index a23af3f7b2..7d1a83a3b4 100644 --- a/fast/stages/02-security/README.md +++ b/fast/stages/02-security/README.md @@ -272,13 +272,10 @@ Some references that might be useful in setting up this stage: | [kms_defaults](variables.tf#L57) | Defaults used for KMS keys. | object({…}) | | {…} | | | [kms_keys](variables.tf#L69) | KMS keys to create, keyed by name. Null attributes will be interpolated with defaults. | map(object({…})) | | {} | | | [outputs_location](variables.tf#L101) | Path where providers, tfvars files, and lists for the following stages are written. Leave empty to disable. | string | | null | | -| [vpc_sc_access_levels](variables.tf#L118) | VPC SC access level definitions. | map(object({…})) | | {} | | -| [vpc_sc_egress_policies](variables.tf#L133) | VPC SC egress policy defnitions. | map(object({…})) | | {} | | -| [vpc_sc_ingress_policies](variables.tf#L151) | VPC SC ingress policy defnitions. | map(object({…})) | | {} | | -| [vpc_sc_perimeter_access_levels](variables.tf#L171) | VPC SC perimeter access_levels. | object({…}) | | null | | -| [vpc_sc_perimeter_egress_policies](variables.tf#L181) | VPC SC egress policies per perimeter, values reference keys defined in the `vpc_sc_ingress_policies` variable. | object({…}) | | null | | -| [vpc_sc_perimeter_ingress_policies](variables.tf#L191) | VPC SC ingress policies per perimeter, values reference keys defined in the `vpc_sc_ingress_policies` variable. | object({…}) | | null | | -| [vpc_sc_perimeter_projects](variables.tf#L201) | VPC SC perimeter resources. | object({…}) | | null | | +| [vpc_sc_access_levels](variables.tf#L118) | VPC SC access level definitions. | map(object({…})) | | {} | | +| [vpc_sc_egress_policies](variables.tf#L147) | VPC SC egress policy defnitions. | map(object({…})) | | {} | | +| [vpc_sc_ingress_policies](variables.tf#L167) | VPC SC ingress policy defnitions. | map(object({…})) | | {} | | +| [vpc_sc_perimeters](variables.tf#L188) | VPC SC regular perimeter definitions. | object({…}) | | {} | | ## Outputs diff --git a/modules/vpc-sc/README.md b/modules/vpc-sc/README.md index 71b86c4259..a78158e176 100644 --- a/modules/vpc-sc/README.md +++ b/modules/vpc-sc/README.md @@ -188,7 +188,7 @@ module "test" { | name | description | type | required | default | |---|---|:---:|:---:|:---:| | [access_policy](variables.tf#L56) | Access Policy name, set to null if creating one. | string | ✓ | | -| [access_levels](variables.tf#L17) | Map of access levels in name => [conditions] format. | map(object({…})) | | {} | +| [access_levels](variables.tf#L17) | Access level definitions. | map(object({…})) | | {} | | [access_policy_create](variables.tf#L61) | Access Policy configuration, fill in to create. Parent is in 'organizations/123456' format. | object({…}) | | null | | [egress_policies](variables.tf#L70) | Egress policy definitions that can be referenced in perimeters. | map(object({…})) | | {} | | [ingress_policies](variables.tf#L99) | Ingress policy definitions that can be referenced in perimeters. | map(object({…})) | | {} | From 53b2f4eeda100a29654796261a49c44132416a1f Mon Sep 17 00:00:00 2001 From: Ludo Date: Thu, 10 Nov 2022 18:19:06 +0100 Subject: [PATCH 7/7] use collections.Counter in tests --- tests/modules/vpc_sc/test_plan.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/modules/vpc_sc/test_plan.py b/tests/modules/vpc_sc/test_plan.py index 326d1e86d4..e4f108e7aa 100644 --- a/tests/modules/vpc_sc/test_plan.py +++ b/tests/modules/vpc_sc/test_plan.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import collections + def test_create_policy(plan_runner): "Test with auto-created policy." @@ -22,10 +24,7 @@ def test_create_policy(plan_runner): _, resources = plan_runner(tf_var_file='test.regular.tfvars', access_policy='null', access_policy_create=access_policy_create) - counts = {} - for r in resources: - n = f'{r["type"]}.{r["name"]}' - counts[n] = counts.get(n, 0) + 1 + counts = collections.Counter(f'{r["type"]}.{r["name"]}' for r in resources) assert counts == { 'google_access_context_manager_access_level.basic': 2, 'google_access_context_manager_access_policy.default': 1, @@ -38,10 +37,7 @@ def test_use_policy(plan_runner): "Test with existing policy." _, resources = plan_runner(tf_var_file='test.regular.tfvars', access_policy="accessPolicies/foobar") - counts = {} - for r in resources: - n = f'{r["type"]}.{r["name"]}' - counts[n] = counts.get(n, 0) + 1 + counts = collections.Counter(f'{r["type"]}.{r["name"]}' for r in resources) assert counts == { 'google_access_context_manager_access_level.basic': 2, 'google_access_context_manager_service_perimeter.bridge': 2,