test(integ): sg-circular-dependency failure-seeking integration test (#842)

Author: go-to-k

docs/_generated/integ-coverage.json

@@ -743,6 +743,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "vpc-lambda",
         "vpc-lambda-cr-race",
         "vpc-nat-gateway"
@@ -812,6 +813,9 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2"
+        ],
         "vpc-lambda": [
           "l2"
         ],
@@ -847,6 +851,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "vpc-lambda",
         "vpc-lambda-cr-race",
         "vpc-nat-gateway"
@@ -915,6 +920,9 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2"
+        ],
         "vpc-lambda": [
           "l2"
         ],
@@ -973,6 +981,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "vpc-lambda",
         "vpc-lambda-cr-race",
         "vpc-nat-gateway"
@@ -1042,6 +1051,9 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2"
+        ],
         "vpc-lambda": [
           "l2"
         ],
@@ -1077,6 +1089,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "vpc-lambda",
         "vpc-lambda-cr-race",
         "vpc-nat-gateway"
@@ -1146,6 +1159,9 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2"
+        ],
         "vpc-lambda": [
           "l2"
         ],
@@ -1178,6 +1194,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "update-replace",
         "vpc-lambda",
         "vpc-lambda-cr-race"
@@ -1238,6 +1255,10 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2",
+          "literal"
+        ],
         "update-replace": [
           "l2",
           "literal"
@@ -1253,11 +1274,15 @@
     {
       "resourceType": "AWS::EC2::SecurityGroupIngress",
       "integs": [
-        "alb"
+        "alb",
+        "sg-circular-dependency"
       ],
       "signals": {
         "alb": [
           "literal"
+        ],
+        "sg-circular-dependency": [
+          "literal"
         ]
       }
     },
@@ -1285,6 +1310,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "vpc-lambda",
         "vpc-lambda-cr-race",
         "vpc-nat-gateway"
@@ -1354,6 +1380,9 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2"
+        ],
         "vpc-lambda": [
           "l2"
         ],
@@ -1400,6 +1429,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "vpc-lambda",
         "vpc-lambda-cr-race",
         "vpc-nat-gateway"
@@ -1469,6 +1499,9 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2"
+        ],
         "vpc-lambda": [
           "l2"
         ],
@@ -1504,6 +1537,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "vpc-lambda",
         "vpc-lambda-cr-race",
         "vpc-nat-gateway"
@@ -1574,6 +1608,9 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2"
+        ],
         "vpc-lambda": [
           "l2"
         ],
@@ -1609,6 +1646,7 @@
         "rds-dbinstance-backfill",
         "remove-protection",
         "rollback-failure-injection",
+        "sg-circular-dependency",
         "vpc-lambda",
         "vpc-lambda-cr-race",
         "vpc-nat-gateway"
@@ -1678,6 +1716,9 @@
         "rollback-failure-injection": [
           "l2"
         ],
+        "sg-circular-dependency": [
+          "l2"
+        ],
         "vpc-lambda": [
           "l2"
         ],

docs/_generated/scenario-coverage.json

@@ -168,6 +168,10 @@
       "tag": "s3-asset-deploy",
       "description": "File/ZIP asset publishing during `cdkd deploy`: a multi-file local directory is zipped + uploaded to the CDK bootstrap asset bucket by `FileAssetPublisher` (content-addressed, skip-if-exists), the Lambda `Code.S3Bucket`/`Code.S3Key` ref is wired to the uploaded object (CodeSize proves it is NOT inline), AND a generic `s3_assets.Asset` upload is read back at runtime via cdkd-resolved bucket/key env vars. Bootstrap-bucket asset objects persist by design across destroy."
     },
+    {
+      "tag": "sg-circular-dependency",
+      "description": "Circular Security Group reference (SG-A ingress from SG-B AND SG-B ingress from SG-A) modeled via standalone AWS::EC2::SecurityGroupIngress resources. DAG builder must not raise a false cycle; destroy must revoke both ingress rules BEFORE deleting either SG (SecurityGroup-after-SecurityGroupIngress implicit-delete-dep) or AWS rejects DeleteSecurityGroup with DependencyViolation."
+    },
     {
       "tag": "state-bucket-region-resolve",
       "description": "State-bucket S3 clients (state backend + lock manager) auto-detect bucket region via `GetBucketLocation` regardless of caller-profile region."
@@ -943,6 +947,13 @@
       "annotated": true,
       "scenarios": []
     },
+    {
+      "name": "sg-circular-dependency",
+      "annotated": true,
+      "scenarios": [
+        "sg-circular-dependency"
+      ]
+    },
     {
       "name": "sns-sqs-event",
       "annotated": true,
@@ -1340,6 +1351,13 @@
         "s3-asset-deploy"
       ]
     },
+    {
+      "scenario": "sg-circular-dependency",
+      "description": "Circular Security Group reference (SG-A ingress from SG-B AND SG-B ingress from SG-A) modeled via standalone AWS::EC2::SecurityGroupIngress resources. DAG builder must not raise a false cycle; destroy must revoke both ingress rules BEFORE deleting either SG (SecurityGroup-after-SecurityGroupIngress implicit-delete-dep) or AWS rejects DeleteSecurityGroup with DependencyViolation.",
+      "fixtures": [
+        "sg-circular-dependency"
+      ]
+    },
     {
       "scenario": "state-bucket-region-resolve",
       "description": "State-bucket S3 clients (state backend + lock manager) auto-detect bucket region via `GetBucketLocation` regardless of caller-profile region.",

docs/changelog-cdkd.md

@@ -19,6 +19,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary
 **Recently Implemented** (2026-06-13):
 
 - ✅ **New `multi-asset` integ stresses the asset-publishing layer when MANY assets of TWO kinds publish concurrently in one deploy** — `tests/integration/multi-asset/**`. The existing `docker-image-asset` and `s3-asset-deploy` fixtures each exercise ONE publisher in isolation (ECR build+push vs a single S3 zip upload), leaving the CONCURRENT multi-asset case — `FileAssetPublisher` + `DockerAssetPublisher` interleaved, ECR + S3 in one run, and several distinct S3 uploads in flight together — without a dedicated regression. The new `CdkdMultiAssetExample` stack (no VPC) publishes **1 ECR image + 4 S3 objects** in a single deploy: a `lambda.DockerImageFunction` (`DockerImageCode.fromImageAsset(docker/, { platform: LINUX_ARM64 })` + `architecture: ARM_64`, matched, to avoid the cross-arch `Runtime.InvalidEntrypoint: ProcessSpawnFailed` trap on Apple-Silicon hosts) goes through `DockerAssetPublisher`; three `lambda.Function`s (`AlphaHandler`/`BetaHandler`/`GammaHandler`, Python 3.12) each `Code.fromAsset('<distinct multi-file dir>')` produce three DISTINCT `FileAssetPublisher` S3 uploads (distinct content -> distinct content-addressed asset hash, confirmed at synth: 5 file assets + 1 docker image in the manifest); and a generic `s3_assets.Asset` (`asset-data/`) is a 4th S3 upload whose resolved `s3BucketName`/`s3ObjectKey` are threaded into the alpha Lambda via `CONFIG_BUCKET`/`CONFIG_KEY` env (asset-ref intrinsic resolution). The load-bearing correctness proof is that **each Lambda returns its OWN distinct marker** (`...-docker` / `...-alpha` / `...-beta` / `...-gamma`): a cross-wired asset (e.g. the beta ZIP uploaded but cdkd pointed the alpha Lambda's `Code.S3Bucket`/`S3Key` at it) would return the WRONG marker and FAIL — so the markers prove not just that all assets uploaded but that each Lambda was wired to the RIGHT one. `verify.sh` (BSD/macOS-portable, real-rc capture + explicit `[verify] PASS`) gracefully SKIPs (exit 0) when `docker info` fails so it is robust on a Docker-less box but runs in a Docker env; it then deploys (printing which asset/resource failed for triage on a deploy error), asserts the Docker Lambda is `PackageType=Image` with OUR pushed image present in ECR by content-tag (parsed from `Code.ImageUri`), asserts each zip Lambda's `CodeSize > 500` bytes (ran from an uploaded ZIP, not inline), invokes all 4 Lambdas and asserts each distinct marker, asserts the alpha Lambda's generic-asset read-back (`configBytes > 0`), then destroys and asserts clean (all 4 Lambdas gone, OUR pushed ECR image by tag gone via a sweep fallback, state file gone). The shared bootstrap container-assets ECR repo + the bootstrap asset bucket OBJECTS persist by design (cdkd does not own CDK bootstrap infra) and are NOT treated as orphans; the EXIT trap sweeps leftover state, the deployment-events sidecar, and (by tag) the pushed image. New scenario tag `multi-asset` in the canonical taxonomy (`scripts/build-scenario-coverage-matrix.ts`); coverage matrices regenerated. (NOTE: not yet run against real AWS — needs `/run-integ multi-asset` (in a Docker env) before merge.)
+- ✅ **New `sg-circular-dependency` integ surfaces create/destroy DAG-ordering bugs with a circular Security Group reference (test-only — no `src/` change)** — `tests/integration/sg-circular-dependency/**`. Models the classic CloudFormation cycle the CFn-safe way: SG-A allows ingress from SG-B AND SG-B allows ingress from SG-A, where each rule is a STANDALONE `AWS::EC2::SecurityGroupIngress` resource (not inline) so the two SGs can exist before the cross-references are added. In CDK, `sgA.addIngressRule(sgB, ...)` + `sgB.addIngressRule(sgA, ...)` against two distinct SG constructs makes CDK emit standalone ingress resources (each `Fn::GetAtt`s `GroupId` on the SG it attaches to and `SourceSecurityGroupId` on the OTHER SG), breaking what would otherwise be a genuine SG-to-SG cycle. The stack is a `natGateways: 0` single-AZ VPC + SG-A + SG-B + the two cross-referencing ingress resources (no EC2 instances). `verify.sh` (BSD/macOS-portable, captures real exit codes, prints an explicit `=== PASS ===` only on full success): **Phase 0** runs `cdkd synth` and asserts >= 2 standalone `AWS::EC2::SecurityGroupIngress` resources each carrying a `SourceSecurityGroupId` (true SG-to-SG cross-ref) so the cycle-breaking shape is confirmed before any AWS call; **Phase 1** deploys (the DAG builder in `src/analyzer/dag-builder.ts` must NOT raise a false `DependencyError` — the standalone ingress resources break the would-be cycle) and asserts both SGs exist with the live cross-reference (SG-A's `IpPermissions[].UserIdGroupPairs[].GroupId` contains SG-B and vice versa); **Phase 2 (the key test)** destroys and asserts 0 errors — if cdkd deletes an SG while its cross-referencing ingress rule is still live, AWS rejects `DeleteSecurityGroup` with `DependencyViolation: resource sg-xxx has a dependent object`, so a wrong delete order fails / orphans here. The post-destroy assertions confirm both SGs + the VPC + the state file are gone. Resources are located by the `cdkd:integ-fixture=sg-circular-dependency` tag (NOT `aws:cdk:path`, which AWS reserves), and the EXIT-trap cleanup revokes-then-deletes both SGs directly (the SAME ordering cdkd must perform) so a destroy-ordering bug never leaks billing resources. The existing `AWS::EC2::SecurityGroup -> AWS::EC2::SecurityGroupIngress` implicit-delete-dep edge in `src/analyzer/implicit-delete-deps.ts` is what this fixture exercises end-to-end on real AWS. New scenario tag `sg-circular-dependency` in the canonical taxonomy (`scripts/build-scenario-coverage-matrix.ts`); integ-coverage + scenario-coverage matrices regenerated. One fix surfaced while authoring: AWS rejects non-ASCII characters (em-dash U+2014) in a SecurityGroup `GroupDescription`, so the two `GroupDescription` strings are ASCII-only (hyphen, not em-dash). Validated green against real AWS (deploy + destroy clean, 0 orphans).
 - ✅ **Hardened post-destroy assertions across three new behavior-class integ fixtures so a silently-skipped delete cannot pass as clean (issue [#831](https://github.com/go-to-k/cdkd/issues/831), test-only — no `src/` change)** — `tests/integration/{update-replace,destroy-interrupt,deployment-events}/verify.sh`. Code review of the new `update-replace` integ (PR #830) surfaced a hardening gap shared by several of the new fixtures: their post-destroy assertions checked only that `state.json` was gone (and S3 buckets), but did NOT explicitly assert that the named NON-bucket resources (Lambda / IAM Role / SecurityGroup / SNS Topic) were actually deleted from AWS. Per the `feedback_protection_integ_must_instantiate_resource` rule ("state-empty misses an orphan carrying no stack name"), a `state destroy` that silently skips a resource (e.g. an SG delete blocked by a lingering ENI, or an IAM role that is not VPC-bound and orphans independently) would leave a real orphan while the test still passed. The fixtures now assert each named resource is NOT-FOUND in AWS after destroy: **`update-replace`** — the Lambda (`WorkerFn`, `aws lambda get-function-configuration`), IAM Role (`WorkerRole`, `aws iam get-role`), and SecurityGroup (`WorkerSg`, `aws ec2 describe-security-groups`) physical ids captured from state in Phase 1 must each error post-destroy, on top of the two existing bucket checks; plus a belt-and-suspenders `aws s3 rb --force` of the predictable `cdkd-update-replace-{account}-{region}-v1`/`-v2` bucket names in the cleanup trap so a re-run is not blocked after a mid-replacement crash that left a v1/v2 bucket behind. **`destroy-interrupt`** — already resolved the backing Lambda + VPC (covering subnets/SG/ENI implicitly) + each SSM parameter id from state; added EXPLICIT not-found assertions for the CR handler's IAM Role (`aws iam get-role` — NOT VPC-bound, so the VPC-gone assert did NOT cover it) and the Lambda SecurityGroup (`aws ec2 describe-security-groups` — belt-and-suspenders on top of the VPC-gone implication). **`deployment-events`** — already asserted the SSM parameter gone; added an SNS Topic not-found assertion (`aws sns get-topic-attributes` against the deterministic `${STACK}-topic` ARN resolved via `sts get-caller-identity`) plus a matching topic delete in the failure-cleanup path. All three scripts stay BSD/macOS-portable (no `grep -P` / `date -d`), use `jq has()` for boolean probes, capture real exit codes, and print `[verify] PASS` only on full success — following the own-`cdkd:integ-fixture`-tag pattern already used by `rollback-failure-injection` (these three resolve physical ids from cdkd state instead, which is equally reliable and needs no `lib/` change). Resource TYPES are unchanged, so the integ-coverage / scenario-coverage matrices regenerated with no diff. (NOTE: `update-replace` and `deployment-events` need a real-AWS re-run to validate the new assertions; `destroy-interrupt` adds only two cheap not-found checks to an already-passing fixture.)
 - ✅ **New `s3-asset-deploy` integ exercises the S3 file/ZIP asset-publishing path during a real `cdkd deploy`** — `tests/integration/s3-asset-deploy/**`. Most Lambda fixtures use inline code (`Code.fromInline`) or never assert the asset upload itself, leaving cdkd's `FileAssetPublisher` (S3 zip + upload, content-addressed skip-if-exists) without a dedicated end-to-end regression. The new `CdkdS3AssetDeployExample` stack closes that gap: a Lambda whose code comes from a **local multi-file directory** (`lambda/` — handler + `helpers/` + `vendored/` Python sub-packages, so the asset is a genuine multi-file ZIP, not a trivial single file) forces cdkd to zip the directory + upload it to the CDK bootstrap asset bucket and wire the function's `Code.S3Bucket`/`Code.S3Key` to the uploaded object; AND a generic `s3_assets.Asset` (`asset-data/`) is uploaded to the same bucket with its resolved `s3BucketName`/`s3ObjectKey` threaded into the Lambda as `CONFIG_BUCKET`/`CONFIG_KEY` env vars (synth confirms both are emitted as `Fn::Sub`-backed bucket + literal key refs, exercising cdkd's intrinsic resolver). `verify.sh` (BSD/macOS-portable, real-rc + explicit `[verify] PASS`) deploys, asserts the function's `CodeSize > 500` bytes (proves it ran from the uploaded ZIP, not inline), invokes it and asserts the handler marker (`cdkd-s3-asset-deploy-marker-v1`) plus a non-zero `configBytes` from the generic-asset S3 read-back (proving that upload reached AWS and the bucket/key env wiring resolved), then destroys and asserts the Lambda + state file are gone with 0 errors. The bootstrap-bucket asset OBJECTS persist by design (cdkd does not own / delete the CDK bootstrap bucket) — the script deliberately does NOT assert their absence and notes this. New scenario tag `s3-asset-deploy` in the canonical taxonomy (`scripts/build-scenario-coverage-matrix.ts`); coverage matrices regenerated. (NOTE: not yet run against real AWS — needs `/run-integ s3-asset-deploy` before merge.)
 - ✅ **New `drift-revert-arrays` integ broadens drift coverage to TAG-heavy / ARRAY-heavy resource types (refs issue [#802](https://github.com/go-to-k/cdkd/issues/802))** — `tests/integration/drift-revert-arrays/` (new fixture; no `src/` change). Issue #802 added `src/analyzer/drift-normalize.ts` (`canonicalizeTagListsDeep` + `canonicalizeIdArraysDeep`) so a benign AWS-side reorder of a tag list (`{Key,Value}[]`) or a resource-id / ARN array no longer surfaces as phantom drift, but the existing `drift-revert` / `drift-revert-vpc` fixtures carry none of those unordered-set array shapes — the canonicalization path had unit coverage only. This fixture (`CdkdDriftArraysExample`) deploys an S3 Bucket, SNS Topic, SQS Queue (each with six user tags), an IAM ManagedPolicy with a multi-statement document carrying multiple `Action[]` (plain scalar arrays — intentionally NOT canonicalized) + multiple `Resource[]` ARN arrays (canonicalized) + six tags, and a VPC (`natGateways: 0`, no NAT cost) + SecurityGroup with four CIDR ingress rules + six tags. `verify.sh` asserts (a) **no false positive on a clean deploy** (`cdkd drift` exit 0 even though AWS reorders the tag lists / ARN arrays on readback), (b) **no false positive on an induced reorder** (`inject-drift.ts reorder` re-PUTs the same six S3 tags reversed; `cdkd drift` still exit 0 — proves `canonicalizeTagListsDeep`), (c) **true drift still detected** (`inject-drift.ts drift` changes a tag VALUE + adds a managed-policy Action + authorizes a new SG ingress rule out of band; `cdkd drift` exit 1), then `cdkd drift --revert -y` reverts and a follow-up `cdkd drift` is clean, and destroy leaves 0 orphans. The script is BSD/macOS-portable (no `grep -P` / `date -d`), captures each `cdkd drift` real exit code, hard-fails with a canonicalizer-naming message, and prints `[verify] PASS` only on full success. The `subnet-…` / `sg-…` resource-id branch of `canonicalizeIdArraysDeep` stays unit-covered (`tests/unit/analyzer/drift-normalize.test.ts`); the integ exercises the ARN branch of the same function end-to-end. New scenario tag `drift-revert-array-canonicalization`.