Skip to content

Commit ba5ba0f

Browse files
authored
test(integ): add drift-revert-arrays integ (tag-list / ARN-array canonicalization, #802) (#833)
1 parent 18d84ae commit ba5ba0f

17 files changed

Lines changed: 1522 additions & 15 deletions

docs/_generated/integ-coverage.json

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,7 @@
728728
"bench-cdk-sample",
729729
"cache-streaming",
730730
"docdb-neptune",
731+
"drift-revert-arrays",
731732
"drift-revert-vpc",
732733
"ec2-instance",
733734
"ec2-vpc",
@@ -763,6 +764,9 @@
763764
"docdb-neptune": [
764765
"l2"
765766
],
767+
"drift-revert-arrays": [
768+
"l2"
769+
],
766770
"drift-revert-vpc": [
767771
"l2"
768772
],
@@ -820,6 +824,7 @@
820824
"bench-cdk-sample",
821825
"cache-streaming",
822826
"docdb-neptune",
827+
"drift-revert-arrays",
823828
"drift-revert-vpc",
824829
"ec2-instance",
825830
"ec2-vpc",
@@ -855,6 +860,9 @@
855860
"docdb-neptune": [
856861
"l2"
857862
],
863+
"drift-revert-arrays": [
864+
"l2"
865+
],
858866
"drift-revert-vpc": [
859867
"l2"
860868
],
@@ -934,6 +942,7 @@
934942
"bench-cdk-sample",
935943
"cache-streaming",
936944
"docdb-neptune",
945+
"drift-revert-arrays",
937946
"drift-revert-vpc",
938947
"ec2-instance",
939948
"ec2-vpc",
@@ -969,6 +978,9 @@
969978
"docdb-neptune": [
970979
"l2"
971980
],
981+
"drift-revert-arrays": [
982+
"l2"
983+
],
972984
"drift-revert-vpc": [
973985
"l2"
974986
],
@@ -1026,6 +1038,7 @@
10261038
"bench-cdk-sample",
10271039
"cache-streaming",
10281040
"docdb-neptune",
1041+
"drift-revert-arrays",
10291042
"drift-revert-vpc",
10301043
"ec2-instance",
10311044
"ec2-vpc",
@@ -1061,6 +1074,9 @@
10611074
"docdb-neptune": [
10621075
"l2"
10631076
],
1077+
"drift-revert-arrays": [
1078+
"l2"
1079+
],
10641080
"drift-revert-vpc": [
10651081
"l2"
10661082
],
@@ -1118,6 +1134,7 @@
11181134
"bench-cdk-sample",
11191135
"cache-streaming",
11201136
"docdb-neptune",
1137+
"drift-revert-arrays",
11211138
"drift-revert-vpc",
11221139
"ec2-instance",
11231140
"ec2-vpc",
@@ -1150,6 +1167,9 @@
11501167
"docdb-neptune": [
11511168
"l2"
11521169
],
1170+
"drift-revert-arrays": [
1171+
"l2"
1172+
],
11531173
"drift-revert-vpc": [
11541174
"l2"
11551175
],
@@ -1210,6 +1230,7 @@
12101230
"bench-cdk-sample",
12111231
"cache-streaming",
12121232
"docdb-neptune",
1233+
"drift-revert-arrays",
12131234
"drift-revert-vpc",
12141235
"ec2-instance",
12151236
"ec2-vpc",
@@ -1245,6 +1266,9 @@
12451266
"docdb-neptune": [
12461267
"l2"
12471268
],
1269+
"drift-revert-arrays": [
1270+
"l2"
1271+
],
12481272
"drift-revert-vpc": [
12491273
"l2"
12501274
],
@@ -1313,6 +1337,7 @@
13131337
"bench-cdk-sample",
13141338
"cache-streaming",
13151339
"docdb-neptune",
1340+
"drift-revert-arrays",
13161341
"drift-revert-vpc",
13171342
"ec2-instance",
13181343
"ec2-vpc",
@@ -1348,6 +1373,9 @@
13481373
"docdb-neptune": [
13491374
"l2"
13501375
],
1376+
"drift-revert-arrays": [
1377+
"l2"
1378+
],
13511379
"drift-revert-vpc": [
13521380
"l2"
13531381
],
@@ -1405,6 +1433,7 @@
14051433
"bench-cdk-sample",
14061434
"cache-streaming",
14071435
"docdb-neptune",
1436+
"drift-revert-arrays",
14081437
"drift-revert-vpc",
14091438
"ec2-instance",
14101439
"ec2-vpc",
@@ -1440,6 +1469,9 @@
14401469
"docdb-neptune": [
14411470
"l2"
14421471
],
1472+
"drift-revert-arrays": [
1473+
"l2"
1474+
],
14431475
"drift-revert-vpc": [
14441476
"l2"
14451477
],
@@ -1498,6 +1530,7 @@
14981530
"bench-cdk-sample",
14991531
"cache-streaming",
15001532
"docdb-neptune",
1533+
"drift-revert-arrays",
15011534
"drift-revert-vpc",
15021535
"ec2-instance",
15031536
"ec2-vpc",
@@ -1533,6 +1566,9 @@
15331566
"docdb-neptune": [
15341567
"l2"
15351568
],
1569+
"drift-revert-arrays": [
1570+
"l2"
1571+
],
15361572
"drift-revert-vpc": [
15371573
"l2"
15381574
],
@@ -2709,6 +2745,7 @@
27092745
"data-analytics",
27102746
"diff-intrinsic-target-change",
27112747
"drift-revert",
2748+
"drift-revert-arrays",
27122749
"event-driven",
27132750
"export",
27142751
"full-stack-demo",
@@ -2765,6 +2802,9 @@
27652802
"drift-revert": [
27662803
"l2"
27672804
],
2805+
"drift-revert-arrays": [
2806+
"l2"
2807+
],
27682808
"event-driven": [
27692809
"l2"
27702810
],
@@ -2922,6 +2962,7 @@
29222962
"composite-stack",
29232963
"deployment-events",
29242964
"drift-revert",
2965+
"drift-revert-arrays",
29252966
"event-driven",
29262967
"export",
29272968
"full-stack-demo",
@@ -2949,6 +2990,9 @@
29492990
"drift-revert": [
29502991
"l2"
29512992
],
2993+
"drift-revert-arrays": [
2994+
"l2"
2995+
],
29522996
"event-driven": [
29532997
"l2"
29542998
],
@@ -3002,6 +3046,7 @@
30023046
"bench-sdk",
30033047
"composite-stack",
30043048
"data-pipeline",
3049+
"drift-revert-arrays",
30053050
"event-driven",
30063051
"eventbridge",
30073052
"full-stack-demo",
@@ -3027,6 +3072,9 @@
30273072
"data-pipeline": [
30283073
"l2"
30293074
],
3075+
"drift-revert-arrays": [
3076+
"l2"
3077+
],
30303078
"event-driven": [
30313079
"l2"
30323080
],

docs/_generated/integ-last-run.tsv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,3 +127,4 @@ custom-resource-provider 2026-06-13T09:16:21Z PASS 290 standard regression sweep
127127
deployment-events 2026-06-13T09:16:21Z PASS 34 verify.sh #808 events sidecar + cdkd events + no-secrets + survives-destroy; 2 deleted 0 err
128128
update-replace 2026-06-13T10:43:46Z PASS 65 verify.sh in-place(S3/Lambda/IAM/SG)+replacement(S3 rename); 0 err 0 orphan
129129
s3-asset-deploy 2026-06-13T10:51:36Z PASS 44 verify.sh S3 zip asset upload + Lambda runs from it + generic Asset read-back; 0 err
130+
drift-revert-arrays 2026-06-13T10:57:27Z PASS 114 verify.sh #802 tag/ARN-array canonicalize; no-false-positive + true-drift detect+revert; 0 err

docs/_generated/scenario-coverage.json

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232
"tag": "deployment-events",
3333
"description": "Structured deployment events to S3 + `cdkd events` command (issue #808): per-run `deployments/{runId}.jsonl` + `index.json` (separate key family from state.json, no schema bump), events survive `cdkd destroy`, and carry error + metadata ONLY (no resource properties / secrets)."
3434
},
35+
{
36+
"tag": "drift-revert-array-canonicalization",
37+
"description": "cdkd drift no-false-positive on tag-list / resource-id / ARN array REORDER (issue #802 `drift-normalize.ts` canonicalization) while still detecting real value / Action / SG-rule drift."
38+
},
3539
{
3640
"tag": "drift-revert-roundtrip",
3741
"description": "cdkd drift detection + `--revert` round-trip via each provider.update()."
@@ -359,6 +363,14 @@
359363
"drift-revert-roundtrip"
360364
]
361365
},
366+
{
367+
"name": "drift-revert-arrays",
368+
"annotated": true,
369+
"scenarios": [
370+
"drift-revert-array-canonicalization",
371+
"drift-revert-roundtrip"
372+
]
373+
},
362374
{
363375
"name": "drift-revert-vpc",
364376
"annotated": true,
@@ -1016,11 +1028,19 @@
10161028
"deployment-events"
10171029
]
10181030
},
1031+
{
1032+
"scenario": "drift-revert-array-canonicalization",
1033+
"description": "cdkd drift no-false-positive on tag-list / resource-id / ARN array REORDER (issue #802 `drift-normalize.ts` canonicalization) while still detecting real value / Action / SG-rule drift.",
1034+
"fixtures": [
1035+
"drift-revert-arrays"
1036+
]
1037+
},
10191038
{
10201039
"scenario": "drift-revert-roundtrip",
10211040
"description": "cdkd drift detection + `--revert` round-trip via each provider.update().",
10221041
"fixtures": [
10231042
"drift-revert",
1043+
"drift-revert-arrays",
10241044
"drift-revert-vpc"
10251045
]
10261046
},

docs/changelog-cdkd.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary
1919
**Recently Implemented** (2026-06-13):
2020

2121
- ✅ **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.)
22+
- ✅ **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`.
2223
- ✅ **Extracted the triplicated `ensureClientForBucket()` state-bucket-region rebuild into one shared helper (issue [#827](https://github.com/go-to-k/cdkd/issues/827))** — `src/utils/bucket-region-client.ts` (new) + `src/state/s3-state-backend.ts` + `src/state/lock-manager.ts` + `src/state/export-index-store.ts`. Pure refactor, no behavior change. The "resolve the state bucket's region via the cached `GetBucketLocation` probe, short-circuit when it already matches the client's region, else rebuild a region-corrected `S3Client` reusing the caller's credentials" pattern had drifted into three near-identical private `ensureClientForBucket()` copies (the state backend PR #60, the lock manager #803, the exports index store #819). They are now one exported helper `rebuildClientForBucketRegion(client, bucket, opts): Promise<S3Client | null>` — it returns `null` to mean "no rebuild needed, keep the original client" and a fresh client otherwise. Every load-bearing per-store difference is preserved via options rather than collapsed: `destroyOldClient` (the state backend OWNS its client and `.destroy()`s the replaced one; the lock manager + exports store share `AwsClients.s3` and must NOT), `profile` + static `credentials` (the state backend threads its constructor `clientOpts` into both the probe and the rebuild) vs `reuseClientCredentials` (the other two authenticate the probe via `client.config.credentials()` best-effort and reuse the original client's `config.credentials` PROVIDER REFERENCE — not a resolved snapshot — for the rebuild), and `tolerateNonStandardClient` (only the exports store gracefully degrades a test double whose `config.region` is not a function by returning `null`; the other two read `config.region()` directly as before). Each store keeps its own per-instance memoization (`clientResolved` flag + single-flight `resolveInFlight` promise) and its own debug log wording via an `onRebuild` callback. The helper deliberately lives in its OWN module rather than alongside `resolveBucketRegion` in `aws-region-resolver.ts`: the three stores' unit tests mock `resolveBucketRegion` via `vi.mock('aws-region-resolver.js', ...)`, and a helper co-located in that module would call its sibling through an in-module binding vitest cannot intercept (a module cannot mock itself) — a separate module imports the mocked binding cross-module so the mock still applies. Tests: all 75 existing unit tests across `tests/unit/state/{s3-state-backend,lock-manager,export-index-store}.test.ts` (incl. each store's 301-rebuild / same-region-no-rebuild / resolve-once-cached suites) pass UNCHANGED, plus 7 new focused tests in `tests/unit/utils/bucket-region-client.test.ts` (region-mismatch → rebuilt client with correct region; same-region → `null`; static-credential carry-over to probe + rebuild; provider-reference reuse; `destroyOldClient` gating; non-standard-client tolerance → `null`; no-credentials → omit `credentials` from the rebuilt client). No runtime behavior change for any of the three cross-region-state-bucket paths (lock acquisition, state read/write, exports index).
2324
- ✅ **`--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).
2425
- ✅ **`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.

0 commit comments

Comments
 (0)