From 898510f4e8eaf44dd95719065c48e658c0ddaef3 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Mon, 19 Oct 2026 05:14:36 +0100 Subject: [PATCH 1/3] fix(iac/aws): route private egress through a persistent fck-nat ENI The private 0.0.0.0/0 routes targeted the primary ENI of whichever fck-nat instance was running when Terraform last read it. When the ASG replaced that instance, the route kept pointing at the dead ENI and IPv4 egress from the private subnets was blackholed until the next apply. Create a Terraform-owned ENI (source/dest check off, in the ASG's only subnet) and point the routes at it. Each instance the ASG launches hands it to the AMI's fck-nat service via /etc/fck-nat.conf, which attaches it at boot and retries until a terminating predecessor has released it. The ASG is replaced when the launch template's user data changes, so the upgrade that introduces the ENI (and any later ENI replacement) launches an instance that attaches it; AMI rotations still do not cycle the instance. The routes wait for the ASG so they move only once that instance is up. Grant the deploy role the ENI create/modify/detach calls, scope the instance role to attaching this ENI, expose the ENI and ASG as outputs, and run the module's new terraform test in CI. Closes #716 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 10 + .../ci-cd-permissions/policy_networking.tf | 9 + terraform/modules/networking/aws/README.md | 5 + terraform/modules/networking/aws/main.tf | 118 ++++++------ terraform/modules/networking/aws/outputs.tf | 17 ++ .../aws/tests/fck_nat_eni.tftest.hcl | 173 ++++++++++++++++++ 6 files changed, 280 insertions(+), 52 deletions(-) create mode 100644 terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b885e287e..7a48d2dc4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -613,6 +613,16 @@ jobs: terraform -chdir="$d" test done + # The fck-nat egress route must target the persistent ENI, not the + # current instance's own, so it survives ASG replacement (#716). + - name: Test AWS networking module + if: matrix.cloud == 'aws' + run: | + d=terraform/modules/networking/aws + terraform -chdir="$d" init -backend=false -lockfile=readonly -input=false + terraform -chdir="$d" fmt -check -recursive + terraform -chdir="$d" test + # The post-deploy /health gate and the GCP uptime check must read the # body's top-level "status", not just the 200 (#649). - name: Test /health gate modules diff --git a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf index de56d2941..423cfc27a 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf @@ -33,6 +33,15 @@ resource "aws_iam_policy" "networking" { "ec2:CreateLaunchTemplateVersion", "ec2:DeleteLaunchTemplateVersions", "ec2:ModifyLaunchTemplate", + # The persistent fck-nat ENI the private routes target (#716): + # CreateNetworkInterface to create it, ModifyNetworkInterfaceAttribute + # because the provider turns source_dest_check off in a second call + # after the create, and DetachNetworkInterface because destroying or + # replacing it detaches it from the NAT instance first. + # DeleteNetworkInterface and DescribeNetworkInterfaces are below. + "ec2:CreateNetworkInterface", + "ec2:ModifyNetworkInterfaceAttribute", + "ec2:DetachNetworkInterface", "ec2:CreateRoute", "ec2:CreateRouteTable", "ec2:CreateSecurityGroup", diff --git a/terraform/modules/networking/aws/README.md b/terraform/modules/networking/aws/README.md index 145cb0528..2804db2b8 100644 --- a/terraform/modules/networking/aws/README.md +++ b/terraform/modules/networking/aws/README.md @@ -114,6 +114,9 @@ The `enable_nat_gateway` option deploys a cost-effective NAT alternative: - Provides IPv4 egress for services without IPv6 support (like AWS SES) - Automatically deployed in the first public subnet - 90% cheaper than AWS NAT Gateway (~$32/month) +- Private IPv4 routes target a persistent ENI that each instance attaches at + boot, so egress survives the ASG replacing the instance. With an existing + VPC, route private subnets to the `nat_network_interface_id` output. ## Security Groups @@ -136,6 +139,8 @@ The module outputs: - `database_security_group_id`: Security group for database - `lambda_vpc_config`: Convenience object for Lambda module - `database_vpc_config`: Convenience object for database module +- `nat_network_interface_id`: Persistent fck-nat ENI to route IPv4 egress to (if enabled) +- `nat_autoscaling_group_name`: fck-nat Auto Scaling group name (if enabled) ## Cost Comparison diff --git a/terraform/modules/networking/aws/main.tf b/terraform/modules/networking/aws/main.tf index 84a3dc797..c6cd16537 100644 --- a/terraform/modules/networking/aws/main.tf +++ b/terraform/modules/networking/aws/main.tf @@ -209,6 +209,27 @@ resource "aws_iam_instance_profile" "fck_nat" { }) } +# Persistent ENI the private routes target (#716). The route tables must not +# name an instance's own primary ENI: the ASG replaces the instance (failed +# health check, AZ event) and the replacement comes up with a new primary ENI, +# so a route pinned to the old one blackholes IPv4 egress until the next apply. +# This ENI outlives every instance. Each one the ASG launches attaches it at +# boot as its second interface (see user_data), so the route target never +# changes. It lives in the ASG's only subnet because an ENI can only attach to +# an instance in its own AZ. +resource "aws_network_interface" "fck_nat" { + count = var.enable_nat_gateway ? 1 : 0 + + description = "${var.stack_name} fck-nat persistent ENI (private route target)" + subnet_id = local.public_subnet_ids[0] + security_groups = [aws_security_group.fck_nat[0].id] + source_dest_check = false # NAT forwards traffic it is neither source nor destination of + + tags = merge(var.tags, { + Name = "${var.stack_name}-fck-nat-eni" + }) +} + # Launch template for fck-nat instance resource "aws_launch_template" "fck_nat" { count = var.enable_nat_gateway ? 1 : 0 @@ -227,15 +248,16 @@ resource "aws_launch_template" "fck_nat" { security_groups = [aws_security_group.fck_nat[0].id] } - # User data to disable source/destination check for NAT functionality + # Hand the persistent ENI to the AMI's own fck-nat service. With eni_id set + # it disables source/destination checking on the primary ENI, attaches + # eni_id as the second interface (retrying until a predecessor that is + # still terminating has released it), and NATs traffic arriving on it out + # of the primary interface. user_data = base64encode(<<-EOF - #!/bin/bash - # Get instance ID and ENI ID - INSTANCE_ID=$(ec2-metadata --instance-id | cut -d " " -f 2) - ENI_ID=$(aws ec2 describe-instances --instance-ids $INSTANCE_ID --region ${var.region} --query 'Reservations[0].Instances[0].NetworkInterfaces[0].NetworkInterfaceId' --output text) - - # Disable source/destination check - aws ec2 modify-network-interface-attribute --network-interface-id $ENI_ID --region ${var.region} --no-source-dest-check + #!/bin/sh + : > /etc/fck-nat.conf + echo "eni_id=${aws_network_interface.fck_nat[0].id}" >> /etc/fck-nat.conf + service fck-nat restart EOF ) @@ -272,7 +294,7 @@ resource "aws_autoscaling_group" "fck_nat" { desired_capacity = 1 min_size = 1 max_size = 1 - vpc_zone_identifier = [local.public_subnet_ids[0]] # Deploy in first public subnet + vpc_zone_identifier = [aws_network_interface.fck_nat[0].subnet_id] # The persistent ENI's subnet (first public subnet) launch_template { id = aws_launch_template.fck_nat[0].id @@ -299,6 +321,16 @@ resource "aws_autoscaling_group" "fck_nat" { lifecycle { create_before_destroy = true + + # A running instance never re-reads user data, and the launch template + # version is "$Latest", so a user_data change alone would leave the + # current instance on the old config. That matters once, on the upgrade + # that introduces the persistent ENI: the routes would move to an ENI no + # instance has attached. Replacing the ASG (create-before-destroy, so the + # new instance is InService before the old one goes) launches one that + # attaches it. user_data only changes when the ENI itself is replaced, so + # AMI rotations (image_id) still do not cycle the instance. + replace_triggered_by = [aws_launch_template.fck_nat[count.index].user_data] } } @@ -311,51 +343,28 @@ resource "aws_iam_role_policy" "fck_nat_ec2" { policy = jsonencode({ Version = "2012-10-17" - Statement = [{ - Effect = "Allow" - Action = [ - "ec2:DescribeInstances", - "ec2:ModifyNetworkInterfaceAttribute" - ] - Resource = "*" - }] + Statement = [ + { + # Attach the persistent ENI to whichever instance the ASG launched. + # AttachNetworkInterface is authorized against both the ENI and the + # instance, so both ARNs are needed; the ENI one is what scopes it. + Sid = "AttachPersistentENI" + Effect = "Allow" + Action = ["ec2:AttachNetworkInterface"] + Resource = [aws_network_interface.fck_nat[0].arn, "arn:aws:ec2:${var.region}:*:instance/*"] + }, + { + # Disable source/destination checking on the instance's own primary + # ENI, whose ID is only known at boot. + Sid = "DisableSourceDestCheck" + Effect = "Allow" + Action = ["ec2:ModifyNetworkInterfaceAttribute"] + Resource = "*" + }, + ] }) } -# Get the instance from the ASG -data "aws_instances" "fck_nat" { - count = var.enable_nat_gateway ? 1 : 0 - - filter { - name = "tag:aws:autoscaling:groupName" - values = [aws_autoscaling_group.fck_nat[0].name] - } - - filter { - name = "instance-state-name" - values = ["running"] - } - - depends_on = [aws_autoscaling_group.fck_nat] -} - -# Get the ENI of the fck-nat instance for routing -data "aws_network_interfaces" "fck_nat" { - count = var.enable_nat_gateway ? 1 : 0 - - filter { - name = "attachment.instance-id" - values = data.aws_instances.fck_nat[0].ids - } - - filter { - name = "attachment.device-index" - values = ["0"] # Primary network interface - } - - depends_on = [data.aws_instances.fck_nat] -} - # ============================================== # Availability Zones # ============================================== @@ -485,7 +494,12 @@ resource "aws_route" "private_internet_ipv4" { route_table_id = aws_route_table.private[count.index].id destination_cidr_block = "0.0.0.0/0" - network_interface_id = data.aws_network_interfaces.fck_nat[0].ids[0] + network_interface_id = aws_network_interface.fck_nat[0].id # Persistent, survives instance replacement (#716) + + # Move the route only once an instance that attaches the ENI is InService, + # so the upgrade that introduces the ENI does not point egress at an + # unattached interface while the replacement ASG is still launching. + depends_on = [aws_autoscaling_group.fck_nat] } # Associate private subnets with private route tables diff --git a/terraform/modules/networking/aws/outputs.tf b/terraform/modules/networking/aws/outputs.tf index 4258a5ea5..d94906640 100644 --- a/terraform/modules/networking/aws/outputs.tf +++ b/terraform/modules/networking/aws/outputs.tf @@ -98,3 +98,20 @@ output "database_vpc_config" { security_group_id = aws_security_group.database.id } } + +# The persistent fck-nat ENI (#716). The module routes its own private route +# tables to it; with use_existing_vpc, point the VPC's private 0.0.0.0/0 +# routes here rather than at the fck-nat instance, whose primary ENI changes +# every time the ASG replaces it. +output "nat_network_interface_id" { + description = "ID of the persistent fck-nat ENI to route private IPv4 egress to (null when enable_nat_gateway is false)" + value = var.enable_nat_gateway ? aws_network_interface.fck_nat[0].id : null +} + +# The fck-nat ASG, for alarms on its instance replacements. It is replaced +# (new name) when the launch template's user data changes; see the ASG's +# replace_triggered_by. +output "nat_autoscaling_group_name" { + description = "Name of the fck-nat Auto Scaling group (null when enable_nat_gateway is false)" + value = var.enable_nat_gateway ? aws_autoscaling_group.fck_nat[0].name : null +} diff --git a/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl b/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl new file mode 100644 index 000000000..544191067 --- /dev/null +++ b/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl @@ -0,0 +1,173 @@ +# fck-nat egress route survives instance replacement (#716). +# +# The private 0.0.0.0/0 routes used to point at the primary ENI of whichever +# fck-nat instance was running when Terraform last read it. The ASG replaces +# that instance on a failed health check or AZ event, the new instance gets a +# new primary ENI, and the route keeps naming the old one until the next +# apply: private-subnet IPv4 egress is blackholed in the meantime. The routes +# must instead target a persistent ENI that Terraform owns and every instance +# the ASG launches attaches at boot, so the route target never changes when +# the instance does. +mock_provider "aws" { + # The AZ list and the VPC's IPv6 block feed cidrsubnet() and indexing, so + # the mock's random defaults cannot stand in for them. + override_data { + target = data.aws_availability_zones.available + values = { + names = ["us-east-1a", "us-east-1b", "us-east-1c"] + } + } + + override_resource { + target = aws_vpc.main + values = { + ipv6_cidr_block = "2600:1f18:1234:5600::/56" + } + } + + # The provider validates the ASG's launch_template.id format. + override_resource { + target = aws_launch_template.fck_nat + values = { + id = "lt-0mock" + } + } +} + +variables { + stack_name = "cudly-test" + permissions_boundary_arn = "arn:aws:iam::123456789012:policy/cudly-deploy-boundary" + region = "us-east-1" + az_count = 3 + enable_nat_gateway = true +} + +run "routes_target_persistent_eni" { + command = apply + + assert { + condition = length(aws_route.private_internet_ipv4) == 3 + error_message = "Every private route table needs its IPv4 default route via fck-nat." + } + + assert { + condition = alltrue([ + for r in aws_route.private_internet_ipv4 : + r.network_interface_id == aws_network_interface.fck_nat[0].id + ]) + error_message = "Private IPv4 default routes must target the persistent fck-nat ENI, not the primary ENI of the instance running at apply time: the ASG replaces that instance and its ENI with it, blackholing egress (#716)." + } + + assert { + condition = output.nat_network_interface_id == aws_network_interface.fck_nat[0].id + error_message = "nat_network_interface_id must expose the persistent ENI so existing-VPC route tables can target it too." + } +} + +run "persistent_eni_is_attachable_by_every_instance" { + command = apply + + # NAT forwards traffic it is neither source nor destination of. + assert { + condition = aws_network_interface.fck_nat[0].source_dest_check == false + error_message = "The persistent ENI must have source/destination checking disabled or it drops forwarded traffic." + } + + # An ENI can only attach to an instance in its own AZ. + assert { + condition = toset(aws_autoscaling_group.fck_nat[0].vpc_zone_identifier) == toset([aws_network_interface.fck_nat[0].subnet_id]) + error_message = "The ASG must launch fck-nat only into the persistent ENI's subnet, or a replacement instance in another AZ cannot attach it." + } + + # The fck-nat AMI's own service attaches eni_id at boot, retrying until a + # terminating predecessor has released it. + assert { + condition = strcontains(base64decode(aws_launch_template.fck_nat[0].user_data), "eni_id=${aws_network_interface.fck_nat[0].id}") + error_message = "Launch template user data must hand the persistent ENI to the fck-nat service (eni_id in /etc/fck-nat.conf) so each replacement instance attaches it." + } + + assert { + condition = strcontains(base64decode(aws_launch_template.fck_nat[0].user_data), "service fck-nat restart") + error_message = "User data must restart the fck-nat service after writing its config, or the ENI is never attached." + } + + assert { + condition = anytrue([ + for s in jsondecode(aws_iam_role_policy.fck_nat_ec2[0].policy).Statement : + contains(flatten([s.Action]), "ec2:AttachNetworkInterface") && contains(flatten([s.Resource]), aws_network_interface.fck_nat[0].arn) + ]) + error_message = "The fck-nat instance role must be allowed ec2:AttachNetworkInterface on the persistent ENI." + } +} + +# A running instance never re-reads user data, so when the ENI it must attach +# changes (the upgrade that introduces it, or the ENI being replaced later), +# the ASG itself has to be replaced to launch an instance that attaches the +# new one. Mock providers do not reproduce the real provider's ForceNew +# planning, so the ENI replacement is requested explicitly here. +run "eni_replacement_replaces_asg" { + command = apply + + plan_options { + replace = [aws_network_interface.fck_nat[0]] + } + + assert { + condition = output.nat_network_interface_id != run.persistent_eni_is_attachable_by_every_instance.nat_network_interface_id + error_message = "Test precondition: the persistent ENI should have been replaced." + } + + assert { + condition = output.nat_autoscaling_group_name != run.persistent_eni_is_attachable_by_every_instance.nat_autoscaling_group_name + error_message = "The fck-nat ASG must be replaced when its user data names a new ENI; updated in place, the running instance keeps the old config and never attaches the ENI the routes now target." + } +} + +# The fck-nat AMI lookup is most_recent, so an upstream AMI release changes the +# launch template's image_id on the next apply with nothing else changing. +# That must not cycle the NAT instance. +run "ami_rotation_keeps_asg" { + command = apply + + override_data { + target = data.aws_ami.fck_nat + values = { + id = "ami-0rotated" + } + } + + assert { + condition = aws_launch_template.fck_nat[0].image_id == "ami-0rotated" + error_message = "Test precondition: the launch template should have picked up the rotated AMI." + } + + assert { + condition = output.nat_autoscaling_group_name == run.eni_replacement_replaces_asg.nat_autoscaling_group_name + error_message = "An AMI rotation alone must not replace the fck-nat ASG (and its running instance)." + } +} + +run "no_nat_no_eni" { + command = plan + + variables { + use_existing_vpc = true + existing_vpc_id = "vpc-0existing" + existing_public_subnet_ids = ["subnet-0public"] + existing_private_subnet_ids = ["subnet-0private"] + enable_nat_gateway = false + } + + override_data { + target = data.aws_vpc.existing + values = { + cidr_block = "10.20.0.0/16" + ipv6_cidr_block = "2600:1f18:abcd:ef00::/56" + } + } + + assert { + condition = length(aws_network_interface.fck_nat) == 0 && output.nat_network_interface_id == null + error_message = "No fck-nat ENI should exist when enable_nat_gateway is false." + } +} From 648ca2ffaa484f853b18c09888d8a0b536000b1c Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Tue, 20 Oct 2026 08:28:34 +0100 Subject: [PATCH 2/3] fix(iac/aws): add linux and darwin_amd64 hashes to networking lock file The networking/aws lock file only recorded the darwin_arm64 h1: hash for hashicorp/aws 5.100.0. CI now runs `terraform test` on this module with `init -lockfile=readonly` on linux_amd64, where the installed package matched none of the recorded checksums and the test step failed. Regenerated with `terraform providers lock` for linux_amd64, darwin_arm64 and darwin_amd64; the pinned version is unchanged. Co-Authored-By: Claude Opus 5.5 --- terraform/modules/networking/aws/.terraform.lock.hcl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/terraform/modules/networking/aws/.terraform.lock.hcl b/terraform/modules/networking/aws/.terraform.lock.hcl index cdc1668d4..8fbb17124 100644 --- a/terraform/modules/networking/aws/.terraform.lock.hcl +++ b/terraform/modules/networking/aws/.terraform.lock.hcl @@ -6,6 +6,8 @@ provider "registry.terraform.io/hashicorp/aws" { constraints = "~> 5.0" hashes = [ "h1:Ijt7pOlB7Tr7maGQIqtsLFbl7pSMIj06TVdkoSBcYOw=", + "h1:edXOJWE4ORX8Fm+dpVpICzMZJat4AX0VRCAy/xkcOc0=", + "h1:hd45qFU5cFuJMpFGdUniU9mVIr5LYVWP1uMeunBpYYs=", "zh:054b8dd49f0549c9a7cc27d159e45327b7b65cf404da5e5a20da154b90b8a644", "zh:0b97bf8d5e03d15d83cc40b0530a1f84b459354939ba6f135a0086c20ebbe6b2", "zh:1589a2266af699cbd5d80737a0fe02e54ec9cf2ca54e7e00ac51c7359056f274", From 65e2799eeab3342aae59d867a835e2e9aa5eee68 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 21 Oct 2026 09:07:33 +0100 Subject: [PATCH 3/3] fix(iac/aws): order fck-nat ASG after its role policy; note untested depends_on Review follow-ups on #1090: - The ASG now depends_on aws_iam_role_policy.fck_nat_ec2, so the AttachNetworkInterface grant exists before a new instance boots and fck-nat's one-shot source/dest-check call is not racing the policy. - The tftest file names the gap a deletion probe found: mock providers expose no apply ordering, so neither the routes' depends_on on the ASG nor the ASG's depends_on on the role policy is covered by a run. Co-Authored-By: Claude Opus 5.5 --- terraform/modules/networking/aws/main.tf | 5 +++++ .../modules/networking/aws/tests/fck_nat_eni.tftest.hcl | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/terraform/modules/networking/aws/main.tf b/terraform/modules/networking/aws/main.tf index c6cd16537..99dc57e2d 100644 --- a/terraform/modules/networking/aws/main.tf +++ b/terraform/modules/networking/aws/main.tf @@ -304,6 +304,11 @@ resource "aws_autoscaling_group" "fck_nat" { health_check_type = "EC2" health_check_grace_period = 60 + # Grant the attach permission before an instance can boot and ask for it. + # fck-nat retries the attach, but its one-shot source/dest-check call on + # the primary ENI does not, so make the order explicit. + depends_on = [aws_iam_role_policy.fck_nat_ec2] + tag { key = "Name" value = "${var.stack_name}-fck-nat-asg" diff --git a/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl b/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl index 544191067..6b6eea5c8 100644 --- a/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl +++ b/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl @@ -42,6 +42,11 @@ variables { enable_nat_gateway = true } +# Not covered here: the routes' depends_on on the ASG, which keeps the +# one-time upgrade from moving egress to the new ENI before an instance that +# attaches it is InService, and the ASG's depends_on on the instance role +# policy. Mock providers expose no apply ordering, so deleting either one +# leaves every run below green; check them by hand when editing main.tf. run "routes_target_persistent_eni" { command = apply