From 174de3a087ed8d2fed06c31db40e10cc535269b6 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Mon, 13 Dec 2021 08:41:02 +0100 Subject: [PATCH 1/3] 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 2/3] 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 3/3] 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 = [] } }