diff --git a/docs/_generated/integ-last-run.tsv b/docs/_generated/integ-last-run.tsv index e184152aa..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-09T13:35:05Z PASS 209 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean +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 36a18a574..bbe339964 100644 --- a/src/provisioning/providers/dynamodb-globaltable-provider.ts +++ b/src/provisioning/providers/dynamodb-globaltable-provider.ts @@ -1298,7 +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; + // 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) { @@ -2787,6 +2826,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..1d27ffaf6 100644 --- a/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts +++ b/tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts @@ -204,8 +204,17 @@ export class DynamoDBGlobalTableStack extends cdk.Stack { partitionKey: { name: 'owner', type: ddb.AttributeType.STRING }, // -> 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 - maxWriteRequestUnits: 60, + ...(updateMode.includes('drop-gsi-ondemand-limits') + ? {} + : { maxWriteRequestUnits: 60 }), }, ], removalPolicy: cdk.RemovalPolicy.DESTROY, diff --git a/tests/integration/dynamodb-globaltable/verify.sh b/tests/integration/dynamodb-globaltable/verify.sh index 36233c96d..fefe05bea 100755 --- a/tests/integration/dynamodb-globaltable/verify.sh +++ b/tests/integration/dynamodb-globaltable/verify.sh @@ -407,6 +407,34 @@ 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 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 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}" \ --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..d7320e479 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,133 @@ 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 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. + 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: [] };