Address review follow-up: egress-IP doc, negative-path tests, cleanups

- README: note that toggling the mode changes the stack's public egress IP
  (NAT EIPs released on disable; egress via the TGW's NAT when enabled), so
  anything allowlisting Quilt's egress address must be updated or it breaks
  silently.
- examples/main.tf: align the IPv6 comment with the corrected mechanism (off =
  no IPv6 default route, clients use IPv4) instead of "IPv4 fallback".
- vpc tests: add negative-path runs — enabled-without-id (attachment
  precondition), malformed id (variable validation), and id-set-without-enable
  (no-op: no attachment/routes).
- vpc main.tf: hoist the repeated `new_network_valid && enable_transit_gateway`
  count gate into a `transit_gateway_enabled` local; add a comment on why the
  attachment lands in intra subnets (ENI placement only).
- quilt: align transit_gateway_id description with the vpc module (the new-VPC
  scoping lives on enable_transit_gateway).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: sir-sigurd

README.md

@@ -823,7 +823,12 @@ delay.
 **Reversibility:** removing `enable_transit_gateway` (or setting it `false`)
 restores the NAT gateways and IPv6 egress-only IGW. Toggling it on or off for an
 already-deployed VPC recreates/destroys NAT gateways and their Elastic IPs and
-briefly interrupts egress, so do it in a maintenance window.
+briefly interrupts egress, so do it in a maintenance window. Either direction
+also **changes the stack's public egress IP** — disabling releases the NAT
+Elastic IPs (AWS won't hand the same ones back), and enabling sends egress out
+through the TGW's NAT instead — so anything that allowlists Quilt's egress
+address (a license endpoint, a partner firewall, a SaaS IP allowlist) must be
+updated, or it breaks silently.
 
 ### Profile
 You may wish to set a specific AWS profile before executing `terraform`

examples/main.tf

@@ -139,7 +139,7 @@ module "quilt" {
   # api_endpoint        = "vpce-YOUR-VPC-ENDPOINT"                         # VPC endpoint (if internal = true)
   # enable_transit_gateway = true                                          # Route private-subnet egress via a TGW instead of NAT (create_new_vpc = true only)
   # transit_gateway_id     = "tgw-YOUR-TRANSIT-GATEWAY-ID"                  # Required when enable_transit_gateway = true; the TGW must route to the internet and back
-  # transit_gateway_ipv6_egress = true                                     # Only if the TGW carries IPv6 egress (otherwise IPv6 is left on IPv4 fallback)
+  # transit_gateway_ipv6_egress = true                                     # Only if the TGW carries IPv6 egress; off = no IPv6 default route (clients use IPv4)
 
   # CloudFormation notifications (optional)
   # stack_notification_arns = ["arn:aws:sns:YOUR-AWS-REGION:YOUR-ACCOUNT-ID:quilt-notifications"]

modules/quilt/variables.tf

@@ -38,7 +38,7 @@ variable "enable_transit_gateway" {
 variable "transit_gateway_id" {
   type        = string
   default     = null
-  description = "Transit Gateway ID for private subnet egress when creating a new VPC. Required when enable_transit_gateway == true; may be a computed value (e.g. a TGW created in the same configuration)."
+  description = "Transit Gateway ID for private subnet egress. Required when enable_transit_gateway == true; may be a computed value (e.g. a TGW created in the same configuration)."
   validation {
     condition     = var.transit_gateway_id == null || can(regex("^tgw-[0-9a-f]+$", var.transit_gateway_id))
     error_message = "transit_gateway_id must be null or a valid Transit Gateway ID (e.g. tgw-0123456789abcdef0)."

modules/vpc/main.tf

@@ -33,6 +33,11 @@ locals {
   new_network_valid      = alltrue(values(local.new_network_requires))
   configuration_error    = !local.existing_network_valid && !local.new_network_valid
 
+  # TGW egress is gated on the bool (not transit_gateway_id != null) so the
+  # resource counts stay known at plan time even when transit_gateway_id is a
+  # computed value (e.g. a TGW created in the same configuration).
+  transit_gateway_enabled = local.new_network_valid && var.enable_transit_gateway
+
   azs          = slice(data.aws_availability_zones.available.names, 0, 2)
   subnet_cidrs = [for k, v in local.azs : cidrsubnet(var.cidr, 1, k)]
 }
@@ -78,11 +83,11 @@ module "vpc" {
 }
 
 resource "aws_ec2_transit_gateway_vpc_attachment" "egress" {
-  # Gate on the bool, not on transit_gateway_id != null: count must be known at
-  # plan time, and transit_gateway_id may be a computed value (e.g. a TGW
-  # created in the same configuration).
-  count = local.new_network_valid && var.enable_transit_gateway ? 1 : 0
+  count = local.transit_gateway_enabled ? 1 : 0
 
+  # Intra subnets only host the attachment ENIs (they have no internet route).
+  # The egress default routes go in the private route tables below — don't move
+  # this to private_subnets.
   subnet_ids         = module.vpc.intra_subnets
   transit_gateway_id = var.transit_gateway_id
   vpc_id             = module.vpc.vpc_id
@@ -101,7 +106,7 @@ resource "aws_ec2_transit_gateway_vpc_attachment" "egress" {
 }
 
 resource "aws_route" "private_tgw_ipv4_egress" {
-  count = local.new_network_valid && var.enable_transit_gateway ? length(module.vpc.private_route_table_ids) : 0
+  count = local.transit_gateway_enabled ? length(module.vpc.private_route_table_ids) : 0
 
   route_table_id         = module.vpc.private_route_table_ids[count.index]
   destination_cidr_block = "0.0.0.0/0"
@@ -111,7 +116,7 @@ resource "aws_route" "private_tgw_ipv4_egress" {
 }
 
 resource "aws_route" "private_tgw_ipv6_egress" {
-  count = local.new_network_valid && var.enable_transit_gateway && var.transit_gateway_ipv6_egress ? length(module.vpc.private_route_table_ids) : 0
+  count = local.transit_gateway_enabled && var.transit_gateway_ipv6_egress ? length(module.vpc.private_route_table_ids) : 0
 
   route_table_id              = module.vpc.private_route_table_ids[count.index]
   destination_ipv6_cidr_block = "::/0"

modules/vpc/tests/validation.tftest.hcl

@@ -293,3 +293,76 @@ run "existing_vpc_with_transit_gateway_is_rejected" {
   # ignore the request.
   expect_failures = [output.configuration_error]
 }
+
+run "transit_gateway_enabled_without_id_is_rejected" {
+  command = plan
+
+  variables {
+    create_new_vpc               = true
+    internal                     = false
+    enable_transit_gateway       = true
+    transit_gateway_id           = null
+    existing_vpc_id              = null
+    existing_api_endpoint        = null
+    existing_intra_subnets       = null
+    existing_private_subnets     = null
+    existing_public_subnets      = null
+    existing_user_security_group = null
+    existing_user_subnets        = null
+  }
+
+  # enable_transit_gateway = true requires a transit_gateway_id; the attachment
+  # precondition must reject a null id.
+  expect_failures = [aws_ec2_transit_gateway_vpc_attachment.egress]
+}
+
+run "transit_gateway_id_invalid_format_is_rejected" {
+  command = plan
+
+  variables {
+    create_new_vpc               = true
+    internal                     = false
+    enable_transit_gateway       = true
+    transit_gateway_id           = "not-a-tgw-id"
+    existing_vpc_id              = null
+    existing_api_endpoint        = null
+    existing_intra_subnets       = null
+    existing_private_subnets     = null
+    existing_public_subnets      = null
+    existing_user_security_group = null
+    existing_user_subnets        = null
+  }
+
+  # A malformed transit_gateway_id must be rejected by the variable validation.
+  expect_failures = [var.transit_gateway_id]
+}
+
+run "transit_gateway_id_without_enable_is_noop" {
+  command = plan
+
+  variables {
+    create_new_vpc               = true
+    internal                     = false
+    enable_transit_gateway       = false
+    transit_gateway_id           = "tgw-00000000000000000"
+    existing_vpc_id              = null
+    existing_api_endpoint        = null
+    existing_intra_subnets       = null
+    existing_private_subnets     = null
+    existing_public_subnets      = null
+    existing_user_security_group = null
+    existing_user_subnets        = null
+  }
+
+  # transit_gateway_id is the value, enable_transit_gateway is the toggle: an id
+  # set without enabling the mode is a no-op — no attachment, no TGW routes.
+  assert {
+    condition     = length(aws_ec2_transit_gateway_vpc_attachment.egress) == 0
+    error_message = "No TGW attachment should be created when enable_transit_gateway is false"
+  }
+
+  assert {
+    condition     = length(aws_route.private_tgw_ipv4_egress) == 0 && length(aws_route.private_tgw_ipv6_egress) == 0
+    error_message = "No TGW egress routes should be created when enable_transit_gateway is false"
+  }
+}