From e8769c38055d3719e0c3e8307994666b7c411a61 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:14:51 +0900 Subject: [PATCH 1/2] fix(dynamodb): reset a removed per-GSI on-demand limit instead of silently no-oping Closes #1423 Removing maxReadRequestUnits / maxWriteRequestUnits from a GlobalTable GSI emitted nothing, so the old ceiling stayed live in AWS forever while cdkd reported success. CloudFormation resets it. This is the absent-field-reset silent-drop class (#1160) one level down inside a nested block (#1225). The blocking unknown was whether -1 is the reset sentinel for the per-GSI Update action: AWS documents it for the TABLE-level OnDemandThroughput only, and reset semantics are field-specific. Settled by a live probe rather than inferred - a real UpdateTable with {-1, -1} on GlobalSecondaryIndexUpdates[].Update was ACCEPTED and DescribeTable then reported the member ABSENT, i.e. genuinely cleared rather than stored as -1. The probe transcript is recorded on the issue. Only the members that were actually SET before are reset; a blanket {-1, -1} would clear a sibling limit the template still declares. Skipped on a billing flip, where absent on-demand fields are just the translation of a PROVISIONED side rather than a template removal. 3 unit tests (2 fail against origin/main; the third is the negative that must NOT emit a reset) plus a new drop-gsi-ondemand-limits integ phase asserting the live DescribeTable readback is ABSENT. Integ PASS, 3 deleted / 0 errors / 0 orphans. --- docs/_generated/integ-last-run.tsv | 2 +- .../dynamodb-globaltable-provider.ts | 43 +++++++- .../lib/dynamodb-globaltable-stack.ts | 17 +++- .../dynamodb-globaltable/verify.sh | 14 +++ ...lobaltable-provider-gsi-throughput.test.ts | 97 +++++++++++++++++++ 5 files changed, 167 insertions(+), 6 deletions(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index e184152aa..65394538b 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:35:05Z PASS 209 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean +dynamodb-globaltable 2026-08-09T14:14:39Z PASS 279 verify.sh issue #1423 per-GSI on-demand limit RESET asserted live (reads back absent); 3 del/0 err, 0 orphans 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 diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index 36a18a574..6b2907498 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -1298,7 +1298,38 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { // 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; + if (gsi.OnDemandThroughput) { + update.OnDemandThroughput = gsi.OnDemandThroughput; + } else if (!billingFlipped) { + // REMOVING a per-GSI on-demand limit from the template has to be sent + // as an explicit reset. Omitting the member leaves the old ceiling + // live in AWS forever while cdkd reports success — the + // absent-field-reset silent-drop class (#1160), one level down inside + // a nested block (#1225). CloudFormation resets it. + // + // `-1` is the reset sentinel, and that is LIVE-VERIFIED rather than + // inferred from the table-level field's docs (issue #1423): a real + // UpdateTable with `{-1, -1}` on the per-GSI Update action was + // ACCEPTED, and DescribeTable afterwards reported the member as + // ABSENT — i.e. genuinely cleared, not stored as -1. Reset semantics + // are field-specific, so this was probed, never assumed. + // + // Only the members that were actually SET before are reset: blanket + // `{-1, -1}` would clear a sibling limit the template still declares. + // Skipped on a billing flip, where "no on-demand fields" is just the + // translation of a PROVISIONED side, not a template removal. + const previousOnDemand = previousSdkByName.get(gsi.IndexName)?.OnDemandThroughput; + if (previousOnDemand) { + const reset: OnDemandThroughput = {}; + if (previousOnDemand.MaxReadRequestUnits !== undefined) { + reset.MaxReadRequestUnits = ON_DEMAND_LIMIT_RESET; + } + if (previousOnDemand.MaxWriteRequestUnits !== undefined) { + reset.MaxWriteRequestUnits = ON_DEMAND_LIMIT_RESET; + } + if (Object.keys(reset).length > 0) update.OnDemandThroughput = reset; + } + } // 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) { @@ -2787,6 +2818,16 @@ function asRecord(value: unknown): Record | undefined { : undefined; } +/** + * The sentinel DynamoDB accepts to CLEAR a per-GSI on-demand request-unit + * ceiling (issue #1423). Live-verified against real AWS: `UpdateTable` with + * `GlobalSecondaryIndexUpdates[].Update.OnDemandThroughput = {-1, -1}` is + * accepted, and the member reads back ABSENT from `DescribeTable` afterwards — + * so it is genuinely reset to unlimited rather than stored as -1. Any drift / + * read-back comparison must therefore expect ABSENCE, not this value. + */ +const ON_DEMAND_LIMIT_RESET = -1; + /** 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; diff --git a/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts b/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts index b5957a694..f16294ac1 100644 --- a/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts +++ b/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts @@ -202,10 +202,19 @@ export class DynamoDBGlobalTableStack extends cdk.Stack { { indexName: 'byOwner', partitionKey: { name: 'owner', type: ddb.AttributeType.STRING }, - // -> Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings - maxReadRequestUnits: 50, - // -> GlobalSecondaryIndexes[].WriteOnDemandThroughputSettings - maxWriteRequestUnits: 60, + // Issue #1423: REMOVING these from the template must reset the live + // ceiling, not silently no-op. Under `drop-gsi-ondemand-limits` both + // are omitted, and verify.sh asserts DescribeTable reports the + // member ABSENT afterwards (the reset reads back as absence, never + // as -1). + ...(updateMode.includes('drop-gsi-ondemand-limits') + ? {} + : { + // -> Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings + maxReadRequestUnits: 50, + // -> GlobalSecondaryIndexes[].WriteOnDemandThroughputSettings + maxWriteRequestUnits: 60, + }), }, ], removalPolicy: cdk.RemovalPolicy.DESTROY, diff --git a/tests/integration/dynamodb-globaltable/verify.sh b/tests/integration/dynamodb-globaltable/verify.sh index 36233c96d..e2aefdc60 100755 --- a/tests/integration/dynamodb-globaltable/verify.sh +++ b/tests/integration/dynamodb-globaltable/verify.sh @@ -407,6 +407,20 @@ echo "[verify] step 13: cdkd deploy with CDKD_TEST_UPDATE=ttl,tags (structural t # cleans up the table regardless of TTL state. CDKD_TEST_UPDATE=ttl,tags ${CLI} deploy "${STACK}" --state-bucket "${STATE_BUCKET}" --verbose +echo "[verify] step 13b: cdkd deploy with drop-gsi-ondemand-limits (issue #1423 — REMOVING a per-GSI on-demand limit must RESET it, not no-op)" +CDKD_TEST_UPDATE=ttl,tags,drop-gsi-ondemand-limits ${CLI} deploy "${STACK}" --state-bucket "${STATE_BUCKET}" --verbose + +# The reset reads back as ABSENCE, never as -1 (live-probed on #1423). Pre-fix +# the template removal emitted nothing at all, so the 50/60 ceiling stayed live +# in AWS forever while cdkd reported success. +OD_AFTER="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \ + --query "Table.GlobalSecondaryIndexes[?IndexName=='byOwner'].OnDemandThroughput | [0]" --output text)" +if [ "${OD_AFTER}" != "None" ]; then + echo "FAIL: issue #1423 — byOwner still carries OnDemandThroughput after the template removed it: ${OD_AFTER}" >&2 + exit 1 +fi +echo " per-GSI on-demand limits reset (OnDemandThroughput absent), issue #1423 closed" + echo "[verify] step 14a: assert DeletionProtectionEnabled flipped back to false on AWS" DP_FINAL="$(aws dynamodb describe-table --table-name "${TABLE_NAME}" --region "${REGION}" \ --query 'Table.DeletionProtectionEnabled' --output text)" 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 9f3117e13..28fdcaa06 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -516,6 +516,103 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { }); describe('update()', () => { + it('resets a REMOVED per-GSI on-demand limit with -1 instead of no-oping (issue #1423)', async () => { + // Deleting `maxReadRequestUnits` / `maxWriteRequestUnits` from a template + // used to emit nothing, so the old ceiling stayed live in AWS forever + // while cdkd reported success — the absent-field-reset silent-drop class + // (#1160). `-1` is the reset sentinel, LIVE-VERIFIED against real AWS: + // UpdateTable accepted it on the per-GSI Update action and DescribeTable + // then reported the member ABSENT. + const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record; + const next = structuredClone(ON_DEMAND_TABLE_PROPS) as Record; + // Drop BOTH limits: write lives on the GSI, read on the local replica. + delete (next['GlobalSecondaryIndexes'] as Record[])[0]![ + 'WriteOnDemandThroughputSettings' + ]; + delete ( + (next['Replicas'] as Record[])[0]!['GlobalSecondaryIndexes'] as Record< + string, + unknown + >[] + )[0]!['ReadOnDemandThroughputSettings']; + + await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous); + + const gsiUpdates = mockSend.mock.calls + .map((c) => c[0]) + .filter( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && + (c.input.GlobalSecondaryIndexUpdates ?? []).some((u) => u.Update !== undefined) + ); + expect(gsiUpdates).toHaveLength(1); + expect(gsiUpdates[0]!.input.GlobalSecondaryIndexUpdates).toEqual([ + { + Update: { + IndexName: 'gsi2', + OnDemandThroughput: { MaxReadRequestUnits: -1, MaxWriteRequestUnits: -1 }, + }, + }, + ]); + }); + + it('resets ONLY the member that was actually set before', async () => { + // A blanket {-1, -1} would clear a sibling limit the template still + // declares. + const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record; + // Previous side has WRITE only (drop the replica-side read limit). + delete ( + (previous['Replicas'] as Record[])[0]![ + 'GlobalSecondaryIndexes' + ] as Record[] + )[0]!['ReadOnDemandThroughputSettings']; + const next = structuredClone(previous) as Record; + delete (next['GlobalSecondaryIndexes'] as Record[])[0]![ + 'WriteOnDemandThroughputSettings' + ]; + + await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous); + + const gsiUpdates = mockSend.mock.calls + .map((c) => c[0]) + .filter( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && + (c.input.GlobalSecondaryIndexUpdates ?? []).some((u) => u.Update !== undefined) + ); + expect(gsiUpdates[0]!.input.GlobalSecondaryIndexUpdates?.[0]?.Update?.OnDemandThroughput).toEqual( + { MaxWriteRequestUnits: -1 } + ); + }); + + it('does NOT emit a reset when the index simply had no limits before', async () => { + const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record; + delete (previous['GlobalSecondaryIndexes'] as Record[])[0]![ + 'WriteOnDemandThroughputSettings' + ]; + delete ( + (previous['Replicas'] as Record[])[0]![ + 'GlobalSecondaryIndexes' + ] as Record[] + )[0]!['ReadOnDemandThroughputSettings']; + const next = structuredClone(previous) as Record; + // Unrelated edit so the table still goes through update(). + next['TableClass'] = 'STANDARD_INFREQUENT_ACCESS'; + + await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous); + + const resets = mockSend.mock.calls + .map((c) => c[0]) + .filter( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && + (c.input.GlobalSecondaryIndexUpdates ?? []).some( + (u) => u.Update?.OnDemandThroughput !== undefined + ) + ); + expect(resets).toHaveLength(0); + }); + it('carries ProvisionedThroughput on a GSI Create action (added index)', async () => { const previous = { ...PROVISIONED_TABLE_PROPS, GlobalSecondaryIndexes: [] }; From ae7dc59f84a1c774c17cb08712ba449a70d7632c Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:34:39 +0900 Subject: [PATCH 2/2] fix(dynamodb): merge the on-demand reset so a PARTIAL removal is not still dropped Review blocker on the first version: the reset lived in an else-if, so it only fired when the new side had NO OnDemandThroughput at all. The read limit comes from Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings and the write limit from GSI.WriteOnDemandThroughputSettings - two INDEPENDENT CDK props - so removing just one left the other live in AWS forever. That is the #1423 bug itself, in the likelier user edit, shipped as fixed. Now merges: start from the desired side and fill -1 for every member the template dropped. Live-probed the MIXED payload before adopting it, since only {-1,-1} had been verified: UpdateTable accepted { MaxReadRequestUnits: 50, MaxWriteRequestUnits: -1 } and DescribeTable read back { MaxReadRequestUnits: 50 } - dropped member cleared, kept member preserved. The integ fixture now drops ONLY the write limit (the harder case) and verify.sh asserts write is absent while read is still 50. It also asserts the index EXISTS first: a '| [0]' query against a missing index also answers None, so the absence check alone would pass if byOwner had vanished. The first integ run of that assertion FAILED because the rewritten queries lost the Table. prefix and length() received null - caught by the integ, fixed, re-run clean (182s, 3 deleted / 0 errors / 0 orphans). Table-level and cross-region-replica on-demand ceilings have the same class and are filed as #1434; each needs its own live probe and the table-level one touches a different code path. --- docs/_generated/integ-last-run.tsv | 2 +- .../dynamodb-globaltable-provider.ts | 68 +++++++++++-------- .../lib/dynamodb-globaltable-stack.ts | 22 +++--- .../dynamodb-globaltable/verify.sh | 28 ++++++-- ...lobaltable-provider-gsi-throughput.test.ts | 30 ++++++++ 5 files changed, 101 insertions(+), 49 deletions(-) diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index 65394538b..6448d3414 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-09T14:14:39Z PASS 279 verify.sh issue #1423 per-GSI on-demand limit RESET asserted live (reads back absent); 3 del/0 err, 0 orphans +dynamodb-globaltable 2026-08-09T14:33:56Z PASS 182 verify.sh #1423 PARTIAL removal asserted live (write reset absent, read kept 50); 3 del/0 err, 0 orphans 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 diff --git a/src/provisioning/providers/dynamodb-globaltable-provider.ts b/src/provisioning/providers/dynamodb-globaltable-provider.ts index 6b2907498..bbe339964 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -1298,38 +1298,46 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider { // 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; - } else if (!billingFlipped) { - // REMOVING a per-GSI on-demand limit from the template has to be sent - // as an explicit reset. Omitting the member leaves the old ceiling - // live in AWS forever while cdkd reports success — the - // absent-field-reset silent-drop class (#1160), one level down inside - // a nested block (#1225). CloudFormation resets it. - // - // `-1` is the reset sentinel, and that is LIVE-VERIFIED rather than - // inferred from the table-level field's docs (issue #1423): a real - // UpdateTable with `{-1, -1}` on the per-GSI Update action was - // ACCEPTED, and DescribeTable afterwards reported the member as - // ABSENT — i.e. genuinely cleared, not stored as -1. Reset semantics - // are field-specific, so this was probed, never assumed. - // - // Only the members that were actually SET before are reset: blanket - // `{-1, -1}` would clear a sibling limit the template still declares. - // Skipped on a billing flip, where "no on-demand fields" is just the - // translation of a PROVISIONED side, not a template removal. - const previousOnDemand = previousSdkByName.get(gsi.IndexName)?.OnDemandThroughput; - if (previousOnDemand) { - const reset: OnDemandThroughput = {}; - if (previousOnDemand.MaxReadRequestUnits !== undefined) { - reset.MaxReadRequestUnits = ON_DEMAND_LIMIT_RESET; - } - if (previousOnDemand.MaxWriteRequestUnits !== undefined) { - reset.MaxWriteRequestUnits = ON_DEMAND_LIMIT_RESET; - } - if (Object.keys(reset).length > 0) update.OnDemandThroughput = reset; + // On-demand limits: MERGE the desired side with an explicit reset for + // every member the template DROPPED. Omitting a removed member leaves + // the old ceiling live in AWS forever while cdkd reports success — the + // absent-field-reset silent-drop class (#1160), one level down inside a + // nested block (#1225). CloudFormation resets it. + // + // Merging rather than branching is load-bearing: the read limit comes + // from `Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughput- + // Settings` and the write limit from `GSI.WriteOnDemandThroughputSettings` + // — two INDEPENDENT CDK props. An `else if` that only fired when the new + // side had no on-demand block at all would still silently drop the + // single-member removal, which is the likelier user edit. + // + // `-1` is the reset sentinel, LIVE-VERIFIED rather than inferred from + // the table-level field's docs (issue #1423). Both payload shapes were + // probed against real AWS: `{-1, -1}` cleared both members, and the + // MIXED `{MaxReadRequestUnits: 50, MaxWriteRequestUnits: -1}` was + // accepted and read back as `{MaxReadRequestUnits: 50}` — the dropped + // member cleared, the kept one preserved. In both cases the reset reads + // back as ABSENCE, never as -1, so drift comparisons must expect that. + // + // Skipped on a billing flip, where "no on-demand fields" is just the + // translation of a PROVISIONED side rather than a template removal. + const previousOnDemand = previousSdkByName.get(gsi.IndexName)?.OnDemandThroughput; + const onDemand: OnDemandThroughput = { ...gsi.OnDemandThroughput }; + if (!billingFlipped && previousOnDemand) { + if ( + previousOnDemand.MaxReadRequestUnits !== undefined && + onDemand.MaxReadRequestUnits === undefined + ) { + onDemand.MaxReadRequestUnits = ON_DEMAND_LIMIT_RESET; + } + if ( + previousOnDemand.MaxWriteRequestUnits !== undefined && + onDemand.MaxWriteRequestUnits === undefined + ) { + onDemand.MaxWriteRequestUnits = ON_DEMAND_LIMIT_RESET; } } + if (Object.keys(onDemand).length > 0) update.OnDemandThroughput = onDemand; // 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) { diff --git a/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts b/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts index f16294ac1..1d27ffaf6 100644 --- a/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts +++ b/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts @@ -202,19 +202,19 @@ export class DynamoDBGlobalTableStack extends cdk.Stack { { indexName: 'byOwner', partitionKey: { name: 'owner', type: ddb.AttributeType.STRING }, - // Issue #1423: REMOVING these from the template must reset the live - // ceiling, not silently no-op. Under `drop-gsi-ondemand-limits` both - // are omitted, and verify.sh asserts DescribeTable reports the - // member ABSENT afterwards (the reset reads back as absence, never - // as -1). + // -> Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings + maxReadRequestUnits: 50, + // Issue #1423: REMOVING this from the template must RESET the live + // ceiling, not silently no-op. `drop-gsi-ondemand-limits` drops ONLY + // the write limit and keeps the read one, which is the harder case: + // the two are independent CDK props, so a fix that only reset when + // the whole on-demand block disappeared would still drop this edit. + // verify.sh asserts the write member is ABSENT while read is still + // 50 (the reset reads back as absence, never as -1). + // -> GlobalSecondaryIndexes[].WriteOnDemandThroughputSettings ...(updateMode.includes('drop-gsi-ondemand-limits') ? {} - : { - // -> Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings - maxReadRequestUnits: 50, - // -> GlobalSecondaryIndexes[].WriteOnDemandThroughputSettings - maxWriteRequestUnits: 60, - }), + : { maxWriteRequestUnits: 60 }), }, ], removalPolicy: cdk.RemovalPolicy.DESTROY, diff --git a/tests/integration/dynamodb-globaltable/verify.sh b/tests/integration/dynamodb-globaltable/verify.sh index e2aefdc60..fefe05bea 100755 --- a/tests/integration/dynamodb-globaltable/verify.sh +++ b/tests/integration/dynamodb-globaltable/verify.sh @@ -411,15 +411,29 @@ echo "[verify] step 13b: cdkd deploy with drop-gsi-ondemand-limits (issue #1423 CDKD_TEST_UPDATE=ttl,tags,drop-gsi-ondemand-limits ${CLI} deploy "${STACK}" --state-bucket "${STATE_BUCKET}" --verbose # The reset reads back as ABSENCE, never as -1 (live-probed on #1423). Pre-fix -# the template removal emitted nothing at all, so the 50/60 ceiling stayed live -# in AWS forever while cdkd reported success. -OD_AFTER="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \ - --query "Table.GlobalSecondaryIndexes[?IndexName=='byOwner'].OnDemandThroughput | [0]" --output text)" -if [ "${OD_AFTER}" != "None" ]; then - echo "FAIL: issue #1423 — byOwner still carries OnDemandThroughput after the template removed it: ${OD_AFTER}" >&2 +# the template removal emitted nothing at all, so the 60 write ceiling stayed +# live in AWS forever while cdkd reported success. +# +# Assert the index still EXISTS first: a `| [0]` query against a MISSING index +# also answers "None", so the absence check alone would pass if `byOwner` had +# vanished entirely. +OD_IDX="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \ + --query "length(Table.GlobalSecondaryIndexes[?IndexName=='byOwner'])" --output text)" +if [ "${OD_IDX}" != "1" ]; then + echo "FAIL: byOwner index missing after the drop-limits update (count=${OD_IDX})" >&2 + exit 1 +fi +# The DROPPED member must be gone... +OD_WRITE="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \ + --query "Table.GlobalSecondaryIndexes[?IndexName=='byOwner'].OnDemandThroughput.MaxWriteRequestUnits | [0]" --output text)" +# ...while the one the template STILL declares must survive untouched. +OD_READ="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \ + --query "Table.GlobalSecondaryIndexes[?IndexName=='byOwner'].OnDemandThroughput.MaxReadRequestUnits | [0]" --output text)" +if [ "${OD_WRITE}" != "None" ] || [ "${OD_READ}" != "50" ]; then + echo "FAIL: issue #1423 — expected write=None (reset) / read=50 (kept), got write=${OD_WRITE} / read=${OD_READ}" >&2 exit 1 fi -echo " per-GSI on-demand limits reset (OnDemandThroughput absent), issue #1423 closed" +echo " per-GSI write limit reset (absent) and read limit kept at 50, issue #1423 closed" echo "[verify] step 14a: assert DeletionProtectionEnabled flipped back to false on AWS" DP_FINAL="$(aws dynamodb describe-table --table-name "${TABLE_NAME}" --region "${REGION}" \ 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 28fdcaa06..d7320e479 100644 --- a/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts +++ b/tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts @@ -556,6 +556,36 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => { ]); }); + it('resets the DROPPED member while KEEPING the one still declared (partial removal)', async () => { + // The likeliest user edit: read and write limits are two independent CDK + // props (read via the local replica, write on the GSI), so removing ONE + // is common. A branch that only reset when the new side had NO on-demand + // block at all would silently leave the other ceiling live — the very + // #1423 bug, shipped as fixed. Live-probed: the MIXED payload + // {MaxReadRequestUnits: 50, MaxWriteRequestUnits: -1} is accepted and + // reads back as {MaxReadRequestUnits: 50}. + const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record; + const next = structuredClone(ON_DEMAND_TABLE_PROPS) as Record; + // Drop ONLY the write limit; the replica-side read limit (50) stays. + delete (next['GlobalSecondaryIndexes'] as Record[])[0]![ + 'WriteOnDemandThroughputSettings' + ]; + + await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous); + + const gsiUpdates = mockSend.mock.calls + .map((c) => c[0]) + .filter( + (c): c is UpdateTableCommand => + c instanceof UpdateTableCommand && + (c.input.GlobalSecondaryIndexUpdates ?? []).some((u) => u.Update !== undefined) + ); + expect(gsiUpdates).toHaveLength(1); + expect( + gsiUpdates[0]!.input.GlobalSecondaryIndexUpdates?.[0]?.Update?.OnDemandThroughput + ).toEqual({ MaxReadRequestUnits: 50, MaxWriteRequestUnits: -1 }); + }); + it('resets ONLY the member that was actually set before', async () => { // A blanket {-1, -1} would clear a sibling limit the template still // declares.