From a5fe5e882849ff50971ea0a9f70d4feb531d4a64 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Wed, 28 Sep 2022 13:42:47 +0200 Subject: [PATCH] refactor net-address modules for 1.3 --- modules/net-address/README.md | 19 ++---- modules/net-address/main.tf | 8 +-- modules/net-address/variables.tf | 14 ++--- tests/modules/net_address/fixture/main.tf | 13 ++-- .../modules/net_address/fixture/variables.tf | 24 ++------ tests/modules/net_address/test_plan.py | 60 +++++++++---------- 6 files changed, 52 insertions(+), 86 deletions(-) diff --git a/modules/net-address/README.md b/modules/net-address/README.md index 76637ccd19..e51383489f 100644 --- a/modules/net-address/README.md +++ b/modules/net-address/README.md @@ -27,22 +27,16 @@ module "addresses" { project_id = var.project_id internal_addresses = { ilb-1 = { + purpose = "SHARED_LOADBALANCER_VIP" region = var.region subnetwork = var.subnet.self_link } ilb-2 = { + address = "10.0.0.2" region = var.region subnetwork = var.subnet.self_link } } - # optional configuration - internal_addresses_config = { - ilb-1 = { - address = null - purpose = "SHARED_LOADBALANCER_VIP" - tier = null - } - } } # tftest modules=1 resources=2 ``` @@ -89,13 +83,12 @@ module "addresses" { | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [project_id](variables.tf#L60) | Project where the addresses will be created. | string | ✓ | | +| [project_id](variables.tf#L54) | Project where the addresses will be created. | string | ✓ | | | [external_addresses](variables.tf#L17) | Map of external address regions, keyed by name. | map(string) | | {} | | [global_addresses](variables.tf#L29) | List of global addresses to create. | list(string) | | [] | -| [internal_addresses](variables.tf#L35) | Map of internal addresses to create, keyed by name. | map(object({…})) | | {} | -| [internal_addresses_config](variables.tf#L44) | Optional configuration for internal addresses, keyed by name. Unused options can be set to null. | map(object({…})) | | {} | -| [psa_addresses](variables.tf#L65) | Map of internal addresses used for Private Service Access. | map(object({…})) | | {} | -| [psc_addresses](variables.tf#L75) | Map of internal addresses used for Private Service Connect. | map(object({…})) | | {} | +| [internal_addresses](variables.tf#L35) | Map of internal addresses to create, keyed by name. | map(object({…})) | | {} | +| [psa_addresses](variables.tf#L59) | Map of internal addresses used for Private Service Access. | map(object({…})) | | {} | +| [psc_addresses](variables.tf#L69) | Map of internal addresses used for Private Service Connect. | map(object({…})) | | {} | ## Outputs diff --git a/modules/net-address/main.tf b/modules/net-address/main.tf index caab92a0c0..8619f95b6e 100644 --- a/modules/net-address/main.tf +++ b/modules/net-address/main.tf @@ -39,10 +39,10 @@ resource "google_compute_address" "internal" { address_type = "INTERNAL" region = each.value.region subnetwork = each.value.subnetwork - address = try(var.internal_addresses_config[each.key].address, null) - network_tier = try(var.internal_addresses_config[each.key].tier, null) - purpose = try(var.internal_addresses_config[each.key].purpose, null) - # labels = lookup(var.internal_address_labels, each.key, {}) + address = each.value.address + network_tier = each.value.tier + purpose = each.value.purpose + labels = coalesce(each.value.labels, {}) } resource "google_compute_global_address" "psc" { diff --git a/modules/net-address/variables.tf b/modules/net-address/variables.tf index bb3043dc8a..35093e83ac 100644 --- a/modules/net-address/variables.tf +++ b/modules/net-address/variables.tf @@ -37,16 +37,10 @@ variable "internal_addresses" { type = map(object({ region = string subnetwork = string - })) - default = {} -} - -variable "internal_addresses_config" { - description = "Optional configuration for internal addresses, keyed by name. Unused options can be set to null." - type = map(object({ - address = string - purpose = string - tier = string + address = optional(string) + labels = optional(map(string)) + purpose = optional(string) + tier = optional(string) })) default = {} } diff --git a/tests/modules/net_address/fixture/main.tf b/tests/modules/net_address/fixture/main.tf index e716ad1a45..4d41b09634 100644 --- a/tests/modules/net_address/fixture/main.tf +++ b/tests/modules/net_address/fixture/main.tf @@ -15,11 +15,10 @@ */ module "test" { - source = "../../../../modules/net-address" - external_addresses = var.external_addresses - global_addresses = var.global_addresses - internal_addresses = var.internal_addresses - internal_addresses_config = var.internal_addresses_config - psa_addresses = var.psa_addresses - project_id = var.project_id + source = "../../../../modules/net-address" + external_addresses = var.external_addresses + global_addresses = var.global_addresses + internal_addresses = var.internal_addresses + psa_addresses = var.psa_addresses + project_id = var.project_id } diff --git a/tests/modules/net_address/fixture/variables.tf b/tests/modules/net_address/fixture/variables.tf index d82728036c..ebc90814f0 100644 --- a/tests/modules/net_address/fixture/variables.tf +++ b/tests/modules/net_address/fixture/variables.tf @@ -15,29 +15,17 @@ */ variable "external_addresses" { - type = map(string) + type = any default = {} } variable "global_addresses" { - type = list(string) + type = any default = [] } variable "internal_addresses" { - type = map(object({ - region = string - subnetwork = string - })) - default = {} -} - -variable "internal_addresses_config" { - type = map(object({ - address = string - purpose = string - tier = string - })) + type = any default = {} } @@ -47,10 +35,6 @@ variable "project_id" { } variable "psa_addresses" { - type = map(object({ - address = string - network = string - prefix_length = number - })) + type = any default = {} } diff --git a/tests/modules/net_address/test_plan.py b/tests/modules/net_address/test_plan.py index 4aa03d270d..2274da6fa7 100644 --- a/tests/modules/net_address/test_plan.py +++ b/tests/modules/net_address/test_plan.py @@ -12,14 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. + def test_external_addresses(plan_runner): addresses = '{one = "europe-west1", two = "europe-west2"}' _, resources = plan_runner(external_addresses=addresses) assert [r['values']['name'] for r in resources] == ['one', 'two'] - assert set(r['values']['address_type'] - for r in resources) == set(['EXTERNAL']) - assert [r['values']['region'] - for r in resources] == ['europe-west1', 'europe-west2'] + assert set(r['values']['address_type'] for r in resources) == set( + ['EXTERNAL']) + assert [r['values']['region'] for r in resources + ] == ['europe-west1', 'europe-west2'] def test_global_addresses(plan_runner): @@ -29,42 +30,37 @@ def test_global_addresses(plan_runner): def test_internal_addresses(plan_runner): - addresses = ( - '{one = {region = "europe-west1", subnetwork = "foobar"}, ' - 'two = {region = "europe-west2", subnetwork = "foobarz"}}' - ) + addresses = ('{one = {region = "europe-west1", subnetwork = "foobar"}, ' + 'two = {region = "europe-west2", subnetwork = "foobarz"}}') _, resources = plan_runner(internal_addresses=addresses) assert [r['values']['name'] for r in resources] == ['one', 'two'] - assert set(r['values']['address_type'] - for r in resources) == set(['INTERNAL']) - assert [r['values']['region'] - for r in resources] == ['europe-west1', 'europe-west2'] + assert set(r['values']['address_type'] for r in resources) == set( + ['INTERNAL']) + assert [r['values']['region'] for r in resources + ] == ['europe-west1', 'europe-west2'] def test_internal_addresses_config(plan_runner): - addresses = ( - '{one = {region = "europe-west1", subnetwork = "foobar"}, ' - 'two = {region = "europe-west2", subnetwork = "foobarz"}}' - ) - config = ( - '{one = {address = "10.0.0.2", purpose = "SHARED_LOADBALANCER_VIP", ' - 'tier=null}}' - ) - _, resources = plan_runner(internal_addresses=addresses, - internal_addresses_config=config) + addresses = '''{ + one = { + region = "europe-west1" + subnetwork = "foobar" + address = "10.0.0.2" + purpose = "SHARED_LOADBALANCER_VIP" + }, + two = {region = "europe-west2", subnetwork = "foobarz"} + }''' + _, resources = plan_runner(internal_addresses=addresses) assert [r['values']['name'] for r in resources] == ['one', 'two'] - assert set(r['values']['address_type'] - for r in resources) == set(['INTERNAL']) - assert [r['values'].get('address') - for r in resources] == ['10.0.0.2', None] - assert [r['values'].get('purpose') - for r in resources] == ['SHARED_LOADBALANCER_VIP', None] + assert set(r['values']['address_type'] for r in resources) == set( + ['INTERNAL']) + assert [r['values'].get('address') for r in resources] == ['10.0.0.2', None] + assert [r['values'].get('purpose') for r in resources + ] == ['SHARED_LOADBALANCER_VIP', None] def test_psa_config(plan_runner): psa_addresses = '{cloudsql-mysql={address="10.199.0.0", network="foobar", prefix_length = 24}}' _, resources = plan_runner(psa_addresses=psa_addresses) - assert set(r['values']['purpose'] - for r in resources) == set(['VPC_PEERING']) - assert set(r['values']['address'] - for r in resources) == set(['10.199.0.0']) + assert set(r['values']['purpose'] for r in resources) == set(['VPC_PEERING']) + assert set(r['values']['address'] for r in resources) == set(['10.199.0.0'])