Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/_generated/integ-last-run.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 50 additions & 1 deletion src/provisioning/providers/dynamodb-globaltable-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -2787,6 +2826,16 @@ function asRecord(value: unknown): Record<string, unknown> | 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 28 additions & 0 deletions tests/integration/dynamodb-globaltable/verify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
const next = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
// Drop BOTH limits: write lives on the GSI, read on the local replica.
delete (next['GlobalSecondaryIndexes'] as Record<string, unknown>[])[0]![
'WriteOnDemandThroughputSettings'
];
delete (
(next['Replicas'] as Record<string, unknown>[])[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<string, unknown>;
const next = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
// Drop ONLY the write limit; the replica-side read limit (50) stays.
delete (next['GlobalSecondaryIndexes'] as Record<string, unknown>[])[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<string, unknown>;
// Previous side has WRITE only (drop the replica-side read limit).
delete (
(previous['Replicas'] as Record<string, unknown>[])[0]![
'GlobalSecondaryIndexes'
] as Record<string, unknown>[]
)[0]!['ReadOnDemandThroughputSettings'];
const next = structuredClone(previous) as Record<string, unknown>;
delete (next['GlobalSecondaryIndexes'] as Record<string, unknown>[])[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<string, unknown>;
delete (previous['GlobalSecondaryIndexes'] as Record<string, unknown>[])[0]![
'WriteOnDemandThroughputSettings'
];
delete (
(previous['Replicas'] as Record<string, unknown>[])[0]![
'GlobalSecondaryIndexes'
] as Record<string, unknown>[]
)[0]!['ReadOnDemandThroughputSettings'];
const next = structuredClone(previous) as Record<string, unknown>;
// 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: [] };
Expand Down