- Fixes based on PR comments

- Movig module under Security
 - Formatting TF files

CHANGELOG.md

@@ -4,10 +4,11 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+- new `vpc-sc` module
+
 ## [2.4.1] - 2020-07-06
 
 - better fix external IP assignment in `compute-vm`
-- new `vpc-sc` module
 
 ## [2.4.0] - 2020-07-06
 

README.md

@@ -34,11 +34,11 @@ The current list of modules supports most of the core foundational and networkin
 Currently available modules:
 
 - **foundational** - [folders](./modules/folders), [log sinks](./modules/logging-sinks), [organization](./modules/organization), [project](./modules/project), [service accounts](./modules/iam-service-accounts)
-- **networking** - [VPC](./modules/net-vpc), [VPC firewall](./modules/net-vpc-firewall), [VPC peering](./modules/net-vpc-peering), [VPC Service Control](./modules/vpc-sc), [VPN static](./modules/net-vpn-static), [VPN dynamic](./modules/net-vpn-dynamic), [VPN HA](./modules/net-vpn-ha), [NAT](./modules/net-cloudnat), [address reservation](./modules/net-address), [DNS](./modules/dns), [L4 ILB](./modules/net-ilb), [Service Directory](./modules/service-directory), [Cloud Endpoints](./modules/cloudenpoints)
+- **networking** - [VPC](./modules/net-vpc), [VPC firewall](./modules/net-vpc-firewall), [VPC peering](./modules/net-vpc-peering), [VPN static](./modules/net-vpn-static), [VPN dynamic](./modules/net-vpn-dynamic), [VPN HA](./modules/net-vpn-ha), [NAT](./modules/net-cloudnat), [address reservation](./modules/net-address), [DNS](./modules/dns), [L4 ILB](./modules/net-ilb), [Service Directory](./modules/service-directory), [Cloud Endpoints](./modules/cloudenpoints)
 - **compute** - [VM/VM group](./modules/compute-vm), [MIG](./modules/compute-mig), [GKE cluster](./modules/gke-cluster), [GKE nodepool](./modules/gke-nodepool), [COS container](./modules/cos-container) (coredns, mysql, onprem, squid)
 - **data** - [GCS](./modules/gcs), [BigQuery dataset](./modules/bigquery-dataset), [Pub/Sub](./modules/pubsub), [Datafusion](./modules/datafusion), [Bigtable instance](./modules/bigtable-instance)
 - **development** - [Cloud Source Repository](./modules/source-repository), [Container Registry](./modules/container-registry), [Artifact Registry](./modules/artifact-registry)
-- **security** - [KMS](./modules/kms), [SecretManager](./modules/secret-manager)
+- **security** - [KMS](./modules/kms), [SecretManager](./modules/secret-manager), [VPC Service Control](./modules/vpc-sc)
 - **serverless** - [Cloud Functions](./cloud-functions)
 
 For more information and usage examples see each module's README file.

modules/README.md

@@ -27,7 +27,6 @@ Specific modules also offer support for non-authoritative bindings (e.g. `google
 - [VPC](./net-vpc)
 - [VPC firewall](./net-vpc-firewall)
 - [VPC peering](./net-vpc-peering)
-- [VPC Service Control](./vpc-sc)
 - [VPN static](./net-vpn-static)
 - [VPN dynamic](./net-vpn-dynamic)
 - [VPN HA](./net-vpn-ha)
@@ -59,6 +58,7 @@ Specific modules also offer support for non-authoritative bindings (e.g. `google
 
 - [Cloud KMS](./kms)
 - [Secret Manager](./secret-manager)
+- [VPC Service Control](./vpc-sc)
 
 ## Serverless
 

modules/vpc-sc/README.md

@@ -6,7 +6,7 @@ This module allows managing VPC Service Control (VPC-SC) properties:
 - [Access Levels](https://cloud.google.com/access-context-manager/docs/manage-access-levels)
 - [VPC-SC Perimeters](https://cloud.google.com/vpc-service-controls/docs/service-perimeters)
 
-Before you begin, check you are running the script with a service account having the [correct permissions](https://cloud.google.com/access-context-manager/docs/access-control) to use Access Context Manager.
+The Use of this module requires credentials with the [correct permissions](https://cloud.google.com/access-context-manager/docs/access-control) to use Access Context Manager.
 
 ## Example VCP-SC standard perimeter
 

modules/vpc-sc/main.tf

@@ -15,7 +15,7 @@
  */
 
 locals {
-  access_policy_name = try(google_access_context_manager_access_policy.default[var.access_policy_title].name, null)
+  access_policy_name = google_access_context_manager_access_policy.default.name
 
   standard_perimeters = {
     for key, value in var.perimeters :
@@ -32,9 +32,8 @@ locals {
 }
 
 resource "google_access_context_manager_access_policy" "default" {
-  for_each  = toset([var.access_policy_title])
+  parent = "organizations/${var.org_id}"
-  parent    = "organizations/${var.org_id}"
+  title  = var.access_policy_title
-  title     = each.key
 }
 
 resource "google_access_context_manager_access_level" "default" {
@@ -48,10 +47,10 @@ resource "google_access_context_manager_access_level" "default" {
 
     content {
       combining_function = try(each.value.combining_function, null)
-      conditions         {
+      conditions {
-        ip_subnetworks   = try(basic.value.ip_subnetworks,null)
+        ip_subnetworks = try(basic.value.ip_subnetworks, null)
-        members          = try(basic.value.members,null)
+        members        = try(basic.value.members, null)
-        negate           = try(basic.value.negate,null)
+        negate         = try(basic.value.negate, null)
       }
     }
   }
@@ -70,18 +69,23 @@ resource "google_access_context_manager_service_perimeter" "standard" {
     for_each = each.value.enforced_config != null ? [""] : []
 
     content {
-        resources           = formatlist("projects/%s", try(lookup(var.perimeter_projects, each.key, {}).enforced, []))
+      resources = formatlist(
-        restricted_services = each.value.enforced_config.restricted_services
+        "projects/%s", try(lookup(var.perimeter_projects, each.key, {}).enforced, [])
-        access_levels       = formatlist("accessPolicies/${local.access_policy_name}/accessLevels/%s", try(lookup(local.perimeter_access_levels_enforced, each.key, []), []))
+      )
+      restricted_services = each.value.enforced_config.restricted_services
+      access_levels = formatlist(
+        "accessPolicies/${local.access_policy_name}/accessLevels/%s",
+        try(lookup(local.perimeter_access_levels_enforced, each.key, []), [])
+      )
 
-        dynamic "vpc_accessible_services" {
+      dynamic "vpc_accessible_services" {
-            for_each = each.value.enforced_config.vpc_accessible_services != [] ? [""] : []
+        for_each = each.value.enforced_config.vpc_accessible_services != [] ? [""] : []
 
-            content {
+        content {
-                enable_restriction = true
+          enable_restriction = true
-                allowed_services = each.value.enforced_config.vpc_accessible_services
+          allowed_services   = each.value.enforced_config.vpc_accessible_services
-            }
         }
+      }
     }
   }
 
@@ -91,22 +95,26 @@ resource "google_access_context_manager_service_perimeter" "standard" {
     for_each = each.value.dry_run_config != null ? [""] : []
 
     content {
-      resources           = formatlist("projects/%s", try(lookup(var.perimeter_projects, each.key, {}).dry_run, []))
+      resources = formatlist(
+        "projects/%s", try(lookup(var.perimeter_projects, each.key, {}).dry_run, [])
+      )
       restricted_services = try(each.value.dry_run_config.restricted_services, null)
-      access_levels       = formatlist("accessPolicies/${local.access_policy_name}/accessLevels/%s", try(lookup(local.perimeter_access_levels_dry_run, each.key, []), []))
+      access_levels = formatlist(
-
+        "accessPolicies/${local.access_policy_name}/accessLevels/%s",
+        try(lookup(local.perimeter_access_levels_dry_run, each.key, []), [])
+      )
 
       dynamic "vpc_accessible_services" {
-        for_each = try(each.value.dry_run_config.vpc_accessible_services != [] ? [""] : [],[])
+        for_each = try(each.value.dry_run_config.vpc_accessible_services != [] ? [""] : [], [])
 
         content {
           enable_restriction = true
-          allowed_services = try(each.value.dry_run_config.vpc_accessible_services, null)
+          allowed_services   = try(each.value.dry_run_config.vpc_accessible_services, null)
         }
       }
     }
   }
-  
+
   # Uncomment if used alongside `google_access_context_manager_service_perimeter_resource`, 
   # so they don't fight over which resources should be in the policy.
   # lifecycle {
@@ -152,6 +160,6 @@ resource "google_access_context_manager_service_perimeter" "bridge" {
 
   depends_on = [
     google_access_context_manager_service_perimeter.standard,
-    google_access_context_manager_access_level.default,    
+    google_access_context_manager_access_level.default,
   ]
 }

modules/vpc-sc/outputs.tf

@@ -17,7 +17,7 @@
 output "org_id" {
   description = "Organization id dependent on module resources."
   value       = var.org_id
-  depends_on  = [
+  depends_on = [
     google_organization_iam_audit_config,
     google_organization_iam_binding.authoritative,
     google_organization_iam_custom_role.roles,
@@ -34,7 +34,7 @@ output "access_policy_name" {
 
 output "access_levels" {
   description = "Access Levels."
-  value       = {
+  value = {
     for key, value in google_access_context_manager_access_level.default :
     key => value
   }
@@ -42,7 +42,7 @@ output "access_levels" {
 
 output "perimeters_standard" {
   description = "VPC-SC standard perimeter resources."
-  value       = {
+  value = {
     for key, value in google_access_context_manager_service_perimeter.standard :
     key => value
   }
@@ -50,7 +50,7 @@ output "perimeters_standard" {
 
 output "perimeters_bridge" {
   description = "VPC-SC bridge perimeter resources."
-  value       = {
+  value = {
     for key, value in google_access_context_manager_service_perimeter.bridge :
     key => value
   }

modules/vpc-sc/variables.tf

@@ -16,12 +16,12 @@
 
 variable "access_levels" {
   description = "Access Levels."
-  type        = map(object({
+  type = map(object({
     combining_function = string
-    conditions         = list(object({
+    conditions = list(object({
-      ip_subnetworks   = list(string)
+      ip_subnetworks = list(string)
-      members          = list(string)
+      members        = list(string)
-      negate           = string
+      negate         = string
     }))
   }))
   default = {}
@@ -46,8 +46,8 @@ variable "org_id" {
 variable "perimeters" {
   description = "Set of Perimeters."
   type = map(object({
-    type            = string
+    type = string
-    dry_run_config  = object({
+    dry_run_config = object({
       restricted_services     = list(string)
       vpc_accessible_services = list(string)
     })