Skip to content

Commit 73442c1

Browse files
authored
fix(dynamodb): translate GlobalTable GSI throughput to the CreateTable SDK shape (#1422)
1 parent aea1471 commit 73442c1

7 files changed

Lines changed: 1629 additions & 65 deletions

File tree

docs/_generated/handled-property-wiring.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4876,6 +4876,7 @@
48764876
"AWS::DynamoDB::GlobalTable"
48774877
],
48784878
"evidence": [
4879+
"delegated",
48794880
"element-read"
48804881
],
48814882
"seededBy": [

docs/_generated/integ-last-run.tsv

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee
7777
drift-revert-vpc 2026-07-30T16:18:56Z PASS 480 verify.sh post-rebase final for PR #1307 (#1299/#1300): 6/6 reverted, 21 del 0 err 0 orphans
7878
dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean
7979
dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean
80-
dynamodb-globaltable 2026-07-21T05:21:32Z PASS 237 verify.sh rc ok, orph clean
80+
dynamodb-globaltable 2026-08-09T13:35:05Z PASS 209 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean
8181
dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean
8282
dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans
8383
dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans

docs/changelog-cdkd.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary
1717
---
1818

1919
**Recently Implemented** (2026-08-09):
20+
- ✅ **`AWS::DynamoDB::GlobalTable`: GSI throughput translated to the `CreateTable` SDK shape, so a PROVISIONED GlobalTable with a GSI stops failing outright (issue [#1387](https://github.com/go-to-k/cdkd/issues/1387))** — `src/provisioning/providers/dynamodb-globaltable-provider.ts`, a new unit-test file, and the `tests/integration/dynamodb-globaltable/` fixture. **The class:** the provider cast the CFn `GlobalSecondaryIndexes` blob RAW to the SDK's `GlobalSecondaryIndex[]`, but the two schemas model per-GSI throughput completely differently and the SDK v3 serializer silently drops unknown members — so a PROVISIONED-billing GlobalTable with a GSI failed `CreateTable` outright (AWS requires `ProvisionedThroughput` on every GSI) and every `TableV2` per-GSI on-demand limit vanished. `TableV2` is the recommended L2 since CDK 2.95, so this is a daily-pattern surface. **The issue's own mapping table turned out to be materially incomplete, and re-deriving it from the authoritative schemas changed the fix twice.** (1) `WriteProvisionedThroughputSettings` has EXACTLY ONE member, `WriteCapacityAutoScalingSettings` — there is no literal `WriteCapacityUnits`, because write capacity on a GlobalTable is always auto-scaled; `CreateTable` needs a concrete number, so the mapping takes `SeedCapacity` (the documented "initial provisioned capacity units") before `MinCapacity`. (2) Per-GSI READ capacity is not on the top-level GSI at all — CDK synthesizes it to `Replicas[?Region==<deploy region>].GlobalSecondaryIndexes[].Read{Provisioned,OnDemand}ThroughputSettings`, so the SDK's single `ProvisionedThroughput` / `OnDemandThroughput` object has to be FUSED from both halves (the GSI-level spellings the schema also permits are honored as a fallback for hand-authored templates). Both facts came from the live `cloudformation:DescribeType` schema cross-checked against a real `cdk synth` under both billing modes; the repo's captured fixture stores top-level names only and could not settle it. **Call sites:** `create()`, `addReplica()`, the `update()` replica-modify action, the `update()` GSI diff (both sides are now translated BEFORE diffing, so emitted `Create` / `Update` actions carry real throughput — a side benefit is that an auto-scaling-only edit, invisible to the DynamoDB API, no longer emits a bare `Update: {IndexName}` that AWS rejects as empty), and one site the issue did not name: the `PAY_PER_REQUEST -> PROVISIONED` **billing-mode flip**, where AWS requires per-index `ProvisionedThroughput` in the SAME `UpdateTable` call — without it the fix would have made create work while leaving the flip broken. **Deliberately left unmapped, recorded in a JSDoc block rather than dropped silently** (this provider has no `unhandledByDesign` map): `Replicas[].ReplicaStreamSpecification.ResourcePolicy` and `Replicas[].ResourcePolicy` (both need `PutResourcePolicy`, not any `UpdateTable` field) and `Replicas[].GlobalSecondaryIndexes[].ContributorInsightsSpecification` (needs a per-index `UpdateContributorInsights`). `handledProperties` is unchanged, so no coverage regeneration was needed. **Three defects in the first cut were caught by review and fixed here, each mutation-proofed:** (a) the billing flip built its index updates from the NEW template but filtered them by the PREVIOUS template's names, so an index the deploy REMOVES got no capacity — yet its `Delete` is issued later, so at flip time it is still a live index on a table becoming PROVISIONED and AWS rejects the whole call; (b) the first attempt at suppressing a false immutable-field warning skipped the `modified` loop wholesale on ANY flip, which silently dropped every per-GSI `Max{Read,Write}RequestUnits` on a `PROVISIONED -> PAY_PER_REQUEST` flip (the flip call carries per-GSI fields in one direction only) — the loop now sends the fields the NEW billing mode needs and suppresses only the warning; (c) a non-array `GlobalSecondaryIndexes` (an unresolved intrinsic) collapsed to `[]` and would have created the table with ZERO indexes while reporting success — the #1387 class one level up — and now throws. `WarmThroughput` also rides the update-ADD path, so a GSI added later matches what `create()` sends for the same template. **Tests:** 29 in a new file, with both property bags copied VERBATIM from a real `cdk synth` rather than hand-invented — which is precisely what surfaced the read-capacity-lives-on-the-replica asymmetry that a hand-written fixture would have encoded wrongly. Every call site and guard is mutation-proofed individually: reverting any one of them kills a specific test. **Integ:** the fixture gains two UNCONDITIONAL `TableV2`s (not gated behind `CDKD_TEST_UPDATE`, so the baseline deploy exercises the previously-failing create path) — two tables because the billing modes cannot coexist on one. L2 was correct here rather than L1, since `TableV2` exposes every property needed. `verify.sh` reads all four GSI values back plus the two table-level ones, and its step 3 stopped taking "the first GlobalTable in state" (which with three tables would have grabbed an arbitrary one) in favor of selecting by logical-id prefix.
2021
- ✅ **`handledProperties` WIRING critic (`gen-handled-property-wiring`) + the two live silent drops its first run caught (issue [#1404](https://github.com/go-to-k/cdkd/issues/1404))** — `scripts/gen-handled-property-wiring.ts` (new, FIFTH codegen'd critic), `docs/_generated/handled-property-wiring.{json,md}` (new matrix), `vite.config.ts` + `.github/workflows/ci.yml` (gen + `--check` steps), `.claude/rules/code-layout.md`. No `src/**` change — pure tooling / docs. **The class:** `handledProperties` can LIE. `gen-property-coverage` proves a property is ACCOUNTED FOR and `gen-nested-key-coverage` audits spellings INSIDE a forwarded blob, but neither proves an entry is WIRED — `ECRProvider` declared `ImageTagMutabilityExclusionFilters` handled while it reached NO API call, so the pre-flight passed on the declaration alone and the value silently vanished (#1392, fixed in #1406). **The critic:** per declared property it requires read evidence in one of four AST shapes (`element-read` / `property-read` / `destructure` / `table-loop`) plus an orthogonal `delegated` tag, CLASS-SCOPED via a taint walk seeded from each method's desired-state parameter and propagated only through calls that pass the bag WHOLE — so a sibling class in the same file, a comment, a `getDriftUnknownPaths` entry, the declaration itself, and a `readCurrentState` write-back all fail to vouch (each pinned by a test). Two rules keep `table-loop` from becoming a rubber stamp, since one syntactic site credits N properties at once: the loop body must **DELIVER, not merely compare** (`EC2Provider.updateSubnet`'s createOnly guard is a change GUARD, and crediting it smuggled the diff-is-not-delivery disguise back in one level up — the rule withdrew the tag from 46 properties across 8 classes, NONE of which became a gap since all are also read individually; `RDSDBProxyProvider` shows the discrimination, its immutable-field loop losing credit while its `mutableFields` loop keeps it), and the table is resolved **LEXICALLY** from the loop outward (a FILE-wide pool let a table local to one class's method vouch for a DIFFERENT class and let two same-named tables override last-wins — `glue-provider.ts` really does declare `result` x12). Each wired property records `seededBy` so a property wired only from a non-delivery member is visible rather than silently green (0 today, fenced). Two strictness calls were forced by the REAL tree, not by fixtures: a whole-bag forward does NOT blanket-excuse un-read declarations (the first draft's excuse silenced the very #1392 property via `hasCdkAutoDeleteTag(properties)` in `delete()`; measured cost 0 of 1063), and a `previousProperties` read is not evidence — with the in-code JSDoc stating honestly that this is NARROWER than it looks (it does not close the disguise for a single `element-read`, only for helpers reached with the previous bag alone; the TABLE case IS closed by the delivery rule). **First-run audit (4 flagged):** TWO real silent drops, seeded as tracked KNOWN GAP allow-list entries and FILED rather than fixed here — `AWS::EC2::NatGateway.MaxDrainDurationSeconds` (issue [#1411](https://github.com/go-to-k/cdkd/issues/1411)) and `AWS::Logs::LogGroup.ResourcePolicyDocument` (issue [#1412](https://github.com/go-to-k/cdkd/issues/1412), already admitted in an in-code comment); `IAMAccessKeyProvider#Serial` and `NestedStackProvider#TemplateURL` are rationale'd NOT-A-BUG entries. Stale entries fail in both modes, so wiring a property forces its entry's removal. **Proven against real code** per the repo's checker rules: reverting the real `ecr-provider.ts` to its pre-#1406 state exits 1 naming the property (a FIRST probe that stripped only the lowercase-`p` reads PASSED — the surviving `previousProperties` read cleared it, and that false clean is what drove the exclusion); stripping the real `VpcId` reads while leaving the comparison-only guard standing must now REJECT; dropping one name from the real Glue / SQS tables must surface a gap; a class appended to the real `glue-provider.ts` must not borrow `buildJobCommonFields`'s local table. The shipped `--check` is driven via `spawnSync` against a scratch COPY of the providers tree (`--providers-dir=` seam), so exit code and failure text are covered without ever writing to `src/`. **Tests:** 68 (shape units, per-SHAPE real-repo floors — 84 classes / 1063 declared / 43 `table-loop` of which 29 sole-evidence — allow-list stale + per-property keying, and the real-code probe set). NO AWS integ (offline static analysis).
2122
- ✅ **`ModifyInstanceFleet` needs BOTH capacities, and every EMR `InstanceTypeConfigs` conversion site now has real-AWS coverage (issue [#1400](https://github.com/go-to-k/cdkd/issues/1400))** — `src/provisioning/providers/emr-instance-fleet-config-provider.ts` (`update()`), new fixture `tests/integration/emr-instance-fleets/`, `.claude/integ-coverage-allowlist.json` (the `AWS::EMR::InstanceFleetConfig` entry removed — it now has an integ). **The gap:** both existing EMR fixtures are instance-GROUP based, and a cluster's instance-collection type is fixed at create (groups XOR fleets), so no fixture could exercise a FLEET. That left all three `InstanceTypeConfigs` conversion sites — `EMRClusterProvider.toInstanceFleetConfig` (inline `Cluster.Instances.{Master,Core}InstanceFleet`), `EMRInstanceFleetConfigProvider.create` (`AddInstanceFleet`), and the same provider's `ModifyInstanceFleet` update — proven only by mocks. #1383 was precisely a send-side-looks-fine / AWS-silently-discards bug (CFn `ConfigurationProperties` vs the SDK's `Properties`), and a unit test can prove cdkd SENDS the block but never that EMR ACCEPTED it. **The fixture:** a fleet-based cluster (master + core inline fleets, `Ec2SubnetIds` plural — `Ec2SubnetId` is the group form) plus a standalone TASK fleet, each carrying a per-`InstanceTypeConfig` `Configurations` marker that `verify.sh` reads back through `ListInstanceFleets` (SDK, `Marker`-paginated). **The bug it found on its first run:** `AddInstanceFleet` tolerates an absent capacity (AWS defaults it to 0) but `ModifyInstanceFleet` rejects the same payload — `"The instance fleet (if-...) should have both targetOnDemandCapacity and targetSpotCapacity specified."` The provider forwarded the template verbatim and the SDK v3 serializer omits `undefined` members, so EVERY resize of an ordinary On-Demand-only or Spot-only fleet failed. The ordinary CDK template declares exactly one of the two, so this was the common case, not an edge case. Fixed by defaulting the undeclared side to `0`, which also matches CFn desired-state semantics (an omitted capacity means zero) and the provider's own `targetCapacity()` helper and delete-path scale-to-0, both of which already sent both members. The pre-existing unit tests all passed a `BASE_PROPS` carrying BOTH keys — which is exactly why the suite agreed with the bug; the added regression test uses the real one-sided shape and asserts both members are PRESENT on the wire (an `Object.keys` check, since `toMatchObject` passes on an absent key whose expected value is `undefined`). A generalization of memory rule `update_api_stricter_than_create_probe_payload`: an AWS update API can REQUIRE a field its create counterpart defaults.
2223
- ✅ **Shape pass for the nested-key critic: `{Quantity, Items}` wrapper + definition-placement divergences mechanically enforced (issue [#1378](https://github.com/go-to-k/cdkd/issues/1378))** — `scripts/gen-nested-key-coverage.ts` (shape pass), `scripts/refresh-cfn-schemas.mjs` (`extractDefinitionShapes` — per-definition member -> terminal type kind, `$ref`-resolved + cycle-guarded, top-level block under the reserved `#top` key; plus the rider `--help` / unknown-flag guard — an unrecognized flag previously fell through to a silent FULL ~135-type re-fetch), target fixtures re-captured, `docs/_generated/nested-key-coverage.{json,md}` extended. **The gap:** the #1373 key pass is structurally blind to divergences whose spelling exists SOMEWHERE in the SDK model — the CloudFront bare-array-vs-`{Quantity, Items}` wrapper family (previously a hand-maintained `QUANTITY_ITEM_FIELDS` list a NEW AWS array member would silently miss) and relocated/renamed members like the `CachedMethods` sibling-vs-nested placement (the hardest #1370 member). **The pass:** SDK interfaces are parsed with member type kinds (`collectSdkInterfaces`; a `Quantity`-bearing interface is a wrapper), and two new CI-blocking buckets fire when neither provider-named (dot-segment-expanded literals — the `'ForwardedValues.Headers'` path idiom) nor allow-listed: `array-vs-wrapper` (a CFn `array` member whose same-spelled SDK members are all wrapper refs) and `definition-member-missing` (a CFn definition member same-spelling an SDK member globally but missing from the same-named SDK interface). Keys with no same-spelled SDK member anywhere stay the key pass's domain (no double-reporting); `ambiguous` shapes and unmatched definitions stay visible non-blocking. **First audit:** no live bug — 62 clean bare-array pairs, 17 provider-handled re-shapings (the whole QUANTITY_ITEM_FIELDS family + `CachedMethods` + `GeoRestriction.Locations`), one new allow-list entry: legacy `S3Origin`, which the key pass could never see because the StreamingDistribution API still carries a same-spelled member — the definition pass catching it is the pass working as designed. **Proven against real code:** full-word-stripping `Aliases` from the REAL provider makes `--check` exit 1 naming `Aliases [array-vs-wrapper]` (live probe + permanent unit probes for both buckets; the interface parse shares the SDK-member floor so a parser collapse fails loudly). **Tests:** +19 (shape-pass synthetic buckets incl. segment-expansion credit and per-key dedup, `extractDefinitionShapes` walker, real-repo shape fences + floors, real-code probes, `--help` guard spawn tests). No `src/**` changes — pure tooling/fixtures/docs.

0 commit comments

Comments
 (0)