From 4124ef41fc9b994c22a798269b51f3fb5be42e41 Mon Sep 17 00:00:00 2001 From: Luca Prete Date: Fri, 18 Nov 2022 15:56:28 +0100 Subject: [PATCH] Add network tags support to the organization module (#979) --- modules/organization/README.md | 63 ++++++++++++++----- modules/organization/outputs.tf | 21 ++++++- modules/organization/tags.tf | 31 +++++---- modules/organization/variables.tf | 53 ++++++++++++---- tests/modules/organization/fixture/main.tf | 1 + .../fixture/test.network_tags.tfvars | 5 ++ .../fixture/test.resource_tags.tfvars | 38 +++++++++++ .../modules/organization/fixture/variables.tf | 5 ++ tests/modules/organization/test_plan_tags.py | 59 +++++------------ 9 files changed, 185 insertions(+), 91 deletions(-) create mode 100644 tests/modules/organization/fixture/test.network_tags.tfvars create mode 100644 tests/modules/organization/fixture/test.resource_tags.tfvars diff --git a/modules/organization/README.md b/modules/organization/README.md index d81611ee..d2e8ef31 100644 --- a/modules/organization/README.md +++ b/modules/organization/README.md @@ -375,7 +375,7 @@ module "org" { "roles/resourcemanager.tagAdmin" = ["group:admins@example.com"] } values = { - dev = null + dev = {} prod = { description = "Environment: production." iam = { @@ -393,6 +393,34 @@ module "org" { # tftest modules=1 resources=7 ``` +You can also define network tags, through a dedicated variable *network_tags*: + +```hcl +module "org" { + source = "./fabric/modules/organization" + organization_id = var.organization_id + network_tags = { + net-environment = { + description = "This is a network tag." + network = "my_project/my_vpc" + iam = { + "roles/resourcemanager.tagAdmin" = ["group:admins@example.com"] + } + values = { + dev = null + prod = { + description = "Environment: production." + iam = { + "roles/resourcemanager.tagUser" = ["user:user1@example.com"] + } + } + } + } + } +} +# tftest modules=1 resources=5 +``` + @@ -415,7 +443,7 @@ module "org" { | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [organization_id](variables.tf#L225) | Organization id in organizations/nnnnnn format. | string | ✓ | | +| [organization_id](variables.tf#L246) | Organization id in organizations/nnnnnn format. | string | ✓ | | | [contacts](variables.tf#L17) | 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](variables.tf#L24) | Map of role name => list of permissions to create in this project. | map(list(string)) | | {} | | [firewall_policies](variables.tf#L31) | Hierarchical firewall policy rules created in the organization. | map(map(object({…}))) | | {} | @@ -430,24 +458,27 @@ module "org" { | [iam_bindings_authoritative](variables.tf#L116) | 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](variables.tf#L122) | Logging exclusions for this organization in the form {NAME -> FILTER}. | map(string) | | {} | | [logging_sinks](variables.tf#L129) | Logging sinks to create for the organization. | map(object({…})) | | {} | -| [org_policies](variables.tf#L159) | Organization policies applied to this organization keyed by policy name. | map(object({…})) | | {} | -| [org_policies_data_path](variables.tf#L199) | Path containing org policies in YAML format. | string | | null | -| [org_policy_custom_constraints](variables.tf#L205) | Organization policiy custom constraints keyed by constraint name. | map(object({…})) | | {} | -| [org_policy_custom_constraints_data_path](variables.tf#L219) | Path containing org policy custom constraints in YAML format. | string | | null | -| [tag_bindings](variables.tf#L235) | Tag bindings for this organization, in key => tag value id format. | map(string) | | null | -| [tags](variables.tf#L241) | Tags by key name. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | null | +| [network_tags](variables.tf#L159) | Network tags by key name. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | +| [org_policies](variables.tf#L180) | Organization policies applied to this organization keyed by policy name. | map(object({…})) | | {} | +| [org_policies_data_path](variables.tf#L220) | Path containing org policies in YAML format. | string | | null | +| [org_policy_custom_constraints](variables.tf#L226) | Organization policiy custom constraints keyed by constraint name. | map(object({…})) | | {} | +| [org_policy_custom_constraints_data_path](variables.tf#L240) | Path containing org policy custom constraints in YAML format. | string | | null | +| [tag_bindings](variables.tf#L275) | Tag bindings for this organization, in key => tag value id format. | map(string) | | null | +| [tags](variables.tf#L255) | Tags by key name. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | ## Outputs | name | description | sensitive | |---|---|:---:| -| [custom_role_id](outputs.tf#L18) | Map of custom role IDs created in the organization. | | -| [custom_roles](outputs.tf#L31) | Map of custom roles resources created in the organization. | | -| [firewall_policies](outputs.tf#L36) | Map of firewall policy resources created in the organization. | | -| [firewall_policy_id](outputs.tf#L41) | Map of firewall policy ids created in the organization. | | -| [organization_id](outputs.tf#L46) | Organization id dependent on module resources. | | -| [sink_writer_identities](outputs.tf#L63) | Writer identities created for each sink. | | -| [tag_keys](outputs.tf#L71) | Tag key resources. | | -| [tag_values](outputs.tf#L78) | Tag value resources. | | +| [custom_role_id](outputs.tf#L17) | Map of custom role IDs created in the organization. | | +| [custom_roles](outputs.tf#L30) | Map of custom roles resources created in the organization. | | +| [firewall_policies](outputs.tf#L35) | Map of firewall policy resources created in the organization. | | +| [firewall_policy_id](outputs.tf#L40) | Map of firewall policy ids created in the organization. | | +| [network_tag_keys](outputs.tf#L45) | Tag key resources. | | +| [network_tag_values](outputs.tf#L52) | Tag value resources. | | +| [organization_id](outputs.tf#L60) | Organization id dependent on module resources. | | +| [sink_writer_identities](outputs.tf#L77) | Writer identities created for each sink. | | +| [tag_keys](outputs.tf#L85) | Tag key resources. | | +| [tag_values](outputs.tf#L92) | Tag value resources. | | diff --git a/modules/organization/outputs.tf b/modules/organization/outputs.tf index 198b3c8d..3617bafb 100644 --- a/modules/organization/outputs.tf +++ b/modules/organization/outputs.tf @@ -14,7 +14,6 @@ * limitations under the License. */ - output "custom_role_id" { description = "Map of custom role IDs created in the organization." value = { @@ -43,6 +42,21 @@ output "firewall_policy_id" { value = { for k, v in google_compute_firewall_policy.policy : k => v.id } } +output "network_tag_keys" { + description = "Tag key resources." + value = { + for k, v in google_tags_tag_key.default : k => v if v.purpose != null + } +} + +output "network_tag_values" { + description = "Tag value resources." + value = { + for k, v in google_tags_tag_value.default + : k => v if google_tags_tag_key.default[split("/", k)[0]].purpose != null + } +} + output "organization_id" { description = "Organization id dependent on module resources." value = var.organization_id @@ -71,13 +85,14 @@ output "sink_writer_identities" { output "tag_keys" { description = "Tag key resources." value = { - for k, v in google_tags_tag_key.default : k => v + for k, v in google_tags_tag_key.default : k => v if v.purpose == null } } output "tag_values" { description = "Tag value resources." value = { - for k, v in google_tags_tag_value.default : k => v + for k, v in google_tags_tag_value.default + : k => v if google_tags_tag_key.default[split("/", k)[0]].purpose == null } } diff --git a/modules/organization/tags.tf b/modules/organization/tags.tf index 7aa94cd1..544b8989 100644 --- a/modules/organization/tags.tf +++ b/modules/organization/tags.tf @@ -55,10 +55,7 @@ locals { tag_values_iam = { for t in local._tag_values_iam : "${t.key}:${t.role}" => t } - tags = { - for k, v in coalesce(var.tags, {}) : - k => v == null ? { description = null, iam = {}, values = null } : v - } + tags = merge(var.tags, var.network_tags) tags_iam = { for t in local._tags_iam : "${t.tag}:${t.role}" => t } @@ -67,13 +64,16 @@ locals { # keys resource "google_tags_tag_key" "default" { - for_each = local.tags - parent = var.organization_id - short_name = each.key - description = coalesce( - each.value.description, - "Managed by the Terraform organization module." + for_each = local.tags + parent = var.organization_id + purpose = ( + lookup(each.value, "network", null) == null ? null : "GCE_FIREWALL" ) + purpose_data = ( + lookup(each.value, "network", null) == null ? null : { network = each.value.network } + ) + short_name = each.key + description = each.value.description depends_on = [ google_organization_iam_binding.authoritative, google_organization_iam_member.additive, @@ -93,13 +93,10 @@ resource "google_tags_tag_key_iam_binding" "default" { # values resource "google_tags_tag_value" "default" { - for_each = local.tag_values - parent = google_tags_tag_key.default[each.value.tag].id - short_name = each.value.name - description = coalesce( - each.value.description, - "Managed by the Terraform organization module." - ) + for_each = local.tag_values + parent = google_tags_tag_key.default[each.value.tag].id + short_name = each.value.name + description = each.value.description } resource "google_tags_tag_value_iam_binding" "default" { diff --git a/modules/organization/variables.tf b/modules/organization/variables.tf index f8a949f5..855c84f6 100644 --- a/modules/organization/variables.tf +++ b/modules/organization/variables.tf @@ -156,6 +156,27 @@ variable "logging_sinks" { } } +variable "network_tags" { + description = "Network tags by key name. The `iam` attribute behaves like the similarly named one at module level." + type = map(object({ + description = optional(string, "Managed by the Terraform organization module.") + iam = optional(map(list(string)), {}) + network = string # project_id/vpc_name + values = optional(map(object({ + description = optional(string, "Managed by the Terraform organization module.") + iam = optional(map(list(string)), {}) + })), {}) + })) + nullable = false + default = {} + validation { + condition = alltrue([ + for k, v in var.network_tags : v != null + ]) + error_message = "Use an empty map instead of null as value." + } +} + variable "org_policies" { description = "Organization policies applied to this organization keyed by policy name." type = map(object({ @@ -231,22 +252,28 @@ variable "organization_id" { } } +variable "tags" { + description = "Tags by key name. The `iam` attribute behaves like the similarly named one at module level." + type = map(object({ + description = optional(string, "Managed by the Terraform organization module.") + iam = optional(map(list(string)), {}) + values = optional(map(object({ + description = optional(string, "Managed by the Terraform organization module.") + iam = optional(map(list(string)), {}) + })), {}) + })) + nullable = false + default = {} + validation { + condition = alltrue([ + for k, v in var.tags : v != null + ]) + error_message = "Use an empty map instead of null as value." + } +} variable "tag_bindings" { description = "Tag bindings for this organization, in key => tag value id format." type = map(string) default = null } - -variable "tags" { - description = "Tags by key name. The `iam` attribute behaves like the similarly named one at module level." - type = map(object({ - description = string - iam = map(list(string)) - values = map(object({ - description = string - iam = map(list(string)) - })) - })) - default = null -} diff --git a/tests/modules/organization/fixture/main.tf b/tests/modules/organization/fixture/main.tf index a620542e..feb49f11 100644 --- a/tests/modules/organization/fixture/main.tf +++ b/tests/modules/organization/fixture/main.tf @@ -28,6 +28,7 @@ module "test" { iam_audit_config = var.iam_audit_config logging_sinks = var.logging_sinks logging_exclusions = var.logging_exclusions + network_tags = var.network_tags org_policies = var.org_policies org_policies_data_path = var.org_policies_data_path org_policy_custom_constraints = var.org_policy_custom_constraints diff --git a/tests/modules/organization/fixture/test.network_tags.tfvars b/tests/modules/organization/fixture/test.network_tags.tfvars new file mode 100644 index 00000000..6c5c3ccd --- /dev/null +++ b/tests/modules/organization/fixture/test.network_tags.tfvars @@ -0,0 +1,5 @@ +network_tags = { + net_environment = { + network = "foobar" + } +} diff --git a/tests/modules/organization/fixture/test.resource_tags.tfvars b/tests/modules/organization/fixture/test.resource_tags.tfvars new file mode 100644 index 00000000..42cf7e0f --- /dev/null +++ b/tests/modules/organization/fixture/test.resource_tags.tfvars @@ -0,0 +1,38 @@ +tags = { + foo = {} + bar = { + description = null + iam = null + values = null + } + foobar = { + description = "Foobar tag." + iam = { + "roles/resourcemanager.tagAdmin" = [ + "user:user1@example.com", "user:user2@example.com" + ] + } + values = { + one = null + two = { + description = "Foobar 2." + iam = { + "roles/resourcemanager.tagViewer" = [ + "user:user3@example.com" + ] + } + } + three = { + description = "Foobar 3." + iam = { + "roles/resourcemanager.tagViewer" = [ + "user:user3@example.com" + ] + "roles/resourcemanager.tagAdmin" = [ + "user:user4@example.com" + ] + } + } + } + } +} diff --git a/tests/modules/organization/fixture/variables.tf b/tests/modules/organization/fixture/variables.tf index c4efa8fb..35c777c4 100644 --- a/tests/modules/organization/fixture/variables.tf +++ b/tests/modules/organization/fixture/variables.tf @@ -69,6 +69,11 @@ variable "logging_exclusions" { default = {} } +variable "network_tags" { + type = any + default = null +} + variable "org_policies" { type = any default = {} diff --git a/tests/modules/organization/test_plan_tags.py b/tests/modules/organization/test_plan_tags.py index 849502f4..bce2208a 100644 --- a/tests/modules/organization/test_plan_tags.py +++ b/tests/modules/organization/test_plan_tags.py @@ -13,47 +13,9 @@ # limitations under the License. -def test_keys(plan_runner): - 'Test tag keys.' - tags = '''{ - foo = null - bar = { - description = null - iam = null - values = null - } - foobar = { - description = "Foobar tag." - iam = { - "roles/resourcemanager.tagAdmin" = [ - "user:user1@example.com", "user:user2@example.com" - ] - } - values = { - one = null - two = { - description = "Foobar 2." - iam = { - "roles/resourcemanager.tagViewer" = [ - "user:user3@example.com" - ] - } - } - three = { - description = "Foobar 3." - iam = { - "roles/resourcemanager.tagViewer" = [ - "user:user3@example.com" - ] - "roles/resourcemanager.tagAdmin" = [ - "user:user4@example.com" - ] - } - } - } - } - }''' - _, resources = plan_runner(tags=tags) +def test_resource_tags(plan_runner): + 'Test resource tags.' + _, resources = plan_runner(tf_var_file='test.resource_tags.tfvars') assert len(resources) == 10 resource_values = {} for r in resources: @@ -64,12 +26,25 @@ def test_keys(plan_runner): r['role'] for r in resource_values['google_tags_tag_value_iam_binding'] ] expected = [ - 'roles/resourcemanager.tagAdmin', 'roles/resourcemanager.tagViewer', + 'roles/resourcemanager.tagAdmin', + 'roles/resourcemanager.tagViewer', 'roles/resourcemanager.tagViewer' ] assert result == expected +def test_network_tags(plan_runner): + 'Test network tags.' + _, resources = plan_runner(tf_var_file='test.network_tags.tfvars') + assert len(resources) == 1 + resource_values = {} + for r in resources: + resource_values.setdefault(r['type'], []).append(r['values']) + google_tags_tag_key = resource_values['google_tags_tag_key'][0] + assert google_tags_tag_key['purpose'] == "GCE_FIREWALL" + assert google_tags_tag_key['purpose_data']['network'] == "foobar" + + def test_bindings(plan_runner): 'Test tag bindings.' tag_bindings = '{foo = "tagValues/123456789012"}'