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

Fix variable enabled=false results in errors #47

Merged
merged 3 commits into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 65 additions & 43 deletions README.md

Large diffs are not rendered by default.

104 changes: 64 additions & 40 deletions docs/terraform.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions examples/complete/fixtures.disabled.tfvars
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
enabled = false
1 change: 1 addition & 0 deletions examples/complete/fixtures.enabled.tfvars
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
enabled = true
2 changes: 2 additions & 0 deletions examples/complete/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ module "vpc" {
module "public_subnets" {
source = "../../"

enabled = var.enabled
availability_zones = var.availability_zones
vpc_id = module.vpc.vpc_id
cidr_block = local.public_cidr_block
Expand All @@ -32,6 +33,7 @@ module "public_subnets" {
module "private_subnets" {
source = "../../"

enabled = var.enabled
availability_zones = var.availability_zones
vpc_id = module.vpc.vpc_id
cidr_block = local.private_cidr_block
Expand Down
5 changes: 3 additions & 2 deletions main.tf
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
locals {
public_enabled = module.this.enabled && var.type == "public"
private_enabled = module.this.enabled && var.type == "private"
public_enabled = module.this.enabled && var.type == "public"
private_enabled = module.this.enabled && var.type == "private"
availability_zones = (module.this.enabled) ? var.availability_zones : []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new style guide is

Suggested change
public_enabled = module.this.enabled && var.type == "public"
private_enabled = module.this.enabled && var.type == "private"
availability_zones = (module.this.enabled) ? var.availability_zones : []
enabled = module.this.enabled
public_enabled = local.enabled && var.type == "public"
private_enabled = local.enabled && var.type == "private"
availability_zones = local.enabled ? var.availability_zones : []

}
32 changes: 16 additions & 16 deletions outputs.tf
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
output "az_subnet_ids" {
value = zipmap(
var.availability_zones,
coalescelist(aws_subnet.private.*.id, aws_subnet.public.*.id),
)
# No ellipsis needed since this module makes either public or private subnets. See the TF 0.15 one function
value = {
for subnet_tuple in concat(local.public_az_subnets, local.private_az_subnets) : subnet_tuple.availability_zone => subnet_tuple.subnet_id
}
description = "Map of AZ names to subnet IDs"
}

output "az_route_table_ids" {
value = zipmap(
var.availability_zones,
coalescelist(aws_route_table.private.*.id, aws_route_table.public.*.id),
)
# No ellipsis needed since this module makes either public or private subnets. See the TF 0.15 one function
value = {
for route_table_tuple in concat(local.public_az_route_table_ids, local.private_az_route_table_ids) : route_table_tuple.availability_zone => route_table_tuple.route_table_id
}
description = " Map of AZ names to Route Table IDs"
}

output "az_ngw_ids" {
value = zipmap(
var.availability_zones,
coalescelist(aws_nat_gateway.public.*.id, local.dummy_az_ngw_ids),
)
# No ellipsis needed since this module makes either public or private subnets. See the TF 0.15 one function
value = {
for nat_gw_tuple in concat(local.public_az_ngw_ids, local.private_az_ngw_ids) : nat_gw_tuple.availability_zone => nat_gw_tuple.nat_gateway_id
}
description = "Map of AZ names to NAT Gateway IDs (only for public subnets)"
}

output "az_subnet_arns" {
value = zipmap(
var.availability_zones,
coalescelist(aws_subnet.private.*.arn, aws_subnet.public.*.arn),
)
# No ellipsis needed since this module makes either public or private subnets. See the TF 0.15 one function
value = {
for subnet_tuple in concat(local.public_az_subnets, local.private_az_subnets) : subnet_tuple.availability_zone => subnet_tuple.subnet_arn
}
description = "Map of AZ names to subnet ARNs"
}

26 changes: 21 additions & 5 deletions private.tf
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ resource "aws_subnet" "private" {
count = local.private_count

vpc_id = var.vpc_id
availability_zone = var.availability_zones[count.index]
availability_zone = local.availability_zones[count.index]
cidr_block = cidrsubnet(var.cidr_block, ceil(log(var.max_subnets, 2)), count.index)

tags = merge(
module.private_label.tags,
{
"Name" = "${module.private_label.id}${module.this.delimiter}${element(var.availability_zones, count.index)}"
"AZ" = var.availability_zones[count.index]
"AZ" = local.availability_zones[count.index]
"Type" = var.type
},
)
Expand Down Expand Up @@ -75,7 +75,7 @@ resource "aws_route_table" "private" {
module.private_label.tags,
{
"Name" = "${module.private_label.id}${module.this.delimiter}${element(var.availability_zones, count.index)}"
"AZ" = element(var.availability_zones, count.index)
"AZ" = element(local.availability_zones, count.index)
"Type" = var.type
},
)
Expand All @@ -96,14 +96,30 @@ resource "aws_route" "default" {
count = local.private_route_count

route_table_id = zipmap(
var.availability_zones,
local.availability_zones,
matchkeys(
aws_route_table.private.*.id,
aws_route_table.private.*.tags.AZ,
var.availability_zones,
local.availability_zones,
),
)[element(keys(var.az_ngw_ids), count.index)]
nat_gateway_id = var.az_ngw_ids[element(keys(var.az_ngw_ids), count.index)]
destination_cidr_block = "0.0.0.0/0"
depends_on = [aws_route_table.private]
}

locals {
private_az_subnets = tolist([for subnet in aws_subnet.private[*] : {
availability_zone = subnet.tags.AZ
subnet = subnet
subnet_id = subnet.id
subnet_arn = subnet.arn
}])
private_az_route_table_ids = tolist([for route_table in aws_route_table.private[*] : {
availability_zone = route_table.tags.AZ
route_table = route_table
route_table_id = route_table.id
}])
# NAT gateways not present in private subnets
private_az_ngw_ids = tolist([])
}
53 changes: 21 additions & 32 deletions public.tf
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
locals {
public_count = local.public_enabled ? length(var.availability_zones) : 0
public_nat_gateways_count = local.public_enabled && var.nat_gateway_enabled ? length(var.availability_zones) : 0
public_count = local.public_enabled ? length(local.availability_zones) : 0
public_nat_gateways_count = local.public_enabled && var.nat_gateway_enabled ? length(local.availability_zones) : 0
}

module "public_label" {
Expand All @@ -16,14 +16,14 @@ resource "aws_subnet" "public" {
count = local.public_count

vpc_id = var.vpc_id
availability_zone = element(var.availability_zones, count.index)
availability_zone = element(local.availability_zones, count.index)
cidr_block = cidrsubnet(var.cidr_block, ceil(log(var.max_subnets, 2)), count.index)

tags = merge(
module.public_label.tags,
{
"Name" = "${module.public_label.id}${module.this.delimiter}${element(var.availability_zones, count.index)}"
"AZ" = element(var.availability_zones, count.index)
"AZ" = element(local.availability_zones, count.index)
"Type" = var.type
},
)
Expand Down Expand Up @@ -74,7 +74,7 @@ resource "aws_route_table" "public" {
module.public_label.tags,
{
"Name" = "${module.public_label.id}${module.this.delimiter}${element(var.availability_zones, count.index)}"
"AZ" = element(var.availability_zones, count.index)
"AZ" = element(local.availability_zones, count.index)
"Type" = var.type
},
)
Expand Down Expand Up @@ -121,38 +121,27 @@ resource "aws_nat_gateway" "public" {
module.public_label.tags,
{
"Name" = "${module.public_label.id}${module.this.delimiter}${element(var.availability_zones, count.index)}"
"AZ" = element(var.availability_zones, count.index)
"AZ" = element(local.availability_zones, count.index)
"Type" = var.type
},
)
}

# Dummy list of NAT Gateway IDs to use in the outputs for private subnets and when `nat_gateway_enabled=false` for public subnets
# Needed due to Terraform limitation of not allowing using conditionals with maps and lists
locals {
dummy_az_ngw_ids = slice(
[
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
"0",
],
0,
length(var.availability_zones),
)
public_az_subnets = tolist([for subnet in aws_subnet.public[*] : {
availability_zone = subnet.tags.AZ
subnet = subnet
subnet_id = subnet.id
subnet_arn = subnet.arn
}])
public_az_route_table_ids = tolist([for route_table in aws_route_table.public[*] : {
availability_zone = route_table.tags.AZ
route_table = route_table
route_table_id = route_table.id
}])
public_az_ngw_ids = tolist([for nat_gateway in aws_nat_gateway.public[*] : {
availability_zone = nat_gateway.tags.AZ
nat_gateway_id = nat_gateway.id
}])
}

94 changes: 64 additions & 30 deletions test/src/examples_complete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ func assertValueStartsWith(t *testing.T, m map[string]string, rx interface{}) {

// Test the Terraform module in examples/complete using Terratest.
func TestExamplesComplete(t *testing.T) {
t.Parallel()
// Init phase module download fails when run in parallel
//t.Parallel()

terraformOptions := &terraform.Options{
// The path to where our Terraform code is located
Expand All @@ -40,35 +41,35 @@ func TestExamplesComplete(t *testing.T) {
// This will run `terraform init` and `terraform apply` and fail the test if there are any errors
terraform.InitAndApply(t, terraformOptions)

/*
Outputs:

private_az_route_table_ids = {
"us-east-2a" = "rtb-0489137a5c668e49b"
"us-east-2b" = "rtb-083c0e942abb4b8a1"
"us-east-2c" = "rtb-0c36484693db5e774"
}
private_az_subnet_ids = {
"us-east-2a" = "subnet-0f56deccfe81c0ea0"
"us-east-2b" = "subnet-05861d30d45e7b675"
"us-east-2c" = "subnet-036d747a2b46857ae"
}
public_az_ngw_ids = {
"us-east-2a" = "nat-0f5057f09b8cd8ddc"
"us-east-2b" = "nat-0971b2505ea6d03f1"
"us-east-2c" = "nat-0dc1cdf91010be057"
}
public_az_route_table_ids = {
"us-east-2a" = "rtb-0642afb4401f1eef1"
"us-east-2b" = "rtb-04f511a28a2d5a6a2"
"us-east-2c" = "rtb-05f0ee4e831b05697"
}
public_az_subnet_ids = {
"us-east-2a" = "subnet-0dcb9e32f1f02a367"
"us-east-2b" = "subnet-0b432a6748ca40638"
"us-east-2c" = "subnet-00a9a6636ca722474"
}
*/
/*
Outputs:

private_az_route_table_ids = {
"us-east-2a" = "rtb-0489137a5c668e49b"
"us-east-2b" = "rtb-083c0e942abb4b8a1"
"us-east-2c" = "rtb-0c36484693db5e774"
}
private_az_subnet_ids = {
"us-east-2a" = "subnet-0f56deccfe81c0ea0"
"us-east-2b" = "subnet-05861d30d45e7b675"
"us-east-2c" = "subnet-036d747a2b46857ae"
}
public_az_ngw_ids = {
"us-east-2a" = "nat-0f5057f09b8cd8ddc"
"us-east-2b" = "nat-0971b2505ea6d03f1"
"us-east-2c" = "nat-0dc1cdf91010be057"
}
public_az_route_table_ids = {
"us-east-2a" = "rtb-0642afb4401f1eef1"
"us-east-2b" = "rtb-04f511a28a2d5a6a2"
"us-east-2c" = "rtb-05f0ee4e831b05697"
}
public_az_subnet_ids = {
"us-east-2a" = "subnet-0dcb9e32f1f02a367"
"us-east-2b" = "subnet-0b432a6748ca40638"
"us-east-2c" = "subnet-00a9a6636ca722474"
}
*/

// Run `terraform output` to get the value of an output variable
privateSubnetIds := terraform.OutputMap(t, terraformOptions, "private_az_subnet_ids")
Expand All @@ -94,3 +95,36 @@ func TestExamplesComplete(t *testing.T) {
assert.Equal(t, expectedAZs, getKeys(publicSubnetIds))
assertValueStartsWith(t, publicSubnetIds, "^subnet-.*")
}

func TestExamplesCompleteDisabledModule(t *testing.T) {
// Init phase module download fails when run in parallel
//t.Parallel()

terraformOptions := &terraform.Options{
// The path to where our Terraform code is located
TerraformDir: "../../examples/complete",
Upgrade: true,
// Variables to pass to our Terraform code using -var-file options
VarFiles: []string{"fixtures.us-east-2.tfvars", "fixtures.disabled.tfvars"},
}

// At the end of the test, run `terraform destroy` to clean up any resources that were created
defer terraform.Destroy(t, terraformOptions)

// This will run `terraform init` and `terraform apply` and fail the test if there are any errors
terraform.InitAndApply(t, terraformOptions)

privateNATGateWayIds := terraform.OutputMap(t, terraformOptions, "private_az_ngw_ids")
privateSubnetIds := terraform.OutputMap(t, terraformOptions, "private_az_subnet_ids")
privateRouteTableIds := terraform.OutputMap(t, terraformOptions, "private_az_route_table_ids")
publicNATGateWayIds := terraform.OutputMap(t, terraformOptions, "public_az_ngw_ids")
publicRouteTableIds := terraform.OutputMap(t, terraformOptions, "public_az_route_table_ids")
publicSubnetIds := terraform.OutputMap(t, terraformOptions, "public_az_subnet_ids")

assert.Empty(t, privateNATGateWayIds)
assert.Empty(t, privateSubnetIds)
assert.Empty(t, privateRouteTableIds)
assert.Empty(t, publicNATGateWayIds)
assert.Empty(t, publicSubnetIds)
assert.Empty(t, publicRouteTableIds)
}