From 898510f4e8eaf44dd95719065c48e658c0ddaef3 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Mon, 19 Oct 2026 05:14:36 +0100 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 From f65a9f55afee2e59cf9d3057953fce0b6b257a2c Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 28 Oct 2026 00:19:39 +0000 Subject: [PATCH 04/10] fix(deps): bump golang.org/x/net to v0.62.0 (GO-2026-6860, GO-2026-6866) govulncheck in the Docker image build now fails on these two advisories, both fixed in x/net v0.62.0. Co-Authored-By: Claude Opus 5.5 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 5fb3a2f61..abcf4fb6c 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( go.opentelemetry.io/otel/metric v1.45.0 // indirect go.opentelemetry.io/otel/trace v1.45.0 // indirect golang.org/x/crypto v0.58.0 - golang.org/x/net v0.61.0 // indirect + golang.org/x/net v0.62.0 // indirect golang.org/x/oauth2 v0.37.0 golang.org/x/sync v0.24.0 golang.org/x/sys v0.49.0 // indirect diff --git a/go.sum b/go.sum index 06deac0b9..83df52e66 100644 --- a/go.sum +++ b/go.sum @@ -367,8 +367,8 @@ golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54= golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w= golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg= golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs= -golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw= -golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= +golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY= +golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98= golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58= golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA= From 9d2e82b4bf7af5a628b3524e96f32d7316f7f5ed Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:45:25 +0100 Subject: [PATCH 05/10] fix(networking): bound the fck-nat restart and terminate on poweroff The fck-nat unit is oneshot with no start timeout and an unbounded attach loop, so a persistent ENI attach failure hung with a green health check. Wrap the restart in timeout 300 and power off on failure; the launch template terminates on shutdown so the ASG replaces the instance. The tftest pins user_data exactly and the shutdown behaviour. Refs #716 Co-Authored-By: Claude Opus 5.5 --- terraform/modules/networking/aws/main.tf | 11 +++++-- .../aws/tests/fck_nat_eni.tftest.hcl | 31 ++++++++++++++++--- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/terraform/modules/networking/aws/main.tf b/terraform/modules/networking/aws/main.tf index 1fca1cc4e..4ecd40c43 100644 --- a/terraform/modules/networking/aws/main.tf +++ b/terraform/modules/networking/aws/main.tf @@ -252,15 +252,22 @@ resource "aws_launch_template" "fck_nat" { # 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. + # of the primary interface. The unit is Type=oneshot with no start timeout + # and an unbounded attach loop, so the restart is bounded: on timeout or + # failure (exit status means attached AND NATing) the instance powers off + # and the ASG replaces it, instead of hanging with a green health check. user_data = base64encode(<<-EOF #!/bin/sh : > /etc/fck-nat.conf echo "eni_id=${aws_network_interface.fck_nat[0].id}" >> /etc/fck-nat.conf - service fck-nat restart + timeout 300 service fck-nat restart || poweroff EOF ) + # The poweroff above must terminate the instance, not stop it, so the ASG + # replaces it at once. Needs no IAM: it is an in-guest shutdown. + instance_initiated_shutdown_behavior = "terminate" + # Disable source/destination check for NAT functionality metadata_options { http_endpoint = "enabled" 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 6b6eea5c8..73ddb51d6 100644 --- a/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl +++ b/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl @@ -42,11 +42,9 @@ 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. +# The routes' and ASG's depends_on, and the attach policy's scoping, are +# pinned by fck_nat_guard_test.go in ci-cd-permissions: mock providers expose +# no apply ordering, so they cannot be asserted here. run "routes_target_persistent_eni" { command = apply @@ -96,6 +94,29 @@ run "persistent_eni_is_attachable_by_every_instance" { error_message = "User data must restart the fck-nat service after writing its config, or the ENI is never attached." } + # user_data is exactly: shebang, conf write, then a restart bounded by + # `timeout` that powers the instance off on failure. The fck-nat unit is + # Type=oneshot with no start timeout and an unbounded attach loop, so + # without the bound a persistent attach failure hangs forever while the EC2 + # health check stays green. The restart's exit status means attached AND + # NATing. Nothing else may run (the string is compared whole). + assert { + condition = base64decode(aws_launch_template.fck_nat[0].user_data) == join("\n", [ + "#!/bin/sh", + ": > /etc/fck-nat.conf", + "echo \"eni_id=${aws_network_interface.fck_nat[0].id}\" >> /etc/fck-nat.conf", + "timeout 300 service fck-nat restart || poweroff", + "", + ]) + error_message = "user_data must be the shebang, the /etc/fck-nat.conf write, then exactly `timeout 300 service fck-nat restart || poweroff`, and nothing else." + } + + # poweroff must terminate (not stop) so the ASG replaces the instance at once. + assert { + condition = aws_launch_template.fck_nat[0].instance_initiated_shutdown_behavior == "terminate" + error_message = "The fck-nat launch template must set instance_initiated_shutdown_behavior = \"terminate\" so the bounded-restart poweroff gets the instance replaced instead of left stopped." + } + assert { condition = anytrue([ for s in jsondecode(aws_iam_role_policy.fck_nat_ec2[0].policy).Statement : From 099de2e75da23637d0a1c9c5a1db9d0e20935464 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:46:57 +0100 Subject: [PATCH 06/10] test(iam): guard fck-nat ENI deploy actions and apply-order wiring Pin the deploy-role EC2 ENI actions and the CreateTags create-action condition, and read the networking module HCL for the depends_on edges, ASG subnet, replace_triggered_by target and attach-policy scoping that mock-provider terraform tests cannot observe. Refs #716 Co-Authored-By: Claude Opus 5.5 --- .../ci-cd-permissions/fck_nat_guard_test.go | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 terraform/environments/aws/ci-cd-permissions/fck_nat_guard_test.go diff --git a/terraform/environments/aws/ci-cd-permissions/fck_nat_guard_test.go b/terraform/environments/aws/ci-cd-permissions/fck_nat_guard_test.go new file mode 100644 index 000000000..38fd4126a --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/fck_nat_guard_test.go @@ -0,0 +1,167 @@ +package cicdpermissions_test + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// networkInterfaceActions are the EC2 actions the module's +// aws_network_interface.fck_nat (#716) needs on the deploy role. The +// ModifyNetworkInterfaceAttribute call is easy to miss: the provider turns +// source_dest_check off in a second call after the create. Without any one of +// these, plan stays green and the first apply that creates, updates or +// replaces the ENI fails with AccessDenied. +var networkInterfaceActions = []string{ + "ec2:CreateNetworkInterface", + "ec2:DeleteNetworkInterface", + "ec2:DescribeNetworkInterfaces", + "ec2:ModifyNetworkInterfaceAttribute", + "ec2:DetachNetworkInterface", + "ec2:CreateTags", + "ec2:DeleteTags", +} + +func TestDeployPolicyAllowsFckNatNetworkInterface(t *testing.T) { + var allowed []string + var createTagsStmts []string + for _, file := range deployPolicyFiles(t) { + for _, stmt := range findStatements(t, file, func(s string) bool { return statementEffect(s) == "Allow" }) { + actions := statementActions(stmt) + allowed = append(allowed, actions...) + if actionPermitted(actions, "ec2:CreateTags") { + createTagsStmts = append(createTagsStmts, stmt) + } + } + } + for _, action := range networkInterfaceActions { + if !actionPermitted(allowed, action) { + t.Errorf("no deploy policy document allows %s, which aws_network_interface.fck_nat needs (#716)", action) + } + } + + // A CreateTags grant conditioned on ec2:CreateAction must include + // CreateNetworkInterface, or tagging the ENI at creation is denied. + // At least one grant must also be usable for it. + usable := false + for _, stmt := range createTagsStmts { + cond := statementConditionBody(stmt) + if cond == "" || (strings.Contains(cond, "ec2:CreateAction") && strings.Contains(cond, "CreateNetworkInterface")) { + usable = true + continue + } + if strings.Contains(cond, "ec2:CreateAction") { + t.Errorf("statement %q grants ec2:CreateTags conditioned on ec2:CreateAction without CreateNetworkInterface; tagging the fck-nat ENI at create time would be denied", statementSid(stmt)) + } + } + if !usable { + t.Errorf("no ec2:CreateTags grant is unconditioned or allows ec2:CreateAction=CreateNetworkInterface") + } +} + +// moduleBlock returns the body of `kind "type" "name" {...}` from comment- +// stripped HCL, or `kind "type" {...}` when name is empty. +func moduleBlock(t *testing.T, src, kind, typ, name string) string { + t.Helper() + head := kind + ` "` + typ + `"` + if name != "" { + head += ` "` + name + `"` + } + re := regexp.MustCompile(`(?m)^` + regexp.QuoteMeta(head) + ` \{`) + loc := re.FindStringIndex(src) + if loc == nil { + t.Fatalf("block %s not found in networking main.tf", head) + } + open := loc[1] - 1 + return src[open:balancedBraceEnd(src, open)] +} + +// bracketValue returns the text of the first `key = [ ... ]` list in block, +// matching nested brackets (index expressions like fck_nat[0]). +func bracketValue(t *testing.T, block, key string) string { + t.Helper() + loc := regexp.MustCompile(regexp.QuoteMeta(key) + `\s*=\s*\[`).FindStringIndex(block) + if loc == nil { + return "" + } + depth := 0 + for i := loc[1] - 1; i < len(block); i++ { + switch block[i] { + case '[': + depth++ + case ']': + depth-- + if depth == 0 { + return block[loc[1]:i] + } + } + } + return "" +} + +// TestFckNatApplyOrderAndWiring pins HCL that mock-provider terraform tests +// cannot see (they expose no apply ordering) and that fails silently in a +// live account: a wrong order only shows as an egress outage. +func TestFckNatApplyOrderAndWiring(t *testing.T) { + path := filepath.Join(modulesDir, "networking", "aws", "main.tf") + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + src := stripCommentLines(string(raw)) + + route := moduleBlock(t, src, "resource", "aws_route", "private_internet_ipv4") + asg := moduleBlock(t, src, "resource", "aws_autoscaling_group", "fck_nat") + eni := moduleBlock(t, src, "resource", "aws_network_interface", "fck_nat") + policy := moduleBlock(t, src, "resource", "aws_iam_role_policy", "fck_nat_ec2") + + dependsOn := func(block, target string) bool { + for _, dep := range strings.Split(bracketValue(t, block, "depends_on"), ",") { + if strings.TrimSpace(dep) == target { + return true + } + } + return false + } + if !dependsOn(route, "aws_autoscaling_group.fck_nat") { + t.Error("aws_route.private_internet_ipv4 must depends_on aws_autoscaling_group.fck_nat: otherwise the routes move to the new ENI before an instance that attaches it is InService") + } + if !dependsOn(asg, "aws_iam_role_policy.fck_nat_ec2") { + t.Error("aws_autoscaling_group.fck_nat must depends_on aws_iam_role_policy.fck_nat_ec2: the attach permission must exist before an instance boots") + } + + if !regexp.MustCompile(`vpc_zone_identifier\s*=\s*\[aws_network_interface\.fck_nat\[0\]\.subnet_id\]`).MatchString(asg) { + t.Error("the fck-nat ASG must launch only into the persistent ENI's subnet; an ENI attaches only within its own AZ") + } + if !regexp.MustCompile(`subnet_id\s*=\s*local\.public_subnet_ids\[0\]`).MatchString(eni) { + t.Error("the persistent ENI must live in local.public_subnet_ids[0]") + } + + if got := strings.TrimSpace(bracketValue(t, asg, "replace_triggered_by")); got != "aws_launch_template.fck_nat[count.index].user_data" { + t.Errorf("replace_triggered_by must be exactly [aws_launch_template.fck_nat[count.index].user_data] (not the ENI, which would also fire on unrelated ENI attribute changes, and not image_id); got [%s]", got) + } + + sid := strings.Index(policy, `Sid = "AttachPersistentENI"`) + if sid < 0 { + t.Fatal("AttachPersistentENI statement not found in aws_iam_role_policy.fck_nat_ec2") + } + stmt := policy[sid:] + if end := strings.Index(stmt, "},"); end >= 0 { + stmt = stmt[:end] + } + res := bracketValue(t, stmt, "Resource") + if res == "" { + t.Fatalf("AttachPersistentENI Resource must be a list (a bare \"*\" string is rejected); statement: %s", stmt) + } + if !strings.Contains(res, "aws_network_interface.fck_nat[0].arn") { + t.Errorf("AttachNetworkInterface must be scoped to the persistent ENI ARN; got %s", res) + } + if !strings.Contains(res, ":instance/*") { + t.Errorf("AttachNetworkInterface is authorized against the instance too; Resource must include an instance/* ARN; got %s", res) + } + if regexp.MustCompile(`"\*"`).MatchString(res) { + t.Errorf("AttachNetworkInterface Resource must not be \"*\"; got %s", res) + } +} From 3fb020319981b10ee940e7f147f803fa2d123156 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:47:13 +0100 Subject: [PATCH 07/10] docs(networking): document ENI deploy actions and add rollback runbook List the three new networking actions in the ci-cd-permissions README and add a rollback runbook (rollback.yml first, hand-run fallbacks second) to the networking module README. Refs #716 Co-Authored-By: Claude Opus 5.5 --- .../aws/ci-cd-permissions/README.md | 2 +- terraform/modules/networking/aws/README.md | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/terraform/environments/aws/ci-cd-permissions/README.md b/terraform/environments/aws/ci-cd-permissions/README.md index bf226ec15..290b55739 100644 --- a/terraform/environments/aws/ci-cd-permissions/README.md +++ b/terraform/environments/aws/ci-cd-permissions/README.md @@ -9,7 +9,7 @@ CI/CD pipeline uses to deploy infrastructure on AWS. It optionally sets up keyle | Resource | Purpose | | --- | --- | | `aws_iam_role.cudly_deploy` | The deploy role; assumed by GitHub Actions or a human operator | -| `aws_iam_policy.networking` | VPC, subnets, security groups, ALB, ECS cluster | +| `aws_iam_policy.networking` | VPC, subnets, security groups, ALB, ECS cluster, and the persistent fck-nat ENI (`ec2:CreateNetworkInterface`, `ec2:ModifyNetworkInterfaceAttribute`, `ec2:DetachNetworkInterface`, added for #716; re-apply before merging the change that creates it) | | `aws_iam_policy.compute` | ECS services/tasks, ECR, CloudWatch Logs, SSM | | `aws_iam_policy.compute_b` | Overflow for `aws_iam_policy.compute`, which is close to the 6144-character managed-policy limit | | `aws_iam_policy.data` | RDS, ElastiCache, S3 (state bucket), Secrets Manager | diff --git a/terraform/modules/networking/aws/README.md b/terraform/modules/networking/aws/README.md index e0815416c..47dadbce5 100644 --- a/terraform/modules/networking/aws/README.md +++ b/terraform/modules/networking/aws/README.md @@ -118,6 +118,37 @@ The `enable_nat_gateway` option deploys a cost-effective NAT alternative: boot, so egress survives the ASG replacing the instance. With an existing VPC, route private subnets to the `nat_network_interface_id` output. +### Rollback runbook (persistent ENI, #716) + +If egress from the private subnets is down after the apply that introduced the +persistent ENI, run `rollback.yml` against the pre-merge `main` SHA first. Note +that `rollback.yml` applies the environment root from that ref's checkout. + +Fallback by hand, when the workflow cannot run. Placeholders: `` is the +`nat_network_interface_id` output, `` the running fck-nat instance +(`nat_autoscaling_group_name` output, then `aws autoscaling describe-auto-scaling-groups`), +`` each private route table. + +```bash +# 1. Is the ENI attached as the instance's second interface? +aws ec2 describe-network-interfaces --network-interface-ids \ + --query 'NetworkInterfaces[0].Attachment' + +# 2. If not, attach it by hand (device index 1; the instance must be in the ENI's AZ). +aws ec2 attach-network-interface --network-interface-id \ + --instance-id --device-index 1 + +# 3. If the route is a blackhole, point it back at a working target +# (the instance's primary ENI, or the ENI once step 2 succeeded). +aws ec2 replace-route --route-table-id \ + --destination-cidr-block 0.0.0.0/0 --network-interface-id +``` + +A persistent attach failure shows as an instance replaced about every 5 minutes +(`aws autoscaling describe-scaling-activities --auto-scaling-group-name `): +user data bounds the service restart at 300 s and powers the instance off, and +the launch template's `terminate` shutdown behaviour has the ASG replace it. + ## Security Groups The module creates: From 0a26a4eb69d634c60d0e71110ad1c0c02c84d6b2 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 01:23:33 +0100 Subject: [PATCH 08/10] test(iam): drop the always-"resource" kind parameter from the fck-nat guard golangci-lint unparam: moduleBlock was only ever called with kind "resource". Renamed to resourceBlock with the kind fixed. Co-Authored-By: Claude Opus 5.5 --- .../aws/ci-cd-permissions/fck_nat_guard_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/terraform/environments/aws/ci-cd-permissions/fck_nat_guard_test.go b/terraform/environments/aws/ci-cd-permissions/fck_nat_guard_test.go index 38fd4126a..854bf45f0 100644 --- a/terraform/environments/aws/ci-cd-permissions/fck_nat_guard_test.go +++ b/terraform/environments/aws/ci-cd-permissions/fck_nat_guard_test.go @@ -61,11 +61,11 @@ func TestDeployPolicyAllowsFckNatNetworkInterface(t *testing.T) { } } -// moduleBlock returns the body of `kind "type" "name" {...}` from comment- -// stripped HCL, or `kind "type" {...}` when name is empty. -func moduleBlock(t *testing.T, src, kind, typ, name string) string { +// resourceBlock returns the body of `resource "type" "name" {...}` from +// comment-stripped HCL, or `resource "type" {...}` when name is empty. +func resourceBlock(t *testing.T, src, typ, name string) string { t.Helper() - head := kind + ` "` + typ + `"` + head := `resource "` + typ + `"` if name != "" { head += ` "` + name + `"` } @@ -112,10 +112,10 @@ func TestFckNatApplyOrderAndWiring(t *testing.T) { } src := stripCommentLines(string(raw)) - route := moduleBlock(t, src, "resource", "aws_route", "private_internet_ipv4") - asg := moduleBlock(t, src, "resource", "aws_autoscaling_group", "fck_nat") - eni := moduleBlock(t, src, "resource", "aws_network_interface", "fck_nat") - policy := moduleBlock(t, src, "resource", "aws_iam_role_policy", "fck_nat_ec2") + route := resourceBlock(t, src, "aws_route", "private_internet_ipv4") + asg := resourceBlock(t, src, "aws_autoscaling_group", "fck_nat") + eni := resourceBlock(t, src, "aws_network_interface", "fck_nat") + policy := resourceBlock(t, src, "aws_iam_role_policy", "fck_nat_ec2") dependsOn := func(block, target string) bool { for _, dep := range strings.Split(bracketValue(t, block, "depends_on"), ",") { From faf5271259ce87488fd0ae319094ee03e6e68655 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 14:29:15 +0100 Subject: [PATCH 09/10] test(networking): pin fck-nat ENI exposure and IMDSv2 Assert the persistent ENI carries exactly the fck-nat security group, that group admits only the VPC CIDR (no 0.0.0.0/0, ::/0 or other source) and the launch template requires IMDSv2. Refs #716 Co-Authored-By: Claude Opus 5.5 --- .../aws/tests/fck_nat_eni.tftest.hcl | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) 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 73ddb51d6..bb5d1212f 100644 --- a/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl +++ b/terraform/modules/networking/aws/tests/fck_nat_eni.tftest.hcl @@ -117,6 +117,26 @@ run "persistent_eni_is_attachable_by_every_instance" { error_message = "The fck-nat launch template must set instance_initiated_shutdown_behavior = \"terminate\" so the bounded-restart poweroff gets the instance replaced instead of left stopped." } + # Network exposure of the NAT: the ENI carries only the fck-nat SG, and that + # SG admits nothing but the VPC's own CIDR. + assert { + condition = toset(aws_network_interface.fck_nat[0].security_groups) == toset([aws_security_group.fck_nat[0].id]) + error_message = "The persistent ENI must carry exactly the fck-nat security group; with none, the VPC default group applies." + } + + assert { + condition = length(aws_security_group.fck_nat[0].ingress) > 0 && alltrue([ + for r in aws_security_group.fck_nat[0].ingress : + toset(r.cidr_blocks) == toset([var.vpc_cidr]) && length(coalesce(r.ipv6_cidr_blocks, [])) == 0 && length(coalesce(r.security_groups, [])) == 0 && length(coalesce(r.prefix_list_ids, [])) == 0 + ]) + error_message = "Every fck-nat security group ingress rule must admit only the VPC CIDR: no 0.0.0.0/0, no ::/0, no other source." + } + + assert { + condition = aws_launch_template.fck_nat[0].metadata_options[0].http_tokens == "required" + error_message = "The fck-nat launch template must require IMDSv2 (http_tokens = required)." + } + assert { condition = anytrue([ for s in jsondecode(aws_iam_role_policy.fck_nat_ec2[0].policy).Statement : From 0cde2b02ab095b49fbda730c02c5d09a1005ccf6 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 14:29:26 +0100 Subject: [PATCH 10/10] docs(networking): correct fck-nat comments The NAT is one instance in total, not one per AZ. The restart bound catches a hung or failed ENI attach, not every NAT misconfiguration: upstream fck-nat.sh has no set -e and exits 0 if the primary ENI modify call fails. Refs #716 Co-Authored-By: Claude Opus 5.5 --- .../aws/ci-cd-permissions/policy_networking.tf | 2 +- terraform/modules/networking/aws/main.tf | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf index a60e61539..3a7b04827 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf @@ -113,7 +113,7 @@ resource "aws_iam_policy" "networking" { }, { # ec2:RunInstances is needed for the fck-nat AutoScaling group - # (one t4g.nano per AZ). Restricting allowed instance types prevents + # (one t4g.nano NAT instance in total). Restricting allowed instance types prevents # the deploy SA from launching arbitrary large instances and attaching # CUDly IAM roles to exfiltrate credentials or run compute at account # cost. diff --git a/terraform/modules/networking/aws/main.tf b/terraform/modules/networking/aws/main.tf index 4ecd40c43..138f5b01c 100644 --- a/terraform/modules/networking/aws/main.tf +++ b/terraform/modules/networking/aws/main.tf @@ -254,8 +254,11 @@ resource "aws_launch_template" "fck_nat" { # still terminating has released it), and NATs traffic arriving on it out # of the primary interface. The unit is Type=oneshot with no start timeout # and an unbounded attach loop, so the restart is bounded: on timeout or - # failure (exit status means attached AND NATing) the instance powers off - # and the ASG replaces it, instead of hanging with a green health check. + # failure the instance powers off and the ASG replaces it, instead of + # hanging with a green health check. This catches a hung or failed ENI + # attach (the script's attach and ip link waits are blocking loops), not + # every NAT misconfiguration: the script has no set -e and exits 0 even if + # the primary ENI's modify-network-interface-attribute call fails. user_data = base64encode(<<-EOF #!/bin/sh : > /etc/fck-nat.conf