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

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

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

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
 }

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

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
-          }])
-  ])

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'
+  ])

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