From 779f585d1d71bf5bdb3da821c2f567b9b9066a69 Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Mon, 23 Nov 2020 18:45:18 +0100 Subject: [PATCH 1/4] Hierarchical firewall policies for folders --- modules/folder/README.md | 46 ++++++++++++++++++++++++++++++ modules/folder/main.tf | 57 +++++++++++++++++++++++++++++++++++++ modules/folder/outputs.tf | 16 +++++++++++ modules/folder/variables.tf | 24 ++++++++++++++++ tests/examples/variables.tf | 2 +- 5 files changed, 144 insertions(+), 1 deletion(-) diff --git a/modules/folder/README.md b/modules/folder/README.md index e9c92f29..76bc2a2a 100644 --- a/modules/folder/README.md +++ b/modules/folder/README.md @@ -41,6 +41,48 @@ module "folder" { # tftest:modules=1:resources=4 ``` +### Hierarchical firewall policies + +```hcl +module "folder1" { + source = "./modules/folder" + parent = var.organization_id + name = "policy-container" + + firewall_policies = { + iap-policy = { + allow-iap-ssh = { + description = "Always allow ssh from IAP" + direction = "INGRESS" + action = "allow" + priority = 100 + ranges = ["35.235.240.0/20"] + ports = { + tcp = ["22"] + } + target_service_accounts = null + target_resources = null + logging = false + } + } + } + firewall_policy_attachments = { + iap-policy = module.folder1.firewall_policy_id["iap-policy"] + } +} + +module "folder2" { + source = "./modules/folder" + parent = var.organization_id + name = "hf2" + firewall_policy_attachments = { + iap-policy = module.folder1.firewall_policy_id["iap-policy"] + } +} +# tftest:modules=2:resources=6 +``` + + ## Variables @@ -48,6 +90,8 @@ module "folder" { |---|---|:---: |:---:|:---:| | name | Folder name. | string | ✓ | | | parent | Parent in folders/folder_id or organizations/org_id format. | 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. | set(string) | | [] | | *iam* | IAM bindings in {ROLE => [MEMBERS]} format. | map(set(string)) | | {} | | *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({...})) | | {} | @@ -56,6 +100,8 @@ module "folder" { | name | description | sensitive | |---|---|:---:| +| firewall_policies | Map of firewall policy resources created in this folder. | | +| firewall_policy_id | Map of firewall policy ids created in this folder. | | | folder | Folder resource. | | | id | Folder id. | | | name | Folder name. | | diff --git a/modules/folder/main.tf b/modules/folder/main.tf index fb018b7a..6fb50acb 100644 --- a/modules/folder/main.tf +++ b/modules/folder/main.tf @@ -14,6 +14,18 @@ * limitations under the License. */ +locals { + 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 + } +} resource "google_folder" "folder" { display_name = var.name @@ -99,3 +111,48 @@ 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 = google_folder.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 = "${google_folder.folder.id}-${each.key}" + attachment_id = google_folder.folder.id + policy_id = each.value +} diff --git a/modules/folder/outputs.tf b/modules/folder/outputs.tf index 4be12eb1..c521367f 100644 --- a/modules/folder/outputs.tf +++ b/modules/folder/outputs.tf @@ -33,3 +33,19 @@ output "name" { description = "Folder name." value = google_folder.folder.display_name } + +output "firewall_policies" { + description = "Map of firewall policy resources created in this folder." + value = { + for name, _ in var.firewall_policies : + name => google_compute_organization_security_policy.policy[name] + } +} + +output "firewall_policy_id" { + description = "Map of firewall policy ids created in this folder." + value = { + for name, _ in var.firewall_policies : + name => google_compute_organization_security_policy.policy[name].id + } +} diff --git a/modules/folder/variables.tf b/modules/folder/variables.tf index 1231be0d..24c0c171 100644 --- a/modules/folder/variables.tf +++ b/modules/folder/variables.tf @@ -50,3 +50,27 @@ variable "policy_list" { })) default = {} } + +variable "firewall_policies" { + description = "Hierarchical firewall policies to *create* in this folder." + type = map(map(object({ + description = string + direction = string + action = string + priority = number + ranges = list(string) + ports = map(list(string)) + target_service_accounts = list(string) + target_resources = list(string) + logging = bool + #preview = bool + }))) + default = {} +} + +variable "firewall_policy_attachments" { + description = "List of hierarchical firewall policy IDs to *attach* to this folder." + # set to avoid manual casting with toset() + type = map(string) + default = {} +} diff --git a/tests/examples/variables.tf b/tests/examples/variables.tf index 69b3cdec..962a2fad 100644 --- a/tests/examples/variables.tf +++ b/tests/examples/variables.tf @@ -15,7 +15,7 @@ # common variables used for examples variable "organization_id" { - default = "organization/organization" + default = "organizations/1122334455" } variable "project_id" { From 254efdd799662ac7f5230a78368882a90147193c Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Mon, 23 Nov 2020 19:01:02 +0100 Subject: [PATCH 2/4] Hierarchical firewall support for organizations --- modules/organization/README.md | 29 ++++++++++++++++ modules/organization/main.tf | 55 +++++++++++++++++++++++++++++++ modules/organization/outputs.tf | 16 +++++++++ modules/organization/variables.tf | 24 ++++++++++++++ 4 files changed, 124 insertions(+) diff --git a/modules/organization/README.md b/modules/organization/README.md index 9547c67d..f7f68a1d 100644 --- a/modules/organization/README.md +++ b/modules/organization/README.md @@ -30,6 +30,35 @@ module "org" { # tftest:modules=1:resources=4 ``` +## Hierarchical firewall rules +```hcl +module "org" { + source = "./modules/organization" + org_id = 11223344 + firewall_policies = { + iap-policy = { + allow-iap-ssh = { + description = "Always allow ssh from IAP" + direction = "INGRESS" + action = "allow" + priority = 100 + ranges = ["35.235.240.0/20"] + ports = { + tcp = ["22"] + } + target_service_accounts = null + target_resources = null + logging = false + } + } + } + firewall_policy_attachments = { + iap_policy = module.org.firewall_policy_id["iap-policy"] + } +} +# tftest:modules=1:resources=3 +``` + ## Variables diff --git a/modules/organization/main.tf b/modules/organization/main.tf index e331d6ae..d87fcab5 100644 --- a/modules/organization/main.tf +++ b/modules/organization/main.tf @@ -29,6 +29,16 @@ locals { 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 + } } resource "google_organization_iam_custom_role" "roles" { @@ -144,3 +154,48 @@ 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 = "organizations/${var.org_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 = "organizations/${var.org_id}-${each.key}" + attachment_id = "organizations/${var.org_id}" + policy_id = each.value +} diff --git a/modules/organization/outputs.tf b/modules/organization/outputs.tf index f33d54f1..f6e52441 100644 --- a/modules/organization/outputs.tf +++ b/modules/organization/outputs.tf @@ -26,3 +26,19 @@ output "org_id" { google_organization_policy.list ] } + +output "firewall_policies" { + description = "Map of firewall policy resources created in the organization." + value = { + for name, _ in var.firewall_policies : + name => google_compute_organization_security_policy.policy[name] + } +} + +output "firewall_policy_id" { + description = "Map of firewall policy ids created in the organization." + value = { + for name, _ in var.firewall_policies : + name => google_compute_organization_security_policy.policy[name].id + } +} diff --git a/modules/organization/variables.tf b/modules/organization/variables.tf index 8c69fcbb..1fe3f1bd 100644 --- a/modules/organization/variables.tf +++ b/modules/organization/variables.tf @@ -70,3 +70,27 @@ variable "policy_list" { })) default = {} } + +variable "firewall_policies" { + description = "Hierarchical firewall policies to *create* in the organization." + type = map(map(object({ + description = string + direction = string + action = string + priority = number + ranges = list(string) + ports = map(list(string)) + target_service_accounts = list(string) + target_resources = list(string) + logging = bool + #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 = {} +} From 6bd299190fb65060eb4ab7ebd40325f3b4cd6535 Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Mon, 23 Nov 2020 19:10:42 +0100 Subject: [PATCH 3/4] Update variable documentation. --- modules/folder/README.md | 2 +- modules/organization/README.md | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/folder/README.md b/modules/folder/README.md index 76bc2a2a..430db3dc 100644 --- a/modules/folder/README.md +++ b/modules/folder/README.md @@ -91,7 +91,7 @@ module "folder2" { | name | Folder name. | string | ✓ | | | parent | Parent in folders/folder_id or organizations/org_id format. | 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. | set(string) | | [] | +| *firewall_policy_attachments* | List of hierarchical firewall policy IDs to *attach* to this folder. | map(string) | | {} | | *iam* | IAM bindings in {ROLE => [MEMBERS]} format. | map(set(string)) | | {} | | *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/README.md b/modules/organization/README.md index f7f68a1d..993c5dcd 100644 --- a/modules/organization/README.md +++ b/modules/organization/README.md @@ -66,6 +66,8 @@ module "org" { |---|---|:---: |:---:|:---:| | org_id | Organization id in nnnnnn format. | number | ✓ | | | *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) | | {} | | *iam* | IAM bindings, in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | | *iam_additive* | Non authoritative IAM bindings, in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | | *iam_additive_members* | IAM additive bindings in {MEMBERS => [ROLE]} format. This might break if members are dynamic values. | map(list(string)) | | {} | @@ -77,5 +79,7 @@ module "org" { | name | description | sensitive | |---|---|:---:| +| firewall_policies | Map of firewall policy resources created in the organization. | | +| firewall_policy_id | Map of firewall policy ids created in the organization. | | | org_id | Organization id dependent on module resources. | | From dc038ad71fe24bd2e78c669a8e965c2882b6977f Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Wed, 25 Nov 2020 16:21:49 +0100 Subject: [PATCH 4/4] Add tests for hierarchical firewall policy --- modules/folder/variables.tf | 6 +- tests/modules/folder/fixture/main.tf | 14 +-- tests/modules/folder/fixture/variables.tf | 20 ++++ .../folder/test_plan_firewall_policy.py | 97 +++++++++++++++++++ tests/modules/organization/fixture/main.tf | 20 ++-- .../modules/organization/fixture/variables.tf | 20 ++++ tests/modules/organization/test_plan.py | 77 ++++++++++++++- 7 files changed, 233 insertions(+), 21 deletions(-) create mode 100644 tests/modules/folder/test_plan_firewall_policy.py diff --git a/modules/folder/variables.tf b/modules/folder/variables.tf index 24c0c171..aba267e6 100644 --- a/modules/folder/variables.tf +++ b/modules/folder/variables.tf @@ -63,14 +63,12 @@ variable "firewall_policies" { target_service_accounts = list(string) target_resources = list(string) logging = bool - #preview = bool }))) default = {} } variable "firewall_policy_attachments" { description = "List of hierarchical firewall policy IDs to *attach* to this folder." - # set to avoid manual casting with toset() - type = map(string) - default = {} + type = map(string) + default = {} } diff --git a/tests/modules/folder/fixture/main.tf b/tests/modules/folder/fixture/main.tf index e9a141f9..5607b366 100644 --- a/tests/modules/folder/fixture/main.tf +++ b/tests/modules/folder/fixture/main.tf @@ -15,10 +15,12 @@ */ module "test" { - source = "../../../../modules/folder" - parent = "organizations/12345678" - name = "folder-a" - iam = var.iam - policy_boolean = var.policy_boolean - policy_list = var.policy_list + source = "../../../../modules/folder" + parent = "organizations/12345678" + name = "folder-a" + iam = var.iam + policy_boolean = var.policy_boolean + policy_list = var.policy_list + firewall_policies = var.firewall_policies + firewall_policy_attachments = var.firewall_policy_attachments } diff --git a/tests/modules/folder/fixture/variables.tf b/tests/modules/folder/fixture/variables.tf index f1cebb93..908b2cb9 100644 --- a/tests/modules/folder/fixture/variables.tf +++ b/tests/modules/folder/fixture/variables.tf @@ -33,3 +33,23 @@ variable "policy_list" { })) default = {} } + +variable "firewall_policies" { + type = map(map(object({ + description = string + direction = string + action = string + priority = number + ranges = list(string) + ports = map(list(string)) + target_service_accounts = list(string) + target_resources = list(string) + logging = bool + }))) + default = {} +} + +variable "firewall_policy_attachments" { + type = map(string) + default = {} +} diff --git a/tests/modules/folder/test_plan_firewall_policy.py b/tests/modules/folder/test_plan_firewall_policy.py new file mode 100644 index 00000000..0bb0204d --- /dev/null +++ b/tests/modules/folder/test_plan_firewall_policy.py @@ -0,0 +1,97 @@ +# Copyright 2020 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_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) == 5 + + 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/fixture/main.tf b/tests/modules/organization/fixture/main.tf index d1b0dd35..90abee75 100644 --- a/tests/modules/organization/fixture/main.tf +++ b/tests/modules/organization/fixture/main.tf @@ -15,13 +15,15 @@ */ module "test" { - source = "../../../../modules/organization" - org_id = 1234567890 - custom_roles = var.custom_roles - iam = var.iam - iam_additive = var.iam_additive - iam_additive_members = var.iam_additive_members - iam_audit_config = var.iam_audit_config - policy_boolean = var.policy_boolean - policy_list = var.policy_list + source = "../../../../modules/organization" + org_id = 1234567890 + custom_roles = var.custom_roles + iam = var.iam + iam_additive = var.iam_additive + iam_additive_members = var.iam_additive_members + iam_audit_config = var.iam_audit_config + policy_boolean = var.policy_boolean + policy_list = var.policy_list + firewall_policies = var.firewall_policies + firewall_policy_attachments = var.firewall_policy_attachments } diff --git a/tests/modules/organization/fixture/variables.tf b/tests/modules/organization/fixture/variables.tf index 7a2ddfdb..7fe88394 100644 --- a/tests/modules/organization/fixture/variables.tf +++ b/tests/modules/organization/fixture/variables.tf @@ -53,3 +53,23 @@ variable "policy_list" { })) default = {} } + +variable "firewall_policies" { + type = map(map(object({ + description = string + direction = string + action = string + priority = number + ranges = list(string) + ports = map(list(string)) + target_service_accounts = list(string) + target_resources = list(string) + logging = bool + }))) + default = {} +} + +variable "firewall_policy_attachments" { + type = map(string) + default = {} +} diff --git a/tests/modules/organization/test_plan.py b/tests/modules/organization/test_plan.py index 8d4152ba..2b42637c 100644 --- a/tests/modules/organization/test_plan.py +++ b/tests/modules/organization/test_plan.py @@ -75,8 +75,6 @@ def test_policy_list(plan_runner): '}' ) _, resources = plan_runner(FIXTURES_DIR, policy_list=policy_list) - # from pprint import pprint - # pprint(resources) assert len(resources) == 3 values = [r['values'] for r in resources] assert [r['constraint'] @@ -86,3 +84,78 @@ 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 + }]) + ])