Skip to content

Commit

Permalink
Fix variable enabled=false results in errors (#47)
Browse files Browse the repository at this point in the history
* Fix module enabled=false produces errors

* Add local availability_zones which is empty if disabled
  * private_count and public_count are either 0 if disabled or the length of the local.availability_zones list
  * therefore aws resource counts will not reference empty list in element function
  * also guaranteed to have same number of elements in zipmap function
* Added convenience local lists of tuples for outputs
* Note az_ngw_ids is now an empty map if disabled - previously a map of constant "0"
 * dummy_az_ngw_ids is no longer referenced so remove
* Transform local lists of tuples to output maps
  * since private_count and public_count are not both >0, no ellipsis needed in transform, producing single map value
  * output maps are all empty if disabled

* Multiple TF 0.13 cleanups

Co-authored-by: Paul Robinson <paul.robinson@internetfusion.co.uk>
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
  • Loading branch information
3 people authored Apr 26, 2021
1 parent 5e04ef2 commit 4e3780d
Show file tree
Hide file tree
Showing 19 changed files with 309 additions and 237 deletions.
4 changes: 2 additions & 2 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

# Cloud Posse must review any changes to standard context definition,
# but some changes can be rubber-stamped.
**/*.tf @cloudposse/engineering @cloudposse/approvers
README.yaml @cloudposse/engineering @cloudposse/approvers
**/*.tf @cloudposse/engineering @cloudposse/contributors @cloudposse/approvers
README.yaml @cloudposse/engineering @cloudposse/contributors @cloudposse/approvers
README.md @cloudposse/engineering @cloudposse/contributors @cloudposse/approvers
docs/*.md @cloudposse/engineering @cloudposse/contributors @cloudposse/approvers

Expand Down
2 changes: 1 addition & 1 deletion .github/auto-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ template: |
replacers:
# Remove irrelevant information from Renovate bot
- search: '/---\s+^#.*Renovate configuration(?:.|\n)*?This PR has been generated .*/gm'
- search: '/(?<=---\s+)+^#.*(Renovate configuration|Configuration)(?:.|\n)*?This PR has been generated .*/gm'
replace: ''
# Remove Renovate bot banner image
- search: '/\[!\[[^\]]*Renovate\][^\]]*\](\([^)]*\))?\s*\n+/gm'
Expand Down
7 changes: 7 additions & 0 deletions .github/mergify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,10 @@ pull_request_rules:
changes_requested: true
approved: true
message: "This Pull Request has been updated, so we're dismissing all reviews."

- name: "close Pull Requests without files changed"
conditions:
- "#files=0"
actions:
close:
message: "This pull request has been automatically closed by Mergify because there are no longer any changes."
4 changes: 3 additions & 1 deletion .github/workflows/auto-format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
jobs:
auto-format:
runs-on: ubuntu-latest
container: cloudposse/build-harness:slim-latest
container: cloudposse/build-harness:latest
steps:
# Checkout the pull request branch
# "An action in a workflow run can’t trigger a new workflow run. For example, if an action pushes code using
Expand All @@ -29,6 +29,8 @@ jobs:
- name: Auto Format
if: github.event.pull_request.state == 'open'
shell: bash
env:
GITHUB_TOKEN: "${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}"
run: make BUILD_HARNESS_PATH=/build-harness PACKAGES_PREFER_HOST=true -f /build-harness/templates/Makefile.build-harness pr/auto-format/host

# Commit changes (if any) to the PR branch
Expand Down
26 changes: 17 additions & 9 deletions .github/workflows/auto-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,25 @@ name: auto-release
on:
push:
branches:
- master
- main
- master
- production

jobs:
publish:
runs-on: ubuntu-latest
steps:
# Drafts your next Release notes as Pull Requests are merged into "master"
- uses: release-drafter/release-drafter@v5
with:
publish: true
prerelease: false
config-name: auto-release.yml
env:
GITHUB_TOKEN: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
# Get PR from merged commit to master
- uses: actions-ecosystem/action-get-merged-pull-request@v1
id: get-merged-pull-request
with:
github_token: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
# Drafts your next Release notes as Pull Requests are merged into "main"
- uses: release-drafter/release-drafter@v5
if: "!contains(steps.get-merged-pull-request.outputs.labels, 'no-release')"
with:
publish: true
prerelease: false
config-name: auto-release.yml
env:
GITHUB_TOKEN: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
2 changes: 2 additions & 0 deletions .github/workflows/validate-codeowners.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
name: Validate Codeowners
on:
workflow_dispatch:

pull_request:

jobs:
Expand Down
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: 1 addition & 1 deletion examples/complete/fixtures.us-east-2.tfvars
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace = "eg"

stage = "test"

name = "multi-az-subnets-only-private"
name = "multi-az-subnets"

availability_zones = ["us-east-2a", "us-east-2b", "us-east-2c"]

Expand Down
4 changes: 3 additions & 1 deletion examples/complete/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ locals {

module "vpc" {
source = "cloudposse/vpc/aws"
version = "0.18.1"
version = "0.21.1"

cidr_block = var.cidr_block

Expand All @@ -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
8 changes: 8 additions & 0 deletions examples/complete/outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ output "public_az_subnet_ids" {
value = module.public_subnets.az_subnet_ids
}

output "private_az_subnet_arns" {
value = module.private_subnets.az_subnet_arns
}

output "public_az_subnet_arns" {
value = module.public_subnets.az_subnet_arns
}

output "private_az_ngw_ids" {
value = module.private_subnets.az_ngw_ids
}
Expand Down
15 changes: 13 additions & 2 deletions main.tf
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
locals {
public_enabled = module.this.enabled && var.type == "public"
private_enabled = module.this.enabled && var.type == "private"
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 : []

output_map = { for az in(local.enabled ? var.availability_zones : []) : az => {
subnet_id = local.public_enabled ? aws_subnet.public[az].id : aws_subnet.private[az].id
subnet_arn = local.public_enabled ? aws_subnet.public[az].arn : aws_subnet.private[az].arn
route_table_id = local.public_enabled ? aws_route_table.public[az].id : aws_route_table.private[az].id
ngw_id = local.public_enabled ? aws_nat_gateway.public[az].id : null
}
}
}
33 changes: 13 additions & 20 deletions outputs.tf
Original file line number Diff line number Diff line change
@@ -1,32 +1,25 @@
output "az_subnet_ids" {
value = zipmap(
var.availability_zones,
coalescelist(aws_subnet.private.*.id, aws_subnet.public.*.id),
)
value = { for az, m in local.output_map : az => m.subnet_id }
description = "Map of AZ names to subnet IDs"
}

output "az_subnet_arns" {
value = { for az, m in local.output_map : az => m.subnet_arn }
description = "Map of AZ names to subnet ARNs"
}

output "az_route_table_ids" {
value = zipmap(
var.availability_zones,
coalescelist(aws_route_table.private.*.id, aws_route_table.public.*.id),
)
value = { for az, m in local.output_map : az => m.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 az, m in local.output_map : az => m.ngw_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),
)
description = "Map of AZ names to subnet ARNs"
}

output "az_subnet_map" {
value = local.output_map
description = "Map of AZ names to map of information about subnets"
}
42 changes: 16 additions & 26 deletions private.tf
Original file line number Diff line number Diff line change
@@ -1,29 +1,27 @@
locals {
private_count = local.private_enabled ? length(var.availability_zones) : 0
private_route_count = length(var.az_ngw_ids)
private_azs = local.private_enabled ? { for idx, az in var.availability_zones : az => idx } : {}
}

module "private_label" {
source = "cloudposse/label/null"
version = "0.22.1"
version = "0.24.1"

attributes = compact(concat(var.attributes, ["private"]))
attributes = ["private"]

context = module.this.context
}

resource "aws_subnet" "private" {
count = local.private_count
for_each = local.private_azs

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

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]
"Name" = "${module.private_label.id}${module.this.delimiter}${each.key}"
"Type" = var.type
},
)
Expand All @@ -33,7 +31,7 @@ resource "aws_network_acl" "private" {
count = local.private_enabled && var.private_network_acl_id == "" ? 1 : 0

vpc_id = var.vpc_id
subnet_ids = aws_subnet.private.*.id
subnet_ids = values(aws_subnet.private)[*].id
dynamic "egress" {
for_each = var.private_network_acl_egress
content {
Expand Down Expand Up @@ -67,43 +65,35 @@ resource "aws_network_acl" "private" {
}

resource "aws_route_table" "private" {
count = local.private_count
for_each = local.private_azs

vpc_id = var.vpc_id

tags = merge(
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)
"Name" = "${module.private_label.id}${module.this.delimiter}${each.key}"
"Type" = var.type
},
)
}

resource "aws_route_table_association" "private" {
count = local.private_count
for_each = local.private_azs

subnet_id = element(aws_subnet.private.*.id, count.index)
route_table_id = element(aws_route_table.private.*.id, count.index)
subnet_id = aws_subnet.private[each.key].id
route_table_id = aws_route_table.private[each.key].id
depends_on = [
aws_subnet.private,
aws_route_table.private,
]
}

resource "aws_route" "default" {
count = local.private_route_count
for_each = local.private_azs

route_table_id = zipmap(
var.availability_zones,
matchkeys(
aws_route_table.private.*.id,
aws_route_table.private.*.tags.AZ,
var.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)]
route_table_id = aws_route_table.private[each.key].id
nat_gateway_id = var.az_ngw_ids[each.key]
destination_cidr_block = "0.0.0.0/0"
depends_on = [aws_route_table.private]
}
Loading

0 comments on commit 4e3780d

Please sign in to comment.