From 66a402083bd497c91ebb1c389035d56b09036000 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Fri, 23 Dec 2022 09:03:31 +0100 Subject: [PATCH] Allow setting no ranges in firewall module custom rules (#1073) * allow setting no ranges in custom firewall rules * fix blueprint * fix example * fix example --- .../data-solutions/sqlserver-alwayson/vpc.tf | 6 +- modules/net-vpc-firewall/README.md | 42 ++++++---- modules/net-vpc-firewall/main.tf | 34 +++++--- modules/net-vpc-firewall/variables.tf | 5 +- .../net_vpc_firewall/auto-rules.tfvars | 4 + .../modules/net_vpc_firewall/auto-rules.yaml | 44 ++++++++++ tests/modules/net_vpc_firewall/common.tfvars | 2 + .../net_vpc_firewall/custom-rules.tfvars | 33 ++++++++ .../net_vpc_firewall/custom-rules.yaml | 83 +++++++++++++++++++ .../config => data}/cidr_template.yaml | 0 .../firewall/load_balancers.yaml | 0 tests/modules/net_vpc_firewall/factory.tfvars | 7 ++ tests/modules/net_vpc_firewall/factory.yaml | 54 ++++++++++++ .../modules/net_vpc_firewall/fixture/main.tf | 25 ------ .../fixture/test.rules.tfvars | 22 ----- .../net_vpc_firewall/fixture/variables.tf | 51 ------------ .../{test_plan.py => test_plan.py_} | 0 tests/modules/net_vpc_firewall/tftest.yaml | 21 +++++ 18 files changed, 301 insertions(+), 132 deletions(-) create mode 100644 tests/modules/net_vpc_firewall/auto-rules.tfvars create mode 100644 tests/modules/net_vpc_firewall/auto-rules.yaml create mode 100644 tests/modules/net_vpc_firewall/common.tfvars create mode 100644 tests/modules/net_vpc_firewall/custom-rules.tfvars create mode 100644 tests/modules/net_vpc_firewall/custom-rules.yaml rename tests/modules/net_vpc_firewall/{fixture/config => data}/cidr_template.yaml (100%) rename tests/modules/net_vpc_firewall/{fixture/config => data}/firewall/load_balancers.yaml (100%) create mode 100644 tests/modules/net_vpc_firewall/factory.tfvars create mode 100644 tests/modules/net_vpc_firewall/factory.yaml delete mode 100644 tests/modules/net_vpc_firewall/fixture/main.tf delete mode 100644 tests/modules/net_vpc_firewall/fixture/test.rules.tfvars delete mode 100644 tests/modules/net_vpc_firewall/fixture/variables.tf rename tests/modules/net_vpc_firewall/{test_plan.py => test_plan.py_} (100%) create mode 100644 tests/modules/net_vpc_firewall/tftest.yaml diff --git a/blueprints/data-solutions/sqlserver-alwayson/vpc.tf b/blueprints/data-solutions/sqlserver-alwayson/vpc.tf index ccc10e1c..0f1e425e 100644 --- a/blueprints/data-solutions/sqlserver-alwayson/vpc.tf +++ b/blueprints/data-solutions/sqlserver-alwayson/vpc.tf @@ -85,6 +85,7 @@ module "firewall" { ingress_rules = { "${var.prefix}-allow-all-between-wsfc-nodes" = { description = "Allow all between WSFC nodes" + source_ranges = [] sources = [module.compute-service-account.email] targets = [module.compute-service-account.email] use_service_accounts = true @@ -96,6 +97,7 @@ module "firewall" { } "${var.prefix}-allow-all-between-wsfc-witness" = { description = "Allow all between WSFC witness nodes" + source_ranges = [] sources = [module.compute-service-account.email] targets = [module.witness-service-account.email] use_service_accounts = true @@ -108,7 +110,7 @@ module "firewall" { "${var.prefix}-allow-sql-to-wsfc-nodes" = { description = "Allow SQL connections to WSFC nodes" targets = [module.compute-service-account.email] - ranges = var.sql_client_cidrs + source_ranges = var.sql_client_cidrs use_service_accounts = true rules = [ { protocol = "tcp", ports = [1433] }, @@ -117,7 +119,7 @@ module "firewall" { "${var.prefix}-allow-health-check-to-wsfc-nodes" = { description = "Allow health checks to WSFC nodes" targets = [module.compute-service-account.email] - ranges = var.health_check_ranges + source_ranges = var.health_check_ranges use_service_accounts = true rules = [ { protocol = "tcp" } diff --git a/modules/net-vpc-firewall/README.md b/modules/net-vpc-firewall/README.md index 38304781..f886035b 100644 --- a/modules/net-vpc-firewall/README.md +++ b/modules/net-vpc-firewall/README.md @@ -33,7 +33,7 @@ Some implicit defaults are used in the rules variable types and can be controlle - action is controlled via the `deny` attribute which defaults to `true` for egress and `false` for ingress - priority defaults to `1000` -- destination ranges (for egress) and source ranges (for ingress) default to `["0.0.0.0/0"]` if not explicitly set +- destination ranges (for egress) and source ranges (for ingress) default to `["0.0.0.0/0"]` if not explicitly set or set to `null`, to disable the behaviour set ranges to the empty list (`[]`) - rules default to all protocols if not set ```hcl @@ -45,31 +45,39 @@ module "firewall" { admin_ranges = ["10.0.0.0/8"] } egress_rules = { - # implicit `deny` action + # implicit deny action allow-egress-rfc1918 = { + deny = false description = "Allow egress to RFC 1918 ranges." destination_ranges = [ "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16" ] - # implicit { protocol = "all" } rule + } + allow-egress-tag = { + deny = false + description = "Allow egress from a specific tag to 0/0." + targets = ["target-tag"] } deny-egress-all = { description = "Block egress." - # implicit ["0.0.0.0/0"] destination ranges - # implicit { protocol = "all" } rule } } ingress_rules = { - # implicit `allow` action + # implicit allow action allow-ingress-ntp = { - description = "Allow NTP service based on tag." - source_ranges = ["0.0.0.0/0"] - targets = ["ntp-svc"] - rules = [{ protocol = "udp", ports = [123] }] + description = "Allow NTP service based on tag." + targets = ["ntp-svc"] + rules = [{ protocol = "udp", ports = [123] }] + } + allow-ingress-tag = { + description = "Allow ingress from a specific tag." + source_ranges = [] + sources = ["client-tag"] + targets = ["target-tag"] } } } -# tftest modules=1 resources=7 +# tftest modules=1 resources=9 ``` ### Controlling or turning off default rules @@ -194,13 +202,13 @@ healthchecks: | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [network](variables.tf#L109) | Name of the network this set of firewall rules applies to. | string | ✓ | | -| [project_id](variables.tf#L114) | Project id of the project that holds the network. | string | ✓ | | +| [network](variables.tf#L108) | Name of the network this set of firewall rules applies to. | string | ✓ | | +| [project_id](variables.tf#L113) | Project id of the project that holds the network. | string | ✓ | | | [default_rules_config](variables.tf#L17) | Optionally created convenience rules. Set the 'disabled' attribute to true, or individual rule attributes to empty lists to disable. | object({…}) | | {} | -| [egress_rules](variables.tf#L37) | List of egress rule definitions, default to deny action. | map(object({…})) | | {} | -| [factories_config](variables.tf#L60) | Paths to data files and folders that enable factory functionality. | object({…}) | | null | -| [ingress_rules](variables.tf#L69) | List of ingress rule definitions, default to allow action. | map(object({…})) | | {} | -| [named_ranges](variables.tf#L92) | Define mapping of names to ranges that can be used in custom rules. | map(list(string)) | | {…} | +| [egress_rules](variables.tf#L37) | List of egress rule definitions, default to deny action. Null destination ranges will be replaced with 0/0. | map(object({…})) | | {} | +| [factories_config](variables.tf#L59) | Paths to data files and folders that enable factory functionality. | object({…}) | | null | +| [ingress_rules](variables.tf#L68) | List of ingress rule definitions, default to allow action. Null source ranges will be replaced with 0/0. | map(object({…})) | | {} | +| [named_ranges](variables.tf#L91) | Define mapping of names to ranges that can be used in custom rules. | map(list(string)) | | {…} | ## Outputs diff --git a/modules/net-vpc-firewall/main.tf b/modules/net-vpc-firewall/main.tf index 708b8844..e525ceb4 100644 --- a/modules/net-vpc-firewall/main.tf +++ b/modules/net-vpc-firewall/main.tf @@ -66,15 +66,23 @@ locals { for name, rule in local._rules : name => merge(rule, { action = rule.deny == true ? "DENY" : "ALLOW" - destination_ranges = flatten([ - for range in coalesce(try(rule.destination_ranges, null), []) : - try(local._named_ranges[range], range) - ]) + destination_ranges = ( + try(rule.destination_ranges, null) == null + ? null + : flatten([ + for range in rule.destination_ranges : + try(local._named_ranges[range], range) + ]) + ) rules = { for k, v in rule.rules : k => v } - source_ranges = flatten([ - for range in coalesce(try(rule.source_ranges, null), []) : - try(local._named_ranges[range], range) - ]) + source_ranges = ( + try(rule.source_ranges, null) == null + ? null + : flatten([ + for range in rule.source_ranges : + try(local._named_ranges[range], range) + ]) + ) }) } } @@ -89,18 +97,20 @@ resource "google_compute_firewall" "custom-rules" { source_ranges = ( each.value.direction == "INGRESS" ? ( - coalesce(each.value.source_ranges, []) == [] + each.value.source_ranges == null ? ["0.0.0.0/0"] : each.value.source_ranges - ) : null + ) + : null ) destination_ranges = ( each.value.direction == "EGRESS" ? ( - coalesce(each.value.destination_ranges, []) == [] + each.value.destination_ranges == null ? ["0.0.0.0/0"] : each.value.destination_ranges - ) : null + ) + : null ) source_tags = ( each.value.use_service_accounts || each.value.direction == "EGRESS" diff --git a/modules/net-vpc-firewall/variables.tf b/modules/net-vpc-firewall/variables.tf index 3e458acd..9f750cc8 100644 --- a/modules/net-vpc-firewall/variables.tf +++ b/modules/net-vpc-firewall/variables.tf @@ -35,7 +35,7 @@ variable "default_rules_config" { } variable "egress_rules" { - description = "List of egress rule definitions, default to deny action." + description = "List of egress rule definitions, default to deny action. Null destination ranges will be replaced with 0/0." type = map(object({ deny = optional(bool, true) description = optional(string) @@ -45,7 +45,6 @@ variable "egress_rules" { include_metadata = optional(bool) })) priority = optional(number, 1000) - sources = optional(list(string)) targets = optional(list(string)) use_service_accounts = optional(bool, false) rules = optional(list(object({ @@ -67,7 +66,7 @@ variable "factories_config" { } variable "ingress_rules" { - description = "List of ingress rule definitions, default to allow action." + description = "List of ingress rule definitions, default to allow action. Null source ranges will be replaced with 0/0." type = map(object({ deny = optional(bool, false) description = optional(string) diff --git a/tests/modules/net_vpc_firewall/auto-rules.tfvars b/tests/modules/net_vpc_firewall/auto-rules.tfvars new file mode 100644 index 00000000..6b991da7 --- /dev/null +++ b/tests/modules/net_vpc_firewall/auto-rules.tfvars @@ -0,0 +1,4 @@ +default_rules_config = { + admin_ranges = ["10.0.0.0/8"] + https_ranges = [] +} diff --git a/tests/modules/net_vpc_firewall/auto-rules.yaml b/tests/modules/net_vpc_firewall/auto-rules.yaml new file mode 100644 index 00000000..ed3c84f2 --- /dev/null +++ b/tests/modules/net_vpc_firewall/auto-rules.yaml @@ -0,0 +1,44 @@ +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +values: + google_compute_firewall.allow-admins[0]: + source_ranges: + - 10.0.0.0/8 + google_compute_firewall.allow-tag-http[0]: + allow: + - ports: + - "80" + protocol: tcp + source_ranges: + - 130.211.0.0/22 + - 209.85.152.0/22 + - 209.85.204.0/22 + - 35.191.0.0/16 + google_compute_firewall.allow-tag-ssh[0]: + allow: + - ports: + - "22" + protocol: tcp + source_ranges: + - 35.235.240.0/20 + +counts: + google_compute_firewall: 3 + modules: 0 + resources: 3 + +outputs: + default_rules: __missing__ + rules: {} diff --git a/tests/modules/net_vpc_firewall/common.tfvars b/tests/modules/net_vpc_firewall/common.tfvars new file mode 100644 index 00000000..fda6ab8f --- /dev/null +++ b/tests/modules/net_vpc_firewall/common.tfvars @@ -0,0 +1,2 @@ +project_id = "test-project" +network = "test-network" diff --git a/tests/modules/net_vpc_firewall/custom-rules.tfvars b/tests/modules/net_vpc_firewall/custom-rules.tfvars new file mode 100644 index 00000000..181a8248 --- /dev/null +++ b/tests/modules/net_vpc_firewall/custom-rules.tfvars @@ -0,0 +1,33 @@ +default_rules_config = { + disabled = true +} +egress_rules = { + allow-egress-rfc1918 = { + deny = false + description = "Allow egress to RFC 1918 ranges." + destination_ranges = [ + "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16" + ] + } + allow-egress-tag = { + deny = false + description = "Allow egress from a specific tag to 0/0." + targets = ["target-tag"] + } + deny-egress-all = { + description = "Block egress." + } +} +ingress_rules = { + allow-ingress-ntp = { + description = "Allow NTP service based on tag." + targets = ["ntp-svc"] + rules = [{ protocol = "udp", ports = [123] }] + } + allow-ingress-tag = { + description = "Allow ingress from a specific tag." + source_ranges = [] + sources = ["client-tag"] + targets = ["target-tag"] + } +} diff --git a/tests/modules/net_vpc_firewall/custom-rules.yaml b/tests/modules/net_vpc_firewall/custom-rules.yaml new file mode 100644 index 00000000..65204897 --- /dev/null +++ b/tests/modules/net_vpc_firewall/custom-rules.yaml @@ -0,0 +1,83 @@ +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +values: + google_compute_firewall.custom-rules["allow-egress-rfc1918"]: + allow: + - ports: [] + protocol: all + deny: [] + description: Allow egress to RFC 1918 ranges. + destination_ranges: + - 10.0.0.0/8 + - 172.16.0.0/12 + - 192.168.0.0/16 + direction: EGRESS + google_compute_firewall.custom-rules["allow-egress-tag"]: + allow: + - ports: [] + protocol: all + deny: [] + description: Allow egress from a specific tag to 0/0. + destination_ranges: + - 0.0.0.0/0 + direction: EGRESS + target_tags: + - target-tag + google_compute_firewall.custom-rules["allow-ingress-ntp"]: + allow: + - ports: + - "123" + protocol: udp + deny: [] + description: Allow NTP service based on tag. + direction: INGRESS + source_ranges: + - 0.0.0.0/0 + source_service_accounts: null + source_tags: null + target_tags: + - ntp-svc + google_compute_firewall.custom-rules["allow-ingress-tag"]: + allow: + - ports: [] + protocol: all + deny: [] + description: Allow ingress from a specific tag. + direction: INGRESS + source_ranges: null + source_tags: + - client-tag + target_tags: + - target-tag + google_compute_firewall.custom-rules["deny-egress-all"]: + allow: [] + deny: + - ports: [] + protocol: all + description: Block egress. + direction: EGRESS + +counts: + google_compute_firewall: 5 + modules: 0 + resources: 5 + +outputs: + default_rules: + admin: [] + http: [] + https: [] + ssh: [] + rules: __missing__ diff --git a/tests/modules/net_vpc_firewall/fixture/config/cidr_template.yaml b/tests/modules/net_vpc_firewall/data/cidr_template.yaml similarity index 100% rename from tests/modules/net_vpc_firewall/fixture/config/cidr_template.yaml rename to tests/modules/net_vpc_firewall/data/cidr_template.yaml diff --git a/tests/modules/net_vpc_firewall/fixture/config/firewall/load_balancers.yaml b/tests/modules/net_vpc_firewall/data/firewall/load_balancers.yaml similarity index 100% rename from tests/modules/net_vpc_firewall/fixture/config/firewall/load_balancers.yaml rename to tests/modules/net_vpc_firewall/data/firewall/load_balancers.yaml diff --git a/tests/modules/net_vpc_firewall/factory.tfvars b/tests/modules/net_vpc_firewall/factory.tfvars new file mode 100644 index 00000000..5d2e1ab7 --- /dev/null +++ b/tests/modules/net_vpc_firewall/factory.tfvars @@ -0,0 +1,7 @@ +default_rules_config = { + disabled = true +} +factories_config = { + cidr_tpl_file = "../../tests/modules/net_vpc_firewall/data/cidr_template.yaml" + rules_folder = "../../tests/modules/net_vpc_firewall/data/firewall" +} diff --git a/tests/modules/net_vpc_firewall/factory.yaml b/tests/modules/net_vpc_firewall/factory.yaml new file mode 100644 index 00000000..26f90bd5 --- /dev/null +++ b/tests/modules/net_vpc_firewall/factory.yaml @@ -0,0 +1,54 @@ +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +values: + google_compute_firewall.custom-rules["allow-healthchecks"]: + allow: + - ports: + - "80" + - "443" + protocol: tcp + deny: [] + description: Allow ingress from healthchecks. + direction: INGRESS + disabled: false + log_config: [] + name: allow-healthchecks + network: test-network + priority: 1000 + project: test-project + source_ranges: + - 130.211.0.0/22 + - 209.85.152.0/22 + - 209.85.204.0/22 + - 35.191.0.0/16 + source_service_accounts: null + source_tags: null + target_service_accounts: null + target_tags: + - lb-backends + timeouts: null + +counts: + google_compute_firewall: 1 + modules: 0 + resources: 1 + +outputs: + default_rules: + admin: [] + http: [] + https: [] + ssh: [] + rules: __missing__ diff --git a/tests/modules/net_vpc_firewall/fixture/main.tf b/tests/modules/net_vpc_firewall/fixture/main.tf deleted file mode 100644 index e69aeff1..00000000 --- a/tests/modules/net_vpc_firewall/fixture/main.tf +++ /dev/null @@ -1,25 +0,0 @@ -/** - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -module "firewall" { - source = "../../../../modules/net-vpc-firewall" - project_id = "test-project" - network = "test-vpc" - default_rules_config = var.default_rules_config - egress_rules = var.egress_rules - ingress_rules = var.ingress_rules - factories_config = var.factories_config -} diff --git a/tests/modules/net_vpc_firewall/fixture/test.rules.tfvars b/tests/modules/net_vpc_firewall/fixture/test.rules.tfvars deleted file mode 100644 index 36944bea..00000000 --- a/tests/modules/net_vpc_firewall/fixture/test.rules.tfvars +++ /dev/null @@ -1,22 +0,0 @@ -egress_rules = { - allow-egress-rfc1918 = { - description = "Allow egress to RFC 1918 ranges." - is_egress = true - destination_ranges = ["10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"] - } - deny-egress-all = { - description = "Block egress." - is_deny = true - is_egress = true - } -} -ingress_rules = { - allow-ingress-ntp = { - description = "Allow NTP service based on tag." - targets = ["ntp-svc"] - rules = [{ protocol = "udp", ports = [123] }] - } -} -default_rules_config = { - disabled = true -} diff --git a/tests/modules/net_vpc_firewall/fixture/variables.tf b/tests/modules/net_vpc_firewall/fixture/variables.tf deleted file mode 100644 index fd71e93b..00000000 --- a/tests/modules/net_vpc_firewall/fixture/variables.tf +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -variable "default_rules_config" { - type = any - default = {} -} - -variable "egress_rules" { - type = any - default = {} -} - -variable "factories_config" { - type = any - default = null -} - -variable "ingress_rules" { - type = any - default = {} -} diff --git a/tests/modules/net_vpc_firewall/test_plan.py b/tests/modules/net_vpc_firewall/test_plan.py_ similarity index 100% rename from tests/modules/net_vpc_firewall/test_plan.py rename to tests/modules/net_vpc_firewall/test_plan.py_ diff --git a/tests/modules/net_vpc_firewall/tftest.yaml b/tests/modules/net_vpc_firewall/tftest.yaml new file mode 100644 index 00000000..e11810c4 --- /dev/null +++ b/tests/modules/net_vpc_firewall/tftest.yaml @@ -0,0 +1,21 @@ +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +module: modules/net-vpc-firewall +common_tfvars: + - common.tfvars +tests: + auto-rules: + custom-rules: + factory: