fix(analyzer): order NAT Gateway deletion before IGW / VPCGatewayAttachment on destroy (#823)

Author: go-to-k

.claude/rules/analyzer.md

@@ -28,5 +28,5 @@ paths:
 - Determines execution order with topological sort
 - **Implicit edge for Custom Resources**: any `AWS::IAM::Policy` / `AWS::IAM::RolePolicy` / `AWS::IAM::ManagedPolicy` attached to a Custom Resource's ServiceToken Lambda execution role automatically gets an edge to the Custom Resource, preventing the handler from being invoked before inline policy attachment returns (avoids mid-deploy AccessDenied race)
 - **Implicit edge for Lambda VpcConfig**: every `AWS::EC2::Subnet` / `AWS::EC2::SecurityGroup` referenced by a Lambda's `Properties.VpcConfig.SubnetIds` / `SecurityGroupIds` gets an explicit edge to the Lambda (`src/analyzer/lambda-vpc-deps.ts`). Defense-in-depth on top of `extractDependencies`; for the reversed deletion traversal this guarantees Lambda is removed before its Subnet/SG so the asynchronous ENI detach has time to complete before EC2 rejects the subnet/SG delete with `DependencyViolation`.
-- **Type-based deletion ordering rules**: `src/analyzer/implicit-delete-deps.ts` centralizes type-pair rules (e.g. VPC after Subnet, Subnet after Lambda) shared by the deploy DELETE phase and the standalone destroy command.
+- **Type-based deletion ordering rules**: `src/analyzer/implicit-delete-deps.ts` centralizes type-pair rules (e.g. VPC after Subnet, Subnet after Lambda, IGW + VPCGatewayAttachment after NatGateway) shared by the deploy DELETE phase and the standalone destroy command. The IGW / VPCGatewayAttachment after NatGateway edge (issue [#817](https://github.com/go-to-k/cdkd/issues/817)) mirrors the NAT-before-IGW ordering CloudFormation enforces: a NAT Gateway holds an Elastic IP mapped to the VPC's public address space, so detaching the IGW before the NAT is gone fails with `Network vpc-xxx has some mapped public address(es)` and the IGW delete then hangs (~19 min observed). No type-based rule is needed for the EIP itself — the NAT Ref's its EIP via `AllocationId`, so the reversed delete traversal already deletes the NAT before the EIP is released.
 - **CDK-defensive DependsOn relaxation (default-on)**: `src/analyzer/cdk-defensive-deps.ts` lists the (depender, dependee) type pairs CDK adds defensively for VPC-Lambda runtime egress (IAM Role / Policy / Lambda::Function / Lambda::Url / Lambda::EventSourceMapping → EC2 Route / SubnetRouteTableAssociation). The deploy code path constructs `DagBuilder({ relaxCdkVpcDefensiveDeps: true })` by default; the matching DependsOn edges are dropped at graph-build time so CloudFront Distribution + Lambda::Url + VPC Lambda dispatch in parallel with NAT Gateway stabilization (~55% faster on `bench-cdk-sample`). Pass `cdkd deploy --no-aggressive-vpc-parallel` to opt out (escape hatch for stacks where the user wants the strict CDK-defensive ordering — e.g. a Custom Resource that synchronously invokes a VPC Lambda outside cdkd's Lambda-ServiceToken Active wait). Only DependsOn entries in the allowlist are dropped — Ref / GetAtt and other DependsOn pairs are untouched.

docs/_generated/integ-last-run.tsv

@@ -1,5 +1,4 @@
 # integ-last-run ledger (update-type: one row per test). cols: test  last_run_iso  result  duration_s  flow  note
-vpc-nat-gateway	2026-06-01T21:12:13Z	PASS		standard	rc ok, orph clean
 dynamodb-streams	2026-06-01T21:12:13Z	PASS		standard	rc ok, orph clean
 composite-stack	2026-06-01T21:12:13Z	PASS		standard	rc ok, orph clean
 nested-stack	2026-06-01T21:12:13Z	PASS		standard	rc ok, orph clean
@@ -124,3 +123,4 @@ ecs-fargate	2026-06-13T04:11:31Z	PASS	353	verify.sh	#807 propagation + #809 writ
 microservices	2026-06-13T04:12:49Z	PASS	39	standard	#804 incremental destroy persistence; 19 deleted 0 err 0 orphan
 lambda	2026-06-13T04:41:51Z	PASS	76	verify.sh	#808 broad integ + cdkd events live-tested (deploy+destroy runs persisted); 9 deleted 0 err
 cross-region-state-bucket	2026-06-13T04:42:50Z	PASS	29	verify.sh	#803 LockManager cross-region; 1 deleted 0 err, temp bucket cleaned (exports-index 301 -> followup #819)
+vpc-nat-gateway	2026-06-13T05:15:45Z	PASS	328	standard	#817 IGW/NAT delete-order; 21 deleted 0 err 0 orphan (NAT before IGW/EIP)

docs/changelog-cdkd.md

@@ -19,6 +19,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary
 **Recently Implemented** (2026-06-13):
 
 - ✅ **`--region` deprecation warning no longer contradicts the actual behavior (issue [#818](https://github.com/go-to-k/cdkd/issues/818))** — `src/cli/options.ts`. `warnIfDeprecatedRegion` and the hidden `deprecatedRegionOption` help text both claimed `--region` "has no effect" on non-bootstrap commands, but every non-bootstrap command (`deploy`, `destroy`, `diff`, `synth`, `list`, `state`, `force-unlock`, `publish-assets`, `import`, `export`, `orphan`, `drift`, `events`, `local *`, …) actually consumes `options.region` as the highest-precedence region source: `const region = options.region || process.env['AWS_REGION'] || 'us-east-1'` feeds the provisioning / state-bucket SDK clients and the `applyRoleArnIfSet` STS hop, and `deploy` / `destroy` / `import` / `export` / `orphan` additionally inject it into `process.env.AWS_REGION` so the CDK synth subprocess inherits it (e.g. `deploy.ts` ~L167/L175/L341). The warning and the code therefore disagreed — a user passing `--region` was told it did nothing while it silently took effect. **Investigation determined `--region` IS legitimately honored everywhere (option B in the issue), so the fix is purely in the warning + help text — no command implementation (`deploy.ts` etc.) was touched**, keeping the change out of the `integ-broad` merge-gate scope and carrying zero behavior-change risk. The warning now reads "`--region is deprecated and will be removed in a future release. It is still honored for now (it overrides AWS_REGION / your AWS profile), but prefer the AWS_REGION environment variable or your AWS profile…`" and the option description drops the false "No effect" claim. The recommended mechanism is still `AWS_REGION` / the AWS profile; the flag stays hidden + deprecated, just honestly described. Docs corrected: two "deprecated and ignored" lines in [docs/cli-reference.md](cli-reference.md) and the `--region` bullet in [.claude/rules/cli-internals.md](../.claude/rules/cli-internals.md). Tests: `tests/unit/cli/options.test.ts` — the existing message assertion updated, plus new assertions that neither the warning nor the option description contains "no effect" and that both mention the flag is "still honored" (issue #818).
+- ✅ **`destroy` waits for NAT Gateway deletion before detaching / deleting the IGW + VPCGatewayAttachment (issue [#817](https://github.com/go-to-k/cdkd/issues/817))** — `src/analyzer/implicit-delete-deps.ts`. Destroying a VPC + NAT Gateway + IGW stack attempted the `VPCGatewayAttachment` detach while the NAT Gateway's Elastic IP was still mapped to the VPC's public address space, failing with `Network vpc-xxx has some mapped public address(es)`, after which the IGW delete hung (~19 min observed). This was the first-run failure split out of the #804 incident as a separate issue. The fix adds two type-based implicit delete-dependency edges so the shared deploy DELETE phase + standalone destroy command order the teardown like CloudFormation does: `AWS::EC2::InternetGateway` gains `AWS::EC2::NatGateway` (alongside its existing `AWS::EC2::VPCGatewayAttachment` dependee) and a new `AWS::EC2::VPCGatewayAttachment` key lists `AWS::EC2::NatGateway` — both are deleted AFTER the NAT Gateway is gone (NAT deletion releases / decouples the EIP). No type-based rule is needed for the EIP itself: the NAT Ref's its EIP via `AllocationId`, so the reversed delete traversal already deletes the NAT before the EIP is released. The injection logic (`destroy-runner.ts` / `deploy-engine.ts`) naturally produces no edge when no NatGateway is in state. Tests: 4 unit assertions in `tests/unit/analyzer/implicit-delete-deps.test.ts` (IGW-after-NAT edge, VPCGatewayAttachment-after-NAT edge, no NatGateway / EIP key registered; the existing no-self-cycle guard covers the new entries). Integ: the existing `vpc-nat-gateway` fixture (VPC + public/private subnets + IGW + NatGateway + EIP) exercises exactly this teardown end-to-end.
 - ✅ **`deploy` retries the ECS CapacityProvider same-stack infrastructure-role IAM-propagation race (issue [#805](https://github.com/go-to-k/cdkd/issues/805))** — `src/deployment/retryable-errors.ts`. cdkd's event-driven DAG dispatches the Cloud Control `CreateResource` for an `AWS::ECS::CapacityProvider` (Managed Instances) as soon as its same-stack infrastructure role finishes creating, and cdkd's fast SDK path creates the IAM role without waiting for propagation — so ECS tried to assume the just-created `InfrastructureRoleArn` before IAM had propagated it and rejected the create with `Caught ServiceAccessDeniedException for ECSInfrastructureRole[arn:...]`. The CC API handler classifies this as a terminal `InvalidRequest` (no internal retry, `SDK Attempt Count: 1`), and none of the existing message patterns matched it, so the deploy failed fast on a transient error. The fix adds `'Caught ServiceAccessDeniedException'` to `RETRYABLE_ERROR_MESSAGE_PATTERNS` — mirroring the `ENHANCED_MONITORING` pattern added for #794 — so the deploy engine's existing `withRetry` (8 attempts, ~47s cumulative) absorbs the propagation window; the phrase is anchored on the CC-API/ECS handler wording so a genuine, permanent role misconfiguration only burns the bounded retries before surfacing. Generic by design: any Cloud-Control-provisioned type that validates a same-stack IAM role at create time and surfaces `ServiceAccessDeniedException` is covered. Tests: the exact wire message from the issue classifies retryable + a plain `AccessDeniedException` (without the handler's "Caught" anchor) stays non-retryable in `retryable-errors.test.ts`. Verified by the issue reporter against the real-world 33-resource stack that surfaced the bug (the capacity provider create retried through the window and completed).
 - ✅ **`AWS::ECS::TaskDefinition` `Volumes[].ConfiguredAtLaunch` no longer silently dropped (issue [#806](https://github.com/go-to-k/cdkd/issues/806))** — `src/provisioning/providers/ecs-provider.ts`. `ECSProvider.convertVolumes` mapped only `Name` / `Host` / `EFSVolumeConfiguration` when converting CFn `Volumes` to the `RegisterTaskDefinition` wire shape; `ConfiguredAtLaunch` was dropped, so the registered task definition had no `configuredAtLaunch` volume and a same-stack `AWS::ECS::Service` carrying `VolumeConfigurations` (a managed EBS volume — CDK's `ServiceManagedVolume`) failed to create with "Volume configuration provided but no matching configuredAtLaunch volume found in task definition". The pre-flight property-coverage gate could not catch this class: it works at top-level property granularity and `Volumes` IS in `handledProperties` — the gap was one level down, inside the handled property. `convertVolumes` now forwards `configuredAtLaunch` via a `coerceBool` helper (same pattern as `EC2Provider`'s) that normalizes CFn boolean-ish values (`true` / `"true"` / `false` / `"false"`) at the wire boundary and returns `undefined` for absent props so the field is omitted from the SDK input (AWS keeps its default). No parallel `update()` change is needed — ECS TaskDefinitions are immutable revisioned resources; property changes route through Replace (CREATE then DELETE). Tests: 4 unit tests (present-true forwarded, string `"true"` / `"false"` coerced, absent omitted, explicit `false` preserved as distinct from omit). The `ecs-fargate` integ fixture gains a `ServiceManagedVolume` (1 GiB gp3, XFS) mounted into the container and attached to the Service via `service.addVolume()` — synthesizing exactly the `ConfiguredAtLaunch` + `VolumeConfigurations` pairing the bug broke (with `desiredCount: 0` no task launches, so no EBS volume is actually created); `verify.sh` asserts the registered task definition's `ebs-data` volume has `configuredAtLaunch == true` (probed via jq `has()` — the `//` operator would map an explicit `false` to the fallback) and that `DescribeServices` shows the deployment carrying the `ebs-data` volume configuration. Remaining `convertVolumes` sub-property gaps of the same class (`DockerVolumeConfiguration` / `FSxWindowsFileServerVolumeConfiguration` unmapped; `Host` / `EFSVolumeConfiguration` cast without PascalCase-to-camelCase conversion) are tracked separately per the issue.
 - ✅ **Cloud Control UPDATE re-includes write-only properties in every patch document (issue [#809](https://github.com/go-to-k/cdkd/issues/809))** — `src/provisioning/cloud-control-provider.ts` + new `src/provisioning/write-only-properties.ts`. Cloud Control applies UPDATE patches read-modify-write: the type's read handler returns the current model, the patch is applied on top, and the result becomes the desired state — but read handlers cannot return **write-only properties**, so any write-only property absent from cdkd's minimal previous-vs-desired patch silently vanished from the desired state on every CC-routed UPDATE. `AWS::ECS::Service` (writeOnlyProperties: `ServiceConnectConfiguration` / `VolumeConfigurations` / `ForceNewDeployment`) hard-failed: a task-definition-only change on a service with a managed EBS volume produced a patch without `VolumeConfigurations`, and `UpdateService` rejected with "Task definition has configuredAtLaunch volume but no volume configuration provided at runtime", wedging the stack (state still recorded the old properties, so every subsequent deploy retried the same failing patch). Types whose handler accepts the write-only-less state lost the configuration silently instead. The fix mirrors `terraform-provider-awscc`: `CloudControlProvider.update` now resolves the type's `writeOnlyProperties` from the registry schema via `cloudformation:DescribeType` (reduced to the top-level containing property — a nested path like `/properties/Foo/Bar` strips to `Foo`), removes those properties from the PREVIOUS side, and regenerates the patch — the generator then naturally emits `add` ops for every write-only property present in the desired properties, which is exactly what the CC read-modify-write contract requires. Only write-only properties are force-included (blanket-upserting all desired properties would risk false replacement signals on `createOnlyProperties` whose read-back form differs from the stored form). Only SUCCESSFUL DescribeType results are cached per resource type for the deploy lifetime in a module-level map, so repeated updates of the same type pay one throttled-API call; a DescribeType failure (missing IAM permission, transient throttle / 5xx) is NOT cached — it warns and falls back to the pre-#809 minimal patch for that update, and a later update of the same type retries DescribeType. Caching failures would let one transient throttle on the first CC-routed UPDATE silently disable write-only re-inclusion for every CC-routed type for the rest of the deploy, reintroducing the exact hard-fail this fixes. No regression for callers permanently without the new `cloudformation:DescribeType` permission — each update simply re-warns and re-falls-back. A DescribeType response without a `Schema` (e.g. a still-registering type) is treated as "no write-only properties" — a successful, cacheable, warning-free lookup. Removal-only write-only diffs skip the update entirely (CC cannot remove what its read handler never returns — pre-fix a `remove` op against a path absent from the current model would have failed), and the no-change fast path still skips without any DescribeType call. Tests: 11 unit tests in `tests/unit/provisioning/cloud-control-provider.test.ts` (unchanged write-only prop rides along as `add`; changed write-only prop not duplicated; nested-path top-level strip; no-write-only type keeps the minimal patch; DescribeType failure warning + fallback; per-type caching of successful lookups; failures NOT cached so a later update retries; retry-after-failure uses the populated set on success; Schema-less response = no write-only props + no warning; no-change skip; removal-only skip). Integ: the `ecs-fargate` fixture's `ServiceManagedVolume` + `CDKD_TEST_UPDATE` pass (issues #806/#807) exercises this exact path end-to-end once both land.

src/analyzer/implicit-delete-deps.ts

@@ -20,8 +20,20 @@
  *    the subnet down first triggers a DependencyViolation.
  */
 export const IMPLICIT_DELETE_DEPENDENCIES: Record<string, readonly string[]> = {
-  // IGW must be deleted AFTER VPCGatewayAttachment
-  'AWS::EC2::InternetGateway': ['AWS::EC2::VPCGatewayAttachment'],
+  // IGW must be deleted AFTER VPCGatewayAttachment, and AFTER the NAT
+  // Gateway. A NAT Gateway holds an Elastic IP mapped to the VPC's public
+  // address space; until the NAT is gone (which releases/decouples the EIP),
+  // EC2 rejects the IGW detach with `Network vpc-xxx has some mapped public
+  // address(es)` and the IGW delete then hangs. CloudFormation enforces this
+  // same NAT-before-IGW ordering. (The EIP itself does not need a type-based
+  // rule: the NAT Ref's the EIP via `AllocationId`, so the reversed delete
+  // traversal already deletes the NAT before the EIP is released.)
+  'AWS::EC2::InternetGateway': ['AWS::EC2::VPCGatewayAttachment', 'AWS::EC2::NatGateway'],
+
+  // VPCGatewayAttachment (the IGW<->VPC attachment) must be detached AFTER the
+  // NAT Gateway is gone — same `mapped public address(es)` rejection as the IGW
+  // delete above (the detach is the operation that actually trips the error).
+  'AWS::EC2::VPCGatewayAttachment': ['AWS::EC2::NatGateway'],
 
   // EventBus must be deleted AFTER Rules on that bus
   'AWS::Events::EventBus': ['AWS::Events::Rule'],

tests/unit/analyzer/implicit-delete-deps.test.ts

@@ -34,6 +34,27 @@ describe('IMPLICIT_DELETE_DEPENDENCIES', () => {
     );
   });
 
+  it('IGW must be deleted after NatGateway (EIP mapped-address release)', () => {
+    expect(IMPLICIT_DELETE_DEPENDENCIES['AWS::EC2::InternetGateway']).toContain(
+      'AWS::EC2::NatGateway'
+    );
+  });
+
+  it('VPCGatewayAttachment must be detached after NatGateway (EIP mapped-address release)', () => {
+    expect(
+      IMPLICIT_DELETE_DEPENDENCIES['AWS::EC2::VPCGatewayAttachment']
+    ).toContain('AWS::EC2::NatGateway');
+  });
+
+  it('does not register a NatGateway implicit-delete key (EIP handled by Ref edge)', () => {
+    // The NAT Ref's its EIP via `AllocationId`, so the reversed delete
+    // traversal already deletes the NAT before the EIP is released. NatGateway
+    // is only ever a dependee here, never a KEY, so no EIP type-based rule is
+    // needed (and there is no need to add one for AWS::EC2::EIP either).
+    expect(IMPLICIT_DELETE_DEPENDENCIES['AWS::EC2::NatGateway']).toBeUndefined();
+    expect(IMPLICIT_DELETE_DEPENDENCIES['AWS::EC2::EIP']).toBeUndefined();
+  });
+
   it('CloudFront OAC must be deleted after Distribution', () => {
     expect(
       IMPLICIT_DELETE_DEPENDENCIES['AWS::CloudFront::OriginAccessControl']