From 27e4aafaf762f161b336f6361ff6a2a0fe189ca3 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:02:16 +0900 Subject: [PATCH 01/18] fix(dynamodb): translate GlobalTable GSI throughput to the CreateTable SDK shape The provider cast the CFn GlobalSecondaryIndexes blob raw to the SDK GlobalSecondaryIndex[], but the two schemas model per-GSI throughput completely differently and the SDK v3 serializer silently drops unknown members. A PROVISIONED-billing GlobalTable with a GSI therefore failed CreateTable outright, 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 mapping table was materially incomplete and re-deriving it from the authoritative schemas changed the fix twice. WriteProvisionedThroughputSettings has exactly one member, WriteCapacityAutoScalingSettings, because write capacity on a GlobalTable is always auto-scaled; CreateTable needs a concrete number, so the mapping takes SeedCapacity before MinCapacity. Per-GSI read capacity is not on the top-level GSI at all, but on the local replica's GSI entry, so the SDK's single throughput object has to be fused from both halves. Applied at create, addReplica, the update replica-modify action, the update GSI diff (both sides translated before diffing, so an auto-scaling-only edit no longer emits an empty Update action AWS rejects), and the PAY_PER_REQUEST to PROVISIONED billing flip, which requires per-index throughput in the same UpdateTable call. Replica resource policies and per-index contributor insights need separate APIs and are recorded in a JSDoc block rather than dropped silently. Closes #1387 --- docs/changelog-cdkd.md | 1 + .../dynamodb-globaltable-provider.ts | 387 ++++++++++- .../lib/dynamodb-globaltable-stack.ts | 90 +++ .../dynamodb-globaltable/verify.sh | 85 ++- ...lobaltable-provider-gsi-throughput.test.ts | 640 ++++++++++++++++++ 5 files changed, 1160 insertions(+), 43 deletions(-) create mode 100644 tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts diff --git a/docs/changelog-cdkd.md b/docs/changelog-cdkd.md index 3195c48ff..aa6f21e4f 100644 --- a/docs/changelog-cdkd.md +++ b/docs/changelog-cdkd.md @@ -17,6 +17,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary --- **Recently Implemented** (2026-08-09): +- ✅ **`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==].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. **Tests:** 23 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. Revert-proofed: restoring the three original raw casts fails 5 of the provider-level tests. **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. - ✅ **`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). - ✅ **`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. - ✅ **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. diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index 7a7e6854f..ac5305e3b 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -19,6 +19,9 @@ import { type GlobalSecondaryIndex, type GlobalSecondaryIndexUpdate, type LocalSecondaryIndex, + type OnDemandThroughput, + type ProvisionedThroughput, + type ReplicaGlobalSecondaryIndex, type StreamSpecification, type Tag, type ReplicationGroupUpdate, @@ -343,10 +346,16 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { } as StreamSpecification; } + // GSIs: the CFn GlobalTable GSI shape models throughput differently + // from the SDK's `GlobalSecondaryIndex` (Issue #1387) — translate + // rather than cast. Pre-fix a PROVISIONED GlobalTable with a GSI + // failed `CreateTable` outright (AWS requires per-GSI + // `ProvisionedThroughput`) and per-GSI on-demand limits were dropped. if (properties['GlobalSecondaryIndexes']) { - createParams.GlobalSecondaryIndexes = properties[ - 'GlobalSecondaryIndexes' - ] as GlobalSecondaryIndex[]; + const sdkIndexes = toSdkGlobalSecondaryIndexes(properties, currentRegion, billingMode); + if (sdkIndexes.length > 0) { + createParams.GlobalSecondaryIndexes = sdkIndexes; + } } if (properties['LocalSecondaryIndexes']) { createParams.LocalSecondaryIndexes = properties[ @@ -565,6 +574,20 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { * `DescribeTable` until the replica's `ReplicaStatus` flips to ACTIVE. * Capped at 10 minutes per replica (AWS Replica provisioning typically * takes 1–5 min). + * + * DELIBERATELY UNMAPPED replica sub-keys (Issue #1387 — recorded rather + * than silently dropped; `CreateReplicationGroupMemberAction` simply has + * no member for them, so they need their own follow-up API calls): + * - `ReplicaStreamSpecification.ResourcePolicy` — the DynamoDB Streams + * resource policy is set by `PutResourcePolicy` against the replica's + * STREAM ARN, not by any `UpdateTable` field. + * - `ResourcePolicy` — the table-level resource policy, likewise a + * separate `PutResourcePolicy` call. + * - `GlobalSecondaryIndexes[].ContributorInsightsSpecification` — a + * per-index `UpdateContributorInsights` call. + * `Replicas` is declared in `handledProperties` at the TOP level, which + * is the granularity cdkd's pre-flight property-coverage check works at; + * these nested keys are tracked as a known gap, not a supported surface. */ private async addReplica( tableName: string, @@ -578,12 +601,13 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { if (replica['KMSMasterKeyId']) { create.KMSMasterKeyId = replica['KMSMasterKeyId'] as string; } - if (replica['GlobalSecondaryIndexes']) { - // Replica-level GSI overrides (per-replica throughput overrides). - // AWS-SDK shape: ReplicaGlobalSecondaryIndex[]. - create.GlobalSecondaryIndexes = replica['GlobalSecondaryIndexes'] as Array<{ - IndexName: string; - }>; + // Replica-level GSI overrides. The CFn + // `ReplicaGlobalSecondaryIndexSpecification` spells the per-replica read + // throughput `Read{Provisioned,OnDemand}ThroughputSettings`, while the SDK + // wants `{Provisioned,OnDemand}ThroughputOverride` — translate (Issue #1387). + const replicaIndexes = toSdkReplicaGlobalSecondaryIndexes(replica['GlobalSecondaryIndexes']); + if (replicaIndexes) { + create.GlobalSecondaryIndexes = replicaIndexes; } if (replica['TableClassOverride']) { create.TableClassOverride = replica['TableClassOverride'] as @@ -847,6 +871,9 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { const oldBilling = (previousProperties['BillingMode'] as string | undefined) ?? 'PAY_PER_REQUEST'; const newBilling = (properties['BillingMode'] as string | undefined) ?? 'PAY_PER_REQUEST'; + // GSIs whose ProvisionedThroughput was already applied as part of the + // BillingMode flip below — step 6 must not re-issue an Update for them. + const gsiHandledByBillingFlip = new Set(); if (oldBilling !== newBilling) { const billingUpdate: UpdateTableCommandInput = { TableName: physicalId, @@ -861,6 +888,39 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { properties, currentRegion ); + // AWS requires per-GSI `ProvisionedThroughput` in the SAME + // UpdateTable call that flips PAY_PER_REQUEST -> PROVISIONED + // (Issue #1387): the throughput-mode change applies to the table + // AND every index atomically. `GlobalSecondaryIndexUpdates` is the + // one companion field UpdateTable accepts alongside `BillingMode`. + const provisionedIndexes = toSdkGlobalSecondaryIndexes( + properties, + currentRegion, + newBilling + ); + // Only indexes that ALREADY exist on AWS can take an `Update` + // action; a GSI introduced by this same deploy is created (with + // its throughput) by step 6's `added` loop. + const existingIndexNames = new Set( + ((previousProperties['GlobalSecondaryIndexes'] ?? []) as unknown[]) + .map((entry) => (entry as Record | null)?.['IndexName']) + .filter((name): name is string => typeof name === 'string') + ); + const indexUpdates: GlobalSecondaryIndexUpdate[] = []; + for (const gsi of provisionedIndexes) { + if (!gsi.IndexName || !gsi.ProvisionedThroughput) continue; + if (!existingIndexNames.has(gsi.IndexName)) continue; + indexUpdates.push({ + Update: { + IndexName: gsi.IndexName, + ProvisionedThroughput: gsi.ProvisionedThroughput, + }, + }); + gsiHandledByBillingFlip.add(gsi.IndexName); + } + if (indexUpdates.length > 0) { + billingUpdate.GlobalSecondaryIndexUpdates = indexUpdates; + } } await this.dynamoDBClient.send(new UpdateTableCommand(billingUpdate)); await this.waitForTableActiveAfterUpdate(physicalId, logicalId); @@ -1050,10 +1110,13 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { if (replica['KMSMasterKeyId'] !== undefined) { updateAction.KMSMasterKeyId = replica['KMSMasterKeyId'] as string; } - if (replica['GlobalSecondaryIndexes']) { - updateAction.GlobalSecondaryIndexes = replica['GlobalSecondaryIndexes'] as Array<{ - IndexName: string; - }>; + // Same CFn -> SDK per-replica GSI throughput translation as the + // create-side `addReplica` (Issue #1387). + const replicaIndexes = toSdkReplicaGlobalSecondaryIndexes( + replica['GlobalSecondaryIndexes'] + ); + if (replicaIndexes) { + updateAction.GlobalSecondaryIndexes = replicaIndexes; } if (replica['TableClassOverride']) { updateAction.TableClassOverride = replica['TableClassOverride'] as @@ -1090,9 +1153,17 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { // 6. GSI diff. New GSI Create may need additional AttributeDefinitions // — AWS allows combining `AttributeDefinitions` and one GSI // `Create` action in the same UpdateTable call. + // + // Both sides are translated CFn -> SDK first (Issue #1387) so the diff + // is over the shapes actually sent to AWS. That is load-bearing twice + // over: the emitted Create / Update actions carry real throughput + // (pre-fix they carried none, so a PROVISIONED GSI add was rejected), + // and an auto-scaling-only template edit — `MaxCapacity` 20 -> 30, + // invisible to DynamoDB's own API — no longer produces a bare + // `Update: { IndexName }` that AWS rejects as an empty update. const gsiDiff = diffGlobalSecondaryIndexes( - (previousProperties['GlobalSecondaryIndexes'] ?? []) as GlobalSecondaryIndex[], - (properties['GlobalSecondaryIndexes'] ?? []) as GlobalSecondaryIndex[] + toSdkGlobalSecondaryIndexes(previousProperties, currentRegion, oldBilling), + toSdkGlobalSecondaryIndexes(properties, currentRegion, newBilling) ); for (const gsi of gsiDiff.removed) { if (!gsi.IndexName) continue; @@ -1131,6 +1202,20 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { } for (const gsi of gsiDiff.modified) { if (!gsi.IndexName) continue; + // Already applied atomically with the BillingMode flip above. + if (gsiHandledByBillingFlip.has(gsi.IndexName)) continue; + // A GSI whose only change is KeySchema / Projection produces no + // throughput fields; AWS rejects an `Update` action with nothing + // but an IndexName, so skip and let the (immutable) change surface + // as a no-op rather than a confusing ValidationException. + if (!gsi.ProvisionedThroughput && !gsi.OnDemandThroughput) { + this.logger.warn( + `GSI '${gsi.IndexName}' on ${physicalId} changed in a way DynamoDB's ` + + `UpdateTable cannot express (KeySchema / Projection are immutable on an ` + + `existing index). Recreate the index under a new name to apply the change.` + ); + continue; + } const gsiUpdate: GlobalSecondaryIndexUpdate = { Update: { IndexName: gsi.IndexName, @@ -2560,32 +2645,264 @@ export function derivePerCallProvisionedThroughput( properties: Record, region: string ): { ReadCapacityUnits: number; WriteCapacityUnits: number } { - const wps = properties['WriteProvisionedThroughputSettings'] as - | Record - | undefined; - const writeAutoScaling = wps?.['WriteCapacityAutoScalingSettings'] as - | Record - | undefined; - const writeCapacity = Number( - wps?.['WriteCapacityUnits'] ?? writeAutoScaling?.['MinCapacity'] ?? 5 - ); const replicas = (properties['Replicas'] ?? []) as Array>; const localReplica = replicas.find((r) => r['Region'] === region); - const localReadSettings = localReplica?.['ReadProvisionedThroughputSettings'] as - | Record - | undefined; - const readAutoScaling = localReadSettings?.['ReadCapacityAutoScalingSettings'] as - | Record - | undefined; - const readCapacity = Number( - localReadSettings?.['ReadCapacityUnits'] ?? readAutoScaling?.['MinCapacity'] ?? 5 - ); return { - ReadCapacityUnits: readCapacity, - WriteCapacityUnits: writeCapacity, + // Read capacity is per-replica on a GlobalTable; the deploy region's + // replica is the one `CreateTable` / `UpdateTable` provisions. (The + // schema also has a TOP-LEVEL `ReadProvisionedThroughputSettings`, but + // cdkd's pre-flight property-coverage check rejects that spelling as + // unsupported, so it can never reach here — do not read it.) + ReadCapacityUnits: + deriveReadCapacityUnits(asRecord(localReplica?.['ReadProvisionedThroughputSettings'])) ?? + DEFAULT_CAPACITY_UNITS, + WriteCapacityUnits: + deriveWriteCapacityUnits(asRecord(properties['WriteProvisionedThroughputSettings'])) ?? + DEFAULT_CAPACITY_UNITS, }; } +/** + * Capacity units applied when a PROVISIONED template carries no usable + * capacity value at all. Matches the pre-existing table-level default. + */ +const DEFAULT_CAPACITY_UNITS = 5; + +/** Narrow an unknown CFn sub-object to a record (or `undefined`). */ +function asRecord(value: unknown): Record | undefined { + return value !== null && typeof value === 'object' && !Array.isArray(value) + ? (value as Record) + : undefined; +} + +/** Coerce a CFn numeric (CFn is stringly-typed) to a finite number. */ +function toFiniteNumber(value: unknown): number | undefined { + if (value === undefined || value === null || value === '') return undefined; + const n = Number(value); + return Number.isFinite(n) ? n : undefined; +} + +/** + * Derive a single WRITE capacity number from a CFn + * `WriteProvisionedThroughputSettings` block. + * + * The `AWS::DynamoDB::GlobalTable` registry schema (verified 2026-08-09 via + * `aws cloudformation describe-type`) declares this block with exactly ONE + * member — `WriteCapacityAutoScalingSettings` — i.e. write capacity on a + * GlobalTable is ALWAYS auto-scaled; there is no literal `WriteCapacityUnits` + * (that is why CDK 2.244's `TableV2` rejects `Capacity.fixed()` for write). + * `CreateTable` / `UpdateTable` still require a concrete starting number, so + * we take `SeedCapacity` ("the initial provisioned capacity units") when the + * template supplies it, else `MinCapacity`. + * + * A literal `WriteCapacityUnits` is still honored first: the CFn schema + * forbids it, but hand-authored / imported templates and cdkd state written + * before this helper existed can carry it, and honoring it is strictly more + * faithful than ignoring it. + */ +export function deriveWriteCapacityUnits( + settings: Record | undefined +): number | undefined { + if (!settings) return undefined; + const literal = toFiniteNumber(settings['WriteCapacityUnits']); + if (literal !== undefined) return literal; + const autoScaling = asRecord(settings['WriteCapacityAutoScalingSettings']); + if (!autoScaling) return undefined; + return toFiniteNumber(autoScaling['SeedCapacity']) ?? toFiniteNumber(autoScaling['MinCapacity']); +} + +/** + * Derive a single READ capacity number from a CFn + * `ReadProvisionedThroughputSettings` block (both the per-replica shape, + * which carries `ReadCapacityUnits` + `ReadCapacityAutoScalingSettings`, and + * the GSI/table-level `GlobalReadProvisionedThroughputSettings` shape, which + * carries only `ReadCapacityUnits`). Same Seed-before-Min precedence as the + * write side. + */ +export function deriveReadCapacityUnits( + settings: Record | undefined +): number | undefined { + if (!settings) return undefined; + const literal = toFiniteNumber(settings['ReadCapacityUnits']); + if (literal !== undefined) return literal; + const autoScaling = asRecord(settings['ReadCapacityAutoScalingSettings']); + if (!autoScaling) return undefined; + return toFiniteNumber(autoScaling['SeedCapacity']) ?? toFiniteNumber(autoScaling['MinCapacity']); +} + +/** + * Translate the CFn `AWS::DynamoDB::GlobalTable` `GlobalSecondaryIndexes[]` + * blob into the SDK's `GlobalSecondaryIndex[]` shape (Issue #1387). + * + * The two schemas model per-GSI throughput COMPLETELY differently, and the + * AWS SDK v3 serializer silently drops unknown members — so the pre-fix raw + * cast meant a PROVISIONED GlobalTable with a GSI failed `CreateTable` + * outright (AWS requires `ProvisionedThroughput` on every GSI), and every + * `TableV2` per-GSI on-demand limit vanished without a trace. + * + * Verified mapping (CFn registry schema `AWS::DynamoDB::GlobalTable` + + * `@aws-sdk/client-dynamodb` `models_0.d.ts`, both read 2026-08-09; the CFn + * side additionally confirmed against a real `cdk synth` of `TableV2` with a + * GSI under both billing modes): + * + * | CFn | SDK | + * | ---------------------------------------------------------------------- | --------------------------------------- | + * | `GSI.WriteProvisionedThroughputSettings.WriteCapacityAutoScalingSettings` | `ProvisionedThroughput.WriteCapacityUnits` | + * | `Replicas[local].GlobalSecondaryIndexes[].ReadProvisionedThroughputSettings.ReadCapacityUnits` | `ProvisionedThroughput.ReadCapacityUnits` | + * | `GSI.WriteOnDemandThroughputSettings.MaxWriteRequestUnits` | `OnDemandThroughput.MaxWriteRequestUnits` | + * | `Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings.MaxReadRequestUnits` | `OnDemandThroughput.MaxReadRequestUnits` | + * | `GSI.WarmThroughput` | `WarmThroughput` (same spelling) | + * + * READ capacity really does live on the REPLICA's GSI entry, not on the + * top-level GSI — CDK's `globalSecondaryIndexes[].readCapacity` synthesizes + * to `Replicas[?Region==].GlobalSecondaryIndexes[]. + * ReadProvisionedThroughputSettings`. `CreateTable` only ever provisions the + * LOCAL replica, so the local entry is the right source. The GSI-level + * `ReadProvisionedThroughputSettings` / `ReadOnDemandThroughputSettings` + * (which the schema also permits) are honored as a fallback for + * hand-authored templates. + * + * Already-SDK-shaped `ProvisionedThroughput` / `OnDemandThroughput` members + * win over the derivation: the CFn schema forbids them, but cdkd state + * written before this fix (and hand-authored templates) can carry them, and + * silently re-deriving over an explicit value would be a regression. + */ +export function toSdkGlobalSecondaryIndexes( + properties: Record, + region: string, + billingMode: string +): GlobalSecondaryIndex[] { + const cfnIndexes = (properties['GlobalSecondaryIndexes'] ?? []) as unknown[]; + if (!Array.isArray(cfnIndexes) || cfnIndexes.length === 0) return []; + + const replicas = (properties['Replicas'] ?? []) as Array>; + const localReplica = Array.isArray(replicas) + ? replicas.find((r) => asRecord(r)?.['Region'] === region) + : undefined; + const localReplicaIndexes = (localReplica?.['GlobalSecondaryIndexes'] ?? []) as unknown[]; + const localByName = new Map>(); + if (Array.isArray(localReplicaIndexes)) { + for (const entry of localReplicaIndexes) { + const record = asRecord(entry); + const name = record?.['IndexName']; + if (record && typeof name === 'string') localByName.set(name, record); + } + } + + const result: GlobalSecondaryIndex[] = []; + for (const entry of cfnIndexes) { + const gsi = asRecord(entry); + // A non-object entry is an unresolved intrinsic or a malformed + // template — pass it through untouched so AWS surfaces the real + // validation error instead of cdkd swallowing it. + if (!gsi) { + result.push(entry as GlobalSecondaryIndex); + continue; + } + const indexName = gsi['IndexName'] as string | undefined; + const localEntry = typeof indexName === 'string' ? localByName.get(indexName) : undefined; + + const sdk: GlobalSecondaryIndex = { + IndexName: indexName, + KeySchema: gsi['KeySchema'] as GlobalSecondaryIndex['KeySchema'], + Projection: gsi['Projection'] as GlobalSecondaryIndex['Projection'], + }; + if (gsi['WarmThroughput'] !== undefined) { + sdk.WarmThroughput = gsi['WarmThroughput'] as GlobalSecondaryIndex['WarmThroughput']; + } + + const explicitProvisioned = asRecord(gsi['ProvisionedThroughput']); + const explicitOnDemand = asRecord(gsi['OnDemandThroughput']); + if (explicitProvisioned) { + sdk.ProvisionedThroughput = explicitProvisioned as unknown as ProvisionedThroughput; + } else if (billingMode === 'PROVISIONED') { + sdk.ProvisionedThroughput = { + ReadCapacityUnits: + deriveReadCapacityUnits(asRecord(localEntry?.['ReadProvisionedThroughputSettings'])) ?? + deriveReadCapacityUnits(asRecord(gsi['ReadProvisionedThroughputSettings'])) ?? + DEFAULT_CAPACITY_UNITS, + WriteCapacityUnits: + deriveWriteCapacityUnits(asRecord(gsi['WriteProvisionedThroughputSettings'])) ?? + DEFAULT_CAPACITY_UNITS, + }; + } + + if (explicitOnDemand) { + sdk.OnDemandThroughput = explicitOnDemand as unknown as OnDemandThroughput; + } else if (billingMode !== 'PROVISIONED') { + const maxWrite = toFiniteNumber( + asRecord(gsi['WriteOnDemandThroughputSettings'])?.['MaxWriteRequestUnits'] + ); + const maxRead = + toFiniteNumber( + asRecord(localEntry?.['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] + ) ?? + toFiniteNumber(asRecord(gsi['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits']); + if (maxWrite !== undefined || maxRead !== undefined) { + sdk.OnDemandThroughput = { + ...(maxRead !== undefined && { MaxReadRequestUnits: maxRead }), + ...(maxWrite !== undefined && { MaxWriteRequestUnits: maxWrite }), + }; + } + } + + result.push(sdk); + } + return result; +} + +/** + * Translate a CFn `Replicas[].GlobalSecondaryIndexes[]` blob (the + * `ReplicaGlobalSecondaryIndexSpecification` shape) into the SDK's + * `ReplicaGlobalSecondaryIndex[]` used by `CreateReplicationGroupMemberAction` + * / `UpdateReplicationGroupMemberAction` (Issue #1387). + * + * Verified mapping: + * + * | CFn | SDK | + * | ---------------------------------------------------------- | ---------------------------------------------- | + * | `ReadProvisionedThroughputSettings.ReadCapacityUnits` | `ProvisionedThroughputOverride.ReadCapacityUnits` | + * | `ReadOnDemandThroughputSettings.MaxReadRequestUnits` | `OnDemandThroughputOverride.MaxReadRequestUnits` | + * + * DELIBERATELY UNMAPPED: `ContributorInsightsSpecification`. The replica-GSI + * SDK shape has no member for it — per-index contributor insights are toggled + * by a separate `UpdateContributorInsights(TableName, IndexName)` call, which + * this provider does not issue for any index (it only reads the TABLE-level + * status back in `readCurrentState`). Recorded here rather than silently + * dropped; tracked with the other GlobalTable nested-key gaps. + */ +export function toSdkReplicaGlobalSecondaryIndexes( + replicaIndexes: unknown +): ReplicaGlobalSecondaryIndex[] | undefined { + if (!Array.isArray(replicaIndexes) || replicaIndexes.length === 0) return undefined; + return replicaIndexes.map((entry) => { + const cfn = asRecord(entry); + if (!cfn) return entry as ReplicaGlobalSecondaryIndex; + const sdk: ReplicaGlobalSecondaryIndex = { + IndexName: cfn['IndexName'] as string | undefined, + }; + const explicitProvisioned = asRecord(cfn['ProvisionedThroughputOverride']); + const explicitOnDemand = asRecord(cfn['OnDemandThroughputOverride']); + const readCapacity = deriveReadCapacityUnits( + asRecord(cfn['ReadProvisionedThroughputSettings']) + ); + const maxReadRequestUnits = toFiniteNumber( + asRecord(cfn['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] + ); + if (explicitProvisioned) { + sdk.ProvisionedThroughputOverride = explicitProvisioned; + } else if (readCapacity !== undefined) { + sdk.ProvisionedThroughputOverride = { ReadCapacityUnits: readCapacity }; + } + if (explicitOnDemand) { + sdk.OnDemandThroughputOverride = explicitOnDemand; + } else if (maxReadRequestUnits !== undefined) { + sdk.OnDemandThroughputOverride = { MaxReadRequestUnits: maxReadRequestUnits }; + } + return sdk; + }); +} + export function diffReplicas( oldReplicas: Array>, newReplicas: Array> diff --git a/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts b/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts index 82d38a636..b5957a694 100644 --- a/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts +++ b/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts @@ -42,6 +42,10 @@ import * as ddb from 'aws-cdk-lib/aws-dynamodb'; * `CDKD_TEST_UPDATE=ttl,tags`. Unknown values are silently ignored so * future verify.sh scenarios can add new keys without touching the * stack. + * + * The stack ALSO deploys two unconditional GSI-carrying tables for Issue + * #1387 (per-GSI throughput CFn -> SDK translation) — see the block at the + * bottom of the constructor. */ export class DynamoDBGlobalTableStack extends cdk.Stack { constructor(scope: Construct, id: string, props: cdk.StackProps) { @@ -129,5 +133,91 @@ export class DynamoDBGlobalTableStack extends cdk.Stack { value: deployRegion, description: 'Deploy (primary) region', }); + + // ─── Issue #1387: per-GSI throughput translation ──────────────────── + // + // The CFn `AWS::DynamoDB::GlobalTable` schema models per-GSI throughput + // completely differently from the DynamoDB SDK's `CreateTable` shape, and + // cdkd used to forward the blob raw. The AWS SDK v3 serializer drops + // unknown members, so: + // + // - a PROVISIONED GlobalTable with a GSI failed `CreateTable` outright + // (AWS requires `ProvisionedThroughput` on every index), and + // - `TableV2`'s per-GSI on-demand limits vanished silently. + // + // Both tables are UNCONDITIONAL (not gated behind `CDKD_TEST_UPDATE`) so + // the very first baseline deploy exercises the create path that used to + // fail. A single table cannot carry both billing modes, hence two. + // + // `TableV2` (L2) is used rather than `CfnGlobalTable` (L1) because the L2 + // exposes every property this fixture needs — per-GSI `readCapacity` / + // `writeCapacity` and per-GSI `maxReadRequestUnits` / + // `maxWriteRequestUnits` — so no escape hatch is required. + // + // NOTE the asymmetry the fix had to get right: per-GSI READ capacity + // synthesizes onto `Replicas[?Region==] + // .GlobalSecondaryIndexes[]`, while WRITE capacity stays on the + // top-level GSI. `CreateTable` needs both halves in ONE + // `ProvisionedThroughput` object. + const gsiProvisionedTable = new ddb.TableV2(this, 'GsiProvisionedTable', { + partitionKey: { name: 'pk', type: ddb.AttributeType.STRING }, + billing: ddb.Billing.provisioned({ + readCapacity: ddb.Capacity.fixed(5), + // TableV2 requires auto-scaled write capacity (the GlobalTable CFn + // shape has no literal WriteCapacityUnits). + writeCapacity: ddb.Capacity.autoscaled({ + minCapacity: 1, + maxCapacity: 10, + targetUtilizationPercent: 70, + }), + }), + globalSecondaryIndexes: [ + { + indexName: 'byStatus', + partitionKey: { name: 'status', type: ddb.AttributeType.STRING }, + // -> Replicas[local].GlobalSecondaryIndexes[].ReadProvisionedThroughputSettings + readCapacity: ddb.Capacity.fixed(7), + // -> GlobalSecondaryIndexes[].WriteProvisionedThroughputSettings + // .WriteCapacityAutoScalingSettings; `seedCapacity` is the + // initial provisioned value AWS starts the index at, so cdkd + // must send WriteCapacityUnits=3 (NOT minCapacity=2). + writeCapacity: ddb.Capacity.autoscaled({ + minCapacity: 2, + maxCapacity: 20, + seedCapacity: 3, + targetUtilizationPercent: 60, + }), + }, + ], + removalPolicy: cdk.RemovalPolicy.DESTROY, + }); + + const gsiOnDemandTable = new ddb.TableV2(this, 'GsiOnDemandTable', { + partitionKey: { name: 'pk', type: ddb.AttributeType.STRING }, + billing: ddb.Billing.onDemand({ + maxReadRequestUnits: 100, + maxWriteRequestUnits: 200, + }), + globalSecondaryIndexes: [ + { + indexName: 'byOwner', + partitionKey: { name: 'owner', type: ddb.AttributeType.STRING }, + // -> Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings + maxReadRequestUnits: 50, + // -> GlobalSecondaryIndexes[].WriteOnDemandThroughputSettings + maxWriteRequestUnits: 60, + }, + ], + removalPolicy: cdk.RemovalPolicy.DESTROY, + }); + + new cdk.CfnOutput(this, 'GsiProvisionedTableName', { + value: gsiProvisionedTable.tableName, + description: 'PROVISIONED GlobalTable whose GSI carries WriteProvisionedThroughputSettings', + }); + new cdk.CfnOutput(this, 'GsiOnDemandTableName', { + value: gsiOnDemandTable.tableName, + description: 'On-demand GlobalTable whose GSI carries per-index on-demand limits', + }); } } diff --git a/tests/integration/dynamodb-globaltable/verify.sh b/tests/integration/dynamodb-globaltable/verify.sh index f2ade084f..36233c96d 100755 --- a/tests/integration/dynamodb-globaltable/verify.sh +++ b/tests/integration/dynamodb-globaltable/verify.sh @@ -18,6 +18,11 @@ # 3. read the deployed table name from cdkd state and assert it starts # with the cdkd `${StackName}-` prefix # 4. assert the deployed table exists on AWS via DescribeTable +# 4b. assert per-GSI throughput round-tripped (Issue #1387): the +# PROVISIONED table's GSI carries ProvisionedThroughput 7/3 and the +# on-demand table's GSI carries OnDemandThroughput 50/60. Pre-fix the +# first table could not be created at all and the second silently lost +# its per-index limits. # 5. cdkd deploy with CDKD_TEST_UPDATE=deletion-protection # 8. assert DeletionProtectionEnabled is now true on AWS # 9. cdkd deploy with CDKD_TEST_UPDATE=deletion-protection,billing-provisioned @@ -131,22 +136,35 @@ echo "[verify] step 2: cdkd deploy (baseline — no UPDATE flags)" unset CDKD_TEST_UPDATE ${CLI} deploy "${STACK}" --state-bucket "${STATE_BUCKET}" --verbose -echo "[verify] step 3: read deployed table name from cdkd state" +echo "[verify] step 3: read deployed table names from cdkd state" STATE_KEY="cdkd/${STACK}/${REGION}/state.json" STATE_JSON="$(aws s3 cp "s3://${STATE_BUCKET}/${STATE_KEY}" -)" -TABLE_NAME="$(echo "${STATE_JSON}" | python3 -c ' +# The stack deploys THREE AWS::DynamoDB::GlobalTable resources (the original +# HistoryTable plus the two Issue #1387 GSI fixtures), so select by logical-id +# prefix rather than "first GlobalTable in state". +table_name_for() { # $1 = construct id prefix -> physical table name + echo "${STATE_JSON}" | python3 -c ' import json, sys +prefix = sys.argv[1] state = json.load(sys.stdin) for logical_id, resource in state.get("resources", {}).items(): - if resource.get("resourceType") == "AWS::DynamoDB::GlobalTable": + if resource.get("resourceType") == "AWS::DynamoDB::GlobalTable" and logical_id.startswith(prefix): print(resource["physicalId"]) break -')" +' "$1" +} +TABLE_NAME="$(table_name_for HistoryTable)" +GSI_PROV_TABLE="$(table_name_for GsiProvisionedTable)" +GSI_OD_TABLE="$(table_name_for GsiOnDemandTable)" if [ -z "${TABLE_NAME}" ]; then - echo "[verify] FAIL: no AWS::DynamoDB::GlobalTable resource in cdkd state" + echo "[verify] FAIL: no HistoryTable AWS::DynamoDB::GlobalTable resource in cdkd state" + exit 1 +fi +if [ -z "${GSI_PROV_TABLE}" ] || [ -z "${GSI_OD_TABLE}" ]; then + echo "[verify] FAIL: Issue #1387 GSI fixture tables missing from cdkd state (prov='${GSI_PROV_TABLE}' on-demand='${GSI_OD_TABLE}')" exit 1 fi -echo "[verify] step 3 ok: deployed table name = ${TABLE_NAME}" +echo "[verify] step 3 ok: deployed table names = ${TABLE_NAME} / ${GSI_PROV_TABLE} / ${GSI_OD_TABLE}" # The canonical bug fix assertion: pre-PR the name was an opaque random # string (`yq2phLewTEUtzr4sy2gYFRU4I-1OGJ0UFLOKOOV`-style); post-PR it @@ -168,6 +186,55 @@ echo "[verify] step 4: assert table exists on AWS" aws dynamodb describe-table --table-name "${TABLE_NAME}" --region "${REGION}" >/dev/null echo "[verify] step 4 ok: DescribeTable succeeded" +echo "[verify] step 4b (Issue #1387): assert per-GSI throughput reached AWS" +# The bug: cdkd cast the CFn `GlobalSecondaryIndexes` blob straight to the +# SDK's `GlobalSecondaryIndex[]`, and the SDK serializer drops unknown +# members. So the PROVISIONED table's CreateTable used to fail outright +# ("Neither ProvisionedThroughput nor OnDemandThroughput was specified for +# index: byStatus") and the on-demand table's per-index limits were dropped +# silently. Read every value back off DescribeTable. +# +# Expected values, and where each half comes from in the synthesized template: +# byStatus.ProvisionedThroughput.ReadCapacityUnits = 7 <- Replicas[local].GlobalSecondaryIndexes[].ReadProvisionedThroughputSettings +# byStatus.ProvisionedThroughput.WriteCapacityUnits = 3 <- GSI.WriteProvisionedThroughputSettings...SeedCapacity +# byOwner.OnDemandThroughput.MaxReadRequestUnits = 50 <- Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings +# byOwner.OnDemandThroughput.MaxWriteRequestUnits = 60 <- GSI.WriteOnDemandThroughputSettings +gsi_field() { # $1 = table, $2 = index name, $3 = JMESPath under the index object + local out + out="$(aws dynamodb describe-table --table-name "$1" --region "${REGION}" \ + --query "Table.GlobalSecondaryIndexes[?IndexName=='$2'] | [0].$3" --output text)" || return 1 + printf '%s' "${out}" +} +assert_gsi_field() { # $1 = table, $2 = index, $3 = JMESPath, $4 = expected + local actual + actual="$(gsi_field "$1" "$2" "$3")" + if [ "${actual}" != "$4" ]; then + echo "[verify] FAIL (issue #1387): ${1} index ${2} ${3} is '${actual}' (expected '$4')" >&2 + echo "[verify] pre-fix cdkd forwarded the CFn-only spelling and the SDK dropped it" >&2 + exit 1 + fi + echo "[verify] step 4b ok: ${2}.${3} = ${actual}" +} +assert_gsi_field "${GSI_PROV_TABLE}" byStatus ProvisionedThroughput.ReadCapacityUnits 7 +assert_gsi_field "${GSI_PROV_TABLE}" byStatus ProvisionedThroughput.WriteCapacityUnits 3 +assert_gsi_field "${GSI_OD_TABLE}" byOwner OnDemandThroughput.MaxReadRequestUnits 50 +assert_gsi_field "${GSI_OD_TABLE}" byOwner OnDemandThroughput.MaxWriteRequestUnits 60 + +# Table-level throughput must survive alongside the per-index values. +PROV_TABLE_READ="$(aws dynamodb describe-table --table-name "${GSI_PROV_TABLE}" --region "${REGION}" \ + --query 'Table.ProvisionedThroughput.ReadCapacityUnits' --output text)" +if [ "${PROV_TABLE_READ}" != "5" ]; then + echo "[verify] FAIL: ${GSI_PROV_TABLE} table-level ReadCapacityUnits is '${PROV_TABLE_READ}' (expected 5)" >&2 + exit 1 +fi +OD_TABLE_MAX_WRITE="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \ + --query 'Table.OnDemandThroughput.MaxWriteRequestUnits' --output text)" +if [ "${OD_TABLE_MAX_WRITE}" != "200" ]; then + echo "[verify] FAIL: ${GSI_OD_TABLE} table-level MaxWriteRequestUnits is '${OD_TABLE_MAX_WRITE}' (expected 200)" >&2 + exit 1 +fi +echo "[verify] step 4b ok: table-level throughput preserved on both GSI fixtures" + echo "[verify] step 5 (was steps 5/6/7): cdkd deploy with CDKD_TEST_UPDATE=deletion-protection (in-place update — Issue #389)" # ORDER NOTE (PR follow-up to #403): TTL toggle is intentionally # deferred to the END of the integ flow. AWS's DynamoDB @@ -383,9 +450,11 @@ echo "[verify] step 15: cdkd destroy --remove-protection --force" # residual state without requiring operator intervention. ${CLI} destroy "${STACK}" --state-bucket "${STATE_BUCKET}" --remove-protection --force -echo "[verify] step 16a: assert table is gone on AWS" +echo "[verify] step 16a: assert tables are gone on AWS" assert_gone "table '${TABLE_NAME}' still exists after destroy" aws dynamodb describe-table --table-name "${TABLE_NAME}" --region "${REGION}" -echo "[verify] step 16a ok: table deleted" +assert_gone "table '${GSI_PROV_TABLE}' still exists after destroy" aws dynamodb describe-table --table-name "${GSI_PROV_TABLE}" --region "${REGION}" +assert_gone "table '${GSI_OD_TABLE}' still exists after destroy" aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" +echo "[verify] step 16a ok: all three tables deleted" echo "[verify] step 16b: assert cdkd state is empty" assert_gone "cdkd state file still exists at s3://${STATE_BUCKET}/${STATE_KEY}" aws s3api head-object --bucket "${STATE_BUCKET}" --key "${STATE_KEY}" diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts new file mode 100644 index 000000000..72b8295c1 --- /dev/null +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -0,0 +1,640 @@ +import { describe, it, expect, vi, beforeEach } from 'vite-plus/test'; +import { CreateTableCommand, UpdateTableCommand } from '@aws-sdk/client-dynamodb'; + +/** + * Issue #1387 regression coverage: the CFn `AWS::DynamoDB::GlobalTable` + * schema models per-GSI throughput completely differently from the SDK's + * `CreateTable` / `UpdateTable` shapes, and the provider used to cast the + * blob raw. The AWS SDK v3 serializer drops unknown members, so: + * + * - a PROVISIONED GlobalTable with a GSI failed `CreateTable` outright + * (AWS requires `ProvisionedThroughput` on every index), and + * - `TableV2`'s per-GSI on-demand limits vanished silently. + * + * EVERY property bag below is copied verbatim from a real `cdk synth` of + * `dynamodb.TableV2` (aws-cdk-lib 2.244.0) — not hand-invented — so the + * tests pin the shape CDK actually emits, including the fact that per-GSI + * READ capacity lives on the LOCAL REPLICA's index entry rather than on the + * top-level GSI. + */ + +const { mockSend, mockAutoScalingSend, warnSpy } = vi.hoisted(() => ({ + mockSend: vi.fn(), + mockAutoScalingSend: vi.fn(), + warnSpy: vi.fn(), +})); + +vi.mock('../../../src/utils/aws-clients.js', () => ({ + getAwsClients: () => ({ + dynamoDB: { send: mockSend, config: { region: () => Promise.resolve('us-east-1') } }, + }), +})); + +vi.mock('@aws-sdk/client-dynamodb', async () => { + const actual = await vi.importActual( + '@aws-sdk/client-dynamodb' + ); + return { + ...actual, + DynamoDBClient: vi.fn().mockImplementation((cfg: { region?: string } | undefined) => ({ + send: mockSend, + config: { region: () => Promise.resolve(cfg?.region ?? 'us-east-1') }, + })), + }; +}); + +vi.mock('@aws-sdk/client-application-auto-scaling', async () => { + const actual = await vi.importActual< + typeof import('@aws-sdk/client-application-auto-scaling') + >('@aws-sdk/client-application-auto-scaling'); + return { + ...actual, + ApplicationAutoScalingClient: vi.fn().mockImplementation(() => ({ + send: mockAutoScalingSend, + })), + }; +}); + +vi.mock('../../../src/utils/logger.js', () => { + const childLogger = { + debug: vi.fn(), + info: vi.fn(), + warn: warnSpy, + error: vi.fn(), + child: vi.fn().mockReturnThis(), + }; + return { + getLogger: () => ({ + child: () => childLogger, + debug: vi.fn(), + info: vi.fn(), + warn: warnSpy, + error: vi.fn(), + }), + }; +}); + +import { + DynamoDBGlobalTableProvider, + deriveReadCapacityUnits, + deriveWriteCapacityUnits, + toSdkGlobalSecondaryIndexes, + toSdkReplicaGlobalSecondaryIndexes, +} from '../../../src/provisioning/providers/dynamodb-globaltable-provider.js'; + +const RESOURCE_TYPE = 'AWS::DynamoDB::GlobalTable'; +const TABLE_ARN = 'arn:aws:dynamodb:us-east-1:123:table/prov-table'; + +/** + * Verbatim `cdk synth` output for: + * + * ```ts + * new ddb.TableV2(stack, 'Prov', { + * partitionKey: { name: 'pk', type: STRING }, + * sortKey: { name: 'sk', type: STRING }, + * billing: Billing.provisioned({ + * readCapacity: Capacity.fixed(5), + * writeCapacity: Capacity.autoscaled({ minCapacity: 1, maxCapacity: 10, targetUtilizationPercent: 70 }), + * }), + * globalSecondaryIndexes: [{ + * indexName: 'gsi1', + * partitionKey: { name: 'g1pk', type: STRING }, + * readCapacity: Capacity.fixed(7), + * writeCapacity: Capacity.autoscaled({ minCapacity: 2, maxCapacity: 20, seedCapacity: 3, targetUtilizationPercent: 60 }), + * }], + * }); + * ``` + * + * Note what is NOT here: the GSI carries no `ReadProvisionedThroughputSettings` + * and no literal `WriteCapacityUnits`. That is the whole bug. + */ +const PROVISIONED_TABLE_PROPS: Record = { + AttributeDefinitions: [ + { AttributeName: 'pk', AttributeType: 'S' }, + { AttributeName: 'sk', AttributeType: 'S' }, + { AttributeName: 'g1pk', AttributeType: 'S' }, + ], + BillingMode: 'PROVISIONED', + GlobalSecondaryIndexes: [ + { + IndexName: 'gsi1', + KeySchema: [{ AttributeName: 'g1pk', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + WriteProvisionedThroughputSettings: { + WriteCapacityAutoScalingSettings: { + MaxCapacity: 20, + MinCapacity: 2, + SeedCapacity: 3, + TargetTrackingScalingPolicyConfiguration: { TargetValue: 60 }, + }, + }, + }, + ], + KeySchema: [ + { AttributeName: 'pk', KeyType: 'HASH' }, + { AttributeName: 'sk', KeyType: 'RANGE' }, + ], + Replicas: [ + { + GlobalSecondaryIndexes: [ + { IndexName: 'gsi1', ReadProvisionedThroughputSettings: { ReadCapacityUnits: 7 } }, + ], + ReadProvisionedThroughputSettings: { ReadCapacityUnits: 5 }, + Region: 'us-east-1', + }, + ], + WriteProvisionedThroughputSettings: { + WriteCapacityAutoScalingSettings: { + MaxCapacity: 10, + MinCapacity: 1, + TargetTrackingScalingPolicyConfiguration: { TargetValue: 70 }, + }, + }, +}; + +/** + * Verbatim `cdk synth` output for the on-demand sibling: + * + * ```ts + * new ddb.TableV2(stack, 'OnDemand', { + * partitionKey: { name: 'pk', type: STRING }, + * billing: Billing.onDemand({ maxReadRequestUnits: 100, maxWriteRequestUnits: 200 }), + * globalSecondaryIndexes: [{ + * indexName: 'gsi2', + * partitionKey: { name: 'g2pk', type: STRING }, + * maxReadRequestUnits: 50, + * maxWriteRequestUnits: 60, + * }], + * }); + * ``` + */ +const ON_DEMAND_TABLE_PROPS: Record = { + AttributeDefinitions: [ + { AttributeName: 'pk', AttributeType: 'S' }, + { AttributeName: 'g2pk', AttributeType: 'S' }, + ], + BillingMode: 'PAY_PER_REQUEST', + GlobalSecondaryIndexes: [ + { + IndexName: 'gsi2', + KeySchema: [{ AttributeName: 'g2pk', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + WriteOnDemandThroughputSettings: { MaxWriteRequestUnits: 60 }, + }, + ], + KeySchema: [{ AttributeName: 'pk', KeyType: 'HASH' }], + Replicas: [ + { + GlobalSecondaryIndexes: [ + { IndexName: 'gsi2', ReadOnDemandThroughputSettings: { MaxReadRequestUnits: 50 } }, + ], + ReadOnDemandThroughputSettings: { MaxReadRequestUnits: 100 }, + Region: 'us-east-1', + }, + ], + WriteOnDemandThroughputSettings: { MaxWriteRequestUnits: 200 }, +}; + +describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { + let provider: DynamoDBGlobalTableProvider; + + beforeEach(() => { + mockSend.mockReset(); + mockAutoScalingSend.mockReset(); + warnSpy.mockReset(); + mockAutoScalingSend.mockResolvedValue({ ScalableTargets: [], ScalingPolicies: [] }); + // Every DynamoDB call in these tests is asserted on its INPUT, so a + // single always-ACTIVE default response covers CreateTable / + // DescribeTable / UpdateTable / the wait pollers alike. Tests that + // needed ordered responses would use `mockResolvedValueOnce`. + mockSend.mockResolvedValue({ + Table: { + TableName: 'prov-table', + TableArn: TABLE_ARN, + TableId: 'tid-1', + TableStatus: 'ACTIVE', + Replicas: [{ RegionName: 'eu-west-1', ReplicaStatus: 'ACTIVE' }], + }, + }); + provider = new DynamoDBGlobalTableProvider(); + }); + + describe('capacity derivation helpers', () => { + it('takes SeedCapacity before MinCapacity on the write side', () => { + expect( + deriveWriteCapacityUnits({ + WriteCapacityAutoScalingSettings: { MinCapacity: 2, MaxCapacity: 20, SeedCapacity: 3 }, + }) + ).toBe(3); + }); + + it('falls back to MinCapacity when no SeedCapacity is present', () => { + expect( + deriveWriteCapacityUnits({ + WriteCapacityAutoScalingSettings: { MinCapacity: 2, MaxCapacity: 20 }, + }) + ).toBe(2); + }); + + it('honors a literal WriteCapacityUnits over auto-scaling settings', () => { + expect( + deriveWriteCapacityUnits({ + WriteCapacityUnits: 42, + WriteCapacityAutoScalingSettings: { MinCapacity: 2, MaxCapacity: 20 }, + }) + ).toBe(42); + }); + + it('reads the per-replica flat ReadCapacityUnits shape', () => { + expect(deriveReadCapacityUnits({ ReadCapacityUnits: 7 })).toBe(7); + expect( + deriveReadCapacityUnits({ + ReadCapacityAutoScalingSettings: { MinCapacity: 4, MaxCapacity: 40 }, + }) + ).toBe(4); + }); + + it('returns undefined (not NaN / 0) for absent or unusable settings', () => { + expect(deriveWriteCapacityUnits(undefined)).toBeUndefined(); + expect(deriveWriteCapacityUnits({})).toBeUndefined(); + expect(deriveReadCapacityUnits({ ReadCapacityUnits: 'not-a-number' })).toBeUndefined(); + }); + }); + + describe('toSdkGlobalSecondaryIndexes', () => { + it('maps the PROVISIONED CDK shape to SDK ProvisionedThroughput (write from the GSI, read from the local replica)', () => { + const [gsi] = toSdkGlobalSecondaryIndexes( + PROVISIONED_TABLE_PROPS, + 'us-east-1', + 'PROVISIONED' + ); + expect(gsi).toBeDefined(); + // The bug: pre-fix this object was the raw CFn blob, so + // `ProvisionedThroughput` was absent and CreateTable 400'd. + expect(gsi!.ProvisionedThroughput).toEqual({ + ReadCapacityUnits: 7, + WriteCapacityUnits: 3, + }); + expect(gsi!.IndexName).toBe('gsi1'); + expect(gsi!.KeySchema).toEqual([{ AttributeName: 'g1pk', KeyType: 'HASH' }]); + expect(gsi!.Projection).toEqual({ ProjectionType: 'ALL' }); + // The CFn-only spellings must not survive into the SDK input. + expect(gsi).not.toHaveProperty('WriteProvisionedThroughputSettings'); + expect(gsi!.OnDemandThroughput).toBeUndefined(); + }); + + it('maps the on-demand CDK shape to SDK OnDemandThroughput (both directions)', () => { + const [gsi] = toSdkGlobalSecondaryIndexes( + ON_DEMAND_TABLE_PROPS, + 'us-east-1', + 'PAY_PER_REQUEST' + ); + expect(gsi!.OnDemandThroughput).toEqual({ + MaxReadRequestUnits: 50, + MaxWriteRequestUnits: 60, + }); + expect(gsi!.ProvisionedThroughput).toBeUndefined(); + expect(gsi).not.toHaveProperty('WriteOnDemandThroughputSettings'); + }); + + it('defaults PROVISIONED capacity to 5/5 when the template carries no GSI throughput at all', () => { + const [gsi] = toSdkGlobalSecondaryIndexes( + { + BillingMode: 'PROVISIONED', + GlobalSecondaryIndexes: [ + { + IndexName: 'bare', + KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], + Projection: { ProjectionType: 'KEYS_ONLY' }, + }, + ], + }, + 'us-east-1', + 'PROVISIONED' + ); + expect(gsi!.ProvisionedThroughput).toEqual({ + ReadCapacityUnits: 5, + WriteCapacityUnits: 5, + }); + }); + + it('honors the GSI-level ReadProvisionedThroughputSettings fallback for hand-authored L1 templates', () => { + const [gsi] = toSdkGlobalSecondaryIndexes( + { + GlobalSecondaryIndexes: [ + { + IndexName: 'l1', + KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + ReadProvisionedThroughputSettings: { ReadCapacityUnits: 9 }, + WriteProvisionedThroughputSettings: { + WriteCapacityAutoScalingSettings: { MinCapacity: 8, MaxCapacity: 80 }, + }, + }, + ], + }, + 'us-east-1', + 'PROVISIONED' + ); + expect(gsi!.ProvisionedThroughput).toEqual({ + ReadCapacityUnits: 9, + WriteCapacityUnits: 8, + }); + }); + + it('passes an already-SDK-shaped ProvisionedThroughput through untouched', () => { + const [gsi] = toSdkGlobalSecondaryIndexes( + { + GlobalSecondaryIndexes: [ + { + IndexName: 'sdk-shaped', + KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + ProvisionedThroughput: { ReadCapacityUnits: 11, WriteCapacityUnits: 12 }, + }, + ], + }, + 'us-east-1', + 'PROVISIONED' + ); + expect(gsi!.ProvisionedThroughput).toEqual({ + ReadCapacityUnits: 11, + WriteCapacityUnits: 12, + }); + }); + + it('forwards WarmThroughput verbatim (same spelling on both sides)', () => { + const [gsi] = toSdkGlobalSecondaryIndexes( + { + GlobalSecondaryIndexes: [ + { + IndexName: 'warm', + KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + WarmThroughput: { ReadUnitsPerSecond: 12000, WriteUnitsPerSecond: 4000 }, + }, + ], + }, + 'us-east-1', + 'PAY_PER_REQUEST' + ); + expect(gsi!.WarmThroughput).toEqual({ + ReadUnitsPerSecond: 12000, + WriteUnitsPerSecond: 4000, + }); + }); + + it('returns an empty array when the template declares no GSIs', () => { + expect(toSdkGlobalSecondaryIndexes({}, 'us-east-1', 'PROVISIONED')).toEqual([]); + }); + }); + + describe('toSdkReplicaGlobalSecondaryIndexes', () => { + it('maps per-replica read settings to the SDK Override members', () => { + const result = toSdkReplicaGlobalSecondaryIndexes([ + { IndexName: 'gsi1', ReadProvisionedThroughputSettings: { ReadCapacityUnits: 7 } }, + { IndexName: 'gsi2', ReadOnDemandThroughputSettings: { MaxReadRequestUnits: 50 } }, + ]); + expect(result).toEqual([ + { IndexName: 'gsi1', ProvisionedThroughputOverride: { ReadCapacityUnits: 7 } }, + { IndexName: 'gsi2', OnDemandThroughputOverride: { MaxReadRequestUnits: 50 } }, + ]); + }); + + it('emits IndexName only when the replica entry carries no throughput override', () => { + expect(toSdkReplicaGlobalSecondaryIndexes([{ IndexName: 'plain' }])).toEqual([ + { IndexName: 'plain' }, + ]); + }); + + it('returns undefined for an absent / empty list so the SDK field stays unset', () => { + expect(toSdkReplicaGlobalSecondaryIndexes(undefined)).toBeUndefined(); + expect(toSdkReplicaGlobalSecondaryIndexes([])).toBeUndefined(); + }); + }); + + describe('create()', () => { + it('sends per-GSI ProvisionedThroughput on CreateTable for a PROVISIONED GlobalTable', async () => { + + await provider.create('Prov', RESOURCE_TYPE, { + ...PROVISIONED_TABLE_PROPS, + TableName: 'prov-table', + }); + + const create = mockSend.mock.calls[0]?.[0] as CreateTableCommand; + expect(create).toBeInstanceOf(CreateTableCommand); + // Without the fix this array was the raw CFn blob and AWS rejected + // the call with "One or more parameter values were invalid: Neither + // ProvisionedThroughput nor OnDemandThroughput was specified for + // index: gsi1". + expect(create.input.GlobalSecondaryIndexes).toEqual([ + { + IndexName: 'gsi1', + KeySchema: [{ AttributeName: 'g1pk', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + ProvisionedThroughput: { ReadCapacityUnits: 7, WriteCapacityUnits: 3 }, + }, + ]); + // Table-level capacity keeps working (seed-before-min applies here too). + expect(create.input.ProvisionedThroughput).toEqual({ + ReadCapacityUnits: 5, + WriteCapacityUnits: 1, + }); + }); + + it('sends per-GSI OnDemandThroughput on CreateTable for an on-demand GlobalTable', async () => { + + await provider.create('OnDemand', RESOURCE_TYPE, { + ...ON_DEMAND_TABLE_PROPS, + TableName: 'od-table', + }); + + const create = mockSend.mock.calls[0]?.[0] as CreateTableCommand; + expect(create.input.GlobalSecondaryIndexes?.[0]?.OnDemandThroughput).toEqual({ + MaxReadRequestUnits: 50, + MaxWriteRequestUnits: 60, + }); + expect(create.input.ProvisionedThroughput).toBeUndefined(); + }); + + it('translates a cross-region replica GSI override on the replica-add UpdateTable', async () => { + + await provider.create('OnDemand', RESOURCE_TYPE, { + ...ON_DEMAND_TABLE_PROPS, + TableName: 'od-table', + Replicas: [ + ...(ON_DEMAND_TABLE_PROPS['Replicas'] as unknown[]), + { + Region: 'eu-west-1', + GlobalSecondaryIndexes: [ + { IndexName: 'gsi2', ReadOnDemandThroughputSettings: { MaxReadRequestUnits: 25 } }, + ], + }, + ], + }); + + const replicaAdd = mockSend.mock.calls[2]?.[0] as UpdateTableCommand; + expect(replicaAdd).toBeInstanceOf(UpdateTableCommand); + expect(replicaAdd.input.ReplicaUpdates?.[0]?.Create?.GlobalSecondaryIndexes).toEqual([ + { IndexName: 'gsi2', OnDemandThroughputOverride: { MaxReadRequestUnits: 25 } }, + ]); + }); + }); + + describe('update()', () => { + it('carries ProvisionedThroughput on a GSI Create action (added index)', async () => { + + const previous = { ...PROVISIONED_TABLE_PROPS, GlobalSecondaryIndexes: [] }; + await provider.update( + 'Prov', + 'prov-table', + RESOURCE_TYPE, + PROVISIONED_TABLE_PROPS, + previous + ); + + const gsiCall = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && c.input.GlobalSecondaryIndexUpdates !== undefined + ); + expect(gsiCall?.input.GlobalSecondaryIndexUpdates).toEqual([ + { + Create: { + IndexName: 'gsi1', + KeySchema: [{ AttributeName: 'g1pk', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + ProvisionedThroughput: { ReadCapacityUnits: 7, WriteCapacityUnits: 3 }, + }, + }, + ]); + }); + + it('carries ProvisionedThroughput on a GSI Update action when the per-replica read capacity changes', async () => { + + const next = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + ( + (next['Replicas'] as Array>)[0]![ + 'GlobalSecondaryIndexes' + ] as Array> + )[0]!['ReadProvisionedThroughputSettings'] = { ReadCapacityUnits: 15 }; + + await provider.update('Prov', 'prov-table', RESOURCE_TYPE, next, PROVISIONED_TABLE_PROPS); + + const gsiCall = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && c.input.GlobalSecondaryIndexUpdates !== undefined + ); + expect(gsiCall?.input.GlobalSecondaryIndexUpdates).toEqual([ + { + Update: { + IndexName: 'gsi1', + ProvisionedThroughput: { ReadCapacityUnits: 15, WriteCapacityUnits: 3 }, + }, + }, + ]); + }); + + it('issues NO GSI UpdateTable for an auto-scaling-only edit DynamoDB cannot express', async () => { + + const next = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + const gsi = (next['GlobalSecondaryIndexes'] as Array>)[0]!; + ( + (gsi['WriteProvisionedThroughputSettings'] as Record)[ + 'WriteCapacityAutoScalingSettings' + ] as Record + )['MaxCapacity'] = 30; + + await provider.update('Prov', 'prov-table', RESOURCE_TYPE, next, PROVISIONED_TABLE_PROPS); + + // Pre-fix the raw-CFn diff saw a "modified" GSI and sent + // `Update: { IndexName }` with no throughput — a ValidationException. + const gsiCalls = mockSend.mock.calls + .map((c) => c[0]) + .filter( + (c) => c instanceof UpdateTableCommand && c.input.GlobalSecondaryIndexUpdates !== undefined + ); + expect(gsiCalls).toHaveLength(0); + }); + + it('includes per-GSI ProvisionedThroughput in the PAY_PER_REQUEST -> PROVISIONED flip call', async () => { + + // Previous deploy: same index, on-demand billing. + const previous: Record = { + ...PROVISIONED_TABLE_PROPS, + BillingMode: 'PAY_PER_REQUEST', + }; + + await provider.update( + 'Prov', + 'prov-table', + RESOURCE_TYPE, + PROVISIONED_TABLE_PROPS, + previous + ); + + const flip = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && c.input.BillingMode !== undefined + ); + // AWS requires the per-index throughput in the SAME call as the + // BillingMode change; a bare flip 400s on a table that has GSIs. + expect(flip?.input.GlobalSecondaryIndexUpdates).toEqual([ + { + Update: { + IndexName: 'gsi1', + ProvisionedThroughput: { ReadCapacityUnits: 7, WriteCapacityUnits: 3 }, + }, + }, + ]); + // ...and step 6 must not re-issue the same Update. + const standaloneGsiCalls = mockSend.mock.calls + .map((c) => c[0]) + .filter( + (c) => + c instanceof UpdateTableCommand && + c.input.GlobalSecondaryIndexUpdates !== undefined && + c.input.BillingMode === undefined + ); + expect(standaloneGsiCalls).toHaveLength(0); + }); + + it('translates the per-replica GSI override on a cross-region UpdateReplica', async () => { + + const euReplica = (override: number): Record => ({ + Region: 'eu-west-1', + GlobalSecondaryIndexes: [ + { + IndexName: 'gsi2', + ReadOnDemandThroughputSettings: { MaxReadRequestUnits: override }, + }, + ], + }); + const previous: Record = { + ...ON_DEMAND_TABLE_PROPS, + Replicas: [...(ON_DEMAND_TABLE_PROPS['Replicas'] as unknown[]), euReplica(25)], + }; + const next: Record = { + ...ON_DEMAND_TABLE_PROPS, + Replicas: [...(ON_DEMAND_TABLE_PROPS['Replicas'] as unknown[]), euReplica(35)], + }; + + await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous); + + const replicaUpdate = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && c.input.ReplicaUpdates !== undefined + ); + expect(replicaUpdate?.input.ReplicaUpdates?.[0]?.Update?.GlobalSecondaryIndexes).toEqual([ + { IndexName: 'gsi2', OnDemandThroughputOverride: { MaxReadRequestUnits: 35 } }, + ]); + }); + }); +}); From f40e3b0875bd0291f0d72f1a030b589c3f094a48 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:08:55 +0900 Subject: [PATCH 02/18] test(integ): record the dynamodb-globaltable run in the integ ledger --- docs/_generated/integ-last-run.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index 8b03f8969..2c739a2a2 100644 --- a/docs/_generated/integ-last-run.tsv +++ b/docs/_generated/integ-last-run.tsv @@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee 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 dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean -dynamodb-globaltable 2026-07-21T05:21:32Z PASS 237 verify.sh rc ok, orph clean +dynamodb-globaltable 2026-08-09T11:07:58Z PASS 168 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans From d26bb9ceb909785caf84ecdf2aa508407527df44 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:22:01 +0900 Subject: [PATCH 03/18] fix(dynamodb): carry throughput for a removed GSI in the billing flip, and stop warning on a flip Two defects found by code review of the GSI throughput translation. The billing-flip call built its index updates from the NEW template but filtered them by the names in the PREVIOUS one, so an index the deploy removes was silently omitted. Its Delete is issued in step 6, which runs after the flip, so at flip time it is still a live index on a table becoming PROVISIONED and AWS rejects the call for having no capacity on it. Its throughput now comes from the previous template, and only surviving indexes are marked handled so the removed one still reaches its Delete. The modified-GSI loop also warned that the index changed in a way UpdateTable cannot express whenever the BillingMode flipped, because the two sides are translated under different billing modes and therefore always differ. That warning is false and sends the user hunting a template problem that does not exist; the loop now skips entirely when the billing mode changed, since step 4 already applied the throughput change atomically. Both are bound by mutation-probed unit tests. Three pre-existing gaps the review surfaced are filed as #1419, #1420 and #1421. --- .../dynamodb-globaltable-provider.ts | 41 +++++- ...lobaltable-provider-gsi-throughput.test.ts | 136 +++++++++++++++++- 2 files changed, 170 insertions(+), 7 deletions(-) diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index ac5305e3b..073b80544 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -906,17 +906,40 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { .map((entry) => (entry as Record | null)?.['IndexName']) .filter((name): name is string => typeof name === 'string') ); - const indexUpdates: GlobalSecondaryIndexUpdate[] = []; + // EVERY index live on AWS needs throughput in this call, including + // one this deploy REMOVES: its `Delete` is issued by step 6, which + // runs AFTER the flip, so at flip time it is still a live index on + // a table becoming PROVISIONED and AWS rejects the call if it has + // no capacity. Its throughput therefore has to come from the + // PREVIOUS template — the new one no longer mentions it. + const byIndexName = new Map(); + for (const gsi of toSdkGlobalSecondaryIndexes( + previousProperties, + currentRegion, + newBilling + )) { + if (gsi.IndexName) byIndexName.set(gsi.IndexName, gsi); + } + const survivingIndexNames = new Set(); for (const gsi of provisionedIndexes) { - if (!gsi.IndexName || !gsi.ProvisionedThroughput) continue; - if (!existingIndexNames.has(gsi.IndexName)) continue; + if (!gsi.IndexName) continue; + survivingIndexNames.add(gsi.IndexName); + byIndexName.set(gsi.IndexName, gsi); + } + const indexUpdates: GlobalSecondaryIndexUpdate[] = []; + for (const indexName of existingIndexNames) { + const gsi = byIndexName.get(indexName); + if (!gsi?.ProvisionedThroughput) continue; indexUpdates.push({ Update: { - IndexName: gsi.IndexName, + IndexName: indexName, ProvisionedThroughput: gsi.ProvisionedThroughput, }, }); - gsiHandledByBillingFlip.add(gsi.IndexName); + // Only a SURVIVING index is "handled" — step 6's `modified` loop + // is what this set suppresses, and a removed index belongs to the + // `removed` loop, which must still run. + if (survivingIndexNames.has(indexName)) gsiHandledByBillingFlip.add(indexName); } if (indexUpdates.length > 0) { billingUpdate.GlobalSecondaryIndexUpdates = indexUpdates; @@ -1204,6 +1227,14 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { if (!gsi.IndexName) continue; // Already applied atomically with the BillingMode flip above. if (gsiHandledByBillingFlip.has(gsi.IndexName)) continue; + // A BillingMode flip re-shapes EVERY index by construction (the two + // sides are translated under different billing modes, so the old + // side carries `ProvisionedThroughput` and the new side carries + // on-demand fields or neither). Those entries are not template + // edits at all, and the flip in step 4 already applied the + // throughput change atomically — emitting anything here, least of + // all the immutable-field warning below, would be actively wrong. + if (oldBilling !== newBilling) continue; // A GSI whose only change is KeySchema / Projection produces no // throughput fields; AWS rejects an `Update` action with nothing // but an IndexName, so skip and let the (immutable) change surface diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts index 72b8295c1..e6cd2e884 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -382,6 +382,13 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { ReadUnitsPerSecond: 12000, WriteUnitsPerSecond: 4000, }); + // Load-bearing: the throughput members must be OMITTED, not emitted + // empty. This index declares no on-demand limits, so relaxing the + // emit-when-present guard would send `OnDemandThroughput: {}`, which + // AWS rejects — and without this assertion that regression is + // invisible to the whole suite. + expect(gsi!.OnDemandThroughput).toBeUndefined(); + expect(gsi!.ProvisionedThroughput).toBeUndefined(); }); it('returns an empty array when the template declares no GSIs', () => { @@ -550,14 +557,61 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { await provider.update('Prov', 'prov-table', RESOURCE_TYPE, next, PROVISIONED_TABLE_PROPS); - // Pre-fix the raw-CFn diff saw a "modified" GSI and sent - // `Update: { IndexName }` with no throughput — a ValidationException. + // NOTE ON WHAT THIS BINDS: under PROVISIONED billing the translated + // GSIs are byte-identical here (MaxCapacity 20 -> 30 does not move + // Seed=3 / read=7), so the diff reports nothing modified and the + // warn-and-skip guard is never reached. Reverting EITHER the + // translation or the guard alone still leaves this green — the + // translation revert is masked by the guard, and the guard revert is + // masked by the translation. It is a both-must-hold end-state + // assertion, not a binding test for either mechanism on its own. + // The translation is bound by the two Create/Update tests above; the + // guard is bound by the PAY_PER_REQUEST case below. + const gsiCalls = mockSend.mock.calls + .map((c) => c[0]) + .filter( + (c) => c instanceof UpdateTableCommand && c.input.GlobalSecondaryIndexUpdates !== undefined + ); + expect(gsiCalls).toHaveLength(0); + }); + + it('warns and skips a modified GSI that yields no throughput at all, instead of sending an empty Update action', async () => { + // Binds the warn-and-skip guard specifically. A PAY_PER_REQUEST GSI + // carrying no on-demand limits translates to a GSI with NEITHER + // ProvisionedThroughput nor OnDemandThroughput, so a KeySchema / + // Projection-only edit reaches the guard with nothing to send. Without + // the guard the provider emits `Update: { IndexName }`, which AWS + // rejects with a ValidationException mid-deploy. + const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record; + const prevGsi = (previous['GlobalSecondaryIndexes'] as Array>)[0]!; + delete prevGsi['ReadOnDemandThroughputSettings']; + delete prevGsi['WriteOnDemandThroughputSettings']; + for (const replica of previous['Replicas'] as Array>) { + for (const rGsi of (replica['GlobalSecondaryIndexes'] ?? []) as Array< + Record + >) { + delete rGsi['ReadOnDemandThroughputSettings']; + delete rGsi['ReadProvisionedThroughputSettings']; + } + } + + // Projection is immutable on an existing index, so this is exactly the + // shape DynamoDB's UpdateTable cannot express. + const next = structuredClone(previous) as Record; + ((next['GlobalSecondaryIndexes'] as Array>)[0]! as Record< + string, + unknown + >)['Projection'] = { ProjectionType: 'KEYS_ONLY' }; + + await provider.update('OnDemand', 'ondemand-table', RESOURCE_TYPE, next, previous); + const gsiCalls = mockSend.mock.calls .map((c) => c[0]) .filter( (c) => c instanceof UpdateTableCommand && c.input.GlobalSecondaryIndexUpdates !== undefined ); expect(gsiCalls).toHaveLength(0); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('UpdateTable cannot express')); }); it('includes per-GSI ProvisionedThroughput in the PAY_PER_REQUEST -> PROVISIONED flip call', async () => { @@ -604,6 +658,84 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { expect(standaloneGsiCalls).toHaveLength(0); }); + it('gives an index this deploy REMOVES throughput in the flip call, since its Delete is issued later', async () => { + // The flip runs in step 4; the `Delete` for a dropped index runs in + // step 6. So at flip time the dropped index is still a LIVE index on + // a table becoming PROVISIONED, and AWS rejects the call unless it + // carries capacity. Its throughput can only come from the PREVIOUS + // template, since the new one no longer mentions it. + const doomed = { + IndexName: 'gsiDoomed', + KeySchema: [{ AttributeName: 'dpk', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + WriteProvisionedThroughputSettings: { + WriteCapacityAutoScalingSettings: { MinCapacity: 2, MaxCapacity: 20, SeedCapacity: 9 }, + }, + }; + const previous = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + previous['BillingMode'] = 'PAY_PER_REQUEST'; + (previous['GlobalSecondaryIndexes'] as unknown[]).push(doomed); + ( + (previous['Replicas'] as Array>)[0]![ + 'GlobalSecondaryIndexes' + ] as unknown[] + ).push({ + IndexName: 'gsiDoomed', + ReadProvisionedThroughputSettings: { ReadCapacityUnits: 4 }, + }); + + // The new template keeps only gsi1 and flips to PROVISIONED. + await provider.update('Prov', 'prov-table', RESOURCE_TYPE, PROVISIONED_TABLE_PROPS, previous); + + const flip = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && c.input.BillingMode !== undefined + ); + expect(flip?.input.GlobalSecondaryIndexUpdates).toEqual( + expect.arrayContaining([ + { + Update: { + IndexName: 'gsiDoomed', + ProvisionedThroughput: { ReadCapacityUnits: 4, WriteCapacityUnits: 9 }, + }, + }, + ]) + ); + // The removed index must still reach step 6's Delete — being handled + // by the flip must not suppress its deletion. + const deletes = mockSend.mock.calls + .map((c) => c[0]) + .filter( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && + (c.input.GlobalSecondaryIndexUpdates ?? []).some((u) => u.Delete !== undefined) + ); + expect(deletes).toHaveLength(1); + expect(deletes[0]!.input.GlobalSecondaryIndexUpdates).toEqual([ + { Delete: { IndexName: 'gsiDoomed' } }, + ]); + }); + + it('does not emit the immutable-field warning when the BillingMode flipped PROVISIONED -> PAY_PER_REQUEST', async () => { + // A flip re-shapes every index by construction: the old side is + // translated under PROVISIONED (so it carries ProvisionedThroughput) + // and the new side under PAY_PER_REQUEST with no on-demand limits (so + // it carries neither field). Every GSI therefore lands in `modified` + // with nothing to send. That is NOT a KeySchema / Projection edit, and + // warning about immutable fields would send the user hunting a + // non-existent template problem. + const next = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + next['BillingMode'] = 'PAY_PER_REQUEST'; + + await provider.update('Prov', 'prov-table', RESOURCE_TYPE, next, PROVISIONED_TABLE_PROPS); + + expect(warnSpy).not.toHaveBeenCalledWith( + expect.stringContaining('UpdateTable cannot express') + ); + }); + it('translates the per-replica GSI override on a cross-region UpdateReplica', async () => { const euReplica = (override: number): Record => ({ From 5d817bec542e12829acea2b0b8b97e2471abd30e Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:26:48 +0900 Subject: [PATCH 04/18] test(integ): record the dynamodb-globaltable re-run in the integ ledger --- docs/_generated/integ-last-run.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index 2c739a2a2..e7f9e9ea2 100644 --- a/docs/_generated/integ-last-run.tsv +++ b/docs/_generated/integ-last-run.tsv @@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee 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 dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean -dynamodb-globaltable 2026-08-09T11:07:58Z PASS 168 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean +dynamodb-globaltable 2026-08-09T11:26:01Z PASS 207 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans From f5dbd379fe94748d7820afff926817be5162f602 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:58:13 +0900 Subject: [PATCH 05/18] chore(audit): regenerate the handled-property-wiring matrix for the new GSI helpers The GSI throughput translation moved the GlobalSecondaryIndexes read into delegated helpers, so the critic now records a delegated evidence tag alongside the existing element-read. CI drift-checks this file. --- docs/_generated/handled-property-wiring.json | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/_generated/handled-property-wiring.json b/docs/_generated/handled-property-wiring.json index 48745762f..c28055caa 100644 --- a/docs/_generated/handled-property-wiring.json +++ b/docs/_generated/handled-property-wiring.json @@ -4876,6 +4876,7 @@ "AWS::DynamoDB::GlobalTable" ], "evidence": [ + "delegated", "element-read" ], "seededBy": [ From f2bd2013c76204ed671482c9e7d2e09622fbdce9 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:18:35 +0900 Subject: [PATCH 06/18] fix(dynamodb): stop the flip-to-on-demand GSI drop, refuse a non-array GSI blob, and carry WarmThroughput on add Three defects found by the 3-axis review of the first cut. Suppressing the false immutable-field warning by skipping the modified loop on any BillingMode flip was too broad. The flip call carries per-GSI fields only in the PAY_PER_REQUEST to PROVISIONED direction, so the reverse flip silently dropped every per-GSI Max Read/Write RequestUnits. The loop now sends the fields the new billing mode needs and suppresses only the warning; WarmThroughput rides along during a flip when it actually changed, so a simultaneous edit is not swallowed and an unchanged value costs no extra round trip. A non-array GlobalSecondaryIndexes value, such as an unresolved intrinsic, collapsed to an empty list and would have created the table with zero indexes while reporting success. That is the silent-drop class this work exists to close, one level up, so it now throws. Absent stays legitimately empty. WarmThroughput was translated by create but not forwarded by the update add path, so the same template produced a different index depending on whether the GSI was in the first deploy or a later one. Each fix is mutation-proofed: reverting any one of them kills a specific test. --- docs/changelog-cdkd.md | 2 +- .../dynamodb-globaltable-provider.ts | 118 +++++++++++++----- ...lobaltable-provider-gsi-throughput.test.ts | 110 ++++++++++++++-- 3 files changed, 190 insertions(+), 40 deletions(-) diff --git a/docs/changelog-cdkd.md b/docs/changelog-cdkd.md index aa6f21e4f..dd7446cf8 100644 --- a/docs/changelog-cdkd.md +++ b/docs/changelog-cdkd.md @@ -17,7 +17,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary --- **Recently Implemented** (2026-08-09): -- ✅ **`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==].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. **Tests:** 23 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. Revert-proofed: restoring the three original raw casts fails 5 of the provider-level tests. **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. +- ✅ **`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==].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. - ✅ **`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). - ✅ **`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. - ✅ **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. diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index 073b80544..1091b77b5 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -26,6 +26,7 @@ import { type Tag, type ReplicationGroupUpdate, type CreateReplicationGroupMemberAction, + type UpdateGlobalSecondaryIndexAction, type UpdateReplicationGroupMemberAction, type UpdateTableCommandInput, } from '@aws-sdk/client-dynamodb'; @@ -1184,10 +1185,22 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { // and an auto-scaling-only template edit — `MaxCapacity` 20 -> 30, // invisible to DynamoDB's own API — no longer produces a bare // `Update: { IndexName }` that AWS rejects as an empty update. + const previousSdkIndexes = toSdkGlobalSecondaryIndexes( + previousProperties, + currentRegion, + oldBilling + ); const gsiDiff = diffGlobalSecondaryIndexes( - toSdkGlobalSecondaryIndexes(previousProperties, currentRegion, oldBilling), + previousSdkIndexes, toSdkGlobalSecondaryIndexes(properties, currentRegion, newBilling) ); + // Needed by the `modified` loop to tell a REAL edit from the reshaping a + // BillingMode flip causes by construction (both sides are translated + // under different billing modes, so every index necessarily differs). + const previousSdkByName = new Map(); + for (const prev of previousSdkIndexes) { + if (prev.IndexName) previousSdkByName.set(prev.IndexName, prev); + } for (const gsi of gsiDiff.removed) { if (!gsi.IndexName) continue; const gsiUpdate: GlobalSecondaryIndexUpdate = { @@ -1212,6 +1225,11 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { ProvisionedThroughput: gsi.ProvisionedThroughput, }), ...(gsi.OnDemandThroughput && { OnDemandThroughput: gsi.OnDemandThroughput }), + // `create()` sends WarmThroughput for a GSI declared up front, so a + // GSI ADDED by a later update must carry it too — otherwise the + // same template yields a different index depending on whether it + // was in the first deploy or a subsequent one. + ...(gsi.WarmThroughput && { WarmThroughput: gsi.WarmThroughput }), }, }; await this.dynamoDBClient.send( @@ -1227,35 +1245,52 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { if (!gsi.IndexName) continue; // Already applied atomically with the BillingMode flip above. if (gsiHandledByBillingFlip.has(gsi.IndexName)) continue; - // A BillingMode flip re-shapes EVERY index by construction (the two - // sides are translated under different billing modes, so the old - // side carries `ProvisionedThroughput` and the new side carries - // on-demand fields or neither). Those entries are not template - // edits at all, and the flip in step 4 already applied the - // throughput change atomically — emitting anything here, least of - // all the immutable-field warning below, would be actively wrong. - if (oldBilling !== newBilling) continue; + // A BillingMode flip re-shapes EVERY index by construction: the two + // sides are translated under different billing modes, so the old side + // carries `ProvisionedThroughput` and the new side on-demand fields or + // neither. Such an entry is not a template edit, so it must not draw + // the immutable-field warning below — but it must NOT be skipped + // wholesale either. `PAY_PER_REQUEST -> PROVISIONED` is applied + // atomically by step 4's flip call (those indexes are filtered out + // above); the REVERSE direction has no per-GSI field in that call, so + // the on-demand limits have to be applied here as their own round trip. + // A blanket skip dropped every per-GSI `Max{Read,Write}RequestUnits` on + // a flip to on-demand — the same silent-drop class #1387 exists to close. + const billingFlipped = oldBilling !== newBilling; + const update: UpdateGlobalSecondaryIndexAction = { IndexName: gsi.IndexName }; + if (gsi.OnDemandThroughput) update.OnDemandThroughput = gsi.OnDemandThroughput; + // Provisioned capacity on a flip is step 4's job, so only a real + // same-billing-mode edit sends it from here. + if (!billingFlipped && gsi.ProvisionedThroughput) { + update.ProvisionedThroughput = gsi.ProvisionedThroughput; + } + // `WarmThroughput` is billing-mode independent, so a flip must not + // swallow a simultaneous change to it — but re-sending an unchanged + // value would cost an extra UpdateTable + wait-for-ACTIVE on every + // flip, so during one it rides along only when it actually differs. + const previousSdk = previousSdkByName.get(gsi.IndexName); + if ( + gsi.WarmThroughput && + (!billingFlipped || + JSON.stringify(gsi.WarmThroughput) !== JSON.stringify(previousSdk?.WarmThroughput)) + ) { + update.WarmThroughput = gsi.WarmThroughput; + } // A GSI whose only change is KeySchema / Projection produces no // throughput fields; AWS rejects an `Update` action with nothing // but an IndexName, so skip and let the (immutable) change surface // as a no-op rather than a confusing ValidationException. - if (!gsi.ProvisionedThroughput && !gsi.OnDemandThroughput) { - this.logger.warn( - `GSI '${gsi.IndexName}' on ${physicalId} changed in a way DynamoDB's ` + - `UpdateTable cannot express (KeySchema / Projection are immutable on an ` + - `existing index). Recreate the index under a new name to apply the change.` - ); + if (!update.ProvisionedThroughput && !update.OnDemandThroughput && !update.WarmThroughput) { + if (!billingFlipped) { + this.logger.warn( + `GSI '${gsi.IndexName}' on ${physicalId} changed in a way DynamoDB's ` + + `UpdateTable cannot express (KeySchema / Projection are immutable on an ` + + `existing index). Recreate the index under a new name to apply the change.` + ); + } continue; } - const gsiUpdate: GlobalSecondaryIndexUpdate = { - Update: { - IndexName: gsi.IndexName, - ...(gsi.ProvisionedThroughput && { - ProvisionedThroughput: gsi.ProvisionedThroughput, - }), - ...(gsi.OnDemandThroughput && { OnDemandThroughput: gsi.OnDemandThroughput }), - }, - }; + const gsiUpdate: GlobalSecondaryIndexUpdate = { Update: update }; await this.dynamoDBClient.send( new UpdateTableCommand({ TableName: physicalId, @@ -2640,11 +2675,16 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { * * Source of truth (CFn `AWS::DynamoDB::GlobalTable` shape): * - WriteCapacityUnits → `properties.WriteProvisionedThroughputSettings` - * (top-level on the table). Literal `WriteCapacityUnits` wins over - * auto-scaling `MinCapacity`; both default to 5 if absent. + * (top-level on the table). That block's ONLY member is + * `WriteCapacityAutoScalingSettings` — write capacity on a GlobalTable is + * always auto-scaled, so there is no literal `WriteCapacityUnits` to + * prefer. `CreateTable` needs one concrete number, so the auto-scaling + * block's `SeedCapacity` (documented as the value used until the scaling + * policies exist) is taken before `MinCapacity`; 5 if the block is absent. * - ReadCapacityUnits → `Replicas[?Region==].ReadProvisionedThroughputSettings` - * (per-replica, the deploy region's setting). Same literal-vs-auto- - * scaling-vs-default-5 precedence. + * (per-replica, the deploy region's setting). That block DOES carry a + * literal `ReadCapacityUnits`, which is taken before its auto-scaling + * `SeedCapacity` / `MinCapacity`; 5 if absent. */ /** * Extract the local replica's `DeletionProtectionEnabled` from a CFn @@ -2803,8 +2843,26 @@ export function toSdkGlobalSecondaryIndexes( region: string, billingMode: string ): GlobalSecondaryIndex[] { - const cfnIndexes = (properties['GlobalSecondaryIndexes'] ?? []) as unknown[]; - if (!Array.isArray(cfnIndexes) || cfnIndexes.length === 0) return []; + const rawIndexes = properties['GlobalSecondaryIndexes']; + // A NON-ARRAY value (an unresolved `Fn::If`, a malformed template) must not + // collapse to "no indexes": that would create the table with ZERO GSIs and + // report success, which is the very silent-drop class this function exists + // to close, one level up. Absent is legitimately empty; present-but-wrong + // is an error, and naming it here beats AWS's opaque 400. Matches the + // per-ENTRY policy below, which also refuses to swallow a bad entry. + if (rawIndexes !== undefined && !Array.isArray(rawIndexes)) { + // A plain Error on purpose: this is a module-level pure helper with no + // logicalId to build a ProvisioningError from. Both callers run inside + // create()/update()'s wrapping catch, which re-throws it as a + // ProvisioningError carrying the resource context. + throw new Error( + `AWS::DynamoDB::GlobalTable GlobalSecondaryIndexes must be an array, got ` + + `${typeof rawIndexes} (${JSON.stringify(rawIndexes)?.slice(0, 200)}). ` + + `An unresolved intrinsic here would otherwise deploy a table with no indexes.` + ); + } + const cfnIndexes = (rawIndexes ?? []) as unknown[]; + if (cfnIndexes.length === 0) return []; const replicas = (properties['Replicas'] ?? []) as Array>; const localReplica = Array.isArray(replicas) diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts index e6cd2e884..cfab412c0 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -394,6 +394,20 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { it('returns an empty array when the template declares no GSIs', () => { expect(toSdkGlobalSecondaryIndexes({}, 'us-east-1', 'PROVISIONED')).toEqual([]); }); + + it('throws on a non-array GlobalSecondaryIndexes instead of deploying a table with none', () => { + // Absent is legitimately empty; present-but-not-an-array (an unresolved + // intrinsic) previously collapsed to [] and created the table with ZERO + // indexes while reporting success — the #1387 silent-drop class one + // level up. + expect(() => + toSdkGlobalSecondaryIndexes( + { GlobalSecondaryIndexes: { 'Fn::If': ['UseGsi', [], []] } }, + 'us-east-1', + 'PROVISIONED' + ) + ).toThrow(/GlobalSecondaryIndexes must be an array/); + }); }); describe('toSdkReplicaGlobalSecondaryIndexes', () => { @@ -518,6 +532,31 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { ]); }); + it('carries WarmThroughput on a GSI Create action, matching what create() sends', async () => { + // create() translates WarmThroughput for a GSI declared up front. If the + // update-add path omits it, the same template yields a different index + // depending on whether the GSI was in the first deploy or a later one. + const next = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + (next['GlobalSecondaryIndexes'] as Array>)[0]!['WarmThroughput'] = { + ReadUnitsPerSecond: 12000, + WriteUnitsPerSecond: 4000, + }; + const previous = { ...PROVISIONED_TABLE_PROPS, GlobalSecondaryIndexes: [] }; + + await provider.update('Prov', 'prov-table', RESOURCE_TYPE, next, previous); + + const gsiCall = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && c.input.GlobalSecondaryIndexUpdates !== undefined + ); + expect(gsiCall?.input.GlobalSecondaryIndexUpdates?.[0]?.Create?.WarmThroughput).toEqual({ + ReadUnitsPerSecond: 12000, + WriteUnitsPerSecond: 4000, + }); + }); + it('carries ProvisionedThroughput on a GSI Update action when the per-replica read capacity changes', async () => { const next = structuredClone(PROVISIONED_TABLE_PROPS) as Record; @@ -693,16 +732,26 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { (c): c is UpdateTableCommand => c instanceof UpdateTableCommand && c.input.BillingMode !== undefined ); - expect(flip?.input.GlobalSecondaryIndexUpdates).toEqual( - expect.arrayContaining([ - { - Update: { - IndexName: 'gsiDoomed', - ProvisionedThroughput: { ReadCapacityUnits: 4, WriteCapacityUnits: 9 }, - }, - }, - ]) + // NOT `arrayContaining`: that passes even if the SURVIVING index is + // dropped from the flip call, which would break the flip just as surely. + // Sorted so the assertion pins the SET, not AWS-irrelevant ordering. + const flipUpdates = [...(flip?.input.GlobalSecondaryIndexUpdates ?? [])].sort((a, b) => + (a.Update?.IndexName ?? '').localeCompare(b.Update?.IndexName ?? '') ); + expect(flipUpdates).toEqual([ + { + Update: { + IndexName: 'gsi1', + ProvisionedThroughput: { ReadCapacityUnits: 7, WriteCapacityUnits: 3 }, + }, + }, + { + Update: { + IndexName: 'gsiDoomed', + ProvisionedThroughput: { ReadCapacityUnits: 4, WriteCapacityUnits: 9 }, + }, + }, + ]); // The removed index must still reach step 6's Delete — being handled // by the flip must not suppress its deletion. const deletes = mockSend.mock.calls @@ -718,6 +767,49 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { ]); }); + it('still applies per-GSI on-demand limits when the BillingMode flips PROVISIONED -> PAY_PER_REQUEST', async () => { + // The flip call carries per-GSI fields only in the PAY_PER_REQUEST -> + // PROVISIONED direction. The reverse has no such field, so the on-demand + // limits must be issued as their own Update — a blanket "skip the + // modified loop on a flip" silently dropped them. + const next = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + next['BillingMode'] = 'PAY_PER_REQUEST'; + const nextGsi = (next['GlobalSecondaryIndexes'] as Array>)[0]!; + delete nextGsi['WriteProvisionedThroughputSettings']; + nextGsi['WriteOnDemandThroughputSettings'] = { MaxWriteRequestUnits: 88 }; + for (const replica of next['Replicas'] as Array>) { + for (const rGsi of (replica['GlobalSecondaryIndexes'] ?? []) as Array< + Record + >) { + delete rGsi['ReadProvisionedThroughputSettings']; + rGsi['ReadOnDemandThroughputSettings'] = { MaxReadRequestUnits: 77 }; + } + } + + await provider.update('Prov', 'prov-table', RESOURCE_TYPE, next, PROVISIONED_TABLE_PROPS); + + const gsiCall = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && + c.input.GlobalSecondaryIndexUpdates !== undefined && + c.input.BillingMode === undefined + ); + expect(gsiCall?.input.GlobalSecondaryIndexUpdates).toEqual([ + { + Update: { + IndexName: 'gsi1', + OnDemandThroughput: { MaxReadRequestUnits: 77, MaxWriteRequestUnits: 88 }, + }, + }, + ]); + // Provisioned capacity must NOT ride along into an on-demand table. + expect(gsiCall?.input.GlobalSecondaryIndexUpdates?.[0]?.Update?.ProvisionedThroughput).toBe( + undefined + ); + }); + it('does not emit the immutable-field warning when the BillingMode flipped PROVISIONED -> PAY_PER_REQUEST', async () => { // A flip re-shapes every index by construction: the old side is // translated under PROVISIONED (so it carries ProvisionedThroughput) From 8f7e50e68219a09a14897e2a26330676056bd339 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:22:45 +0900 Subject: [PATCH 07/18] test(integ): record the dynamodb-globaltable run in the integ ledger --- docs/_generated/integ-last-run.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index e7f9e9ea2..355d48db0 100644 --- a/docs/_generated/integ-last-run.tsv +++ b/docs/_generated/integ-last-run.tsv @@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee 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 dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean -dynamodb-globaltable 2026-08-09T11:26:01Z PASS 207 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean +dynamodb-globaltable 2026-08-09T12:22:09Z PASS 175 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans From 18887ea2b4087303447640e597da922e5a4532c4 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:30:27 +0900 Subject: [PATCH 08/18] fix(dynamodb): convert the GSI array error in create(), and stop dropping WarmThroughput on a flip Follow-up to the delta re-review of the previous round. create() has no wrapping catch at the translation point, so the helper's plain Error escaped untyped into the deploy engine's retry loop; the call site now converts it to a ProvisioningError. The same call was behind a truthiness gate, so a falsy-but-present value such as null skipped the non-array guard entirely and would have deployed a zero-GSI table. It is now called unconditionally, which is safe because the helper returns an empty list for an absent value. Indexes handled by the PAY_PER_REQUEST to PROVISIONED flip are skipped by the modified loop, so a simultaneous WarmThroughput change vanished with no warning. It now rides the flip's own Update action when it differs. The differs-check is applied in every case rather than only during a flip, because warm throughput is increase-only on the AWS side and re-asserting the current value on an unrelated capacity edit is a needless risk. The hand-rolled stringify comparison was replaced with the module's existing deepEqual. Each fix is mutation-proofed: reverting any one of them kills a specific test. --- .../dynamodb-globaltable-provider.ts | 59 ++++++++++----- ...lobaltable-provider-gsi-throughput.test.ts | 71 +++++++++++++++++++ 2 files changed, 114 insertions(+), 16 deletions(-) diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index 1091b77b5..d55df7f56 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -352,11 +352,26 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { // rather than cast. Pre-fix a PROVISIONED GlobalTable with a GSI // failed `CreateTable` outright (AWS requires per-GSI // `ProvisionedThroughput`) and per-GSI on-demand limits were dropped. - if (properties['GlobalSecondaryIndexes']) { - const sdkIndexes = toSdkGlobalSecondaryIndexes(properties, currentRegion, billingMode); - if (sdkIndexes.length > 0) { - createParams.GlobalSecondaryIndexes = sdkIndexes; - } + // Called UNCONDITIONALLY (it returns [] for an absent value): a truthiness + // gate here would let `null` / `''` / `0` skip the helper's non-array guard + // and deploy a zero-GSI table — the very case that guard exists for. + // `create()` has no wrapping catch around this point, so the helper's plain + // Error is converted to a ProvisioningError here rather than escaping + // untyped into the deploy engine's retry loop. + let sdkIndexes: GlobalSecondaryIndex[]; + try { + sdkIndexes = toSdkGlobalSecondaryIndexes(properties, currentRegion, billingMode); + } catch (error) { + throw new ProvisioningError( + error instanceof Error ? error.message : String(error), + resourceType, + logicalId, + undefined, + error instanceof Error ? error : undefined + ); + } + if (sdkIndexes.length > 0) { + createParams.GlobalSecondaryIndexes = sdkIndexes; } if (properties['LocalSecondaryIndexes']) { createParams.LocalSecondaryIndexes = properties[ @@ -913,14 +928,15 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { // a table becoming PROVISIONED and AWS rejects the call if it has // no capacity. Its throughput therefore has to come from the // PREVIOUS template — the new one no longer mentions it. - const byIndexName = new Map(); + const previousByIndexName = new Map(); for (const gsi of toSdkGlobalSecondaryIndexes( previousProperties, currentRegion, newBilling )) { - if (gsi.IndexName) byIndexName.set(gsi.IndexName, gsi); + if (gsi.IndexName) previousByIndexName.set(gsi.IndexName, gsi); } + const byIndexName = new Map(previousByIndexName); const survivingIndexNames = new Set(); for (const gsi of provisionedIndexes) { if (!gsi.IndexName) continue; @@ -931,10 +947,19 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { for (const indexName of existingIndexNames) { const gsi = byIndexName.get(indexName); if (!gsi?.ProvisionedThroughput) continue; + // Every index handled here is skipped by step 6's `modified` loop, + // so a simultaneous WarmThroughput change has to ride along or it + // is dropped with no warning. Sent only when it actually differs — + // warm throughput is increase-only, so re-asserting the current + // value on an unrelated capacity edit is a needless AWS-side risk. + const previousWarm = previousByIndexName.get(indexName)?.WarmThroughput; + const warmChanged = + gsi.WarmThroughput !== undefined && !deepEqual(gsi.WarmThroughput, previousWarm); indexUpdates.push({ Update: { IndexName: indexName, ProvisionedThroughput: gsi.ProvisionedThroughput, + ...(warmChanged && { WarmThroughput: gsi.WarmThroughput }), }, }); // Only a SURVIVING index is "handled" — step 6's `modified` loop @@ -1265,14 +1290,15 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { update.ProvisionedThroughput = gsi.ProvisionedThroughput; } // `WarmThroughput` is billing-mode independent, so a flip must not - // swallow a simultaneous change to it — but re-sending an unchanged - // value would cost an extra UpdateTable + wait-for-ACTIVE on every - // flip, so during one it rides along only when it actually differs. + // swallow a simultaneous change to it. Sent only when it actually + // differs, in EVERY case — DynamoDB warm throughput is increase-only, + // so re-asserting the current value on an unrelated capacity edit is a + // needless AWS-side risk, and the extra UpdateTable would also cost a + // wait-for-ACTIVE. const previousSdk = previousSdkByName.get(gsi.IndexName); if ( - gsi.WarmThroughput && - (!billingFlipped || - JSON.stringify(gsi.WarmThroughput) !== JSON.stringify(previousSdk?.WarmThroughput)) + gsi.WarmThroughput !== undefined && + !deepEqual(gsi.WarmThroughput, previousSdk?.WarmThroughput) ) { update.WarmThroughput = gsi.WarmThroughput; } @@ -2852,9 +2878,10 @@ export function toSdkGlobalSecondaryIndexes( // per-ENTRY policy below, which also refuses to swallow a bad entry. if (rawIndexes !== undefined && !Array.isArray(rawIndexes)) { // A plain Error on purpose: this is a module-level pure helper with no - // logicalId to build a ProvisioningError from. Both callers run inside - // create()/update()'s wrapping catch, which re-throws it as a - // ProvisioningError carrying the resource context. + // logicalId to build a ProvisioningError from. `update()` runs inside a + // wrapping catch that converts it; `create()` does NOT, so its call site + // converts it explicitly. Both paths therefore surface a ProvisioningError + // carrying the resource context. throw new Error( `AWS::DynamoDB::GlobalTable GlobalSecondaryIndexes must be an array, got ` + `${typeof rawIndexes} (${JSON.stringify(rawIndexes)?.slice(0, 200)}). ` + diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts index cfab412c0..edff5a662 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -395,6 +395,19 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { expect(toSdkGlobalSecondaryIndexes({}, 'us-east-1', 'PROVISIONED')).toEqual([]); }); + it('surfaces a non-array GlobalSecondaryIndexes from create() as a ProvisioningError', async () => { + // create() has no wrapping catch at the translation point, so it converts + // the helper's plain Error itself. A falsy-but-present value (null) is the + // case a truthiness gate would have skipped entirely, deploying a + // zero-GSI table. + await expect( + provider.create('Prov', RESOURCE_TYPE, { + ...PROVISIONED_TABLE_PROPS, + GlobalSecondaryIndexes: null, + }) + ).rejects.toThrow(/GlobalSecondaryIndexes must be an array/); + }); + it('throws on a non-array GlobalSecondaryIndexes instead of deploying a table with none', () => { // Absent is legitimately empty; present-but-not-an-array (an unresolved // intrinsic) previously collapsed to [] and created the table with ZERO @@ -767,6 +780,64 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { ]); }); + it('carries a changed WarmThroughput in the PAY_PER_REQUEST -> PROVISIONED flip call', async () => { + // Indexes handled by the flip are skipped by the `modified` loop, so a + // simultaneous WarmThroughput change has to ride the flip's own Update + // or it vanishes with no warning. + const previous = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + previous['BillingMode'] = 'PAY_PER_REQUEST'; + (previous['GlobalSecondaryIndexes'] as Array>)[0]!['WarmThroughput'] = + { ReadUnitsPerSecond: 12000, WriteUnitsPerSecond: 4000 }; + + const next = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + (next['GlobalSecondaryIndexes'] as Array>)[0]!['WarmThroughput'] = { + ReadUnitsPerSecond: 24000, + WriteUnitsPerSecond: 4000, + }; + + await provider.update('Prov', 'prov-table', RESOURCE_TYPE, next, previous); + + const flip = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && c.input.BillingMode !== undefined + ); + expect(flip?.input.GlobalSecondaryIndexUpdates?.[0]?.Update?.WarmThroughput).toEqual({ + ReadUnitsPerSecond: 24000, + WriteUnitsPerSecond: 4000, + }); + }); + + it('does not re-send an unchanged WarmThroughput on an unrelated capacity edit', async () => { + // Warm throughput is increase-only on the AWS side, so re-asserting the + // current value on a capacity-only edit is a needless risk. + const previous = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + (previous['GlobalSecondaryIndexes'] as Array>)[0]!['WarmThroughput'] = + { ReadUnitsPerSecond: 12000, WriteUnitsPerSecond: 4000 }; + const next = structuredClone(previous) as Record; + ( + (next['Replicas'] as Array>)[0]![ + 'GlobalSecondaryIndexes' + ] as Array> + )[0]!['ReadProvisionedThroughputSettings'] = { ReadCapacityUnits: 19 }; + + await provider.update('Prov', 'prov-table', RESOURCE_TYPE, next, previous); + + const gsiCall = mockSend.mock.calls + .map((c) => c[0]) + .find( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && c.input.GlobalSecondaryIndexUpdates !== undefined + ); + expect(gsiCall?.input.GlobalSecondaryIndexUpdates?.[0]?.Update?.ProvisionedThroughput).toEqual( + { ReadCapacityUnits: 19, WriteCapacityUnits: 3 } + ); + expect(gsiCall?.input.GlobalSecondaryIndexUpdates?.[0]?.Update?.WarmThroughput).toBe( + undefined + ); + }); + it('still applies per-GSI on-demand limits when the BillingMode flips PROVISIONED -> PAY_PER_REQUEST', async () => { // The flip call carries per-GSI fields only in the PAY_PER_REQUEST -> // PROVISIONED direction. The reverse has no such field, so the on-demand From 97e1e44abda732095a49bffe99dad00dfafcd243 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:34:29 +0900 Subject: [PATCH 09/18] test(integ): record the dynamodb-globaltable run in the integ ledger --- docs/_generated/integ-last-run.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index 355d48db0..808e31ec1 100644 --- a/docs/_generated/integ-last-run.tsv +++ b/docs/_generated/integ-last-run.tsv @@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee 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 dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean -dynamodb-globaltable 2026-08-09T12:22:09Z PASS 175 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean +dynamodb-globaltable 2026-08-09T12:33:52Z PASS 173 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans From 4795a7899e5208d58e62601b848b3ce35ac1c58b Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:43:15 +0900 Subject: [PATCH 10/18] fix(dynamodb): guard the pre-translation GSI scan, correct the UpdateTable comments, coerce explicit throughput numbers Addresses the remaining findings from the 3-axis review carried out by a separate session. The existing-index scan in the billing flip runs before the translation, so a non-array GlobalSecondaryIndexes hit .map on a plain object and died with a bare TypeError instead of the named error the translator raises a few lines later. It is now Array.isArray-guarded like every other new site. Two comments claimed UpdateTable accepts only one of BillingMode, ReplicaUpdates and GlobalSecondaryIndexUpdates per call. That contradicted the flip path, which deliberately sends BillingMode and GlobalSecondaryIndexUpdates together because AWS requires per-GSI capacity in the same call. Both now state the real constraint and its documented exception. An explicitly supplied, already-SDK-shaped throughput block was cast straight through, the one path skipping toFiniteNumber, so a stringly-typed CFn value would reach the SDK unnormalized while every derived value was coerced. The three such sites now go through a shared helper that also drops junk keys. --- .../dynamodb-globaltable-provider.ts | 70 +++++++++++++++---- ...lobaltable-provider-gsi-throughput.test.ts | 29 ++++++++ 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index d55df7f56..9f4d8e211 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -80,10 +80,15 @@ import type { * - `update()` covers every mutable surface — Tags, DeletionProtection, * TableClass, SSE, StreamSpec, OnDemand throughput, BillingMode flip, * Replica add / remove / modify, GSI add / remove / modify, TTL toggle. - * - The serialization is load-bearing: AWS's `UpdateTable` accepts only - * ONE of `{BillingMode, ReplicaUpdates, GlobalSecondaryIndexUpdates}` - * per call, so each category is its own SDK round-trip with a wait-for - * -ACTIVE in between. Immutable property changes (TableName, KeySchema, + * - The serialization is load-bearing: AWS's `UpdateTable` does not accept + * `ReplicaUpdates` alongside `BillingMode` / `GlobalSecondaryIndexUpdates`, + * so each category is its own SDK round-trip with a wait-for-ACTIVE in + * between. The ONE documented exception is the `PAY_PER_REQUEST -> + * PROVISIONED` flip, where AWS REQUIRES per-GSI `ProvisionedThroughput` in + * the SAME call as `BillingMode` ("you must specify read and write capacity + * unit values for the table and for each global secondary index") — so that + * path deliberately sends both together (Issue #1387). Immutable property + * changes (TableName, KeySchema, * AttributeDefinitions removal, LocalSecondaryIndexes) throw * `ProvisioningError` naming the offending field — the deploy engine's * diff classification should catch these as REPLACEMENT before ever @@ -646,10 +651,13 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { /** * Update a DynamoDB Global Table in place. * - * AWS-side state-machine constraint: `UpdateTable` accepts only ONE of - * `{BillingMode, ReplicaUpdates, GlobalSecondaryIndexUpdates}` per call, + * AWS-side state-machine constraint: `UpdateTable` does not accept + * `ReplicaUpdates` alongside `BillingMode` / `GlobalSecondaryIndexUpdates`, * so each category must serialize into its own SDK round-trip with a - * `waitForTableActiveAfterUpdate` between every step. Order: + * `waitForTableActiveAfterUpdate` between every step. The ONE exception is + * step 4's `PAY_PER_REQUEST -> PROVISIONED` flip, which AWS requires to + * carry per-GSI `ProvisionedThroughput` in the same call (Issue #1387). + * Order: * 1. Wait for current ACTIVE (defensive). * 2. Tags diff (TagResource / UntagResource — no wait needed). * 3. Non-conflicting flat fields (DeletionProtectionEnabled / TableClass @@ -917,8 +925,13 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { // Only indexes that ALREADY exist on AWS can take an `Update` // action; a GSI introduced by this same deploy is created (with // its throughput) by step 6's `added` loop. + // `Array.isArray` rather than a bare cast: this scan runs BEFORE the + // translation below, so a non-array value (an unresolved intrinsic) + // would hit `.map` on a plain object and die with a bare TypeError + // instead of the named error the translator raises a few lines down. + const previousCfnIndexes = previousProperties['GlobalSecondaryIndexes']; const existingIndexNames = new Set( - ((previousProperties['GlobalSecondaryIndexes'] ?? []) as unknown[]) + (Array.isArray(previousCfnIndexes) ? (previousCfnIndexes as unknown[]) : []) .map((entry) => (entry as Record | null)?.['IndexName']) .filter((name): name is string => typeof name === 'string') ); @@ -2773,6 +2786,29 @@ function asRecord(value: unknown): Record | undefined { } /** Coerce a CFn numeric (CFn is stringly-typed) to a finite number. */ +/** + * Normalize an explicitly-supplied, already-SDK-shaped throughput block. + * + * The CFn schema forbids these members, but cdkd state written before Issue + * #1387 and hand-authored templates can carry them. Forwarding the raw record + * would be the ONE path that skips {@link toFiniteNumber}, so a stringly-typed + * CFn `"5"` would reach the SDK unnormalized while every DERIVED value is + * coerced — an inconsistency that only shows up on the rarest input. Picking + * the known members explicitly also drops junk keys the SDK serializer would + * discard silently anyway. + */ +function coerceThroughputNumbers( + block: Record, + members: readonly K[] +): Record { + const out = {} as Record; + for (const member of members) { + const n = toFiniteNumber(block[member]); + if (n !== undefined) out[member] = n; + } + return out; +} + function toFiniteNumber(value: unknown): number | undefined { if (value === undefined || value === null || value === '') return undefined; const n = Number(value); @@ -2930,7 +2966,10 @@ export function toSdkGlobalSecondaryIndexes( const explicitProvisioned = asRecord(gsi['ProvisionedThroughput']); const explicitOnDemand = asRecord(gsi['OnDemandThroughput']); if (explicitProvisioned) { - sdk.ProvisionedThroughput = explicitProvisioned as unknown as ProvisionedThroughput; + sdk.ProvisionedThroughput = coerceThroughputNumbers(explicitProvisioned, [ + 'ReadCapacityUnits', + 'WriteCapacityUnits', + ]) as ProvisionedThroughput; } else if (billingMode === 'PROVISIONED') { sdk.ProvisionedThroughput = { ReadCapacityUnits: @@ -2944,7 +2983,10 @@ export function toSdkGlobalSecondaryIndexes( } if (explicitOnDemand) { - sdk.OnDemandThroughput = explicitOnDemand as unknown as OnDemandThroughput; + sdk.OnDemandThroughput = coerceThroughputNumbers(explicitOnDemand, [ + 'MaxReadRequestUnits', + 'MaxWriteRequestUnits', + ]) as OnDemandThroughput; } else if (billingMode !== 'PROVISIONED') { const maxWrite = toFiniteNumber( asRecord(gsi['WriteOnDemandThroughputSettings'])?.['MaxWriteRequestUnits'] @@ -3006,12 +3048,16 @@ export function toSdkReplicaGlobalSecondaryIndexes( asRecord(cfn['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] ); if (explicitProvisioned) { - sdk.ProvisionedThroughputOverride = explicitProvisioned; + sdk.ProvisionedThroughputOverride = coerceThroughputNumbers(explicitProvisioned, [ + 'ReadCapacityUnits', + ]); } else if (readCapacity !== undefined) { sdk.ProvisionedThroughputOverride = { ReadCapacityUnits: readCapacity }; } if (explicitOnDemand) { - sdk.OnDemandThroughputOverride = explicitOnDemand; + sdk.OnDemandThroughputOverride = coerceThroughputNumbers(explicitOnDemand, [ + 'MaxReadRequestUnits', + ]); } else if (maxReadRequestUnits !== undefined) { sdk.OnDemandThroughputOverride = { MaxReadRequestUnits: maxReadRequestUnits }; } diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts index edff5a662..8b651fa49 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -408,6 +408,35 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { ).rejects.toThrow(/GlobalSecondaryIndexes must be an array/); }); + it('coerces a stringly-typed explicit ProvisionedThroughput instead of forwarding it raw', () => { + // CFn is stringly typed, and an explicitly-supplied already-SDK-shaped + // block is the one path that would otherwise skip toFiniteNumber, so a + // "5" would reach the SDK unnormalized while every derived value is a + // number. Junk keys are dropped too. + const [gsi] = toSdkGlobalSecondaryIndexes( + { + GlobalSecondaryIndexes: [ + { + IndexName: 'explicit', + KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + ProvisionedThroughput: { + ReadCapacityUnits: '5', + WriteCapacityUnits: '11', + NotAnSdkMember: 'junk', + }, + }, + ], + }, + 'us-east-1', + 'PROVISIONED' + ); + expect(gsi!.ProvisionedThroughput).toEqual({ + ReadCapacityUnits: 5, + WriteCapacityUnits: 11, + }); + }); + it('throws on a non-array GlobalSecondaryIndexes instead of deploying a table with none', () => { // Absent is legitimately empty; present-but-not-an-array (an unresolved // intrinsic) previously collapsed to [] and created the table with ZERO From db3bc07d06f297d691032575a5cebddd103a5b65 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:45:06 +0900 Subject: [PATCH 11/18] test(dynamodb): bind the non-array guard on the pre-translation GSI scan The guard shipped in the previous commit without a test. Removing it makes the failure surface as 'previousCfnIndexes.map is not a function', an opaque error naming nothing, instead of the translator's named message. --- ...odb-globaltable-provider-gsi-throughput.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts index 8b651fa49..997c299c6 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -809,6 +809,20 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { ]); }); + it('reports the named error, not a bare TypeError, for a non-array previous GlobalSecondaryIndexes during a flip', async () => { + // The existing-index scan in the flip runs BEFORE the translation, so an + // unguarded `.map` on a plain object died with `.map is not a function` + // — an opaque failure that names nothing. The scan is guarded, so the + // translator's own named error is what surfaces. + const previous = structuredClone(PROVISIONED_TABLE_PROPS) as Record; + previous['BillingMode'] = 'PAY_PER_REQUEST'; + previous['GlobalSecondaryIndexes'] = { 'Fn::If': ['UseGsi', [], []] }; + + await expect( + provider.update('Prov', 'prov-table', RESOURCE_TYPE, PROVISIONED_TABLE_PROPS, previous) + ).rejects.toThrow(/GlobalSecondaryIndexes must be an array/); + }); + it('carries a changed WarmThroughput in the PAY_PER_REQUEST -> PROVISIONED flip call', async () => { // Indexes handled by the flip are skipped by the `modified` loop, so a // simultaneous WarmThroughput change has to ride the flip's own Update From ad56c6226329d580d8ce8eca411c846f361546bc Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:58:01 +0900 Subject: [PATCH 12/18] fix(dynamodb): do not send an empty coerced throughput block, which AWS reads as inherit The numeric coercion added in the previous commit returned an empty object when no member parsed, and the call sites assigned it. AWS documents an empty ProvisionedThroughputOverride / OnDemandThroughputOverride as inherit the source table's settings, so an unparseable explicit value silently changed replica behavior instead of failing loudly. On the GSI side an empty block also suppressed the derived fallback, letting a garbage explicit value beat a valid derived one. The helper now returns undefined when nothing survives coercion, and every call site treats that as no explicit block and falls through. Found by review; both directions are bound by tests. Also moves the toFiniteNumber doc comment back onto toFiniteNumber, drops two redundant casts, and rewords the UpdateTable comments so the flip reads as the one case where BillingMode and GlobalSecondaryIndexUpdates combine rather than as an exception to the ReplicaUpdates rule. --- .../dynamodb-globaltable-provider.ts | 86 ++++++++++++------- ...lobaltable-provider-gsi-throughput.test.ts | 34 ++++++++ 2 files changed, 88 insertions(+), 32 deletions(-) diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index 9f4d8e211..8576b3837 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -83,12 +83,12 @@ import type { * - The serialization is load-bearing: AWS's `UpdateTable` does not accept * `ReplicaUpdates` alongside `BillingMode` / `GlobalSecondaryIndexUpdates`, * so each category is its own SDK round-trip with a wait-for-ACTIVE in - * between. The ONE documented exception is the `PAY_PER_REQUEST -> - * PROVISIONED` flip, where AWS REQUIRES per-GSI `ProvisionedThroughput` in - * the SAME call as `BillingMode` ("you must specify read and write capacity - * unit values for the table and for each global secondary index") — so that - * path deliberately sends both together (Issue #1387). Immutable property - * changes (TableName, KeySchema, + * between. `BillingMode` and `GlobalSecondaryIndexUpdates` DO combine, and + * only in the `PAY_PER_REQUEST -> PROVISIONED` flip, where AWS REQUIRES + * per-GSI `ProvisionedThroughput` in the SAME call as `BillingMode` ("you + * must specify read and write capacity unit values for the table and for + * each global secondary index") — so that path deliberately sends both + * together (Issue #1387). Immutable property changes (TableName, KeySchema, * AttributeDefinitions removal, LocalSecondaryIndexes) throw * `ProvisioningError` naming the offending field — the deploy engine's * diff classification should catch these as REPLACEMENT before ever @@ -654,9 +654,10 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { * AWS-side state-machine constraint: `UpdateTable` does not accept * `ReplicaUpdates` alongside `BillingMode` / `GlobalSecondaryIndexUpdates`, * so each category must serialize into its own SDK round-trip with a - * `waitForTableActiveAfterUpdate` between every step. The ONE exception is - * step 4's `PAY_PER_REQUEST -> PROVISIONED` flip, which AWS requires to - * carry per-GSI `ProvisionedThroughput` in the same call (Issue #1387). + * `waitForTableActiveAfterUpdate` between every step. `BillingMode` and + * `GlobalSecondaryIndexUpdates` DO combine, and only in step 4's + * `PAY_PER_REQUEST -> PROVISIONED` flip, which AWS requires to carry + * per-GSI `ProvisionedThroughput` in the same call (Issue #1387). * Order: * 1. Wait for current ACTIVE (defensive). * 2. Tags diff (TagResource / UntagResource — no wait needed). @@ -2785,7 +2786,6 @@ function asRecord(value: unknown): Record | undefined { : undefined; } -/** Coerce a CFn numeric (CFn is stringly-typed) to a finite number. */ /** * Normalize an explicitly-supplied, already-SDK-shaped throughput block. * @@ -2796,19 +2796,32 @@ function asRecord(value: unknown): Record | undefined { * coerced — an inconsistency that only shows up on the rarest input. Picking * the known members explicitly also drops junk keys the SDK serializer would * discard silently anyway. + * + * Returns `undefined` — NOT an empty object — when no member survives + * coercion. That distinction is load-bearing on the replica overrides: AWS + * documents an EMPTY `{Provisioned,OnDemand}ThroughputOverride` as "inherit + * the source table's settings", so assigning `{}` for an unparseable value + * would turn a loud serialization failure into a silent inherit, and on the + * GSI side would additionally suppress the derived fallback. Callers treat + * `undefined` as "no explicit block" and fall through. */ function coerceThroughputNumbers( block: Record, members: readonly K[] -): Record { - const out = {} as Record; +): Record | undefined { + const out = {} as Record; + let any = false; for (const member of members) { const n = toFiniteNumber(block[member]); - if (n !== undefined) out[member] = n; + if (n !== undefined) { + out[member] = n; + any = true; + } } - return out; + return any ? out : undefined; } +/** Coerce a CFn numeric (CFn is stringly-typed) to a finite number. */ function toFiniteNumber(value: unknown): number | undefined { if (value === undefined || value === null || value === '') return undefined; const n = Number(value); @@ -2963,13 +2976,20 @@ export function toSdkGlobalSecondaryIndexes( sdk.WarmThroughput = gsi['WarmThroughput'] as GlobalSecondaryIndex['WarmThroughput']; } - const explicitProvisioned = asRecord(gsi['ProvisionedThroughput']); - const explicitOnDemand = asRecord(gsi['OnDemandThroughput']); + const explicitProvisioned = asRecord(gsi['ProvisionedThroughput']) + ? coerceThroughputNumbers(asRecord(gsi['ProvisionedThroughput'])!, [ + 'ReadCapacityUnits', + 'WriteCapacityUnits', + ]) + : undefined; + const explicitOnDemand = asRecord(gsi['OnDemandThroughput']) + ? coerceThroughputNumbers(asRecord(gsi['OnDemandThroughput'])!, [ + 'MaxReadRequestUnits', + 'MaxWriteRequestUnits', + ]) + : undefined; if (explicitProvisioned) { - sdk.ProvisionedThroughput = coerceThroughputNumbers(explicitProvisioned, [ - 'ReadCapacityUnits', - 'WriteCapacityUnits', - ]) as ProvisionedThroughput; + sdk.ProvisionedThroughput = explicitProvisioned; } else if (billingMode === 'PROVISIONED') { sdk.ProvisionedThroughput = { ReadCapacityUnits: @@ -2983,10 +3003,7 @@ export function toSdkGlobalSecondaryIndexes( } if (explicitOnDemand) { - sdk.OnDemandThroughput = coerceThroughputNumbers(explicitOnDemand, [ - 'MaxReadRequestUnits', - 'MaxWriteRequestUnits', - ]) as OnDemandThroughput; + sdk.OnDemandThroughput = explicitOnDemand; } else if (billingMode !== 'PROVISIONED') { const maxWrite = toFiniteNumber( asRecord(gsi['WriteOnDemandThroughputSettings'])?.['MaxWriteRequestUnits'] @@ -3039,8 +3056,17 @@ export function toSdkReplicaGlobalSecondaryIndexes( const sdk: ReplicaGlobalSecondaryIndex = { IndexName: cfn['IndexName'] as string | undefined, }; - const explicitProvisioned = asRecord(cfn['ProvisionedThroughputOverride']); - const explicitOnDemand = asRecord(cfn['OnDemandThroughputOverride']); + // Coerced to `undefined` when nothing parses: AWS reads an EMPTY override + // as "inherit the source table's settings", so assigning `{}` here would + // silently change replica behavior instead of failing loudly. + const explicitProvisionedRaw = asRecord(cfn['ProvisionedThroughputOverride']); + const explicitOnDemandRaw = asRecord(cfn['OnDemandThroughputOverride']); + const explicitProvisioned = explicitProvisionedRaw + ? coerceThroughputNumbers(explicitProvisionedRaw, ['ReadCapacityUnits']) + : undefined; + const explicitOnDemand = explicitOnDemandRaw + ? coerceThroughputNumbers(explicitOnDemandRaw, ['MaxReadRequestUnits']) + : undefined; const readCapacity = deriveReadCapacityUnits( asRecord(cfn['ReadProvisionedThroughputSettings']) ); @@ -3048,16 +3074,12 @@ export function toSdkReplicaGlobalSecondaryIndexes( asRecord(cfn['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] ); if (explicitProvisioned) { - sdk.ProvisionedThroughputOverride = coerceThroughputNumbers(explicitProvisioned, [ - 'ReadCapacityUnits', - ]); + sdk.ProvisionedThroughputOverride = explicitProvisioned; } else if (readCapacity !== undefined) { sdk.ProvisionedThroughputOverride = { ReadCapacityUnits: readCapacity }; } if (explicitOnDemand) { - sdk.OnDemandThroughputOverride = coerceThroughputNumbers(explicitOnDemand, [ - 'MaxReadRequestUnits', - ]); + sdk.OnDemandThroughputOverride = explicitOnDemand; } else if (maxReadRequestUnits !== undefined) { sdk.OnDemandThroughputOverride = { MaxReadRequestUnits: maxReadRequestUnits }; } diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts index 997c299c6..868ed1259 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -437,6 +437,40 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { }); }); + it('falls through to the derived value when an explicit block coerces to nothing', () => { + // An explicit block whose every member is unparseable must NOT become an + // empty object: AWS reads an empty throughput block as "inherit", which + // would turn a loud failure into a silent wrong value, and on the GSI + // side it would also beat a perfectly valid derived setting. + const [gsi] = toSdkGlobalSecondaryIndexes( + { + GlobalSecondaryIndexes: [ + { + IndexName: 'garbage', + KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + OnDemandThroughput: { MaxReadRequestUnits: 'abc' }, + ReadOnDemandThroughputSettings: { MaxReadRequestUnits: 41 }, + WriteOnDemandThroughputSettings: { MaxWriteRequestUnits: 42 }, + }, + ], + }, + 'us-east-1', + 'PAY_PER_REQUEST' + ); + expect(gsi!.OnDemandThroughput).toEqual({ + MaxReadRequestUnits: 41, + MaxWriteRequestUnits: 42, + }); + }); + + it('leaves a replica override unset when it coerces to nothing, rather than sending an inherit-me empty block', () => { + const [replica] = toSdkReplicaGlobalSecondaryIndexes([ + { IndexName: 'r', OnDemandThroughputOverride: { MaxReadRequestUnits: 'abc' } }, + ])!; + expect(replica!.OnDemandThroughputOverride).toBe(undefined); + }); + it('throws on a non-array GlobalSecondaryIndexes instead of deploying a table with none', () => { // Absent is legitimately empty; present-but-not-an-array (an unresolved // intrinsic) previously collapsed to [] and created the table with ZERO From 46988680b96d0b60644da4221262622ca2c88c84 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:03:56 +0900 Subject: [PATCH 13/18] fix(dynamodb): drop the two type imports left unused by the cast removal Dropping the redundant as-casts left ProvisionedThroughput and OnDemandThroughput imported but unreferenced. Caught by CI because the previous round ran vp check --fix but not vp run check, and only the latter is CI parity. --- src/provisioning/providers/dynamodb-globaltable-provider.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index 8576b3837..1535fd58f 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -19,8 +19,6 @@ import { type GlobalSecondaryIndex, type GlobalSecondaryIndexUpdate, type LocalSecondaryIndex, - type OnDemandThroughput, - type ProvisionedThroughput, type ReplicaGlobalSecondaryIndex, type StreamSpecification, type Tag, From a9e26df0c2df62175666b73b91c98266724916b9 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:08:52 +0900 Subject: [PATCH 14/18] test(integ): record the dynamodb-globaltable run in the integ ledger --- docs/_generated/integ-last-run.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index 808e31ec1..3fc1acea6 100644 --- a/docs/_generated/integ-last-run.tsv +++ b/docs/_generated/integ-last-run.tsv @@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee 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 dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean -dynamodb-globaltable 2026-08-09T12:33:52Z PASS 173 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean +dynamodb-globaltable 2026-08-09T13:08:16Z PASS 226 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans From 94febf467f3036a6d59d40567bf7d358efc2cdd7 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:17:32 +0900 Subject: [PATCH 15/18] fix(dynamodb): merge explicit throughput per member and throw on an unparseable one The previous round's all-or-nothing coercion was wrong in both directions, found by review. A partial explicit block replaced the derived block wholesale, so an explicit MaxReadRequestUnits silently suppressed a valid derived MaxWriteRequestUnits. Explicit members now merge over derived ones per member instead. A fully unparseable explicit block fell through to the derived branch, which defaults to 5 read and write units, so a table the template explicitly sized was quietly deployed at the default. A present member that will not coerce now throws and names the field, matching what this provider already does for a non-array GlobalSecondaryIndexes. Absent members still fall through, which is the only case that should. Both directions are mutation-proofed. The first probe for the merge case did not actually apply, so it was re-run against a verified mutation. --- .../dynamodb-globaltable-provider.ts | 139 +++++++++++------- ...lobaltable-provider-gsi-throughput.test.ts | 48 ++++-- 2 files changed, 120 insertions(+), 67 deletions(-) diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index 1535fd58f..cc11088ef 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -914,8 +914,9 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { // AWS requires per-GSI `ProvisionedThroughput` in the SAME // UpdateTable call that flips PAY_PER_REQUEST -> PROVISIONED // (Issue #1387): the throughput-mode change applies to the table - // AND every index atomically. `GlobalSecondaryIndexUpdates` is the - // one companion field UpdateTable accepts alongside `BillingMode`. + // AND every index atomically, so the table-level + // `ProvisionedThroughput` set above and these per-index updates ride + // the same call. const provisionedIndexes = toSdkGlobalSecondaryIndexes( properties, currentRegion, @@ -2795,28 +2796,38 @@ function asRecord(value: unknown): Record | undefined { * the known members explicitly also drops junk keys the SDK serializer would * discard silently anyway. * - * Returns `undefined` — NOT an empty object — when no member survives - * coercion. That distinction is load-bearing on the replica overrides: AWS - * documents an EMPTY `{Provisioned,OnDemand}ThroughputOverride` as "inherit - * the source table's settings", so assigning `{}` for an unparseable value - * would turn a loud serialization failure into a silent inherit, and on the - * GSI side would additionally suppress the derived fallback. Callers treat - * `undefined` as "no explicit block" and fall through. + * THROWS on a member that is PRESENT but will not coerce (an unresolved + * intrinsic, a malformed value). Silently omitting it would let the caller's + * `??` chain fall through to a derived value or to + * {@link DEFAULT_CAPACITY_UNITS}, i.e. quietly deploy 5/5 for a table whose + * template explicitly asked to be sized — the same silent-substitution class + * this provider throws on for a non-array `GlobalSecondaryIndexes`. + * + * Absent members are simply omitted, so the result is a PARTIAL block the + * caller merges per-member over the derived values. Returning a whole-block + * winner instead would let a half-filled explicit block suppress a valid + * derived sibling. */ function coerceThroughputNumbers( block: Record, - members: readonly K[] -): Record | undefined { - const out = {} as Record; - let any = false; + members: readonly K[], + blockName: string +): Partial> { + const out: Partial> = {}; for (const member of members) { - const n = toFiniteNumber(block[member]); - if (n !== undefined) { - out[member] = n; - any = true; + const raw = block[member]; + if (raw === undefined || raw === null || raw === '') continue; + const n = toFiniteNumber(raw); + if (n === undefined) { + throw new Error( + `AWS::DynamoDB::GlobalTable ${blockName}.${member} must be a number, got ` + + `${JSON.stringify(raw)?.slice(0, 120)}. Leaving it out would silently ` + + `substitute a derived or default capacity for an explicitly sized table.` + ); } + out[member] = n; } - return any ? out : undefined; + return out; } /** Coerce a CFn numeric (CFn is stringly-typed) to a finite number. */ @@ -2974,39 +2985,50 @@ export function toSdkGlobalSecondaryIndexes( sdk.WarmThroughput = gsi['WarmThroughput'] as GlobalSecondaryIndex['WarmThroughput']; } - const explicitProvisioned = asRecord(gsi['ProvisionedThroughput']) - ? coerceThroughputNumbers(asRecord(gsi['ProvisionedThroughput'])!, [ - 'ReadCapacityUnits', - 'WriteCapacityUnits', - ]) - : undefined; - const explicitOnDemand = asRecord(gsi['OnDemandThroughput']) - ? coerceThroughputNumbers(asRecord(gsi['OnDemandThroughput'])!, [ - 'MaxReadRequestUnits', - 'MaxWriteRequestUnits', - ]) - : undefined; - if (explicitProvisioned) { - sdk.ProvisionedThroughput = explicitProvisioned; - } else if (billingMode === 'PROVISIONED') { + // Explicit already-SDK-shaped blocks merge over the derived ones PER + // MEMBER. Whole-block replacement was wrong in both directions: a partial + // explicit block suppressed a perfectly valid derived sibling (silent + // loss), and a fully-unparseable one fell through to the derived defaults, + // silently deploying 5/5 for a table the template asked to size. + // `coerceThroughputNumbers` throws on a PRESENT member that will not + // coerce, so an unresolved intrinsic is loud rather than defaulted. + const explicitProvisionedRaw = asRecord(gsi['ProvisionedThroughput']); + const explicitOnDemandRaw = asRecord(gsi['OnDemandThroughput']); + const explicitProvisioned = explicitProvisionedRaw + ? coerceThroughputNumbers( + explicitProvisionedRaw, + ['ReadCapacityUnits', 'WriteCapacityUnits'], + 'GlobalSecondaryIndexes[].ProvisionedThroughput' + ) + : {}; + const explicitOnDemand = explicitOnDemandRaw + ? coerceThroughputNumbers( + explicitOnDemandRaw, + ['MaxReadRequestUnits', 'MaxWriteRequestUnits'], + 'GlobalSecondaryIndexes[].OnDemandThroughput' + ) + : {}; + + if (billingMode === 'PROVISIONED' || Object.keys(explicitProvisioned).length > 0) { sdk.ProvisionedThroughput = { ReadCapacityUnits: + explicitProvisioned.ReadCapacityUnits ?? deriveReadCapacityUnits(asRecord(localEntry?.['ReadProvisionedThroughputSettings'])) ?? deriveReadCapacityUnits(asRecord(gsi['ReadProvisionedThroughputSettings'])) ?? DEFAULT_CAPACITY_UNITS, WriteCapacityUnits: + explicitProvisioned.WriteCapacityUnits ?? deriveWriteCapacityUnits(asRecord(gsi['WriteProvisionedThroughputSettings'])) ?? DEFAULT_CAPACITY_UNITS, }; } - if (explicitOnDemand) { - sdk.OnDemandThroughput = explicitOnDemand; - } else if (billingMode !== 'PROVISIONED') { - const maxWrite = toFiniteNumber( - asRecord(gsi['WriteOnDemandThroughputSettings'])?.['MaxWriteRequestUnits'] - ); + if (billingMode !== 'PROVISIONED' || Object.keys(explicitOnDemand).length > 0) { + const maxWrite = + explicitOnDemand.MaxWriteRequestUnits ?? + toFiniteNumber(asRecord(gsi['WriteOnDemandThroughputSettings'])?.['MaxWriteRequestUnits']); const maxRead = + explicitOnDemand.MaxReadRequestUnits ?? toFiniteNumber( asRecord(localEntry?.['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] ) ?? @@ -3054,32 +3076,41 @@ export function toSdkReplicaGlobalSecondaryIndexes( const sdk: ReplicaGlobalSecondaryIndex = { IndexName: cfn['IndexName'] as string | undefined, }; - // Coerced to `undefined` when nothing parses: AWS reads an EMPTY override - // as "inherit the source table's settings", so assigning `{}` here would - // silently change replica behavior instead of failing loudly. + // An explicit override merges over the derived value per member (each of + // these blocks has exactly one member today, so that reduces to "explicit + // wins"). An unparseable member THROWS rather than being dropped: omitting + // the override entirely has the same "inherit the source table's settings" + // meaning as sending `{}`, so a dropped value would not fail loudly — it + // would just quietly ignore what the template asked for. const explicitProvisionedRaw = asRecord(cfn['ProvisionedThroughputOverride']); const explicitOnDemandRaw = asRecord(cfn['OnDemandThroughputOverride']); const explicitProvisioned = explicitProvisionedRaw - ? coerceThroughputNumbers(explicitProvisionedRaw, ['ReadCapacityUnits']) - : undefined; + ? coerceThroughputNumbers( + explicitProvisionedRaw, + ['ReadCapacityUnits'], + 'Replicas[].GlobalSecondaryIndexes[].ProvisionedThroughputOverride' + ) + : {}; const explicitOnDemand = explicitOnDemandRaw - ? coerceThroughputNumbers(explicitOnDemandRaw, ['MaxReadRequestUnits']) - : undefined; + ? coerceThroughputNumbers( + explicitOnDemandRaw, + ['MaxReadRequestUnits'], + 'Replicas[].GlobalSecondaryIndexes[].OnDemandThroughputOverride' + ) + : {}; const readCapacity = deriveReadCapacityUnits( asRecord(cfn['ReadProvisionedThroughputSettings']) ); const maxReadRequestUnits = toFiniteNumber( asRecord(cfn['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] ); - if (explicitProvisioned) { - sdk.ProvisionedThroughputOverride = explicitProvisioned; - } else if (readCapacity !== undefined) { - sdk.ProvisionedThroughputOverride = { ReadCapacityUnits: readCapacity }; + const mergedReadCapacity = explicitProvisioned.ReadCapacityUnits ?? readCapacity; + const mergedMaxRead = explicitOnDemand.MaxReadRequestUnits ?? maxReadRequestUnits; + if (mergedReadCapacity !== undefined) { + sdk.ProvisionedThroughputOverride = { ReadCapacityUnits: mergedReadCapacity }; } - if (explicitOnDemand) { - sdk.OnDemandThroughputOverride = explicitOnDemand; - } else if (maxReadRequestUnits !== undefined) { - sdk.OnDemandThroughputOverride = { MaxReadRequestUnits: maxReadRequestUnits }; + if (mergedMaxRead !== undefined) { + sdk.OnDemandThroughputOverride = { MaxReadRequestUnits: mergedMaxRead }; } return sdk; }); diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts index 868ed1259..a04052f00 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -437,20 +437,39 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { }); }); - it('falls through to the derived value when an explicit block coerces to nothing', () => { - // An explicit block whose every member is unparseable must NOT become an - // empty object: AWS reads an empty throughput block as "inherit", which - // would turn a loud failure into a silent wrong value, and on the GSI - // side it would also beat a perfectly valid derived setting. + it('throws on a present-but-unparseable explicit member instead of substituting a default', () => { + // Dropping it would let the `??` chain fall through to a derived value or + // to DEFAULT_CAPACITY_UNITS, i.e. quietly deploy 5/5 for a table the + // template explicitly sized. An unresolved intrinsic must be loud. + expect(() => + toSdkGlobalSecondaryIndexes( + { + GlobalSecondaryIndexes: [ + { + IndexName: 'garbage', + KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], + Projection: { ProjectionType: 'ALL' }, + ProvisionedThroughput: { ReadCapacityUnits: { Ref: 'Unresolved' } }, + }, + ], + }, + 'us-east-1', + 'PROVISIONED' + ) + ).toThrow(/ProvisionedThroughput\.ReadCapacityUnits must be a number/); + }); + + it('merges a PARTIAL explicit block over the derived values instead of suppressing them', () => { + // Whole-block replacement let a half-filled explicit block silently drop + // a perfectly valid derived sibling — the exact class this work closes. const [gsi] = toSdkGlobalSecondaryIndexes( { GlobalSecondaryIndexes: [ { - IndexName: 'garbage', + IndexName: 'partial', KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], Projection: { ProjectionType: 'ALL' }, - OnDemandThroughput: { MaxReadRequestUnits: 'abc' }, - ReadOnDemandThroughputSettings: { MaxReadRequestUnits: 41 }, + OnDemandThroughput: { MaxReadRequestUnits: 41 }, WriteOnDemandThroughputSettings: { MaxWriteRequestUnits: 42 }, }, ], @@ -464,11 +483,14 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { }); }); - it('leaves a replica override unset when it coerces to nothing, rather than sending an inherit-me empty block', () => { - const [replica] = toSdkReplicaGlobalSecondaryIndexes([ - { IndexName: 'r', OnDemandThroughputOverride: { MaxReadRequestUnits: 'abc' } }, - ])!; - expect(replica!.OnDemandThroughputOverride).toBe(undefined); + it('throws on an unparseable replica override rather than quietly ignoring it', () => { + // Omitting the override means "inherit the source table's settings" — + // identical to sending `{}` — so a dropped value would NOT fail loudly. + expect(() => + toSdkReplicaGlobalSecondaryIndexes([ + { IndexName: 'r', OnDemandThroughputOverride: { MaxReadRequestUnits: 'abc' } }, + ]) + ).toThrow(/OnDemandThroughputOverride\.MaxReadRequestUnits must be a number/); }); it('throws on a non-array GlobalSecondaryIndexes instead of deploying a table with none', () => { From cf4afefa469c85130dad6fe1672a0d0b0ee726f0 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:23:10 +0900 Subject: [PATCH 16/18] test(integ): record the dynamodb-globaltable run in the integ ledger --- docs/_generated/integ-last-run.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index 3fc1acea6..b4fe23fc0 100644 --- a/docs/_generated/integ-last-run.tsv +++ b/docs/_generated/integ-last-run.tsv @@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee 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 dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean -dynamodb-globaltable 2026-08-09T13:08:16Z PASS 226 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean +dynamodb-globaltable 2026-08-09T13:22:12Z PASS 222 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans From e6d942123e546fd00ec6ac583c0f3331deed5556 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:31:00 +0900 Subject: [PATCH 17/18] revert(dynamodb): drop the explicit-throughput coercion and record it as issue #1428 The coercion started as a nit (a stringly-typed CFn value skipping toFiniteNumber on the one explicit-block path) and produced a new defect in each of three designs, every one of which passed my own review and was caught independently. v1 assigned an empty coerced block, which AWS reads as inherit the source table's settings, turning a loud failure into a silent one. v2 returned undefined and fell through to the derived branch, silently deploying the default 5 read and write units for a table the template explicitly sized. v3 threw on an uncoercible member, but the same helper also runs against previousProperties from state, so a garbage value already in state made every later update throw including the one that would remove it, and the replica path threw after CreateTable had already run. The three underlying defects are real but need one coherent fix (coerce, merge per member, gate on billing mode) with strictness applied only to the desired side and validation before any mutating call. That is issue #1428, which records all three failed designs so the next attempt does not rediscover them. Reverting restores the behavior this PR started from for that edge case, which the CFn schema forbids and CDK never emits. --- .../dynamodb-globaltable-provider.ts | 134 ++++-------------- ...lobaltable-provider-gsi-throughput.test.ts | 85 ----------- 2 files changed, 29 insertions(+), 190 deletions(-) diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index cc11088ef..36a18a574 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -19,6 +19,8 @@ import { type GlobalSecondaryIndex, type GlobalSecondaryIndexUpdate, type LocalSecondaryIndex, + type OnDemandThroughput, + type ProvisionedThroughput, type ReplicaGlobalSecondaryIndex, type StreamSpecification, type Tag, @@ -2785,51 +2787,6 @@ function asRecord(value: unknown): Record | undefined { : undefined; } -/** - * Normalize an explicitly-supplied, already-SDK-shaped throughput block. - * - * The CFn schema forbids these members, but cdkd state written before Issue - * #1387 and hand-authored templates can carry them. Forwarding the raw record - * would be the ONE path that skips {@link toFiniteNumber}, so a stringly-typed - * CFn `"5"` would reach the SDK unnormalized while every DERIVED value is - * coerced — an inconsistency that only shows up on the rarest input. Picking - * the known members explicitly also drops junk keys the SDK serializer would - * discard silently anyway. - * - * THROWS on a member that is PRESENT but will not coerce (an unresolved - * intrinsic, a malformed value). Silently omitting it would let the caller's - * `??` chain fall through to a derived value or to - * {@link DEFAULT_CAPACITY_UNITS}, i.e. quietly deploy 5/5 for a table whose - * template explicitly asked to be sized — the same silent-substitution class - * this provider throws on for a non-array `GlobalSecondaryIndexes`. - * - * Absent members are simply omitted, so the result is a PARTIAL block the - * caller merges per-member over the derived values. Returning a whole-block - * winner instead would let a half-filled explicit block suppress a valid - * derived sibling. - */ -function coerceThroughputNumbers( - block: Record, - members: readonly K[], - blockName: string -): Partial> { - const out: Partial> = {}; - for (const member of members) { - const raw = block[member]; - if (raw === undefined || raw === null || raw === '') continue; - const n = toFiniteNumber(raw); - if (n === undefined) { - throw new Error( - `AWS::DynamoDB::GlobalTable ${blockName}.${member} must be a number, got ` + - `${JSON.stringify(raw)?.slice(0, 120)}. Leaving it out would silently ` + - `substitute a derived or default capacity for an explicitly sized table.` - ); - } - out[member] = n; - } - return out; -} - /** Coerce a CFn numeric (CFn is stringly-typed) to a finite number. */ function toFiniteNumber(value: unknown): number | undefined { if (value === undefined || value === null || value === '') return undefined; @@ -2985,50 +2942,34 @@ export function toSdkGlobalSecondaryIndexes( sdk.WarmThroughput = gsi['WarmThroughput'] as GlobalSecondaryIndex['WarmThroughput']; } - // Explicit already-SDK-shaped blocks merge over the derived ones PER - // MEMBER. Whole-block replacement was wrong in both directions: a partial - // explicit block suppressed a perfectly valid derived sibling (silent - // loss), and a fully-unparseable one fell through to the derived defaults, - // silently deploying 5/5 for a table the template asked to size. - // `coerceThroughputNumbers` throws on a PRESENT member that will not - // coerce, so an unresolved intrinsic is loud rather than defaulted. - const explicitProvisionedRaw = asRecord(gsi['ProvisionedThroughput']); - const explicitOnDemandRaw = asRecord(gsi['OnDemandThroughput']); - const explicitProvisioned = explicitProvisionedRaw - ? coerceThroughputNumbers( - explicitProvisionedRaw, - ['ReadCapacityUnits', 'WriteCapacityUnits'], - 'GlobalSecondaryIndexes[].ProvisionedThroughput' - ) - : {}; - const explicitOnDemand = explicitOnDemandRaw - ? coerceThroughputNumbers( - explicitOnDemandRaw, - ['MaxReadRequestUnits', 'MaxWriteRequestUnits'], - 'GlobalSecondaryIndexes[].OnDemandThroughput' - ) - : {}; - - if (billingMode === 'PROVISIONED' || Object.keys(explicitProvisioned).length > 0) { + // An explicit already-SDK-shaped block is forwarded as-is. The CFn schema + // forbids these members, so this only fires for pre-#1387 cdkd state and + // hand-authored templates. Normalizing them (numeric coercion, per-member + // merge over the derived values, billing-mode gating) is deliberately NOT + // attempted here — see issue #1428, which records three failed designs. + const explicitProvisioned = asRecord(gsi['ProvisionedThroughput']); + const explicitOnDemand = asRecord(gsi['OnDemandThroughput']); + if (explicitProvisioned) { + sdk.ProvisionedThroughput = explicitProvisioned as unknown as ProvisionedThroughput; + } else if (billingMode === 'PROVISIONED') { sdk.ProvisionedThroughput = { ReadCapacityUnits: - explicitProvisioned.ReadCapacityUnits ?? deriveReadCapacityUnits(asRecord(localEntry?.['ReadProvisionedThroughputSettings'])) ?? deriveReadCapacityUnits(asRecord(gsi['ReadProvisionedThroughputSettings'])) ?? DEFAULT_CAPACITY_UNITS, WriteCapacityUnits: - explicitProvisioned.WriteCapacityUnits ?? deriveWriteCapacityUnits(asRecord(gsi['WriteProvisionedThroughputSettings'])) ?? DEFAULT_CAPACITY_UNITS, }; } - if (billingMode !== 'PROVISIONED' || Object.keys(explicitOnDemand).length > 0) { - const maxWrite = - explicitOnDemand.MaxWriteRequestUnits ?? - toFiniteNumber(asRecord(gsi['WriteOnDemandThroughputSettings'])?.['MaxWriteRequestUnits']); + if (explicitOnDemand) { + sdk.OnDemandThroughput = explicitOnDemand as unknown as OnDemandThroughput; + } else if (billingMode !== 'PROVISIONED') { + const maxWrite = toFiniteNumber( + asRecord(gsi['WriteOnDemandThroughputSettings'])?.['MaxWriteRequestUnits'] + ); const maxRead = - explicitOnDemand.MaxReadRequestUnits ?? toFiniteNumber( asRecord(localEntry?.['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] ) ?? @@ -3076,41 +3017,24 @@ export function toSdkReplicaGlobalSecondaryIndexes( const sdk: ReplicaGlobalSecondaryIndex = { IndexName: cfn['IndexName'] as string | undefined, }; - // An explicit override merges over the derived value per member (each of - // these blocks has exactly one member today, so that reduces to "explicit - // wins"). An unparseable member THROWS rather than being dropped: omitting - // the override entirely has the same "inherit the source table's settings" - // meaning as sending `{}`, so a dropped value would not fail loudly — it - // would just quietly ignore what the template asked for. - const explicitProvisionedRaw = asRecord(cfn['ProvisionedThroughputOverride']); - const explicitOnDemandRaw = asRecord(cfn['OnDemandThroughputOverride']); - const explicitProvisioned = explicitProvisionedRaw - ? coerceThroughputNumbers( - explicitProvisionedRaw, - ['ReadCapacityUnits'], - 'Replicas[].GlobalSecondaryIndexes[].ProvisionedThroughputOverride' - ) - : {}; - const explicitOnDemand = explicitOnDemandRaw - ? coerceThroughputNumbers( - explicitOnDemandRaw, - ['MaxReadRequestUnits'], - 'Replicas[].GlobalSecondaryIndexes[].OnDemandThroughputOverride' - ) - : {}; + // Forwarded as-is, same rationale as the GSI-level blocks above (#1428). + const explicitProvisioned = asRecord(cfn['ProvisionedThroughputOverride']); + const explicitOnDemand = asRecord(cfn['OnDemandThroughputOverride']); const readCapacity = deriveReadCapacityUnits( asRecord(cfn['ReadProvisionedThroughputSettings']) ); const maxReadRequestUnits = toFiniteNumber( asRecord(cfn['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] ); - const mergedReadCapacity = explicitProvisioned.ReadCapacityUnits ?? readCapacity; - const mergedMaxRead = explicitOnDemand.MaxReadRequestUnits ?? maxReadRequestUnits; - if (mergedReadCapacity !== undefined) { - sdk.ProvisionedThroughputOverride = { ReadCapacityUnits: mergedReadCapacity }; + if (explicitProvisioned) { + sdk.ProvisionedThroughputOverride = explicitProvisioned; + } else if (readCapacity !== undefined) { + sdk.ProvisionedThroughputOverride = { ReadCapacityUnits: readCapacity }; } - if (mergedMaxRead !== undefined) { - sdk.OnDemandThroughputOverride = { MaxReadRequestUnits: mergedMaxRead }; + if (explicitOnDemand) { + sdk.OnDemandThroughputOverride = explicitOnDemand; + } else if (maxReadRequestUnits !== undefined) { + sdk.OnDemandThroughputOverride = { MaxReadRequestUnits: maxReadRequestUnits }; } return sdk; }); diff --git a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts index a04052f00..9f3117e13 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -408,91 +408,6 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { ).rejects.toThrow(/GlobalSecondaryIndexes must be an array/); }); - it('coerces a stringly-typed explicit ProvisionedThroughput instead of forwarding it raw', () => { - // CFn is stringly typed, and an explicitly-supplied already-SDK-shaped - // block is the one path that would otherwise skip toFiniteNumber, so a - // "5" would reach the SDK unnormalized while every derived value is a - // number. Junk keys are dropped too. - const [gsi] = toSdkGlobalSecondaryIndexes( - { - GlobalSecondaryIndexes: [ - { - IndexName: 'explicit', - KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], - Projection: { ProjectionType: 'ALL' }, - ProvisionedThroughput: { - ReadCapacityUnits: '5', - WriteCapacityUnits: '11', - NotAnSdkMember: 'junk', - }, - }, - ], - }, - 'us-east-1', - 'PROVISIONED' - ); - expect(gsi!.ProvisionedThroughput).toEqual({ - ReadCapacityUnits: 5, - WriteCapacityUnits: 11, - }); - }); - - it('throws on a present-but-unparseable explicit member instead of substituting a default', () => { - // Dropping it would let the `??` chain fall through to a derived value or - // to DEFAULT_CAPACITY_UNITS, i.e. quietly deploy 5/5 for a table the - // template explicitly sized. An unresolved intrinsic must be loud. - expect(() => - toSdkGlobalSecondaryIndexes( - { - GlobalSecondaryIndexes: [ - { - IndexName: 'garbage', - KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], - Projection: { ProjectionType: 'ALL' }, - ProvisionedThroughput: { ReadCapacityUnits: { Ref: 'Unresolved' } }, - }, - ], - }, - 'us-east-1', - 'PROVISIONED' - ) - ).toThrow(/ProvisionedThroughput\.ReadCapacityUnits must be a number/); - }); - - it('merges a PARTIAL explicit block over the derived values instead of suppressing them', () => { - // Whole-block replacement let a half-filled explicit block silently drop - // a perfectly valid derived sibling — the exact class this work closes. - const [gsi] = toSdkGlobalSecondaryIndexes( - { - GlobalSecondaryIndexes: [ - { - IndexName: 'partial', - KeySchema: [{ AttributeName: 'g', KeyType: 'HASH' }], - Projection: { ProjectionType: 'ALL' }, - OnDemandThroughput: { MaxReadRequestUnits: 41 }, - WriteOnDemandThroughputSettings: { MaxWriteRequestUnits: 42 }, - }, - ], - }, - 'us-east-1', - 'PAY_PER_REQUEST' - ); - expect(gsi!.OnDemandThroughput).toEqual({ - MaxReadRequestUnits: 41, - MaxWriteRequestUnits: 42, - }); - }); - - it('throws on an unparseable replica override rather than quietly ignoring it', () => { - // Omitting the override means "inherit the source table's settings" — - // identical to sending `{}` — so a dropped value would NOT fail loudly. - expect(() => - toSdkReplicaGlobalSecondaryIndexes([ - { IndexName: 'r', OnDemandThroughputOverride: { MaxReadRequestUnits: 'abc' } }, - ]) - ).toThrow(/OnDemandThroughputOverride\.MaxReadRequestUnits must be a number/); - }); - it('throws on a non-array GlobalSecondaryIndexes instead of deploying a table with none', () => { // Absent is legitimately empty; present-but-not-an-array (an unresolved // intrinsic) previously collapsed to [] and created the table with ZERO From 8b50ec02f753e5dca988f14fa64272e62aaa94aa Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:35:44 +0900 Subject: [PATCH 18/18] test(integ): record the dynamodb-globaltable run in the integ ledger --- docs/_generated/integ-last-run.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index b4fe23fc0..3485c3f39 100644 --- a/docs/_generated/integ-last-run.tsv +++ b/docs/_generated/integ-last-run.tsv @@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee 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 dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean -dynamodb-globaltable 2026-08-09T13:22:12Z PASS 222 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean +dynamodb-globaltable 2026-08-09T13:35:05Z PASS 209 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans