Refactor VPC firewall module for Terraform 1.3 (#949)

* module and module tests/examples

* align blueprints and fast

* fix null ranges

* make ports optional

* tfdoc

* make rules optional defaulting to all protocols

* review comments

* last round of comments

* invert precedence of template variables

* add option to disable all default rules

* add option to disable all default rules

* split egress/ingress

* tests

* fix tests

tests/modules/net_vpc_firewall/fixture/config/firewall/load_balancers.yaml

@@ -12,17 +12,14 @@
 # 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
+ingress:
+  allow-healthchecks:
+    description: Allow ingress from healthchecks.
+    source_ranges:
+      - healthchecks
+    targets: ["lb-backends"]
+    rules:
+      - protocol: tcp
+        ports:
+          - 80
+          - 443

tests/modules/net_vpc_firewall/fixture/main.tf

@@ -15,14 +15,11 @@
  */
 
 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
+  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
 }

tests/modules/net_vpc_firewall/fixture/test.rules.tfvars

@@ -0,0 +1,22 @@
+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
+}

tests/modules/net_vpc_firewall/fixture/variables.tf

@@ -14,84 +14,38 @@
  * limitations under the License.
  */
 
-variable "admin_ranges" {
-  description = "IP CIDR ranges that have complete access to all subnets."
-  type        = list(string)
-  default     = []
-}
+/**
+ * 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 "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)
-  }))
+variable "default_rules_config" {
+  type    = any
   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 "egress_rules" {
+  type    = any
+  default = {}
 }
 
-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 "factories_config" {
+  type    = any
+  default = null
 }
 
-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 "ingress_rules" {
+  type    = any
+  default = {}
 }
-
-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"]
-}
-

tests/modules/net_vpc_firewall/test_plan.py

@@ -12,27 +12,48 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-def test_vpc_firewall_simple(plan_runner):
-  "Test vpc with no extra options."
+import pytest
+
+
+def test_defaults(plan_runner):
+  "Test variable defaults."
   _, resources = plan_runner()
   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'])
+  assert set([r['type'] for r in resources]) == set(['google_compute_firewall'])
+  assert set([r['values']['name'] for r in resources]) == set([
+      'test-vpc-ingress-tag-http', 'test-vpc-ingress-tag-https',
+      'test-vpc-ingress-tag-ssh'
+  ])
+  assert set([r['values']['project'] for r in resources
+             ]) == set(['test-project'])
+  assert set([r['values']['network'] for r in resources]) == set(['test-vpc'])
 
 
-def test_vpc_firewall_factory(plan_runner):
-  "Test shared vpc variables."
-  _, resources = plan_runner(
-    data_folder="config/firewall",
-    cidr_template_file="config/cidr_template.yaml"
-  )
+def test_rules(plan_runner):
+  "Test custom rules."
+  tfvars = 'test.rules.tfvars'
+  _, resources = plan_runner(extra_files=[tfvars], tf_var_file=tfvars)
+  assert len(resources) == 3
+  rules = {r['index']: r['values'] for r in resources}
+  rule = rules['allow-ingress-ntp']
+  assert rule['source_ranges'] == ['0.0.0.0/0']
+  assert rule['allow'] == [{'ports': ['123'], 'protocol': 'udp'}]
+  rule = rules['deny-egress-all']
+  assert rule['destination_ranges'] == ['0.0.0.0/0']
+  assert rule['deny'] == [{'ports': [], 'protocol': 'all'}]
+
+
+def test_factory(plan_runner):
+  "Test factory."
+  factories_config = '''{
+    cidr_tpl_file = "config/cidr_template.yaml"
+    rules_folder  = "config/firewall"
+  }'''
+  _, resources = plan_runner(factories_config=factories_config)
   assert len(resources) == 4
-  factory_rule = [r for r in resources if r["values"]
-                  ["name"] == "allow-healthchecks"][0]["values"]
+  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"])