Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use for_each instead of count #3

Merged
merged 4 commits into from
Sep 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,14 @@ Each environment configuration contains the following fields:
Specify either `route_to` or `route_to_cidr_blocks`. `route_to_cidr_blocks` supersedes `route_to`.
- `transit_gateway_vpc_attachment_id` - An existing Transit Gateway Attachment ID. If provided, the module will use it instead of creating a new one.

You now have the option to have Terraform manage route table entries by key, whereas previously they were only managed by index. The advantage
of managing them by key is that if a route table ID or destination CIDR changes, only that entry is affected, whereas when managed by index,
all the entries after the first affected index may be destroyed and re-created at a different index. The reason this is left as an option,
with the default being to manage the entries by index, is that if you are creating the VPC or subnets at the same time you are creating
the Transit Gateway, then Terraform will not be able to generate the keys during the plan phase and the plan will fail with the error
`The "for_each" value depends on resource attributes that cannot be determined until apply...`. We recommend setting `route_keys_enabled` to
`true` unless you get this error, in which case you must leave it set to its default value of `false`.

__NOTE:__ This module requires Terraform 0.13 and newer since it uses [module expansion with `for_each`](https://www.hashicorp.com/blog/announcing-hashicorp-terraform-0-13/).

## Usage
Expand Down Expand Up @@ -238,6 +246,7 @@ Available targets:
| ram\_principal | The principal to associate with the resource share. Possible values are an AWS account ID, an Organization ARN, or an Organization Unit ARN. If this is not provided and `ram_resource_share_enabled` is set to `true`, the Organization ARN will be used | `string` | `null` | no |
| ram\_resource\_share\_enabled | Whether to enable sharing the Transit Gateway with the Organization using Resource Access Manager (RAM) | `bool` | `false` | no |
| regex\_replace\_chars | Regex to replace chars with empty string in `namespace`, `environment`, `stage` and `name`.<br>If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no |
| route\_keys\_enabled | If true, Terraform will use keys to label routes, preventing unnecessary changes,<br>but this requires that the VPCs and subnets already exist before using this module.<br>If false, Terraform will use numbers to label routes, and a single change may<br>cascade to a long list of changes because the index or order has changed, but<br>this will work when the `true` setting generates the error `The "for_each" value depends on resource attributes...` | `bool` | `false` | no |
| stage | Stage, e.g. 'prod', 'staging', 'dev', OR 'source', 'build', 'test', 'deploy', 'release' | `string` | `null` | no |
| tags | Additional tags (e.g. `map('BusinessUnit','XYZ')` | `map(string)` | `{}` | no |
| vpc\_attachment\_dns\_support | Whether resource attachments automatically propagate routes to the default propagation route table. Valid values: `disable`, `enable`. Default value: `enable` | `string` | `"enable"` | no |
Expand Down
8 changes: 8 additions & 0 deletions README.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,14 @@ introduction: |-
Specify either `route_to` or `route_to_cidr_blocks`. `route_to_cidr_blocks` supersedes `route_to`.
- `transit_gateway_vpc_attachment_id` - An existing Transit Gateway Attachment ID. If provided, the module will use it instead of creating a new one.

You now have the option to have Terraform manage route table entries by key, whereas previously they were only managed by index. The advantage
of managing them by key is that if a route table ID or destination CIDR changes, only that entry is affected, whereas when managed by index,
all the entries after the first affected index may be destroyed and re-created at a different index. The reason this is left as an option,
with the default being to manage the entries by index, is that if you are creating the VPC or subnets at the same time you are creating
the Transit Gateway, then Terraform will not be able to generate the keys during the plan phase and the plan will fail with the error
`The "for_each" value depends on resource attributes that cannot be determined until apply...`. We recommend setting `route_keys_enabled` to
`true` unless you get this error, in which case you must leave it set to its default value of `false`.

__NOTE:__ This module requires Terraform 0.13 and newer since it uses [module expansion with `for_each`](https://www.hashicorp.com/blog/announcing-hashicorp-terraform-0-13/).


Expand Down
1 change: 1 addition & 0 deletions docs/terraform.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
| ram\_principal | The principal to associate with the resource share. Possible values are an AWS account ID, an Organization ARN, or an Organization Unit ARN. If this is not provided and `ram_resource_share_enabled` is set to `true`, the Organization ARN will be used | `string` | `null` | no |
| ram\_resource\_share\_enabled | Whether to enable sharing the Transit Gateway with the Organization using Resource Access Manager (RAM) | `bool` | `false` | no |
| regex\_replace\_chars | Regex to replace chars with empty string in `namespace`, `environment`, `stage` and `name`.<br>If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no |
| route\_keys\_enabled | If true, Terraform will use keys to label routes, preventing unnecessary changes,<br>but this requires that the VPCs and subnets already exist before using this module.<br>If false, Terraform will use numbers to label routes, and a single change may<br>cascade to a long list of changes because the index or order has changed, but<br>this will work when the `true` setting generates the error `The "for_each" value depends on resource attributes...` | `bool` | `false` | no |
| stage | Stage, e.g. 'prod', 'staging', 'dev', OR 'source', 'build', 'test', 'deploy', 'release' | `string` | `null` | no |
| tags | Additional tags (e.g. `map('BusinessUnit','XYZ')` | `map(string)` | `{}` | no |
| vpc\_attachment\_dns\_support | Whether resource attachments automatically propagate routes to the default propagation route table. Valid values: `disable`, `enable`. Default value: `enable` | `string` | `"enable"` | no |
Expand Down
44 changes: 33 additions & 11 deletions main.tf
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
locals {
transit_gateway_id = var.existing_transit_gateway_id != null && var.existing_transit_gateway_id != "" ? var.existing_transit_gateway_id : try(aws_ec2_transit_gateway.default[0].id, "")
transit_gateway_route_table_id = var.existing_transit_gateway_route_table_id != null && var.existing_transit_gateway_route_table_id != "" ? var.existing_transit_gateway_route_table_id : try(aws_ec2_transit_gateway_route_table.default[0].id, "")
transit_gateway_id = var.existing_transit_gateway_id != null && var.existing_transit_gateway_id != "" ? var.existing_transit_gateway_id : (
var.create_transit_gateway ? aws_ec2_transit_gateway.default[0].id : null
)
transit_gateway_route_table_id = var.existing_transit_gateway_route_table_id != null && var.existing_transit_gateway_route_table_id != "" ? var.existing_transit_gateway_route_table_id : (
var.create_transit_gateway_route_table ? aws_ec2_transit_gateway_route_table.default[0].id : null
)
}

resource "aws_ec2_transit_gateway" "default" {
Expand All @@ -20,16 +24,33 @@ resource "aws_ec2_transit_gateway_route_table" "default" {
tags = module.this.tags
}

# Need to find out if VPC is in same account as Transit Gateway.
# See resource "aws_ec2_transit_gateway_vpc_attachment" below.
data "aws_ec2_transit_gateway" "this" {
id = local.transit_gateway_id
}

data "aws_vpc" "default" {
for_each = var.create_transit_gateway_vpc_attachment && var.config != null ? var.config : {}
id = each.value["vpc_id"]
}

resource "aws_ec2_transit_gateway_vpc_attachment" "default" {
for_each = var.create_transit_gateway_vpc_attachment && var.config != null ? var.config : {}
transit_gateway_id = local.transit_gateway_id
vpc_id = each.value["vpc_id"]
subnet_ids = each.value["subnet_ids"]
dns_support = var.vpc_attachment_dns_support
ipv6_support = var.vpc_attachment_ipv6_support
tags = module.this.tags
transit_gateway_default_route_table_association = false
transit_gateway_default_route_table_propagation = false
for_each = var.create_transit_gateway_vpc_attachment && var.config != null ? var.config : {}
transit_gateway_id = local.transit_gateway_id
vpc_id = each.value["vpc_id"]
subnet_ids = each.value["subnet_ids"]
dns_support = var.vpc_attachment_dns_support
ipv6_support = var.vpc_attachment_ipv6_support
tags = module.this.tags

# transit_gateway_default_route_table_association and transit_gateway_default_route_table_propagation
# must be set to `false` if the VPC is in the same account as the Transit Gateway, and `null` otherwise
# https://github.com/terraform-providers/terraform-provider-aws/issues/13512
# https://github.com/terraform-providers/terraform-provider-aws/issues/8383
# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ec2_transit_gateway_vpc_attachment
transit_gateway_default_route_table_association = data.aws_ec2_transit_gateway.this.owner_id == data.aws_vpc.default[each.key].owner_id ? false : null
transit_gateway_default_route_table_propagation = data.aws_ec2_transit_gateway.this.owner_id == data.aws_vpc.default[each.key].owner_id ? false : null
}

# Allow traffic from the VPC attachments to the Transit Gateway
Expand Down Expand Up @@ -67,4 +88,5 @@ module "subnet_route" {
transit_gateway_id = local.transit_gateway_id
route_table_ids = each.value["subnet_route_table_ids"] != null ? each.value["subnet_route_table_ids"] : []
destination_cidr_blocks = each.value["route_to_cidr_blocks"] != null ? each.value["route_to_cidr_blocks"] : ([for i in setintersection(keys(var.config), (each.value["route_to"] != null ? each.value["route_to"] : [])) : var.config[i]["vpc_cidr"]])
route_keys_enabled = var.route_keys_enabled
}
18 changes: 13 additions & 5 deletions modules/subnet_route/main.tf
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
locals {
route_config_provided = var.route_table_ids != null && length(var.route_table_ids) > 0 && var.destination_cidr_blocks != null && length(var.destination_cidr_blocks) > 0
route_config = local.route_config_provided ? [for i in setproduct(var.route_table_ids, var.destination_cidr_blocks) : i] : []
route_config_list = local.route_config_provided ? [for i in setproduct(var.route_table_ids, var.destination_cidr_blocks) : i] : []
route_config_map = local.route_config_provided ? { for i in local.route_config_list : format("%v:%v", i[0], i[1]) => i } : {}
}

resource "aws_route" "default" {
count = length(local.route_config)
resource "aws_route" "keys" {
for_each = var.route_keys_enabled ? local.route_config_map : {}
transit_gateway_id = var.transit_gateway_id
route_table_id = local.route_config[count.index][0]
destination_cidr_block = local.route_config[count.index][1]
route_table_id = each.value[0]
destination_cidr_block = each.value[1]
}

resource "aws_route" "count" {
count = var.route_keys_enabled ? 0 : length(local.route_config_list)
transit_gateway_id = var.transit_gateway_id
route_table_id = local.route_config_list[count.index][0]
destination_cidr_block = local.route_config_list[count.index][1]
}
2 changes: 1 addition & 1 deletion modules/subnet_route/outputs.tf
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
output "subnet_route_ids" {
value = try(aws_route.default[*].id, [])
value = compact(concat(values(aws_route.keys)[*].id, aws_route.count[*].id))
description = "Subnet route identifiers combined with destinations"
}
5 changes: 5 additions & 0 deletions modules/subnet_route/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,8 @@ variable "destination_cidr_blocks" {
type = list(string)
description = "Destination CIDR blocks"
}

variable "route_keys_enabled" {
type = bool
default = false
}
11 changes: 7 additions & 4 deletions modules/transit_gateway_route/main.tf
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
locals {
route_config = { for rc in var.route_config : format("%v%v", rc.destination_cidr_block, rc.blackhole ? ":bh" : "") => rc }
}
resource "aws_ec2_transit_gateway_route" "default" {
count = length(var.route_config)
blackhole = var.route_config[count.index]["blackhole"]
destination_cidr_block = var.route_config[count.index]["destination_cidr_block"]
transit_gateway_attachment_id = var.route_config[count.index]["blackhole"] == true ? null : var.transit_gateway_attachment_id
for_each = local.route_config
blackhole = each.value["blackhole"]
destination_cidr_block = each.value["destination_cidr_block"]
transit_gateway_attachment_id = each.value["blackhole"] ? null : var.transit_gateway_attachment_id
transit_gateway_route_table_id = var.transit_gateway_route_table_id
}
2 changes: 1 addition & 1 deletion modules/transit_gateway_route/outputs.tf
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
output "transit_gateway_route_ids" {
value = try(aws_ec2_transit_gateway_route.default[*].id, [])
value = values(aws_ec2_transit_gateway_route.default)[*].id
description = "Transit Gateway route identifiers combined with destinations"
}
12 changes: 12 additions & 0 deletions variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,15 @@ variable "create_transit_gateway_route_table_association_and_propagation" {
default = true
description = "Whether to create Transit Gateway Route Table associations and propagations"
}

variable "route_keys_enabled" {
type = bool
default = false
description = <<-EOT
If true, Terraform will use keys to label routes, preventing unnecessary changes,
but this requires that the VPCs and subnets already exist before using this module.
If false, Terraform will use numbers to label routes, and a single change may
cascade to a long list of changes because the index or order has changed, but
this will work when the `true` setting generates the error `The "for_each" value depends on resource attributes...`
EOT
}