From 90718bf134c9d97943ac149b5ea6d83e9648ce7c Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Mon, 6 Dec 2021 16:41:08 +0100 Subject: [PATCH 01/24] net-vpc module: added subnet mini-factory --- modules/net-vpc/main.tf | 57 ++++++++++++++++++++++++++++++++---- modules/net-vpc/variables.tf | 6 ++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/modules/net-vpc/main.tf b/modules/net-vpc/main.tf index 0695968f..5e13a167 100644 --- a/modules/net-vpc/main.tf +++ b/modules/net-vpc/main.tf @@ -64,10 +64,11 @@ locals { : [] ) } - subnets = { + subnets = merge({ for subnet in var.subnets : "${subnet.region}/${subnet.name}" => subnet - } + }, local.subnet_data) + subnets_l7ilb = { for subnet in var.subnets_l7ilb : "${subnet.region}/${subnet.name}" => subnet @@ -77,6 +78,52 @@ locals { ? try(google_compute_network.network.0, null) : try(data.google_compute_network.network.0, null) ) + + # subnet_data = var.data_folder == null ? {} : { + # for k, v in local._subnet_data : k => merge(v, + # { + # network_users : concat( + # formatlist("group:%s", try(v.iam_groups, [])), + # formatlist("user:%s", try(v.iam_users, [])), + # formatlist("serviceAccount:%s", try(v.iam_service_accounts, [])) + # ) + # } + # ) + # } + + _subnet_data = var.data_folder == null ? {} : { + for f in fileset(var.data_folder, "**/*.yaml") : + trimsuffix(basename(f), ".yaml") => merge( + yamldecode(file("${var.data_folder}/${f}")), + { + project_id = var.project_id + network = local.network.name + } + ) + } + + + subnet_data = var.data_folder == null ? {} : { + for k, v in local._subnet_data : "${v.region}/${k}" => { + ip_cidr_range = v.ip_cidr_range + name = k + region = v.region + secondary_ip_range = v.secondary_ip_range + } + } + + subnet_data_descriptions = { + for k, v in local._subnet_data : "${v.region}/${k}" => try(v.description, null) + } + + subnet_descriptions = merge(var.subnet_descriptions, local.subnet_data_descriptions) + + subnet_data_private_access = { + for k, v in local._subnet_data : "${v.region}/${k}" => try(v.private_ip_google_access, true) + } + + subnet_private_access = merge(var.subnet_private_access, local.subnet_data_private_access) + } data "google_compute_network" "network" { @@ -146,12 +193,12 @@ resource "google_compute_subnetwork" "subnetwork" { { range_name = name, ip_cidr_range = range } ] description = lookup( - var.subnet_descriptions, + local.subnet_descriptions, "${each.value.region}/${each.value.name}", "Terraform-managed." ) private_ip_google_access = lookup( - var.subnet_private_access, "${each.value.region}/${each.value.name}", true + local.subnet_private_access, "${each.value.region}/${each.value.name}", true ) dynamic "log_config" { for_each = local.subnet_log_configs["${each.value.region}/${each.value.name}"] @@ -177,7 +224,7 @@ resource "google_compute_subnetwork" "l7ilb" { each.value.active || each.value.active == null ? "ACTIVE" : "BACKUP" ) description = lookup( - var.subnet_descriptions, + local.subnet_descriptions, "${each.value.region}/${each.value.name}", "Terraform-managed." ) diff --git a/modules/net-vpc/variables.tf b/modules/net-vpc/variables.tf index 462c8a4d..04b0fdb1 100644 --- a/modules/net-vpc/variables.tf +++ b/modules/net-vpc/variables.tf @@ -20,6 +20,12 @@ variable "auto_create_subnetworks" { default = false } +variable "data_folder" { + description = "An optional folder containing the subnet configurations in YaML format." + type = string + default = null +} + variable "delete_default_routes_on_create" { description = "Set to true to delete the default routes at creation time." type = bool From ca03a8aea758dcbd4790f38a08368a55f59c4ff6 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Mon, 6 Dec 2021 16:44:33 +0100 Subject: [PATCH 02/24] Update README.md --- modules/net-vpc/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/net-vpc/README.md b/modules/net-vpc/README.md index 75462ff6..6b00ac59 100644 --- a/modules/net-vpc/README.md +++ b/modules/net-vpc/README.md @@ -178,6 +178,7 @@ module "vpc" { | name | The name of the network being created | string | ✓ | | | project_id | The ID of the project where this VPC will be created | string | ✓ | | | *auto_create_subnetworks* | Set to true to create an auto mode subnet, defaults to custom mode. | bool | | false | +| *data_folder* | An optional folder containing the subnet configurations in YaML format. | string | | null | | *delete_default_routes_on_create* | Set to true to delete the default routes at creation time. | bool | | false | | *description* | An optional description of this resource (triggers recreation on change). | string | | Terraform-managed. | | *dns_policy* | DNS policy setup for the VPC. | object({...}) | | null | From a2d5f6bfa76cf0e3ca0d7671d88d289ab65294bb Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Mon, 6 Dec 2021 16:46:46 +0100 Subject: [PATCH 03/24] Code cleanup --- modules/net-vpc/main.tf | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/modules/net-vpc/main.tf b/modules/net-vpc/main.tf index 5e13a167..3cfe00f2 100644 --- a/modules/net-vpc/main.tf +++ b/modules/net-vpc/main.tf @@ -79,18 +79,6 @@ locals { : try(data.google_compute_network.network.0, null) ) - # subnet_data = var.data_folder == null ? {} : { - # for k, v in local._subnet_data : k => merge(v, - # { - # network_users : concat( - # formatlist("group:%s", try(v.iam_groups, [])), - # formatlist("user:%s", try(v.iam_users, [])), - # formatlist("serviceAccount:%s", try(v.iam_service_accounts, [])) - # ) - # } - # ) - # } - _subnet_data = var.data_folder == null ? {} : { for f in fileset(var.data_folder, "**/*.yaml") : trimsuffix(basename(f), ".yaml") => merge( @@ -102,7 +90,6 @@ locals { ) } - subnet_data = var.data_folder == null ? {} : { for k, v in local._subnet_data : "${v.region}/${k}" => { ip_cidr_range = v.ip_cidr_range From dfe1bad689803ee5f537de253a18963a7dfcb6d5 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Mon, 6 Dec 2021 17:32:15 +0100 Subject: [PATCH 04/24] Adds support for IAM per-subnet IAM bindings via factory --- modules/net-vpc/main.tf | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/modules/net-vpc/main.tf b/modules/net-vpc/main.tf index 3cfe00f2..21d6dd23 100644 --- a/modules/net-vpc/main.tf +++ b/modules/net-vpc/main.tf @@ -15,17 +15,6 @@ */ locals { - iam_members = var.iam == null ? {} : var.iam - subnet_iam_members = flatten([ - for subnet, roles in local.iam_members : [ - for role, members in roles : { - subnet = subnet - role = role - members = members - } - ] - ]) - log_configs = var.log_configs == null ? {} : var.log_configs peer_network = ( var.peering_config == null @@ -98,19 +87,41 @@ locals { secondary_ip_range = v.secondary_ip_range } } - subnet_data_descriptions = { for k, v in local._subnet_data : "${v.region}/${k}" => try(v.description, null) } - subnet_descriptions = merge(var.subnet_descriptions, local.subnet_data_descriptions) subnet_data_private_access = { for k, v in local._subnet_data : "${v.region}/${k}" => try(v.private_ip_google_access, true) } - subnet_private_access = merge(var.subnet_private_access, local.subnet_data_private_access) + iam_members = var.iam == null ? {} : var.iam + subnet_data_iam_members = [ + for k, v in local._subnet_data : { + subnet = "${v.region}/${k}" + role = "roles/compute.networkUser" + members = concat( + formatlist("group:%s", try(v.iam_groups, [])), + formatlist("user:%s", try(v.iam_users, [])), + formatlist("serviceAccount:%s", try(v.iam_service_accounts, [])) + ) + } + ] + subnet_iam_members = concat(local.subnet_data_iam_members, flatten([ + for subnet, roles in local.iam_members : [ + for role, members in roles : { + subnet = subnet + role = role + members = members + } + ] + ])) + + + + } data "google_compute_network" "network" { From 04cf75d446fde7d53775402c13d962eb295b2286 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Mon, 6 Dec 2021 17:47:39 +0100 Subject: [PATCH 05/24] Gracefully handle a null secondary_ip_range --- modules/net-vpc/main.tf | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/modules/net-vpc/main.tf b/modules/net-vpc/main.tf index 21d6dd23..deff4d10 100644 --- a/modules/net-vpc/main.tf +++ b/modules/net-vpc/main.tf @@ -84,7 +84,7 @@ locals { ip_cidr_range = v.ip_cidr_range name = k region = v.region - secondary_ip_range = v.secondary_ip_range + secondary_ip_range = try(v.secondary_ip_range, []) } } subnet_data_descriptions = { @@ -119,9 +119,6 @@ locals { ] ])) - - - } data "google_compute_network" "network" { From 7b07d1fb8defc3943c1866503eb202e3290f3840 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 09:00:04 +0100 Subject: [PATCH 06/24] Update README.md --- factories/subnets/README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/factories/subnets/README.md b/factories/subnets/README.md index 07cec90a..13df44d4 100644 --- a/factories/subnets/README.md +++ b/factories/subnets/README.md @@ -10,7 +10,7 @@ This module implements a resource factory which allows the creation and manageme ```hcl module "subnets" { - source = "./modules/resource-factories/subnets" + source = "./factories/subnets" config_folder = "subnets" } # tftest:skip @@ -48,9 +48,8 @@ iam_groups: ["lorem@example.com"] # Opt- Groups to grant compute/networkUser to iam_service_accounts: ["foobar@project-id.iam.gserviceaccount.com"] # Opt- SAs to grant compute/networkUser to secondary_ip_ranges: # Opt- List of secondary IP ranges - - name: secondary-range-a # Name for the secondary range - ip_cidr_range: 192.168.0.0/24 # IP range for the secondary range - + - secondary-range-a: 192.168.0.0/24 + # Secondary ranges in name: cidr format ``` From 490f500ac277b2c9b8433e67f9367bf7298bd925 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 09:01:32 +0100 Subject: [PATCH 07/24] Update README.md --- factories/firewall-hierarchical-policies/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/factories/firewall-hierarchical-policies/README.md b/factories/firewall-hierarchical-policies/README.md index da317a6f..db374ebf 100644 --- a/factories/firewall-hierarchical-policies/README.md +++ b/factories/firewall-hierarchical-policies/README.md @@ -12,7 +12,7 @@ This module also allows defining custom template variables, to centralize common ```hcl module "hierarchical" { - source = "./modules/resource-factories/hierarchical-firewall" + source = "./factories/hierarchical-firewall-policies" config_folder = "firewall/hierarchical" templates_folder = "firewall/templates" } From 8a2ad25a453b8e9a78380f5bf4c846a461b63014 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 09:35:24 +0100 Subject: [PATCH 08/24] Updated README.md. Added tests. --- modules/net-vpc/README.md | 29 +++++++++++++++++++ .../net_vpc/fixture/data/factory-subnet.yaml | 9 ++++++ tests/modules/net_vpc/fixture/main.tf | 1 + tests/modules/net_vpc/fixture/variables.tf | 6 ++++ tests/modules/net_vpc/test_plan_subnets.py | 19 ++++++++++-- 5 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 tests/modules/net_vpc/fixture/data/factory-subnet.yaml diff --git a/modules/net-vpc/README.md b/modules/net-vpc/README.md index 6b00ac59..98c6e99e 100644 --- a/modules/net-vpc/README.md +++ b/modules/net-vpc/README.md @@ -170,6 +170,35 @@ module "vpc" { # tftest:modules=1:resources=3 ``` +### Subnet Factory +The `net-vpc` module includes a subnet factory (see [Resource Factories](../../factories/)) for the massive creation of subnets leveraging one configuration file per subnet. + + +```hcl +module "vpc" { + source = "./modules/net-vpc" + project_id = "my-project" + name = "my-network" + data_folder = "config/subnets" +} +# tftest:skip +``` + +```yaml +# ./config/subnets/subnet-name.yaml +region: europe-west1 # Region where the subnet will be creted +description: Sample description # Description +ip_cidr_range: 10.0.0.0/24 # Primary IP range for the subnet +private_ip_google_access: false # Opt- Enables PGA. Defaults to true +iam_users: ["foobar@example.com"] # Opt- Users to grant compute/networkUser to +iam_groups: ["lorem@example.com"] # Opt- Groups to grant compute/networkUser to +iam_service_accounts: ["foobar@project-id.iam.gserviceaccount.com"] + # Opt- SAs to grant compute/networkUser to +secondary_ip_ranges: # Opt- List of secondary IP ranges + - secondary-range-a: 192.168.0.0/24 + # Secondary ranges in name: cidr format +``` + ## Variables diff --git a/tests/modules/net_vpc/fixture/data/factory-subnet.yaml b/tests/modules/net_vpc/fixture/data/factory-subnet.yaml new file mode 100644 index 00000000..bdb3c4c9 --- /dev/null +++ b/tests/modules/net_vpc/fixture/data/factory-subnet.yaml @@ -0,0 +1,9 @@ +region: europe-west1 +description: Sample description +ip_cidr_range: 10.128.0.0/24 +private_ip_google_access: false +iam_users: ["foobar@example.com"] +iam_groups: ["lorem@example.com"] +iam_service_accounts: ["foobar@project-id.iam.gserviceaccount.com"] +secondary_ip_range: + secondary-range-a: 192.168.128.0/24 \ No newline at end of file diff --git a/tests/modules/net_vpc/fixture/main.tf b/tests/modules/net_vpc/fixture/main.tf index 42217101..5812bba6 100644 --- a/tests/modules/net_vpc/fixture/main.tf +++ b/tests/modules/net_vpc/fixture/main.tf @@ -31,4 +31,5 @@ module "test" { subnet_private_access = var.subnet_private_access auto_create_subnetworks = var.auto_create_subnetworks private_service_networking_range = var.private_service_networking_range + data_folder = var.data_folder } diff --git a/tests/modules/net_vpc/fixture/variables.tf b/tests/modules/net_vpc/fixture/variables.tf index efa2ba7a..80628473 100644 --- a/tests/modules/net_vpc/fixture/variables.tf +++ b/tests/modules/net_vpc/fixture/variables.tf @@ -125,3 +125,9 @@ variable "private_service_networking_range" { type = string default = null } + +variable "data_folder" { + description = "An optional folder containing the subnet configurations in YaML format." + type = string + default = null +} diff --git a/tests/modules/net_vpc/test_plan_subnets.py b/tests/modules/net_vpc/test_plan_subnets.py index 99375e8b..166487b7 100644 --- a/tests/modules/net_vpc/test_plan_subnets.py +++ b/tests/modules/net_vpc/test_plan_subnets.py @@ -29,6 +29,19 @@ _VAR_SUBNETS = ( ']' ) +_VAR_DATA_FOLDER = "data" + + +def test_subnet_factory(plan_runner): + "Test subnet factory." + _, resources = plan_runner(FIXTURES_DIR, data_folder=_VAR_DATA_FOLDER) + assert len(resources) == 3 + subnets = [r['values'] + for r in resources if r['type'] == 'google_compute_subnetwork'] + assert set(s['name'] for s in subnets) == set( + ['factory-subnet']) + assert set(len(s['secondary_ip_range']) for s in subnets) == set([1]) + def test_subnets_simple(plan_runner): "Test subnets variable." @@ -58,9 +71,9 @@ def test_subnet_log_configs(plan_runner): for r in resources: if r['type'] != 'google_compute_subnetwork': continue - flow_logs[r['values']['name']] = [{key: config[key] for key in config.keys() - & {'aggregation_interval', 'flow_sampling', 'metadata'}} - for config in r['values']['log_config']] + flow_logs[r['values']['name']] = [{key: config[key] for key in config.keys() + & {'aggregation_interval', 'flow_sampling', 'metadata'}} + for config in r['values']['log_config']] assert flow_logs == { # enable, override one default option 'a': [{ From 3b79b79d923bcce16644d6e1ceb8e01bed63441c Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 09:37:51 +0100 Subject: [PATCH 09/24] Added license boilerplate to subnet yaml data in tests --- .../net_vpc/fixture/data/factory-subnet.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/modules/net_vpc/fixture/data/factory-subnet.yaml b/tests/modules/net_vpc/fixture/data/factory-subnet.yaml index bdb3c4c9..dd70105a 100644 --- a/tests/modules/net_vpc/fixture/data/factory-subnet.yaml +++ b/tests/modules/net_vpc/fixture/data/factory-subnet.yaml @@ -1,3 +1,17 @@ +# Copyright 2021 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. + region: europe-west1 description: Sample description ip_cidr_range: 10.128.0.0/24 From a0f15931a506b01bd20f478b282586ca9b0bcb8f Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 10:04:25 +0100 Subject: [PATCH 10/24] Removed dead code --- modules/net-vpc/main.tf | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/modules/net-vpc/main.tf b/modules/net-vpc/main.tf index deff4d10..988141d1 100644 --- a/modules/net-vpc/main.tf +++ b/modules/net-vpc/main.tf @@ -70,16 +70,10 @@ locals { _subnet_data = var.data_folder == null ? {} : { for f in fileset(var.data_folder, "**/*.yaml") : - trimsuffix(basename(f), ".yaml") => merge( - yamldecode(file("${var.data_folder}/${f}")), - { - project_id = var.project_id - network = local.network.name - } - ) + trimsuffix(basename(f), ".yaml") => yamldecode(file("${var.data_folder}/${f}")) } - subnet_data = var.data_folder == null ? {} : { + subnet_data = { for k, v in local._subnet_data : "${v.region}/${k}" => { ip_cidr_range = v.ip_cidr_range name = k From b1ff59299018a17320bd5136790d1923462b27a9 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 13:26:21 +0100 Subject: [PATCH 11/24] net-vpc-firewall mini rules-factory --- modules/net-vpc-firewall/README.md | 45 +++++++++++++++++++++++++++ modules/net-vpc-firewall/main.tf | 30 +++++++++++++++++- modules/net-vpc-firewall/variables.tf | 13 ++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/modules/net-vpc-firewall/README.md b/modules/net-vpc-firewall/README.md index 83f8f7cd..a3ad750f 100644 --- a/modules/net-vpc-firewall/README.md +++ b/modules/net-vpc-firewall/README.md @@ -81,6 +81,49 @@ module "firewall" { # tftest:modules=1:resources=1 ``` + +### Rules Factory +The module includes a rules factory (see [Resource Factories](../../factories/)) for the massive creation of rules leveraging YaML configuration files. Each configuration file can optionally contain more than one rule which a structure that reflects the `custom_rules` variable. + +```hcl +module "firewall" { + source = "./modules/net-vpc-firewall" + project_id = "my-project" + network = "my-network" + data_folder = "config/firewall" + cidr_template_file = "config/cidr_template.yaml" +} +# tftest:skip +``` + +```yaml +# ./config/firewall/load_balancers.yaml +allow-healthchecks: + description: Allow ingress from healthchecks. + direction: INGRESS + action: allow + sources: [] + ranges: + - $healthchecks + targets: ["lb-backends"] + use_service_accounts: false + rules: + - protocol: tcp + ports: + - 80 + - 443 +``` + +```yaml +# ./config/cidr_template.yaml +healthchecks: + - 35.191.0.0/16 + - 130.211.0.0/22 + - 209.85.152.0/22 + - 209.85.204.0/22 + +``` + ## Variables @@ -89,7 +132,9 @@ module "firewall" { | network | Name of the network this set of firewall rules applies to. | string | ✓ | | | project_id | Project id of the project that holds the network. | string | ✓ | | | *admin_ranges* | IP CIDR ranges that have complete access to all subnets. | list(string) | | [] | +| *cidr_template_file* | Path for optional file containing name->cidr_list map to be used by the rules factory. | string | | null | | *custom_rules* | List of custom rule definitions (refer to variables file for syntax). | map(object({...})) | | {} | +| *data_folder* | Path for optional folder containing firewall rules defined as YaML objects used by the rules factory. | string | | null | | *http_source_ranges* | List of IP CIDR ranges for tag-based HTTP rule, defaults to the health checkers ranges. | list(string) | | ["35.191.0.0/16", "130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22"] | | *https_source_ranges* | List of IP CIDR ranges for tag-based HTTPS rule, defaults to the health checkers ranges. | list(string) | | ["35.191.0.0/16", "130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22"] | | *named_ranges* | Names that can be used of valid values for the `ranges` field of `custom_rules` | map(list(string)) | | ... | diff --git a/modules/net-vpc-firewall/main.tf b/modules/net-vpc-firewall/main.tf index c196e470..aac7b929 100644 --- a/modules/net-vpc-firewall/main.tf +++ b/modules/net-vpc-firewall/main.tf @@ -15,7 +15,7 @@ */ locals { - custom_rules = { + _custom_rules = { for id, rule in var.custom_rules : id => merge(rule, { # make rules a map so we use it in a for_each @@ -27,8 +27,36 @@ locals { ]) }) } + + cidrs = try({ for name, cidrs in yamldecode(file("${var.cidr_template_file}")) : + name => cidrs + }, {}) + + _factory_rules_raw = flatten([ + for file in try(fileset(var.data_folder, "**/*.yaml"), []) : [ + for key, ruleset in yamldecode(file("${var.data_folder}/${file}")) : + merge(ruleset, { + name = "${key}" + rules = { for index, ports in ruleset.rules : index => ports } + ranges = try(ruleset.ranges, null) == null ? null : flatten( + [for cidr in ruleset.ranges : + can(regex("^\\$", cidr)) + ? local.cidrs[trimprefix(cidr, "$")] + : [cidr] + ]) + extra_attributes = try(ruleset.extra_attributes, {}) + }) + ] + ]) + + _factory_rules = { + for d in local._custom_rules_raw : d["name"] => d + } + + custom_rules = merge(local._custom_rules, local._factory_rules) } + ############################################################################### # rules based on IP ranges ############################################################################### diff --git a/modules/net-vpc-firewall/variables.tf b/modules/net-vpc-firewall/variables.tf index 94755d2e..7a392a6d 100644 --- a/modules/net-vpc-firewall/variables.tf +++ b/modules/net-vpc-firewall/variables.tf @@ -39,6 +39,18 @@ variable "custom_rules" { default = {} } +variable "cidr_template_file" { + description = "Path for optional file containing name->cidr_list map to be used by the rules factory." + type = string + default = null +} + +variable "data_folder" { + description = "Path for optional folder containing firewall rules defined as YaML objects used by the rules factory." + type = string + default = null +} + variable "http_source_ranges" { description = "List of IP CIDR ranges for tag-based HTTP rule, defaults to the health checkers ranges." type = list(string) @@ -80,3 +92,4 @@ variable "ssh_source_ranges" { type = list(string) default = ["35.235.240.0/20"] } + From 5c8557a29d0ed8504c51a076247b08bf709f8b69 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 13:29:01 +0100 Subject: [PATCH 12/24] Paying tributes to the linting gods --- modules/net-vpc-firewall/variables.tf | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/net-vpc-firewall/variables.tf b/modules/net-vpc-firewall/variables.tf index 7a392a6d..7682a3f3 100644 --- a/modules/net-vpc-firewall/variables.tf +++ b/modules/net-vpc-firewall/variables.tf @@ -20,6 +20,12 @@ variable "admin_ranges" { default = [] } +variable "cidr_template_file" { + description = "Path for optional file containing name->cidr_list map to be used by the rules factory." + type = string + default = null +} + variable "custom_rules" { description = "List of custom rule definitions (refer to variables file for syntax)." type = map(object({ @@ -39,12 +45,6 @@ variable "custom_rules" { default = {} } -variable "cidr_template_file" { - description = "Path for optional file containing name->cidr_list map to be used by the rules factory." - type = string - default = null -} - variable "data_folder" { description = "Path for optional folder containing firewall rules defined as YaML objects used by the rules factory." type = string From 83485040678189bc11032c1e1596a358a965b366 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 13:40:35 +0100 Subject: [PATCH 13/24] Fixed small bug on locals --- modules/net-vpc-firewall/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/net-vpc-firewall/main.tf b/modules/net-vpc-firewall/main.tf index aac7b929..23dc0588 100644 --- a/modules/net-vpc-firewall/main.tf +++ b/modules/net-vpc-firewall/main.tf @@ -50,7 +50,7 @@ locals { ]) _factory_rules = { - for d in local._custom_rules_raw : d["name"] => d + for d in local._factory_rules_raw : d["name"] => d } custom_rules = merge(local._custom_rules, local._factory_rules) From 28d84c120a9960a40b69f6c4b8d81f503d9a28b1 Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 14:40:23 +0100 Subject: [PATCH 14/24] Tests for net-vpc-firewall module --- tests/modules/net_vpc_firewall/__init__.py | 13 +++ .../fixture/config/cidr_template.yaml | 19 ++++ .../config/firewall/load_balancers.yaml | 28 ++++++ .../modules/net_vpc_firewall/fixture/main.tf | 28 ++++++ .../net_vpc_firewall/fixture/variables.tf | 97 +++++++++++++++++++ tests/modules/net_vpc_firewall/test_plan.py | 44 +++++++++ 6 files changed, 229 insertions(+) create mode 100644 tests/modules/net_vpc_firewall/__init__.py create mode 100644 tests/modules/net_vpc_firewall/fixture/config/cidr_template.yaml create mode 100644 tests/modules/net_vpc_firewall/fixture/config/firewall/load_balancers.yaml create mode 100644 tests/modules/net_vpc_firewall/fixture/main.tf create mode 100644 tests/modules/net_vpc_firewall/fixture/variables.tf create mode 100644 tests/modules/net_vpc_firewall/test_plan.py diff --git a/tests/modules/net_vpc_firewall/__init__.py b/tests/modules/net_vpc_firewall/__init__.py new file mode 100644 index 00000000..d46dbae5 --- /dev/null +++ b/tests/modules/net_vpc_firewall/__init__.py @@ -0,0 +1,13 @@ +# Copyright 2021 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. diff --git a/tests/modules/net_vpc_firewall/fixture/config/cidr_template.yaml b/tests/modules/net_vpc_firewall/fixture/config/cidr_template.yaml new file mode 100644 index 00000000..b33125de --- /dev/null +++ b/tests/modules/net_vpc_firewall/fixture/config/cidr_template.yaml @@ -0,0 +1,19 @@ +# Copyright 2021 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. + +healthchecks: + - 35.191.0.0/16 + - 130.211.0.0/22 + - 209.85.152.0/22 + - 209.85.204.0/22 diff --git a/tests/modules/net_vpc_firewall/fixture/config/firewall/load_balancers.yaml b/tests/modules/net_vpc_firewall/fixture/config/firewall/load_balancers.yaml new file mode 100644 index 00000000..558e65b1 --- /dev/null +++ b/tests/modules/net_vpc_firewall/fixture/config/firewall/load_balancers.yaml @@ -0,0 +1,28 @@ +# Copyright 2021 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. + +allow-healthchecks: + description: Allow ingress from healthchecks. + direction: INGRESS + action: allow + sources: [] + ranges: + - $healthchecks + targets: ["lb-backends"] + use_service_accounts: false + rules: + - protocol: tcp + ports: + - 80 + - 443 diff --git a/tests/modules/net_vpc_firewall/fixture/main.tf b/tests/modules/net_vpc_firewall/fixture/main.tf new file mode 100644 index 00000000..59201cb2 --- /dev/null +++ b/tests/modules/net_vpc_firewall/fixture/main.tf @@ -0,0 +1,28 @@ +/** + * Copyright 2021 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. + */ + +module "firewall" { + source = "../../../../modules/net-vpc-firewall" + project_id = var.project_id + network = var.network + admin_ranges = var.admin_ranges + http_source_ranges = var.http_source_ranges + https_source_ranges = var.https_source_ranges + ssh_source_ranges = var.ssh_source_ranges + custom_rules = var.custom_rules + data_folder = var.data_folder + cidr_template_file = var.cidr_template_file +} diff --git a/tests/modules/net_vpc_firewall/fixture/variables.tf b/tests/modules/net_vpc_firewall/fixture/variables.tf new file mode 100644 index 00000000..7f261a6f --- /dev/null +++ b/tests/modules/net_vpc_firewall/fixture/variables.tf @@ -0,0 +1,97 @@ +/** + * Copyright 2021 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 "admin_ranges" { + description = "IP CIDR ranges that have complete access to all subnets." + type = list(string) + default = [] +} + +variable "cidr_template_file" { + description = "Path for optional file containing name->cidr_list map to be used by the rules factory." + type = string + default = null +} + +variable "custom_rules" { + description = "List of custom rule definitions (refer to variables file for syntax)." + type = map(object({ + description = string + direction = string + action = string # (allow|deny) + ranges = list(string) + sources = list(string) + targets = list(string) + use_service_accounts = bool + rules = list(object({ + protocol = string + ports = list(string) + })) + extra_attributes = map(string) + })) + default = {} +} + +variable "data_folder" { + description = "Path for optional folder containing firewall rules defined as YaML objects used by the rules factory." + type = string + default = null +} + +variable "http_source_ranges" { + description = "List of IP CIDR ranges for tag-based HTTP rule, defaults to the health checkers ranges." + type = list(string) + default = ["35.191.0.0/16", "130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22"] +} + +variable "https_source_ranges" { + description = "List of IP CIDR ranges for tag-based HTTPS rule, defaults to the health checkers ranges." + type = list(string) + default = ["35.191.0.0/16", "130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22"] +} + +variable "named_ranges" { + description = "Names that can be used of valid values for the `ranges` field of `custom_rules`" + type = map(list(string)) + default = { + any = ["0.0.0.0/0"] + dns-forwarders = ["35.199.192.0/19"] + health-checkers = ["35.191.0.0/16", "130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22"] + iap-forwarders = ["35.235.240.0/20"] + private-googleapis = ["199.36.153.8/30"] + restricted-googleapis = ["199.36.153.4/30"] + rfc1918 = ["10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"] + } +} + +variable "network" { + description = "Name of the network this set of firewall rules applies to." + type = string + default = "vpc" +} + +variable "project_id" { + description = "Project id of the project that holds the network." + type = string + default = "project" +} + +variable "ssh_source_ranges" { + description = "List of IP CIDR ranges for tag-based SSH rule, defaults to the IAP forwarders range." + type = list(string) + default = ["35.235.240.0/20"] +} + diff --git a/tests/modules/net_vpc_firewall/test_plan.py b/tests/modules/net_vpc_firewall/test_plan.py new file mode 100644 index 00000000..8159a7fc --- /dev/null +++ b/tests/modules/net_vpc_firewall/test_plan.py @@ -0,0 +1,44 @@ +# Copyright 2021 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. + + +import os +import pytest + + +FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture') + + +def test_vpc_firewall_simple(plan_runner): + "Test vpc with no extra options." + _, resources = plan_runner(FIXTURES_DIR) + assert len(resources) == 3 + assert set([r['type'] for r in resources]) == set( + ['google_compute_firewall']) + assert set([r['values']['name'] for r in resources]) == set( + ['vpc-ingress-tag-http', 'vpc-ingress-tag-https', 'vpc-ingress-tag-ssh']) + assert set([r['values']['project'] for r in resources]) == set(['project']) + assert set([r['values']['network'] for r in resources]) == set(['vpc']) + + +def test_vpc_firewall_factory(plan_runner): + "Test shared vpc variables." + _, resources = plan_runner( + FIXTURES_DIR, data_folder="config/firewall", cidr_template_file="config/cidr_template.yaml") + assert len(resources) == 4 + factory_rule = [r for r in resources if r["values"] + ["name"] == "allow-healthchecks"][0]["values"] + assert set(factory_rule["source_ranges"]) == set( + ["130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22", "35.191.0.0/16"]) + assert set(factory_rule["target_tags"]) == set(["lb-backends"]) From dfe83c2cdfa95246db0c282a9831da71fa7fb15b Mon Sep 17 00:00:00 2001 From: Simone Ruffilli Date: Tue, 7 Dec 2021 17:46:49 +0100 Subject: [PATCH 15/24] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 108b5a99..e0e0181f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file. ## [Unreleased] - new `cloud-run` module - added gVNIC support to `compute-vm` module +- added a rule factory to `net-vpc-firewall` module +- added a subnet factory to `net-vpc` module ## [8.0.0] - 2021-10-21 From fcc8741cd2af2abc1b8e80979bd9746f3abe15ce Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Tue, 7 Dec 2021 19:26:24 +0100 Subject: [PATCH 16/24] refactor locals (#382) --- modules/net-vpc/README.md | 27 +++--- modules/net-vpc/main.tf | 178 ++++++++++++++++++-------------------- 2 files changed, 101 insertions(+), 104 deletions(-) diff --git a/modules/net-vpc/README.md b/modules/net-vpc/README.md index 98c6e99e..652edc74 100644 --- a/modules/net-vpc/README.md +++ b/modules/net-vpc/README.md @@ -171,7 +171,7 @@ module "vpc" { ``` ### Subnet Factory -The `net-vpc` module includes a subnet factory (see [Resource Factories](../../factories/)) for the massive creation of subnets leveraging one configuration file per subnet. +The `net-vpc` module includes a subnet factory (see [Resource Factories](../../factories/)) for the massive creation of subnets leveraging one configuration file per subnet. ```hcl @@ -186,17 +186,20 @@ module "vpc" { ```yaml # ./config/subnets/subnet-name.yaml -region: europe-west1 # Region where the subnet will be creted -description: Sample description # Description -ip_cidr_range: 10.0.0.0/24 # Primary IP range for the subnet -private_ip_google_access: false # Opt- Enables PGA. Defaults to true -iam_users: ["foobar@example.com"] # Opt- Users to grant compute/networkUser to -iam_groups: ["lorem@example.com"] # Opt- Groups to grant compute/networkUser to -iam_service_accounts: ["foobar@project-id.iam.gserviceaccount.com"] - # Opt- SAs to grant compute/networkUser to -secondary_ip_ranges: # Opt- List of secondary IP ranges - - secondary-range-a: 192.168.0.0/24 - # Secondary ranges in name: cidr format +region: europe-west1 +description: Sample description +ip_cidr_range: 10.0.0.0/24 +# optional attributes +private_ip_google_access: false # defaults to true +iam_users: ["foobar@example.com"] # grant compute/networkUser to users +iam_groups: ["lorem@example.com"] # grant compute/networkUser to groups +iam_service_accounts: ["fbz@prj.iam.gserviceaccount.com"] +secondary_ip_ranges: # map of secondary ip ranges + - secondary-range-a: 192.168.0.0/24 +flow_logs: # enable, set to empty map to use defaults + - aggregation_interval: "INTERVAL_5_SEC" + - flow_sampling: 0.5 + - metadata: "INCLUDE_ALL_METADATA" ``` diff --git a/modules/net-vpc/main.tf b/modules/net-vpc/main.tf index 988141d1..a48f2d0a 100644 --- a/modules/net-vpc/main.tf +++ b/modules/net-vpc/main.tf @@ -15,86 +15,17 @@ */ locals { - log_configs = var.log_configs == null ? {} : var.log_configs - peer_network = ( - var.peering_config == null - ? null - : element(reverse(split("/", var.peering_config.peer_vpc_self_link)), 0) - ) - routes = var.routes == null ? {} : var.routes - routes_gateway = { - for name, data in local.routes : - name => data if data.next_hop_type == "gateway" - } - routes_ilb = { - for name, data in local.routes : - name => data if data.next_hop_type == "ilb" - } - routes_instance = { - for name, data in local.routes : - name => data if data.next_hop_type == "instance" - } - routes_ip = { - for name, data in local.routes : - name => data if data.next_hop_type == "ip" - } - routes_vpn_tunnel = { - for name, data in local.routes : - name => data if data.next_hop_type == "vpn_tunnel" - } - subnet_log_configs = { - for name, attrs in { for s in local.subnets : format("%s/%s", s.region, s.name) => s } : name => ( - lookup(var.subnet_flow_logs, name, false) - ? [{ - for key, value in var.log_config_defaults : key => lookup( - lookup(local.log_configs, name, {}), key, value - ) - }] - : [] - ) - } - subnets = merge({ - for subnet in var.subnets : - "${subnet.region}/${subnet.name}" => subnet - }, local.subnet_data) - - subnets_l7ilb = { - for subnet in var.subnets_l7ilb : - "${subnet.region}/${subnet.name}" => subnet - } - network = ( - var.vpc_create - ? try(google_compute_network.network.0, null) - : try(data.google_compute_network.network.0, null) - ) - - _subnet_data = var.data_folder == null ? {} : { + _factory_data = var.data_folder == null ? {} : { for f in fileset(var.data_folder, "**/*.yaml") : trimsuffix(basename(f), ".yaml") => yamldecode(file("${var.data_folder}/${f}")) } - - subnet_data = { - for k, v in local._subnet_data : "${v.region}/${k}" => { - ip_cidr_range = v.ip_cidr_range - name = k - region = v.region - secondary_ip_range = try(v.secondary_ip_range, []) - } + _factory_descriptions = { + for k, v in local._factory_data : + "${v.region}/${k}" => try(v.description, null) } - subnet_data_descriptions = { - for k, v in local._subnet_data : "${v.region}/${k}" => try(v.description, null) - } - subnet_descriptions = merge(var.subnet_descriptions, local.subnet_data_descriptions) - - subnet_data_private_access = { - for k, v in local._subnet_data : "${v.region}/${k}" => try(v.private_ip_google_access, true) - } - subnet_private_access = merge(var.subnet_private_access, local.subnet_data_private_access) - - iam_members = var.iam == null ? {} : var.iam - subnet_data_iam_members = [ - for k, v in local._subnet_data : { - subnet = "${v.region}/${k}" + _factory_iam_members = [ + for k, v in local._factory_subnets : { + subnet = k role = "roles/compute.networkUser" members = concat( formatlist("group:%s", try(v.iam_groups, [])), @@ -103,16 +34,77 @@ locals { ) } ] - subnet_iam_members = concat(local.subnet_data_iam_members, flatten([ - for subnet, roles in local.iam_members : [ + _factory_flow_logs = { + for k, v in local._factory_data : "${v.region}/${k}" => merge( + var.log_config_defaults, try(v.flow_logs, {}) + ) if try(v.flow_logs, false) + } + _factory_private_access = { + for k, v in local._factory_data : "${v.region}/${k}" => try( + v.private_ip_google_access, true + ) + } + _factory_subnets = { + for k, v in local._factory_data : "${v.region}/${k}" => { + ip_cidr_range = v.ip_cidr_range + name = k + region = v.region + secondary_ip_range = try(v.secondary_ip_range, []) + } + } + _iam = var.iam == null ? {} : var.iam + _routes = var.routes == null ? {} : var.routes + _subnet_flow_logs = { + for k, v in var.subnet_flow_logs : k => merge( + var.log_config_defaults, try(var.log_configs[k], {}) + ) + } + _subnet_iam_members = flatten([ + for subnet, roles in local._iam : [ for role, members in roles : { - subnet = subnet - role = role members = members + role = role + subnet = subnet } ] - ])) - + ]) + network = ( + var.vpc_create + ? try(google_compute_network.network.0, null) + : try(data.google_compute_network.network.0, null) + ) + peer_network = ( + var.peering_config == null + ? null + : element(reverse(split("/", var.peering_config.peer_vpc_self_link)), 0) + ) + routes = { + gateway = { for k, v in local._routes : k => v if v.next_hop_type == "gateway" } + ilb = { for k, v in local._routes : k => v if v.next_hop_type == "ilb" } + instance = { for k, v in local._routes : k => v if v.next_hop_type == "instance" } + ip = { for k, v in local._routes : k => v if v.next_hop_type == "ip" } + vpn_tunnel = { for k, v in local._routes : k => v if v.next_hop_type == "vpn_tunnel" } + } + subnet_descriptions = merge( + local._factory_descriptions, var.subnet_descriptions + ) + subnet_iam_members = concat( + local._factory_iam_members, local._subnet_iam_members + ) + subnet_flow_logs = merge( + local._factory_flow_logs, local._subnet_flow_logs + ) + subnet_private_access = merge( + local._factory_private_access, var.subnet_private_access + ) + subnets = merge( + { for subnet in var.subnets : "${subnet.region}/${subnet.name}" => subnet }, + local._factory_subnets + ) + subnets_l7ilb = { + for subnet in var.subnets_l7ilb : + "${subnet.region}/${subnet.name}" => subnet + } } data "google_compute_network" "network" { @@ -182,15 +174,17 @@ resource "google_compute_subnetwork" "subnetwork" { { range_name = name, ip_cidr_range = range } ] description = lookup( - local.subnet_descriptions, - "${each.value.region}/${each.value.name}", - "Terraform-managed." + local.subnet_descriptions, each.key, "Terraform-managed." ) private_ip_google_access = lookup( - local.subnet_private_access, "${each.value.region}/${each.value.name}", true + local.subnet_private_access, each.key, true ) dynamic "log_config" { - for_each = local.subnet_log_configs["${each.value.region}/${each.value.name}"] + for_each = toset( + try(local.subnet_flow_logs[each.key], {}) != {} + ? [local.subnet_flow_logs[each.key]] + : [] + ) iterator = config content { aggregation_interval = config.value.aggregation_interval @@ -232,7 +226,7 @@ resource "google_compute_subnetwork_iam_binding" "binding" { } resource "google_compute_route" "gateway" { - for_each = local.routes_gateway + for_each = local.routes.gateway project = var.project_id network = local.network.name name = "${var.name}-${each.key}" @@ -244,7 +238,7 @@ resource "google_compute_route" "gateway" { } resource "google_compute_route" "ilb" { - for_each = local.routes_ilb + for_each = local.routes.ilb project = var.project_id network = local.network.name name = "${var.name}-${each.key}" @@ -256,7 +250,7 @@ resource "google_compute_route" "ilb" { } resource "google_compute_route" "instance" { - for_each = local.routes_instance + for_each = local.routes.instance project = var.project_id network = local.network.name name = "${var.name}-${each.key}" @@ -270,7 +264,7 @@ resource "google_compute_route" "instance" { } resource "google_compute_route" "ip" { - for_each = local.routes_ip + for_each = local.routes.ip project = var.project_id network = local.network.name name = "${var.name}-${each.key}" @@ -282,7 +276,7 @@ resource "google_compute_route" "ip" { } resource "google_compute_route" "vpn_tunnel" { - for_each = local.routes_vpn_tunnel + for_each = local.routes.vpn_tunnel project = var.project_id network = local.network.name name = "${var.name}-${each.key}" From 546385d3ee2073d5827c5e4e7d31e3fd8da6d0dc Mon Sep 17 00:00:00 2001 From: lcaggio Date: Thu, 9 Dec 2021 09:55:47 +0100 Subject: [PATCH 17/24] Add support for partitioned tables on Organization sinks (#380) * Add support for partioned tables on Organization sinks * Update changelog * Fix lint * Fix lint * Use simple bool instead of block * fix README * Fix Readme * Rename variable Co-authored-by: Ludovico Magnocavallo --- CHANGELOG.md | 1 + modules/organization/README.md | 54 ++-- modules/organization/main.tf | 7 + modules/organization/variables.tf | 11 +- .../modules/organization/fixture/variables.tf | 13 +- .../modules/organization/test_plan_logging.py | 256 +++++++++--------- 6 files changed, 181 insertions(+), 161 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0e0181f..48eeae80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- **incompatible change** Add support for partitioned tables on Organization sinks - new `cloud-run` module - added gVNIC support to `compute-vm` module - added a rule factory to `net-vpc-firewall` module diff --git a/modules/organization/README.md b/modules/organization/README.md index 5e60be8c..5b9108c3 100644 --- a/modules/organization/README.md +++ b/modules/organization/README.md @@ -110,36 +110,40 @@ module "org" { logging_sinks = { warnings = { - type = "gcs" - destination = module.gcs.name - filter = "severity=WARNING" - iam = false - include_children = true - exclusions = {} + type = "gcs" + destination = module.gcs.name + filter = "severity=WARNING" + iam = false + include_children = true + bq_partitioned_table = null + exclusions = {} } info = { - type = "bigquery" - destination = module.dataset.id - filter = "severity=INFO" - iam = false - include_children = true - exclusions = {} + type = "bigquery" + destination = module.dataset.id + filter = "severity=INFO" + iam = false + include_children = true + bq_partitioned_table = true + exclusions = {} } notice = { - type = "pubsub" - destination = module.pubsub.id - filter = "severity=NOTICE" - iam = true - include_children = true - exclusions = {} + type = "pubsub" + destination = module.pubsub.id + filter = "severity=NOTICE" + iam = true + include_children = true + bq_partitioned_table = null + exclusions = {} } debug = { - type = "logging" - destination = module.bucket.id - filter = "severity=DEBUG" - iam = true - include_children = false - exclusions = { + type = "logging" + destination = module.bucket.id + filter = "severity=DEBUG" + iam = true + include_children = false + bq_partitioned_table = null + exclusions = { no-compute = "logName:compute" } } @@ -186,7 +190,7 @@ module "org" { | *iam_audit_config_authoritative* | IAM Authoritative service audit logging configuration. Service as key, map of log permission (eg DATA_READ) and excluded members as value for each service. Audit config should also be authoritative when using authoritative bindings. Use with caution. | map(map(list(string))) | | null | | *iam_bindings_authoritative* | IAM authoritative bindings, in {ROLE => [MEMBERS]} format. Roles and members not explicitly listed will be cleared. Bindings should also be authoritative when using authoritative audit config. Use with caution. | map(list(string)) | | null | | *logging_exclusions* | Logging exclusions for this organization in the form {NAME -> FILTER}. | map(string) | | {} | -| *logging_sinks* | Logging sinks to create for this organization. | map(object({...})) | | {} | +| *logging_sinks* | Logging sinks to create for this organization. | map(object({...})) | | {} | | *policy_boolean* | Map of boolean org policies and enforcement value, set value to null for policy restore. | map(bool) | | {} | | *policy_list* | Map of list org policies, status is true for allow, false for deny, null for restore. Values can only be used for allow or deny. | map(object({...})) | | {} | diff --git a/modules/organization/main.tf b/modules/organization/main.tf index e845c549..5f6737d0 100644 --- a/modules/organization/main.tf +++ b/modules/organization/main.tf @@ -289,6 +289,13 @@ resource "google_logging_organization_sink" "sink" { filter = each.value.filter include_children = each.value.include_children + dynamic "bigquery_options" { + for_each = each.value.bq_partitioned_table == true ? [""] : [] + content { + use_partitioned_tables = each.value.bq_partitioned_table + } + } + dynamic "exclusions" { for_each = each.value.exclusions iterator = exclusion diff --git a/modules/organization/variables.tf b/modules/organization/variables.tf index a98492d3..e01c1e46 100644 --- a/modules/organization/variables.tf +++ b/modules/organization/variables.tf @@ -111,11 +111,12 @@ variable "logging_exclusions" { variable "logging_sinks" { description = "Logging sinks to create for this organization." type = map(object({ - destination = string - type = string - filter = string - iam = bool - include_children = bool + destination = string + type = string + filter = string + iam = bool + include_children = bool + bq_partitioned_table = bool # TODO exclusions also support description and disabled exclusions = map(string) })) diff --git a/tests/modules/organization/fixture/variables.tf b/tests/modules/organization/fixture/variables.tf index eb4e807c..b9463346 100644 --- a/tests/modules/organization/fixture/variables.tf +++ b/tests/modules/organization/fixture/variables.tf @@ -81,12 +81,13 @@ variable "firewall_policy_attachments" { variable "logging_sinks" { type = map(object({ - destination = string - type = string - filter = string - iam = bool - include_children = bool - exclusions = map(string) + destination = string + type = string + filter = string + iam = bool + include_children = bool + bq_partitioned_table = bool + exclusions = map(string) })) default = {} } diff --git a/tests/modules/organization/test_plan_logging.py b/tests/modules/organization/test_plan_logging.py index 1ad6aec4..98163afc 100644 --- a/tests/modules/organization/test_plan_logging.py +++ b/tests/modules/organization/test_plan_logging.py @@ -22,148 +22,154 @@ FIXTURES_DIR = os.path.join(os.path.dirname(__file__), "fixture") def test_sinks(plan_runner): - "Test folder-level sinks." - logging_sinks = """ { + "Test folder-level sinks." + logging_sinks = """ { warning = { - type = "gcs" - destination = "mybucket" - filter = "severity=WARNING" - iam = true - include_children = true - exclusions = {} + type = "gcs" + destination = "mybucket" + filter = "severity=WARNING" + iam = true + include_children = true + bq_partitioned_table = null + exclusions = {} } info = { - type = "bigquery" - destination = "projects/myproject/datasets/mydataset" - filter = "severity=INFO" - iam = true - include_children = true - exclusions = {} + type = "bigquery" + destination = "projects/myproject/datasets/mydataset" + filter = "severity=INFO" + iam = true + include_children = true + bq_partitioned_table = false + exclusions = {} } notice = { - type = "pubsub" - destination = "projects/myproject/topics/mytopic" - filter = "severity=NOTICE" - iam = true - include_children = false - exclusions = {} + type = "pubsub" + destination = "projects/myproject/topics/mytopic" + filter = "severity=NOTICE" + iam = true + include_children = false + bq_partitioned_table = null + exclusions = {} } debug = { - type = "logging" - destination = "projects/myproject/locations/global/buckets/mybucket" - filter = "severity=DEBUG" - iam = true - include_children = false - exclusions = { + type = "logging" + destination = "projects/myproject/locations/global/buckets/mybucket" + filter = "severity=DEBUG" + iam = true + include_children = false + bq_partitioned_table = null + exclusions = { no-compute = "logName:compute" no-container = "logName:container" } } } """ - _, resources = plan_runner(FIXTURES_DIR, logging_sinks=logging_sinks) - assert len(resources) == 8 + _, resources = plan_runner(FIXTURES_DIR, logging_sinks=logging_sinks) + assert len(resources) == 8 - resource_types = Counter([r["type"] for r in resources]) - assert resource_types == { - "google_logging_organization_sink": 4, - "google_bigquery_dataset_iam_member": 1, - "google_project_iam_member": 1, - "google_pubsub_topic_iam_member": 1, - "google_storage_bucket_iam_member": 1, - } + resource_types = Counter([r["type"] for r in resources]) + assert resource_types == { + "google_logging_organization_sink": 4, + "google_bigquery_dataset_iam_member": 1, + "google_project_iam_member": 1, + "google_pubsub_topic_iam_member": 1, + "google_storage_bucket_iam_member": 1, + } - sinks = [r for r in resources if r["type"] == "google_logging_organization_sink"] - assert sorted([r["index"] for r in sinks]) == [ - "debug", - "info", - "notice", - "warning", - ] - values = [ - ( - r["index"], - r["values"]["filter"], - r["values"]["destination"], - r["values"]["include_children"], - ) - for r in sinks - ] - assert sorted(values) == [ - ( - "debug", - "severity=DEBUG", - "logging.googleapis.com/projects/myproject/locations/global/buckets/mybucket", - False, - ), - ( - "info", - "severity=INFO", - "bigquery.googleapis.com/projects/myproject/datasets/mydataset", - True, - ), - ( - "notice", - "severity=NOTICE", - "pubsub.googleapis.com/projects/myproject/topics/mytopic", - False, - ), - ("warning", "severity=WARNING", "storage.googleapis.com/mybucket", True), - ] + sinks = [r for r in resources if r["type"] + == "google_logging_organization_sink"] + assert sorted([r["index"] for r in sinks]) == [ + "debug", + "info", + "notice", + "warning", + ] + values = [ + ( + r["index"], + r["values"]["filter"], + r["values"]["destination"], + r["values"]["include_children"], + ) + for r in sinks + ] + assert sorted(values) == [ + ( + "debug", + "severity=DEBUG", + "logging.googleapis.com/projects/myproject/locations/global/buckets/mybucket", + False, + ), + ( + "info", + "severity=INFO", + "bigquery.googleapis.com/projects/myproject/datasets/mydataset", + True, + ), + ( + "notice", + "severity=NOTICE", + "pubsub.googleapis.com/projects/myproject/topics/mytopic", + False, + ), + ("warning", "severity=WARNING", "storage.googleapis.com/mybucket", True), + ] - bindings = [r for r in resources if "member" in r["type"]] - values = [(r["index"], r["type"], r["values"]["role"]) for r in bindings] - assert sorted(values) == [ - ("debug", "google_project_iam_member", "roles/logging.bucketWriter"), - ("info", "google_bigquery_dataset_iam_member", "roles/bigquery.dataEditor"), - ("notice", "google_pubsub_topic_iam_member", "roles/pubsub.publisher"), - ("warning", "google_storage_bucket_iam_member", "roles/storage.objectCreator"), - ] + bindings = [r for r in resources if "member" in r["type"]] + values = [(r["index"], r["type"], r["values"]["role"]) for r in bindings] + assert sorted(values) == [ + ("debug", "google_project_iam_member", "roles/logging.bucketWriter"), + ("info", "google_bigquery_dataset_iam_member", "roles/bigquery.dataEditor"), + ("notice", "google_pubsub_topic_iam_member", "roles/pubsub.publisher"), + ("warning", "google_storage_bucket_iam_member", "roles/storage.objectCreator"), + ] - exclusions = [(r["index"], r["values"]["exclusions"]) for r in sinks] - assert sorted(exclusions) == [ - ( - "debug", - [ - { - "description": None, - "disabled": False, - "filter": "logName:compute", - "name": "no-compute", - }, - { - "description": None, - "disabled": False, - "filter": "logName:container", - "name": "no-container", - }, - ], - ), - ("info", []), - ("notice", []), - ("warning", []), - ] + exclusions = [(r["index"], r["values"]["exclusions"]) for r in sinks] + assert sorted(exclusions) == [ + ( + "debug", + [ + { + "description": None, + "disabled": False, + "filter": "logName:compute", + "name": "no-compute", + }, + { + "description": None, + "disabled": False, + "filter": "logName:container", + "name": "no-container", + }, + ], + ), + ("info", []), + ("notice", []), + ("warning", []), + ] def test_exclusions(plan_runner): - "Test folder-level logging exclusions." - logging_exclusions = ( - "{" - 'exclusion1 = "resource.type=gce_instance", ' - 'exclusion2 = "severity=NOTICE", ' - "}" - ) - _, resources = plan_runner(FIXTURES_DIR, logging_exclusions=logging_exclusions) - assert len(resources) == 2 - exclusions = [ - r for r in resources if r["type"] == "google_logging_organization_exclusion" - ] - assert sorted([r["index"] for r in exclusions]) == [ - "exclusion1", - "exclusion2", - ] - values = [(r["index"], r["values"]["filter"]) for r in exclusions] - assert sorted(values) == [ - ("exclusion1", "resource.type=gce_instance"), - ("exclusion2", "severity=NOTICE"), - ] + "Test folder-level logging exclusions." + logging_exclusions = ( + "{" + 'exclusion1 = "resource.type=gce_instance", ' + 'exclusion2 = "severity=NOTICE", ' + "}" + ) + _, resources = plan_runner( + FIXTURES_DIR, logging_exclusions=logging_exclusions) + assert len(resources) == 2 + exclusions = [ + r for r in resources if r["type"] == "google_logging_organization_exclusion" + ] + assert sorted([r["index"] for r in exclusions]) == [ + "exclusion1", + "exclusion2", + ] + values = [(r["index"], r["values"]["filter"]) for r in exclusions] + assert sorted(values) == [ + ("exclusion1", "resource.type=gce_instance"), + ("exclusion2", "severity=NOTICE"), + ] From 3758c8f3b0739292a3bd74954e286f03ea31e657 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Thu, 9 Dec 2021 17:26:37 +0100 Subject: [PATCH 18/24] Improve PSN support in net-vpc module (#384) * improve PSN support * fix variable order * fix example test * fix cloudsql example --- modules/cloudsql-instance/README.md | 2 +- modules/net-vpc/README.md | 4 +- modules/net-vpc/main.tf | 43 +++++++++++++--------- modules/net-vpc/variables.tf | 26 ++++++------- tests/modules/net_vpc/fixture/main.tf | 34 ++++++++--------- tests/modules/net_vpc/fixture/variables.tf | 5 +++ tests/modules/net_vpc/test_plan.py | 17 --------- tests/modules/net_vpc/test_plan_psn.py | 42 +++++++++++++++++++++ 8 files changed, 106 insertions(+), 67 deletions(-) create mode 100644 tests/modules/net_vpc/test_plan_psn.py diff --git a/modules/cloudsql-instance/README.md b/modules/cloudsql-instance/README.md index 1c2e4364..34290eeb 100644 --- a/modules/cloudsql-instance/README.md +++ b/modules/cloudsql-instance/README.md @@ -25,7 +25,7 @@ module "vpc" { source = "./modules/net-vpc" project_id = module.project.project_id name = "my-network" - private_service_networking_range = "10.60.0.0/16" + psn_ranges = ["10.60.0.0/16"] } module "db" { diff --git a/modules/net-vpc/README.md b/modules/net-vpc/README.md index 652edc74..41fc347d 100644 --- a/modules/net-vpc/README.md +++ b/modules/net-vpc/README.md @@ -138,7 +138,7 @@ module "vpc" { secondary_ip_range = null } ] - private_service_networking_range = "10.10.0.0/16" + psn_ranges = ["10.10.0.0/16"] } # tftest:modules=1:resources=4 ``` @@ -220,7 +220,7 @@ flow_logs: # enable, set to empty map to use defaults | *mtu* | Maximum Transmission Unit in bytes. The minimum value for this field is 1460 and the maximum value is 1500 bytes. | | | null | | *peering_config* | VPC peering configuration. | object({...}) | | null | | *peering_create_remote_end* | Skip creation of peering on the remote end when using peering_config | bool | | true | -| *private_service_networking_range* | RFC1919 CIDR range used for Google services that support private service networking. | string | | ... | +| *psn_ranges* | CIDR ranges used for Google services that support Private Service Networking. | list(string) | | ... | | *routes* | Network routes, keyed by name. | map(object({...})) | | {} | | *routing_mode* | The network routing mode (default 'GLOBAL') | string | | ... | | *shared_vpc_host* | Enable shared VPC for this project. | bool | | false | diff --git a/modules/net-vpc/main.tf b/modules/net-vpc/main.tf index a48f2d0a..ad663d31 100644 --- a/modules/net-vpc/main.tf +++ b/modules/net-vpc/main.tf @@ -78,6 +78,13 @@ locals { ? null : element(reverse(split("/", var.peering_config.peer_vpc_self_link)), 0) ) + psn_ranges = { + for r in(var.psn_ranges == null ? [] : var.psn_ranges) : r => { + address = split("/", r)[0] + name = replace(split("/", r)[0], ".", "-") + prefix_length = split("/", r)[1] + } + } routes = { gateway = { for k, v in local._routes : k => v if v.next_hop_type == "gateway" } ilb = { for k, v in local._routes : k => v if v.next_hop_type == "ilb" } @@ -287,17 +294,6 @@ resource "google_compute_route" "vpn_tunnel" { next_hop_vpn_tunnel = each.value.next_hop } -resource "google_compute_global_address" "psn_range" { - count = var.private_service_networking_range == null ? 0 : 1 - project = var.project_id - name = "${var.name}-google-psn" - purpose = "VPC_PEERING" - address_type = "INTERNAL" - address = split("/", var.private_service_networking_range)[0] - prefix_length = split("/", var.private_service_networking_range)[1] - network = local.network.id -} - resource "google_dns_policy" "default" { count = var.dns_policy == null ? 0 : 1 enable_inbound_forwarding = var.dns_policy.inbound @@ -309,7 +305,7 @@ resource "google_dns_policy" "default" { } dynamic "alternative_name_server_config" { - for_each = var.dns_policy.outbound == null ? [] : [1] + for_each = toset(var.dns_policy.outbound == null ? [] : [""]) content { dynamic "target_name_servers" { for_each = toset(var.dns_policy.outbound.private_ns) @@ -330,9 +326,22 @@ resource "google_dns_policy" "default" { } } -resource "google_service_networking_connection" "psn_connection" { - count = var.private_service_networking_range == null ? 0 : 1 - network = local.network.id - service = "servicenetworking.googleapis.com" - reserved_peering_ranges = [google_compute_global_address.psn_range.0.name] +resource "google_compute_global_address" "psn_ranges" { + for_each = local.psn_ranges + project = var.project_id + name = "${var.name}-psn-${each.value.name}" + purpose = "VPC_PEERING" + address_type = "INTERNAL" + address = each.value.address + prefix_length = each.value.prefix_length + network = local.network.id +} + +resource "google_service_networking_connection" "psn_connection" { + for_each = toset(local.psn_ranges == {} ? [] : [""]) + network = local.network.id + service = "servicenetworking.googleapis.com" + reserved_peering_ranges = [ + for k, v in google_compute_global_address.psn_ranges : v.name + ] } diff --git a/modules/net-vpc/variables.tf b/modules/net-vpc/variables.tf index 04b0fdb1..5be9c923 100644 --- a/modules/net-vpc/variables.tf +++ b/modules/net-vpc/variables.tf @@ -103,24 +103,24 @@ variable "peering_create_remote_end" { default = true } -variable "private_service_networking_range" { - description = "RFC1919 CIDR range used for Google services that support private service networking." - type = string - default = null - validation { - condition = ( - var.private_service_networking_range == null || - can(cidrnetmask(var.private_service_networking_range)) - ) - error_message = "Specify a valid RFC1918 CIDR range for private service networking." - } -} - variable "project_id" { description = "The ID of the project where this VPC will be created" type = string } +variable "psn_ranges" { + description = "CIDR ranges used for Google services that support Private Service Networking." + type = list(string) + default = null + validation { + condition = alltrue([ + for r in(var.psn_ranges == null ? [] : var.psn_ranges) : + can(cidrnetmask(r)) + ]) + error_message = "Specify a valid RFC1918 CIDR range for Private Service Networking." + } +} + variable "routes" { description = "Network routes, keyed by name." type = map(object({ diff --git a/tests/modules/net_vpc/fixture/main.tf b/tests/modules/net_vpc/fixture/main.tf index 5812bba6..f959c3bb 100644 --- a/tests/modules/net_vpc/fixture/main.tf +++ b/tests/modules/net_vpc/fixture/main.tf @@ -15,21 +15,21 @@ */ module "test" { - source = "../../../../modules/net-vpc" - project_id = var.project_id - name = var.name - iam = var.iam - log_configs = var.log_configs - log_config_defaults = var.log_config_defaults - peering_config = var.peering_config - routes = var.routes - shared_vpc_host = var.shared_vpc_host - shared_vpc_service_projects = var.shared_vpc_service_projects - subnets = var.subnets - subnet_descriptions = var.subnet_descriptions - subnet_flow_logs = var.subnet_flow_logs - subnet_private_access = var.subnet_private_access - auto_create_subnetworks = var.auto_create_subnetworks - private_service_networking_range = var.private_service_networking_range - data_folder = var.data_folder + source = "../../../../modules/net-vpc" + project_id = var.project_id + name = var.name + iam = var.iam + log_configs = var.log_configs + log_config_defaults = var.log_config_defaults + peering_config = var.peering_config + routes = var.routes + shared_vpc_host = var.shared_vpc_host + shared_vpc_service_projects = var.shared_vpc_service_projects + subnets = var.subnets + subnet_descriptions = var.subnet_descriptions + subnet_flow_logs = var.subnet_flow_logs + subnet_private_access = var.subnet_private_access + auto_create_subnetworks = var.auto_create_subnetworks + psn_ranges = var.psn_ranges + data_folder = var.data_folder } diff --git a/tests/modules/net_vpc/fixture/variables.tf b/tests/modules/net_vpc/fixture/variables.tf index 80628473..d23497e2 100644 --- a/tests/modules/net_vpc/fixture/variables.tf +++ b/tests/modules/net_vpc/fixture/variables.tf @@ -61,6 +61,11 @@ variable "peering_config" { default = null } +variable "psn_ranges" { + type = list(string) + default = null +} + variable "routes" { type = map(object({ dest_range = string diff --git a/tests/modules/net_vpc/test_plan.py b/tests/modules/net_vpc/test_plan.py index 28c3657f..e04d62a5 100644 --- a/tests/modules/net_vpc/test_plan.py +++ b/tests/modules/net_vpc/test_plan.py @@ -88,20 +88,3 @@ def test_vpc_routes(plan_runner): resource = [r for r in resources if r['values'] ['name'] == 'my-vpc-next-hop-test'][0] assert resource['values']['next_hop_%s' % next_hop_type] - - -def test_vpc_psn(plan_runner): - _, resources = plan_runner( - FIXTURES_DIR, private_service_networking_range="10.10.0.0/16" - ) - assert len(resources) == 3 - - address = [r["values"] for r in resources if r["type"] == "google_compute_global_address"][0] - assert address["address"] == "10.10.0.0" - assert address["address_type"] == "INTERNAL" - assert address["prefix_length"] == 16 - assert address["purpose"] == "VPC_PEERING" - - connection = [r["values"] for r in resources if r["type"] == "google_service_networking_connection"][0] - assert connection["service"] == "servicenetworking.googleapis.com" - assert connection["reserved_peering_ranges"] == ["my-vpc-google-psn"] diff --git a/tests/modules/net_vpc/test_plan_psn.py b/tests/modules/net_vpc/test_plan_psn.py new file mode 100644 index 00000000..f80cd1d9 --- /dev/null +++ b/tests/modules/net_vpc/test_plan_psn.py @@ -0,0 +1,42 @@ +# Copyright 2021 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. + + +import os +import pytest +import tftest + + +FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture') + + +def test_single_range(plan_runner): + "Test single PSN range." + _, resources = plan_runner(FIXTURES_DIR, psn_ranges='["172.16.100.0/24"]') + assert len(resources) == 3 + + +def test_multi_range(plan_runner): + "Test multiple PSN ranges." + _, resources = plan_runner(FIXTURES_DIR, + psn_ranges='["172.16.100.0/24", "172.16.101.0/24"]') + assert len(resources) == 4 + + +def test_validation(plan_runner): + "Test PSN variable validation." + try: + plan_runner(FIXTURES_DIR, psn_ranges='["foobar"]') + except tftest.TerraformTestError as e: + assert 'Invalid value for variable' in e.args[0] From 9b7850485d71f4ac103de06074544d759894b304 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Thu, 9 Dec 2021 17:33:33 +0100 Subject: [PATCH 19/24] Update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48eeae80..b67c748d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,12 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -- **incompatible change** Add support for partitioned tables on Organization sinks +- **incompatible change** add support for partitioned tables on Organization sinks - new `cloud-run` module - added gVNIC support to `compute-vm` module - added a rule factory to `net-vpc-firewall` module - added a subnet factory to `net-vpc` module +- **incompatible change** renamed `private_service_networking_range` variable to `psc_ranges` in `net-vpc`module, and changed its type to `list(string)` ## [8.0.0] - 2021-10-21 From 402385b955e4b245009a25d5cc2f81ef8ceb2ff4 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Thu, 9 Dec 2021 17:33:54 +0100 Subject: [PATCH 20/24] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b67c748d..59b7f6eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,11 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -- **incompatible change** add support for partitioned tables on Organization sinks - new `cloud-run` module - added gVNIC support to `compute-vm` module - added a rule factory to `net-vpc-firewall` module - added a subnet factory to `net-vpc` module +- **incompatible change** add support for partitioned tables on organization sinks - **incompatible change** renamed `private_service_networking_range` variable to `psc_ranges` in `net-vpc`module, and changed its type to `list(string)` ## [8.0.0] - 2021-10-21 From 026d6348125a038f120e8a69b0031bfb65ff6cef Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Thu, 9 Dec 2021 17:34:39 +0100 Subject: [PATCH 21/24] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59b7f6eb..567ccb00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. - added gVNIC support to `compute-vm` module - added a rule factory to `net-vpc-firewall` module - added a subnet factory to `net-vpc` module -- **incompatible change** add support for partitioned tables on organization sinks +- **incompatible change** added support for partitioned tables to `organization` module sinks - **incompatible change** renamed `private_service_networking_range` variable to `psc_ranges` in `net-vpc`module, and changed its type to `list(string)` ## [8.0.0] - 2021-10-21 From 174de3a087ed8d2fed06c31db40e10cc535269b6 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Mon, 13 Dec 2021 08:41:02 +0100 Subject: [PATCH 22/24] Organization module refactor, in-module firewall policy factory for organization and folder (#385) * move iam and logging to separate files, minimal refactoring * update README * fix example * factory * tfdoc * boilerplate * remove data_folder variable * tfdoc * fix default factory name * add firewall policy to folder module * add factory example --- .../firewall-hierarchical-policies/main.tf | 3 +- modules/folder/README.md | 5 +- modules/folder/firewall-policy.tf | 97 +++++++ modules/folder/main.tf | 55 ---- modules/folder/variables.tf | 22 +- modules/net-vpc-firewall/main.tf | 3 +- modules/organization/README.md | 74 +++++- modules/organization/firewall-policy.tf | 104 ++++++++ modules/organization/iam.tf | 117 +++++++++ modules/organization/logging.tf | 95 +++++++ modules/organization/main.tf | 244 ------------------ modules/organization/variables.tf | 36 ++- .../fixture/data/firewall-cidrs.yaml | 18 ++ .../fixture/data/firewall-rules.yaml | 37 +++ tests/modules/organization/fixture/main.tf | 1 + .../modules/organization/fixture/variables.tf | 9 + tests/modules/organization/test_plan.py | 75 ------ .../organization/test_plan_firewall.py | 137 ++++++++++ .../modules/organization/test_plan_logging.py | 2 +- 19 files changed, 734 insertions(+), 400 deletions(-) create mode 100644 modules/folder/firewall-policy.tf create mode 100644 modules/organization/firewall-policy.tf create mode 100644 modules/organization/iam.tf create mode 100644 modules/organization/logging.tf create mode 100644 tests/modules/organization/fixture/data/firewall-cidrs.yaml create mode 100644 tests/modules/organization/fixture/data/firewall-rules.yaml create mode 100644 tests/modules/organization/test_plan_firewall.py diff --git a/factories/firewall-hierarchical-policies/main.tf b/factories/firewall-hierarchical-policies/main.tf index 3b049236..b63c9f5a 100644 --- a/factories/firewall-hierarchical-policies/main.tf +++ b/factories/firewall-hierarchical-policies/main.tf @@ -15,7 +15,8 @@ */ locals { - cidrs = try({ for name, cidrs in yamldecode(file("${var.templates_folder}/cidrs.yaml")) : + cidrs = try({ + for name, cidrs in yamldecode(file("${var.templates_folder}/cidrs.yaml")) : name => cidrs }, {}) diff --git a/modules/folder/README.md b/modules/folder/README.md index 1f572a79..612d092b 100644 --- a/modules/folder/README.md +++ b/modules/folder/README.md @@ -170,8 +170,9 @@ module "folder2" { | name | description | type | required | default | |---|---|:---: |:---:|:---:| | *contacts* | List of essential contacts for this resource. Must be in the form EMAIL -> [NOTIFICATION_TYPES]. Valid notification types are ALL, SUSPENSION, SECURITY, TECHNICAL, BILLING, LEGAL, PRODUCT_UPDATES | map(list(string)) | | {} | -| *firewall_policies* | Hierarchical firewall policies to *create* in this folder. | map(map(object({...}))) | | {} | -| *firewall_policy_attachments* | List of hierarchical firewall policy IDs to *attach* to this folder. | map(string) | | {} | +| *firewall_policies* | Hierarchical firewall policies created in this folder. | map(map(object({...}))) | | {} | +| *firewall_policy_attachments* | List of hierarchical firewall policy IDs to attached to this folder. | map(string) | | {} | +| *firewall_policy_factory* | Configuration for the firewall policy factory. | object({...}) | | null | | *folder_create* | Create folder. When set to false, uses id to reference an existing folder. | bool | | true | | *group_iam* | Authoritative IAM binding for organization groups, in {GROUP_EMAIL => [ROLES]} format. Group emails need to be static. Can be used in combination with the `iam` variable. | map(list(string)) | | {} | | *iam* | IAM bindings in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | diff --git a/modules/folder/firewall-policy.tf b/modules/folder/firewall-policy.tf new file mode 100644 index 00000000..383fdffe --- /dev/null +++ b/modules/folder/firewall-policy.tf @@ -0,0 +1,97 @@ +/** + * Copyright 2021 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. + */ + +locals { + _factory_cidrs = try( + yamldecode(file(var.firewall_policy_factory.cidr_file)), {} + ) + _factory_name = ( + try(var.firewall_policy_factory.policy_name, null) == null + ? "factory" + : var.firewall_policy_factory.policy_name + ) + _factory_rules = try( + yamldecode(file(var.firewall_policy_factory.rules_file)), {} + ) + _factory_rules_parsed = { + for name, rule in local._factory_rules : name => merge(rule, { + ranges = flatten([ + for r in(rule.ranges == null ? [] : rule.ranges) : + lookup(local._factory_cidrs, trimprefix(r, "$"), r) + ]) + }) + } + _merged_rules = flatten([ + for policy, rules in local.firewall_policies : [ + for name, rule in rules : merge(rule, { + policy = policy + name = name + }) + ] + ]) + firewall_policies = merge(var.firewall_policies, ( + length(local._factory_rules) == 0 + ? {} + : { (local._factory_name) = local._factory_rules_parsed } + )) + firewall_rules = { + for r in local._merged_rules : "${r.policy}-${r.name}" => r + } +} + +resource "google_compute_organization_security_policy" "policy" { + provider = google-beta + for_each = local.firewall_policies + display_name = each.key + parent = local.folder.id +} + +resource "google_compute_organization_security_policy_rule" "rule" { + provider = google-beta + for_each = local.firewall_rules + policy_id = google_compute_organization_security_policy.policy[each.value.policy].id + action = each.value.action + direction = each.value.direction + priority = try(each.value.priority, null) + target_resources = try(each.value.target_resources, null) + target_service_accounts = try(each.value.target_service_accounts, null) + enable_logging = try(each.value.logging, null) + # preview = each.value.preview + match { + description = each.value.description + config { + src_ip_ranges = each.value.direction == "INGRESS" ? each.value.ranges : null + dest_ip_ranges = each.value.direction == "EGRESS" ? each.value.ranges : null + dynamic "layer4_config" { + for_each = each.value.ports + iterator = port + content { + ip_protocol = port.key + ports = port.value + } + } + } + } +} + +resource "google_compute_organization_security_policy_association" "attachment" { + provider = google-beta + for_each = var.firewall_policy_attachments + name = "${local.folder.id}-${each.key}" + attachment_id = local.folder.id + policy_id = each.value +} + diff --git a/modules/folder/main.tf b/modules/folder/main.tf index 3c23d861..917e2769 100644 --- a/modules/folder/main.tf +++ b/modules/folder/main.tf @@ -15,12 +15,6 @@ */ locals { - extended_rules = flatten([ - for policy, rules in var.firewall_policies : [ - for rule_name, rule in rules : - merge(rule, { policy = policy, name = rule_name }) - ] - ]) group_iam_roles = distinct(flatten(values(var.group_iam))) group_iam = { for r in local.group_iam_roles : r => [ @@ -34,10 +28,6 @@ locals { try(local.group_iam[role], []) ) } - rules_map = { - for rule in local.extended_rules : - "${rule.policy}-${rule.name}" => rule - } logging_sinks = coalesce(var.logging_sinks, {}) sink_type_destination = { gcs = "storage.googleapis.com" @@ -151,51 +141,6 @@ resource "google_folder_organization_policy" "list" { } } -resource "google_compute_organization_security_policy" "policy" { - provider = google-beta - for_each = var.firewall_policies - - display_name = each.key - parent = local.folder.id -} - -resource "google_compute_organization_security_policy_rule" "rule" { - provider = google-beta - for_each = local.rules_map - - policy_id = google_compute_organization_security_policy.policy[each.value.policy].id - action = each.value.action - direction = each.value.direction - priority = each.value.priority - target_resources = each.value.target_resources - target_service_accounts = each.value.target_service_accounts - enable_logging = each.value.logging - # preview = each.value.preview - match { - description = each.value.description - config { - src_ip_ranges = each.value.direction == "INGRESS" ? each.value.ranges : null - dest_ip_ranges = each.value.direction == "EGRESS" ? each.value.ranges : null - dynamic "layer4_config" { - for_each = each.value.ports - iterator = port - content { - ip_protocol = port.key - ports = port.value - } - } - } - } -} - -resource "google_compute_organization_security_policy_association" "attachment" { - provider = google-beta - for_each = var.firewall_policy_attachments - name = "${local.folder.id}-${each.key}" - attachment_id = local.folder.id - policy_id = each.value -} - resource "google_logging_folder_sink" "sink" { for_each = local.logging_sinks name = each.key diff --git a/modules/folder/variables.tf b/modules/folder/variables.tf index 139bcde2..0abcc48e 100644 --- a/modules/folder/variables.tf +++ b/modules/folder/variables.tf @@ -21,27 +21,37 @@ variable "contacts" { } variable "firewall_policies" { - description = "Hierarchical firewall policies to *create* in this folder." + description = "Hierarchical firewall policies created in this folder." type = map(map(object({ + action = string description = string direction = string - action = string + logging = bool + ports = map(list(string)) priority = number ranges = list(string) - ports = map(list(string)) - target_service_accounts = list(string) target_resources = list(string) - logging = bool + target_service_accounts = list(string) }))) default = {} } variable "firewall_policy_attachments" { - description = "List of hierarchical firewall policy IDs to *attach* to this folder." + description = "List of hierarchical firewall policy IDs to attached to this folder." type = map(string) default = {} } +variable "firewall_policy_factory" { + description = "Configuration for the firewall policy factory." + type = object({ + cidr_file = string + policy_name = string + rules_file = string + }) + default = null +} + variable "folder_create" { description = "Create folder. When set to false, uses id to reference an existing folder." type = bool diff --git a/modules/net-vpc-firewall/main.tf b/modules/net-vpc-firewall/main.tf index 23dc0588..158e0991 100644 --- a/modules/net-vpc-firewall/main.tf +++ b/modules/net-vpc-firewall/main.tf @@ -28,7 +28,8 @@ locals { }) } - cidrs = try({ for name, cidrs in yamldecode(file("${var.cidr_template_file}")) : + cidrs = try({ + for name, cidrs in yamldecode(file(var.cidr_template_file)) : name => cidrs }, {}) diff --git a/modules/organization/README.md b/modules/organization/README.md index 5b9108c3..5f437b86 100644 --- a/modules/organization/README.md +++ b/modules/organization/README.md @@ -45,7 +45,16 @@ There are several mutually exclusive ways of managing IAM in this module Some care must be takend with the `groups_iam` variable (and in some situations with the additive variables) to ensure that variable keys are static values, so that Terraform is able to compute the dependency graph. -## Hierarchical firewall rules +## Hierarchical firewall policies + +Hirerarchical firewall policies can be managed in two ways: + +- via the `firewall_policies` variable, to directly define policies and rules in Terraform +- via the `firewall_policy_factory` variable, to leverage external YaML files via a simple "factory" embedded in the module ([see here](../../factories) for more context on factories) + +Once you have policies (either created via the module or externally), you can attach them using the `firewall_policy_attachments` variable. + +### Directly defined firewall policies ```hcl module "org" { @@ -75,6 +84,60 @@ module "org" { # tftest:modules=1:resources=3 ``` +### Firewall policy factory + +The in-built factory allows you to define a single policy, using one file for rules, and an optional file for CIDR range substitution variables. Remember that non-absolute paths are relative to the root module (the folder where you run `terraform`). + +```hcl +module "org" { + source = "./modules/organization" + organization_id = var.organization_id + firewall_policy_factory = { + cidr_file = "data/cidrs.yaml + policy_name = null + rules_file = "data/rules.yaml" + } +} +# tftest:skip +``` + +```yaml +# cidrs.yaml + +rfc1918: + - 10.0.0.0/8 + - 172.168.0.0/12 + - 192.168.0.0/16 +``` + +```yaml +# rules.yaml + +allow-admins: + description: Access from the admin subnet to all subnets + direction: INGRESS + action: allow + priority: 1000 + ranges: + - $rfc1918 + ports: + all: [] + target_resources: null + enable_logging: false + +allow-ssh-from-iap: + description: Enable SSH from IAP + direction: INGRESS + action: allow + priority: 1002 + ranges: + - 35.235.240.0/20 + ports: + tcp: ["22"] + target_resources: null + enable_logging: false +``` + ## Logging Sinks ```hcl @@ -110,7 +173,7 @@ module "org" { logging_sinks = { warnings = { - type = "gcs" + type = "storage" destination = module.gcs.name filter = "severity=WARNING" iam = false @@ -180,8 +243,9 @@ module "org" { | organization_id | Organization id in organizations/nnnnnn format. | string | ✓ | | | *contacts* | List of essential contacts for this resource. Must be in the form EMAIL -> [NOTIFICATION_TYPES]. Valid notification types are ALL, SUSPENSION, SECURITY, TECHNICAL, BILLING, LEGAL, PRODUCT_UPDATES | map(list(string)) | | {} | | *custom_roles* | Map of role name => list of permissions to create in this project. | map(list(string)) | | {} | -| *firewall_policies* | Hierarchical firewall policies to *create* in the organization. | map(map(object({...}))) | | {} | -| *firewall_policy_attachments* | List of hierarchical firewall policy IDs to *attach* to the organization | map(string) | | {} | +| *firewall_policies* | Hierarchical firewall policy rules created in the organization. | map(map(object({...}))) | | {} | +| *firewall_policy_attachments* | List of hierarchical firewall policy IDs attached to the organization. | map(string) | | {} | +| *firewall_policy_factory* | Configuration for the firewall policy factory. | object({...}) | | null | | *group_iam* | Authoritative IAM binding for organization groups, in {GROUP_EMAIL => [ROLES]} format. Group emails need to be static. Can be used in combination with the `iam` variable. | map(list(string)) | | {} | | *iam* | IAM bindings, in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | | *iam_additive* | Non authoritative IAM bindings, in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | @@ -190,7 +254,7 @@ module "org" { | *iam_audit_config_authoritative* | IAM Authoritative service audit logging configuration. Service as key, map of log permission (eg DATA_READ) and excluded members as value for each service. Audit config should also be authoritative when using authoritative bindings. Use with caution. | map(map(list(string))) | | null | | *iam_bindings_authoritative* | IAM authoritative bindings, in {ROLE => [MEMBERS]} format. Roles and members not explicitly listed will be cleared. Bindings should also be authoritative when using authoritative audit config. Use with caution. | map(list(string)) | | null | | *logging_exclusions* | Logging exclusions for this organization in the form {NAME -> FILTER}. | map(string) | | {} | -| *logging_sinks* | Logging sinks to create for this organization. | map(object({...})) | | {} | +| *logging_sinks* | Logging sinks to create for this organization. | map(object({...})) | | {} | | *policy_boolean* | Map of boolean org policies and enforcement value, set value to null for policy restore. | map(bool) | | {} | | *policy_list* | Map of list org policies, status is true for allow, false for deny, null for restore. Values can only be used for allow or deny. | map(object({...})) | | {} | diff --git a/modules/organization/firewall-policy.tf b/modules/organization/firewall-policy.tf new file mode 100644 index 00000000..084da343 --- /dev/null +++ b/modules/organization/firewall-policy.tf @@ -0,0 +1,104 @@ +/** + * Copyright 2021 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. + */ + +locals { + _factory_cidrs = try( + yamldecode(file(var.firewall_policy_factory.cidr_file)), {} + ) + _factory_name = ( + try(var.firewall_policy_factory.policy_name, null) == null + ? "factory" + : var.firewall_policy_factory.policy_name + ) + _factory_rules = try( + yamldecode(file(var.firewall_policy_factory.rules_file)), {} + ) + _factory_rules_parsed = { + for name, rule in local._factory_rules : name => merge(rule, { + ranges = flatten([ + for r in(rule.ranges == null ? [] : rule.ranges) : + lookup(local._factory_cidrs, trimprefix(r, "$"), r) + ]) + }) + } + _merged_rules = flatten([ + for policy, rules in local.firewall_policies : [ + for name, rule in rules : merge(rule, { + policy = policy + name = name + }) + ] + ]) + firewall_policies = merge(var.firewall_policies, ( + length(local._factory_rules) == 0 + ? {} + : { (local._factory_name) = local._factory_rules_parsed } + )) + firewall_rules = { + for r in local._merged_rules : "${r.policy}-${r.name}" => r + } +} + +resource "google_compute_organization_security_policy" "policy" { + provider = google-beta + for_each = local.firewall_policies + display_name = each.key + parent = var.organization_id + depends_on = [ + google_organization_iam_audit_config.config, + google_organization_iam_binding.authoritative, + google_organization_iam_custom_role.roles, + google_organization_iam_member.additive, + google_organization_iam_policy.authoritative, + ] +} + +resource "google_compute_organization_security_policy_rule" "rule" { + provider = google-beta + for_each = local.firewall_rules + policy_id = google_compute_organization_security_policy.policy[each.value.policy].id + action = each.value.action + direction = each.value.direction + priority = try(each.value.priority, null) + target_resources = try(each.value.target_resources, null) + target_service_accounts = try(each.value.target_service_accounts, null) + enable_logging = try(each.value.logging, null) + # preview = each.value.preview + match { + description = each.value.description + config { + src_ip_ranges = each.value.direction == "INGRESS" ? each.value.ranges : null + dest_ip_ranges = each.value.direction == "EGRESS" ? each.value.ranges : null + dynamic "layer4_config" { + for_each = each.value.ports + iterator = port + content { + ip_protocol = port.key + ports = port.value + } + } + } + } +} + +resource "google_compute_organization_security_policy_association" "attachment" { + provider = google-beta + for_each = var.firewall_policy_attachments + name = "${var.organization_id}-${each.key}" + attachment_id = var.organization_id + policy_id = each.value +} + diff --git a/modules/organization/iam.tf b/modules/organization/iam.tf new file mode 100644 index 00000000..3e594935 --- /dev/null +++ b/modules/organization/iam.tf @@ -0,0 +1,117 @@ +/** + * Copyright 2021 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. + */ + +locals { + _group_iam_roles = distinct(flatten(values(var.group_iam))) + _group_iam = { + for r in local._group_iam_roles : r => [ + for k, v in var.group_iam : "group:${k}" if try(index(v, r), null) != null + ] + } + _iam_additive_pairs = flatten([ + for role, members in var.iam_additive : [ + for member in members : { role = role, member = member } + ] + ]) + _iam_additive_member_pairs = flatten([ + for member, roles in var.iam_additive_members : [ + for role in roles : { role = role, member = member } + ] + ]) + iam = { + for role in distinct(concat(keys(var.iam), keys(local._group_iam))) : + role => concat( + try(var.iam[role], []), + try(local._group_iam[role], []) + ) + } + iam_additive = { + for pair in concat(local._iam_additive_pairs, local._iam_additive_member_pairs) : + "${pair.role}-${pair.member}" => pair + } +} + +resource "google_organization_iam_custom_role" "roles" { + for_each = var.custom_roles + org_id = local.organization_id_numeric + role_id = each.key + title = "Custom role ${each.key}" + description = "Terraform-managed" + permissions = each.value +} + +resource "google_organization_iam_binding" "authoritative" { + for_each = local.iam + org_id = local.organization_id_numeric + role = each.key + members = each.value +} + +resource "google_organization_iam_member" "additive" { + for_each = ( + length(var.iam_additive) + length(var.iam_additive_members) > 0 + ? local.iam_additive + : {} + ) + org_id = local.organization_id_numeric + role = each.value.role + member = each.value.member +} + +resource "google_organization_iam_policy" "authoritative" { + count = var.iam_bindings_authoritative != null || var.iam_audit_config_authoritative != null ? 1 : 0 + org_id = local.organization_id_numeric + policy_data = data.google_iam_policy.authoritative.policy_data +} + +data "google_iam_policy" "authoritative" { + dynamic "binding" { + for_each = var.iam_bindings_authoritative != null ? var.iam_bindings_authoritative : {} + content { + role = binding.key + members = binding.value + } + } + + dynamic "audit_config" { + for_each = var.iam_audit_config_authoritative != null ? var.iam_audit_config_authoritative : {} + content { + service = audit_config.key + dynamic "audit_log_configs" { + for_each = audit_config.value + iterator = config + content { + log_type = config.key + exempted_members = config.value + } + } + } + } +} + +resource "google_organization_iam_audit_config" "config" { + for_each = var.iam_audit_config + org_id = local.organization_id_numeric + service = each.key + dynamic "audit_log_config" { + for_each = each.value + iterator = config + content { + log_type = config.key + exempted_members = config.value + } + } +} diff --git a/modules/organization/logging.tf b/modules/organization/logging.tf new file mode 100644 index 00000000..ad9d8534 --- /dev/null +++ b/modules/organization/logging.tf @@ -0,0 +1,95 @@ +/** + * Copyright 2021 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. + */ + +locals { + logging_sinks = coalesce(var.logging_sinks, {}) + sink_bindings = { + for type in ["bigquery", "logging", "pubsub", "storage"] : + type => { + for name, sink in local.logging_sinks : + name => sink if sink.iam && sink.type == type + } + } +} + +resource "google_logging_organization_sink" "sink" { + for_each = local.logging_sinks + name = each.key + org_id = local.organization_id_numeric + destination = "${each.value.type}.googleapis.com/${each.value.destination}" + filter = each.value.filter + include_children = each.value.include_children + + dynamic "bigquery_options" { + for_each = each.value.bq_partitioned_table == true ? [""] : [] + content { + use_partitioned_tables = each.value.bq_partitioned_table + } + } + + dynamic "exclusions" { + for_each = each.value.exclusions + iterator = exclusion + content { + name = exclusion.key + filter = exclusion.value + } + } + depends_on = [ + google_organization_iam_binding.authoritative, + google_organization_iam_member.additive, + google_organization_iam_policy.authoritative, + ] +} + +resource "google_storage_bucket_iam_member" "storage-sinks-binding" { + for_each = local.sink_bindings["storage"] + bucket = each.value.destination + role = "roles/storage.objectCreator" + member = google_logging_organization_sink.sink[each.key].writer_identity +} + +resource "google_bigquery_dataset_iam_member" "bq-sinks-binding" { + for_each = local.sink_bindings["bigquery"] + project = split("/", each.value.destination)[1] + dataset_id = split("/", each.value.destination)[3] + role = "roles/bigquery.dataEditor" + member = google_logging_organization_sink.sink[each.key].writer_identity +} + +resource "google_pubsub_topic_iam_member" "pubsub-sinks-binding" { + for_each = local.sink_bindings["pubsub"] + project = split("/", each.value.destination)[1] + topic = split("/", each.value.destination)[3] + role = "roles/pubsub.publisher" + member = google_logging_organization_sink.sink[each.key].writer_identity +} + +resource "google_project_iam_member" "bucket-sinks-binding" { + for_each = local.sink_bindings["logging"] + project = split("/", each.value.destination)[1] + role = "roles/logging.bucketWriter" + member = google_logging_organization_sink.sink[each.key].writer_identity + # TODO(jccb): use a condition to limit writer-identity only to this bucket +} + +resource "google_logging_organization_exclusion" "logging-exclusion" { + for_each = coalesce(var.logging_exclusions, {}) + name = each.key + org_id = local.organization_id_numeric + description = "${each.key} (Terraform-managed)" + filter = each.value +} diff --git a/modules/organization/main.tf b/modules/organization/main.tf index 5f6737d0..4f19c524 100644 --- a/modules/organization/main.tf +++ b/modules/organization/main.tf @@ -16,130 +16,6 @@ locals { organization_id_numeric = split("/", var.organization_id)[1] - group_iam_roles = distinct(flatten(values(var.group_iam))) - group_iam = { - for r in local.group_iam_roles : r => [ - for k, v in var.group_iam : "group:${k}" if try(index(v, r), null) != null - ] - } - iam = { - for role in distinct(concat(keys(var.iam), keys(local.group_iam))) : - role => concat( - try(var.iam[role], []), - try(local.group_iam[role], []) - ) - } - iam_additive_pairs = flatten([ - for role, members in var.iam_additive : [ - for member in members : { role = role, member = member } - ] - ]) - iam_additive_member_pairs = flatten([ - for member, roles in var.iam_additive_members : [ - for role in roles : { role = role, member = member } - ] - ]) - iam_additive = { - for pair in concat(local.iam_additive_pairs, local.iam_additive_member_pairs) : - "${pair.role}-${pair.member}" => pair - } - extended_rules = flatten([ - for policy, rules in var.firewall_policies : [ - for rule_name, rule in rules : - merge(rule, { policy = policy, name = rule_name }) - ] - ]) - rules_map = { - for rule in local.extended_rules : - "${rule.policy}-${rule.name}" => rule - } - logging_sinks = coalesce(var.logging_sinks, {}) - sink_type_destination = { - gcs = "storage.googleapis.com" - bigquery = "bigquery.googleapis.com" - pubsub = "pubsub.googleapis.com" - logging = "logging.googleapis.com" - } - sink_bindings = { - for type in ["gcs", "bigquery", "pubsub", "logging"] : - type => { - for name, sink in local.logging_sinks : - name => sink - if sink.iam && sink.type == type - } - } -} - -resource "google_organization_iam_custom_role" "roles" { - for_each = var.custom_roles - org_id = local.organization_id_numeric - role_id = each.key - title = "Custom role ${each.key}" - description = "Terraform-managed" - permissions = each.value -} - -resource "google_organization_iam_binding" "authoritative" { - for_each = local.iam - org_id = local.organization_id_numeric - role = each.key - members = each.value -} - -resource "google_organization_iam_member" "additive" { - for_each = ( - length(var.iam_additive) + length(var.iam_additive_members) > 0 - ? local.iam_additive - : {} - ) - org_id = local.organization_id_numeric - role = each.value.role - member = each.value.member -} - -resource "google_organization_iam_policy" "authoritative" { - count = var.iam_bindings_authoritative != null || var.iam_audit_config_authoritative != null ? 1 : 0 - org_id = local.organization_id_numeric - policy_data = data.google_iam_policy.authoritative.policy_data -} - -data "google_iam_policy" "authoritative" { - dynamic "binding" { - for_each = var.iam_bindings_authoritative != null ? var.iam_bindings_authoritative : {} - content { - role = binding.key - members = binding.value - } - } - - dynamic "audit_config" { - for_each = var.iam_audit_config_authoritative != null ? var.iam_audit_config_authoritative : {} - content { - service = audit_config.key - dynamic "audit_log_configs" { - for_each = audit_config.value - iterator = config - content { - log_type = config.key - exempted_members = config.value - } - } - } - } -} - -resource "google_organization_iam_audit_config" "config" { - for_each = var.iam_audit_config - org_id = local.organization_id_numeric - service = each.key - dynamic "audit_log_config" { - for_each = each.value - iterator = config - content { - log_type = config.key - exempted_members = config.value - } - } } resource "google_organization_policy" "boolean" { @@ -231,126 +107,6 @@ resource "google_organization_policy" "list" { ] } -resource "google_compute_organization_security_policy" "policy" { - provider = google-beta - for_each = var.firewall_policies - display_name = each.key - parent = var.organization_id - depends_on = [ - google_organization_iam_audit_config.config, - google_organization_iam_binding.authoritative, - google_organization_iam_custom_role.roles, - google_organization_iam_member.additive, - google_organization_iam_policy.authoritative, - ] -} - -resource "google_compute_organization_security_policy_rule" "rule" { - provider = google-beta - for_each = local.rules_map - policy_id = google_compute_organization_security_policy.policy[each.value.policy].id - action = each.value.action - direction = each.value.direction - priority = each.value.priority - target_resources = each.value.target_resources - target_service_accounts = each.value.target_service_accounts - enable_logging = each.value.logging - # preview = each.value.preview - match { - description = each.value.description - config { - src_ip_ranges = each.value.direction == "INGRESS" ? each.value.ranges : null - dest_ip_ranges = each.value.direction == "EGRESS" ? each.value.ranges : null - dynamic "layer4_config" { - for_each = each.value.ports - iterator = port - content { - ip_protocol = port.key - ports = port.value - } - } - } - } -} - -resource "google_compute_organization_security_policy_association" "attachment" { - provider = google-beta - for_each = var.firewall_policy_attachments - name = "${var.organization_id}-${each.key}" - attachment_id = var.organization_id - policy_id = each.value -} - -resource "google_logging_organization_sink" "sink" { - for_each = local.logging_sinks - name = each.key - org_id = local.organization_id_numeric - destination = "${local.sink_type_destination[each.value.type]}/${each.value.destination}" - filter = each.value.filter - include_children = each.value.include_children - - dynamic "bigquery_options" { - for_each = each.value.bq_partitioned_table == true ? [""] : [] - content { - use_partitioned_tables = each.value.bq_partitioned_table - } - } - - dynamic "exclusions" { - for_each = each.value.exclusions - iterator = exclusion - content { - name = exclusion.key - filter = exclusion.value - } - } - depends_on = [ - google_organization_iam_binding.authoritative, - google_organization_iam_member.additive, - google_organization_iam_policy.authoritative, - ] -} - -resource "google_storage_bucket_iam_member" "gcs-sinks-binding" { - for_each = local.sink_bindings["gcs"] - bucket = each.value.destination - role = "roles/storage.objectCreator" - member = google_logging_organization_sink.sink[each.key].writer_identity -} - -resource "google_bigquery_dataset_iam_member" "bq-sinks-binding" { - for_each = local.sink_bindings["bigquery"] - project = split("/", each.value.destination)[1] - dataset_id = split("/", each.value.destination)[3] - role = "roles/bigquery.dataEditor" - member = google_logging_organization_sink.sink[each.key].writer_identity -} - -resource "google_pubsub_topic_iam_member" "pubsub-sinks-binding" { - for_each = local.sink_bindings["pubsub"] - project = split("/", each.value.destination)[1] - topic = split("/", each.value.destination)[3] - role = "roles/pubsub.publisher" - member = google_logging_organization_sink.sink[each.key].writer_identity -} - -resource "google_project_iam_member" "bucket-sinks-binding" { - for_each = local.sink_bindings["logging"] - project = split("/", each.value.destination)[1] - role = "roles/logging.bucketWriter" - member = google_logging_organization_sink.sink[each.key].writer_identity - # TODO(jccb): use a condition to limit writer-identity only to this - # bucket -} - -resource "google_logging_organization_exclusion" "logging-exclusion" { - for_each = coalesce(var.logging_exclusions, {}) - name = each.key - org_id = local.organization_id_numeric - description = "${each.key} (Terraform-managed)" - filter = each.value -} - resource "google_essential_contacts_contact" "contact" { provider = google-beta for_each = var.contacts diff --git a/modules/organization/variables.tf b/modules/organization/variables.tf index e01c1e46..2ca79a6d 100644 --- a/modules/organization/variables.tf +++ b/modules/organization/variables.tf @@ -27,27 +27,36 @@ variable "custom_roles" { } variable "firewall_policies" { - description = "Hierarchical firewall policies to *create* in the organization." + description = "Hierarchical firewall policy rules created in the organization." type = map(map(object({ + action = string description = string direction = string - action = string + logging = bool + ports = map(list(string)) priority = number ranges = list(string) - ports = map(list(string)) - target_service_accounts = list(string) target_resources = list(string) - logging = bool - #preview = bool + target_service_accounts = list(string) + # preview = bool }))) default = {} } variable "firewall_policy_attachments" { - description = "List of hierarchical firewall policy IDs to *attach* to the organization" - # set to avoid manual casting with toset() - type = map(string) - default = {} + description = "List of hierarchical firewall policy IDs attached to the organization." + type = map(string) + default = {} +} + +variable "firewall_policy_factory" { + description = "Configuration for the firewall policy factory." + type = object({ + cidr_file = string + policy_name = string + rules_file = string + }) + default = null } variable "group_iam" { @@ -120,6 +129,13 @@ variable "logging_sinks" { # TODO exclusions also support description and disabled exclusions = map(string) })) + validation { + condition = alltrue([ + for k, v in(var.logging_sinks == null ? {} : var.logging_sinks) : + contains(["bigquery", "logging", "pubsub", "storage"], v.type) + ]) + error_message = "Type must be one of 'bigquery', 'logging', 'pubsub', 'storage'." + } default = {} } diff --git a/tests/modules/organization/fixture/data/firewall-cidrs.yaml b/tests/modules/organization/fixture/data/firewall-cidrs.yaml new file mode 100644 index 00000000..61291fcd --- /dev/null +++ b/tests/modules/organization/fixture/data/firewall-cidrs.yaml @@ -0,0 +1,18 @@ +# Copyright 2021 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. + +rfc1918: + - 10.0.0.0/8 + - 172.168.0.0/12 + - 192.168.0.0/16 diff --git a/tests/modules/organization/fixture/data/firewall-rules.yaml b/tests/modules/organization/fixture/data/firewall-rules.yaml new file mode 100644 index 00000000..62d4798b --- /dev/null +++ b/tests/modules/organization/fixture/data/firewall-rules.yaml @@ -0,0 +1,37 @@ +# Copyright 2021 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. + +allow-admins: + description: Access from the admin subnet to all subnets + direction: INGRESS + action: allow + priority: 1000 + ranges: + - $rfc1918 + ports: + all: [] + target_resources: null + enable_logging: false + +allow-ssh-from-iap: + description: Enable SSH from IAP + direction: INGRESS + action: allow + priority: 1002 + ranges: + - 35.235.240.0/20 + ports: + tcp: ["22"] + target_resources: null + enable_logging: false diff --git a/tests/modules/organization/fixture/main.tf b/tests/modules/organization/fixture/main.tf index b17a6535..6c948c38 100644 --- a/tests/modules/organization/fixture/main.tf +++ b/tests/modules/organization/fixture/main.tf @@ -27,6 +27,7 @@ module "test" { policy_list = var.policy_list firewall_policies = var.firewall_policies firewall_policy_attachments = var.firewall_policy_attachments + firewall_policy_factory = var.firewall_policy_factory logging_sinks = var.logging_sinks logging_exclusions = var.logging_exclusions } diff --git a/tests/modules/organization/fixture/variables.tf b/tests/modules/organization/fixture/variables.tf index b9463346..8e04290c 100644 --- a/tests/modules/organization/fixture/variables.tf +++ b/tests/modules/organization/fixture/variables.tf @@ -79,6 +79,15 @@ variable "firewall_policy_attachments" { default = {} } +variable "firewall_policy_factory" { + type = object({ + cidr_file = string + policy_name = string + rules_file = string + }) + default = null +} + variable "logging_sinks" { type = map(object({ destination = string diff --git a/tests/modules/organization/test_plan.py b/tests/modules/organization/test_plan.py index 168b836a..5fd1b0ab 100644 --- a/tests/modules/organization/test_plan.py +++ b/tests/modules/organization/test_plan.py @@ -110,78 +110,3 @@ def test_policy_list(plan_runner): assert values[1]['list_policy'][0]['deny'] == [ {'all': False, 'values': ["bar"]}] assert values[2]['restore_policy'] == [{'default': True}] - - -def test_firweall_policy(plan_runner): - "Test boolean folder policy." - policy = """ - { - policy1 = { - allow-ingress = { - description = "" - direction = "INGRESS" - action = "allow" - priority = 100 - ranges = ["10.0.0.0/8"] - ports = { - tcp = ["22"] - } - target_service_accounts = null - target_resources = null - logging = false - } - deny-egress = { - description = "" - direction = "EGRESS" - action = "deny" - priority = 200 - ranges = ["192.168.0.0/24"] - ports = { - tcp = ["443"] - } - target_service_accounts = null - target_resources = null - logging = false - } - } - } - """ - attachment = '{ iap_policy = "policy1" }' - _, resources = plan_runner(FIXTURES_DIR, firewall_policies=policy, - firewall_policy_attachments=attachment) - assert len(resources) == 4 - - policies = [r for r in resources - if r['type'] == 'google_compute_organization_security_policy'] - assert len(policies) == 1 - - rules = [r for r in resources - if r['type'] == 'google_compute_organization_security_policy_rule'] - assert len(rules) == 2 - - rule_values = [] - for rule in rules: - name = rule['name'] - index = rule['index'] - action = rule['values']['action'] - direction = rule['values']['direction'] - priority = rule['values']['priority'] - config = rule['values']['match'] - assert len(config) == 1 - config = config[0]['config'] - rule_values.append((name, index, action, direction, priority, config)) - - assert sorted(rule_values) == sorted([ - ('rule', 'policy1-allow-ingress', 'allow', 'INGRESS', 100, [ - { - 'dest_ip_ranges': None, - 'layer4_config': [{'ip_protocol': 'tcp', 'ports': ['22']}], - 'src_ip_ranges': ['10.0.0.0/8'] - }]), - ('rule', 'policy1-deny-egress', 'deny', 'EGRESS', 200, [ - { - 'dest_ip_ranges': ['192.168.0.0/24'], - 'layer4_config': [{'ip_protocol': 'tcp', 'ports': ['443']}], - 'src_ip_ranges': None - }]) - ]) diff --git a/tests/modules/organization/test_plan_firewall.py b/tests/modules/organization/test_plan_firewall.py new file mode 100644 index 00000000..8998bf7b --- /dev/null +++ b/tests/modules/organization/test_plan_firewall.py @@ -0,0 +1,137 @@ +# Copyright 2021 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. + + +import os +import pytest + + +FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture') + +_FACTORY = ''' +{ + cidr_file = "data/firewall-cidrs.yaml" + policy_name = "factory-1" + rules_file = "data/firewall-rules.yaml" +} +''' +_POLICIES = ''' +{ + policy1 = { + allow-ingress = { + description = "" + direction = "INGRESS" + action = "allow" + priority = 100 + ranges = ["10.0.0.0/8"] + ports = { + tcp = ["22"] + } + target_service_accounts = null + target_resources = null + logging = false + } + deny-egress = { + description = "" + direction = "EGRESS" + action = "deny" + priority = 200 + ranges = ["192.168.0.0/24"] + ports = { + tcp = ["443"] + } + target_service_accounts = null + target_resources = null + logging = false + } + } + policy2 = { + allow-ingress = { + description = "" + direction = "INGRESS" + action = "allow" + priority = 100 + ranges = ["10.0.0.0/8"] + ports = { + tcp = ["22"] + } + target_service_accounts = null + target_resources = null + logging = false + } + } + } +''' + + +def test_custom(plan_runner): + 'Test custom firewall policies.' + _, resources = plan_runner(FIXTURES_DIR, firewall_policies=_POLICIES) + assert len(resources) == 5 + policies = [r for r in resources + if r['type'] == 'google_compute_organization_security_policy'] + rules = [r for r in resources + if r['type'] == 'google_compute_organization_security_policy_rule'] + assert set(r['index'] for r in policies) == set([ + 'policy1', 'policy2' + ]) + assert set(r['index'] for r in rules) == set([ + 'policy1-deny-egress', 'policy2-allow-ingress', 'policy1-allow-ingress' + ]) + + +def test_factory(plan_runner): + 'Test firewall policy factory.' + _, resources = plan_runner(FIXTURES_DIR, firewall_policy_factory=_FACTORY) + assert len(resources) == 3 + policies = [r for r in resources + if r['type'] == 'google_compute_organization_security_policy'] + rules = [r for r in resources + if r['type'] == 'google_compute_organization_security_policy_rule'] + assert set(r['index'] for r in policies) == set([ + 'factory-1' + ]) + assert set(r['index'] for r in rules) == set([ + 'factory-1-allow-admins', 'factory-1-allow-ssh-from-iap' + ]) + + +def test_factory_name(plan_runner): + 'Test firewall policy factory default name.' + factory = _FACTORY.replace('"factory-1"', 'null') + _, resources = plan_runner(FIXTURES_DIR, firewall_policy_factory=factory) + assert len(resources) == 3 + policies = [r for r in resources + if r['type'] == 'google_compute_organization_security_policy'] + assert set(r['index'] for r in policies) == set([ + 'factory' + ]) + + +def test_combined(plan_runner): + 'Test combined rules.' + _, resources = plan_runner(FIXTURES_DIR, firewall_policies=_POLICIES, + firewall_policy_factory=_FACTORY) + assert len(resources) == 8 + policies = [r for r in resources + if r['type'] == 'google_compute_organization_security_policy'] + rules = [r for r in resources + if r['type'] == 'google_compute_organization_security_policy_rule'] + assert set(r['index'] for r in policies) == set([ + 'factory-1', 'policy1', 'policy2' + ]) + assert set(r['index'] for r in rules) == set([ + 'factory-1-allow-admins', 'factory-1-allow-ssh-from-iap', + 'policy1-deny-egress', 'policy2-allow-ingress', 'policy1-allow-ingress' + ]) diff --git a/tests/modules/organization/test_plan_logging.py b/tests/modules/organization/test_plan_logging.py index 98163afc..fdb592e4 100644 --- a/tests/modules/organization/test_plan_logging.py +++ b/tests/modules/organization/test_plan_logging.py @@ -25,7 +25,7 @@ def test_sinks(plan_runner): "Test folder-level sinks." logging_sinks = """ { warning = { - type = "gcs" + type = "storage" destination = "mybucket" filter = "severity=WARNING" iam = true From 750bb9f7e08f082e75756801673ccc52a9686ce3 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Mon, 13 Dec 2021 08:41:59 +0100 Subject: [PATCH 23/24] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 567ccb00..75f76836 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file. - added a subnet factory to `net-vpc` module - **incompatible change** added support for partitioned tables to `organization` module sinks - **incompatible change** renamed `private_service_networking_range` variable to `psc_ranges` in `net-vpc`module, and changed its type to `list(string)` +- added a firewall policy factory to `organization` module ## [8.0.0] - 2021-10-21 From 38b8ea175796b0f629414ea46189142aaa18b3ff Mon Sep 17 00:00:00 2001 From: lcaggio Date: Mon, 13 Dec 2021 11:26:09 +0100 Subject: [PATCH 24/24] Fix VPC-SC module, add example (#387) --- modules/vpc-sc/README.md | 47 ++++++++++++++++++++++++++++++++++++++++ modules/vpc-sc/main.tf | 7 ++++-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/modules/vpc-sc/README.md b/modules/vpc-sc/README.md index 01bcf59d..1537c119 100644 --- a/modules/vpc-sc/README.md +++ b/modules/vpc-sc/README.md @@ -136,6 +136,53 @@ module "vpc-sc" { # tftest:modules=1:resources=3 ``` +## Example VCP-SC: 2 standard perimeters with one bridge between the two (dry run mode). +```hcl +module "vpc-sc" { + source = "./modules/vpc-sc" + organization_id = "organizations/112233" + access_policy_title = "My Access Policy" + perimeters = { + perimeter_1 = { + type = "PERIMETER_TYPE_REGULAR" + dry_run_config = { + restricted_services = ["storage.googleapis.com", "bigquery.googleapis.com"] + vpc_accessible_services = ["storage.googleapis.com", "bigquery.googleapis.com"] + } + enforced_config = null + } + perimeter_2 = { + type = "PERIMETER_TYPE_REGULAR" + dry_run_config = { + restricted_services = ["storage.googleapis.com", "bigquery.googleapis.com"] + vpc_accessible_services = ["storage.googleapis.com", "bigquery.googleapis.com"] + } + enforced_config = null + } + perimeter_bridge = { + type = "PERIMETER_TYPE_BRIDGE" + dry_run_config = null + enforced_config = null + } + } + perimeter_projects = { + perimeter_1 = { + enforced = [] + dry_run = [111111111] + } + perimeter_2 = { + enforced = [] + dry_run = [222222222] + } + perimeter_bridge = { + enforced = [] + dry_run = [111111111, 222222222] + } + } +} +# tftest:modules=1:resources=4 +``` + ## Example VCP-SC standard perimeter with one service and one project in dry run mode in a Organization with an already existent access policy ```hcl module "vpc-sc-first" { diff --git a/modules/vpc-sc/main.tf b/modules/vpc-sc/main.tf index abae2e35..3e6c2801 100644 --- a/modules/vpc-sc/main.tf +++ b/modules/vpc-sc/main.tf @@ -330,11 +330,14 @@ resource "google_access_context_manager_service_perimeter" "bridge" { } # Dry run mode configuration + use_explicit_dry_run_spec = try(lookup(var.perimeter_projects, each.key, null).dry_run, null) != null ? true : null dynamic "spec" { - for_each = try(lookup(var.perimeter_projects, each.key, {}).dry_run, []) != null ? [""] : [] + for_each = try(lookup(var.perimeter_projects, each.key, null).dry_run, null) != null ? [""] : [] content { - resources = formatlist("projects/%s", try(lookup(var.perimeter_projects, each.key, {}).dry_run, [])) + resources = try(formatlist("projects/%s", lookup(var.perimeter_projects, each.key, {}).dry_run), null) + restricted_services = [] + access_levels = [] } }