From 1abfdacd56acfe901774d6b8dff6288f75ae25a6 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Mon, 11 Jul 2022 21:11:52 +0200 Subject: [PATCH 1/2] Avoid setting empty IAM binding in subnet factory (#731) * avoid setting empty IAM binding in subnet factory * fix tests --- modules/net-vpc/subnets.tf | 9 +++---- tests/modules/net_vpc/test_plan_subnets.py | 28 +++++++++++----------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/modules/net-vpc/subnets.tf b/modules/net-vpc/subnets.tf index 204b69d8..02576e8e 100644 --- a/modules/net-vpc/subnets.tf +++ b/modules/net-vpc/subnets.tf @@ -30,9 +30,9 @@ locals { subnet = k role = "roles/compute.networkUser" members = concat( - formatlist("group:%s", try(v.iam_groups, [])), - formatlist("user:%s", try(v.iam_users, [])), - formatlist("serviceAccount:%s", try(v.iam_service_accounts, [])) + formatlist("group:%s", lookup(v, "iam_groups", [])), + formatlist("user:%s", lookup(v, "iam_users", [])), + formatlist("serviceAccount:%s", lookup(v, "iam_service_accounts", [])) ) } ] @@ -73,7 +73,8 @@ locals { local._factory_descriptions, var.subnet_descriptions ) subnet_iam_members = concat( - local._factory_iam_members, local._subnet_iam_members + [for k in local._factory_iam_members : k if length(k.members) > 0], + local._subnet_iam_members ) subnet_flow_logs = merge( local._factory_flow_logs, local._subnet_flow_logs diff --git a/tests/modules/net_vpc/test_plan_subnets.py b/tests/modules/net_vpc/test_plan_subnets.py index 0ff9d999..992d4a98 100644 --- a/tests/modules/net_vpc/test_plan_subnets.py +++ b/tests/modules/net_vpc/test_plan_subnets.py @@ -20,8 +20,7 @@ _VAR_SUBNETS = ( ' secondary_ip_range=null},' '{name = "c", region = "europe-west1", ip_cidr_range = "10.0.2.0/24",' ' secondary_ip_range={a="192.168.0.0/24", b="192.168.1.0/24"}},' - ']' -) + ']') _VAR_DATA_FOLDER = "data" @@ -29,9 +28,10 @@ _VAR_DATA_FOLDER = "data" def test_subnet_factory(plan_runner): "Test subnet factory." _, resources = plan_runner(data_folder=_VAR_DATA_FOLDER) - assert len(resources) == 5 - subnets = [r['values'] - for r in resources if r['type'] == 'google_compute_subnetwork'] + assert len(resources) == 3 + subnets = [ + r['values'] for r in resources if r['type'] == 'google_compute_subnetwork' + ] assert {s['name'] for s in subnets} == {'factory-subnet', 'factory-subnet2'} assert {len(s['secondary_ip_range']) for s in subnets} == {0, 1} @@ -40,8 +40,9 @@ def test_subnets_simple(plan_runner): "Test subnets variable." _, resources = plan_runner(subnets=_VAR_SUBNETS) assert len(resources) == 4 - subnets = [r['values'] - for r in resources if r['type'] == 'google_compute_subnetwork'] + subnets = [ + r['values'] for r in resources if r['type'] == 'google_compute_subnetwork' + ] assert {s['name'] for s in subnets} == {'a', 'b', 'c'} assert {len(s['secondary_ip_range']) for s in subnets} == {0, 0, 2} @@ -51,11 +52,9 @@ def test_subnet_log_configs(plan_runner): log_config = '{"europe-west1/a" = { flow_sampling = 0.1 }}' log_config_defaults = ( '{aggregation_interval = "INTERVAL_10_MIN", flow_sampling = 0.5, ' - 'metadata = "INCLUDE_ALL_METADATA"}' - ) + 'metadata = "INCLUDE_ALL_METADATA"}') subnet_flow_logs = '{"europe-west1/a"=true, "europe-west1/b"=true}' - _, resources = plan_runner(subnets=_VAR_SUBNETS, - log_configs=log_config, + _, resources = plan_runner(subnets=_VAR_SUBNETS, log_configs=log_config, log_config_defaults=log_config_defaults, subnet_flow_logs=subnet_flow_logs) assert len(resources) == 4 @@ -63,9 +62,10 @@ def test_subnet_log_configs(plan_runner): for r in resources: if r['type'] != 'google_compute_subnetwork': continue - flow_logs[r['values']['name']] = [{key: config[key] for key in config.keys() - & {'aggregation_interval', 'flow_sampling', 'metadata'}} - for config in r['values']['log_config']] + flow_logs[r['values']['name']] = [{ + key: config[key] for key in config.keys() & + {'aggregation_interval', 'flow_sampling', 'metadata'} + } for config in r['values']['log_config']] assert flow_logs == { # enable, override one default option 'a': [{ From d9ea825ea56bb9e8e3971591460e987a528d6e2e Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Tue, 12 Jul 2022 09:23:23 +0200 Subject: [PATCH 2/2] add support for topic message duration (#732) --- modules/pubsub/README.md | 13 +++++++------ modules/pubsub/main.tf | 9 +++++---- modules/pubsub/variables.tf | 6 ++++++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/modules/pubsub/README.md b/modules/pubsub/README.md index a321c662..74d0b8ac 100644 --- a/modules/pubsub/README.md +++ b/modules/pubsub/README.md @@ -94,17 +94,18 @@ module "pubsub" { | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [name](variables.tf#L62) | PubSub topic name. | string | ✓ | | -| [project_id](variables.tf#L67) | Project used for resources. | string | ✓ | | +| [name](variables.tf#L68) | PubSub topic name. | string | ✓ | | +| [project_id](variables.tf#L73) | Project used for resources. | string | ✓ | | | [dead_letter_configs](variables.tf#L17) | Per-subscription dead letter policy configuration. | map(object({…})) | | {} | | [defaults](variables.tf#L26) | Subscription defaults for options. | object({…}) | | {…} | | [iam](variables.tf#L44) | IAM bindings for topic in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | | [kms_key](variables.tf#L50) | KMS customer managed encryption key. | string | | null | | [labels](variables.tf#L56) | Labels. | map(string) | | {} | -| [push_configs](variables.tf#L72) | Push subscription configurations. | map(object({…})) | | {} | -| [regions](variables.tf#L85) | List of regions used to set persistence policy. | list(string) | | [] | -| [subscription_iam](variables.tf#L91) | IAM bindings for subscriptions in {SUBSCRIPTION => {ROLE => [MEMBERS]}} format. | map(map(list(string))) | | {} | -| [subscriptions](variables.tf#L97) | Topic subscriptions. Also define push configs for push subscriptions. If options is set to null subscription defaults will be used. Labels default to topic labels if set to null. | map(object({…})) | | {} | +| [message_retention_duration](variables.tf#L62) | Minimum duration to retain a message after it is published to the topic. | string | | null | +| [push_configs](variables.tf#L78) | Push subscription configurations. | map(object({…})) | | {} | +| [regions](variables.tf#L91) | List of regions used to set persistence policy. | list(string) | | [] | +| [subscription_iam](variables.tf#L97) | IAM bindings for subscriptions in {SUBSCRIPTION => {ROLE => [MEMBERS]}} format. | map(map(list(string))) | | {} | +| [subscriptions](variables.tf#L103) | Topic subscriptions. Also define push configs for push subscriptions. If options is set to null subscription defaults will be used. Labels default to topic labels if set to null. | map(object({…})) | | {} | ## Outputs diff --git a/modules/pubsub/main.tf b/modules/pubsub/main.tf index af08de38..2645de02 100644 --- a/modules/pubsub/main.tf +++ b/modules/pubsub/main.tf @@ -36,10 +36,11 @@ locals { } resource "google_pubsub_topic" "default" { - project = var.project_id - name = var.name - kms_key_name = var.kms_key - labels = var.labels + project = var.project_id + name = var.name + kms_key_name = var.kms_key + labels = var.labels + message_retention_duration = var.message_retention_duration dynamic "message_storage_policy" { for_each = length(var.regions) > 0 ? [var.regions] : [] diff --git a/modules/pubsub/variables.tf b/modules/pubsub/variables.tf index 592d8009..009a12fb 100644 --- a/modules/pubsub/variables.tf +++ b/modules/pubsub/variables.tf @@ -59,6 +59,12 @@ variable "labels" { default = {} } +variable "message_retention_duration" { + description = "Minimum duration to retain a message after it is published to the topic." + type = string + default = null +} + variable "name" { description = "PubSub topic name." type = string