Skip to content

Commit abd2760

Browse files
TheOrangePuffTheOrangePuff
authored
Merge pull request #1672 from aligent/fix/dynamo-defaults-aspect-billing-mode
fix(cdk-aspects): prevent DynamoDbDefaultsAspect injecting incompatible throughput settings
2 parents 648fb79 + 1691748 commit abd2760

4 files changed

Lines changed: 222 additions & 1 deletion

File tree

‎.changeset/fix-dynamo-defaults-aspect-billing-mode.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@aligent/cdk-aspects": patch
3+
---
4+
5+
Fix `DynamoDbDefaultsAspect` injecting `ProvisionedThroughput` onto `PAY_PER_REQUEST` tables and `OnDemandThroughput` onto `PROVISIONED` tables

‎CLAUDE.md‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,27 @@ yarn nx lint <package-name> --fix
103103
5. Commit with descriptive message
104104
6. Push to remote
105105

106-
**Never push code that fails linting checks** - this will cause GitHub Actions to fail and block the PR.
106+
**Never push code that fails linting checks** - this will cause GitHub Actions to fail and block the PR.
107+
108+
### Changesets
109+
110+
This repo uses [Changesets](https://github.com/changesets/changesets) for versioning. Every PR that modifies a package must include a changeset file in `.changeset/`.
111+
112+
Before writing a changeset, confirm with the user:
113+
- which package(s) are affected
114+
- the bump type (`patch`, `minor`, or `major`)
115+
- the description
116+
117+
Then create `.changeset/<descriptive-slug>.md`:
118+
119+
```markdown
120+
---
121+
"@aligent/<package-name>": patch | minor | major
122+
---
123+
124+
Short description of the change.
125+
```
126+
127+
- `patch` — bug fixes, non-breaking tweaks
128+
- `minor` — new backwards-compatible features
129+
- `major` — breaking changes

‎packages/cdk-aspects/lib/defaults/dynamodb.test.ts‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
import { App, Aspects, Stack } from "aws-cdk-lib";
2+
import { Template } from "aws-cdk-lib/assertions";
3+
import {
4+
AttributeType,
5+
BillingMode,
6+
CfnTable,
7+
Table,
8+
} from "aws-cdk-lib/aws-dynamodb";
9+
import { DynamoDbDefaultsAspect } from "./dynamodb";
10+
11+
const makeTable = (
12+
stack: Stack,
13+
id: string,
14+
billingMode?: BillingMode
15+
): Table =>
16+
new Table(stack, id, {
17+
partitionKey: { name: "pk", type: AttributeType.STRING },
18+
...(billingMode !== undefined && { billingMode }),
19+
});
20+
21+
describe("DynamoDbDefaultsAspect", () => {
22+
let app: App;
23+
let stack: Stack;
24+
25+
const setup = (duration: "SHORT" | "MEDIUM" | "LONG") => {
26+
app = new App();
27+
stack = new Stack(app, "TestStack");
28+
Aspects.of(stack).add(new DynamoDbDefaultsAspect({ duration }));
29+
};
30+
31+
describe("SHORT duration", () => {
32+
beforeEach(() => setup("SHORT"));
33+
34+
it("sets PROVISIONED billing mode on tables with no explicit billing mode", () => {
35+
makeTable(stack, "MyTable");
36+
app.synth();
37+
38+
Template.fromStack(stack).hasResourceProperties("AWS::DynamoDB::Table", {
39+
BillingMode: "PROVISIONED",
40+
});
41+
});
42+
43+
it("does not inject ProvisionedThroughput onto a PAY_PER_REQUEST table", () => {
44+
makeTable(stack, "OnDemandTable", BillingMode.PAY_PER_REQUEST);
45+
app.synth();
46+
47+
const resources = Template.fromStack(stack).findResources(
48+
"AWS::DynamoDB::Table"
49+
);
50+
const props = Object.values(resources)[0].Properties;
51+
expect(props["ProvisionedThroughput"]).toBeUndefined();
52+
});
53+
54+
it("preserves PAY_PER_REQUEST billing mode on an explicit on-demand table", () => {
55+
makeTable(stack, "OnDemandTable", BillingMode.PAY_PER_REQUEST);
56+
app.synth();
57+
58+
Template.fromStack(stack).hasResourceProperties("AWS::DynamoDB::Table", {
59+
BillingMode: "PAY_PER_REQUEST",
60+
});
61+
});
62+
63+
it("does not override an existing ProvisionedThroughput", () => {
64+
const table = makeTable(stack, "MyTable");
65+
const cfnTable = table.node.defaultChild as CfnTable;
66+
cfnTable.provisionedThroughput = {
67+
readCapacityUnits: 10,
68+
writeCapacityUnits: 10,
69+
};
70+
app.synth();
71+
72+
Template.fromStack(stack).hasResourceProperties("AWS::DynamoDB::Table", {
73+
ProvisionedThroughput: {
74+
ReadCapacityUnits: 10,
75+
WriteCapacityUnits: 10,
76+
},
77+
});
78+
});
79+
80+
it("applies DESTROY removal policy", () => {
81+
makeTable(stack, "MyTable");
82+
app.synth();
83+
84+
const resources = Template.fromStack(stack).findResources(
85+
"AWS::DynamoDB::Table"
86+
);
87+
expect(Object.values(resources)[0].DeletionPolicy).toBe("Delete");
88+
});
89+
});
90+
91+
describe("MEDIUM duration", () => {
92+
beforeEach(() => setup("MEDIUM"));
93+
94+
it("sets PAY_PER_REQUEST billing mode on tables with no explicit billing mode", () => {
95+
makeTable(stack, "MyTable");
96+
app.synth();
97+
98+
Template.fromStack(stack).hasResourceProperties("AWS::DynamoDB::Table", {
99+
BillingMode: "PAY_PER_REQUEST",
100+
});
101+
});
102+
103+
it("sets on-demand throughput limits on tables with no explicit billing mode", () => {
104+
makeTable(stack, "MyTable");
105+
app.synth();
106+
107+
Template.fromStack(stack).hasResourceProperties("AWS::DynamoDB::Table", {
108+
OnDemandThroughput: {
109+
MaxReadRequestUnits: 100,
110+
MaxWriteRequestUnits: 100,
111+
},
112+
});
113+
});
114+
115+
it("does not inject OnDemandThroughput when billing mode is forced to PROVISIONED at L1", () => {
116+
// CDK L2 omits billingMode for PROVISIONED (it's the CF default), so we force it at L1
117+
// to simulate a table where someone has explicitly pinned PROVISIONED via cfnTable override
118+
const table = makeTable(stack, "ProvisionedTable");
119+
const cfnTable = table.node.defaultChild as CfnTable;
120+
cfnTable.billingMode = "PROVISIONED";
121+
app.synth();
122+
123+
const resources = Template.fromStack(stack).findResources(
124+
"AWS::DynamoDB::Table"
125+
);
126+
const props = Object.values(resources)[0].Properties;
127+
expect(props["OnDemandThroughput"]).toBeUndefined();
128+
});
129+
130+
it("does not override an existing OnDemandThroughput", () => {
131+
const table = makeTable(stack, "MyTable");
132+
const cfnTable = table.node.defaultChild as CfnTable;
133+
cfnTable.onDemandThroughput = {
134+
maxReadRequestUnits: 50,
135+
maxWriteRequestUnits: 50,
136+
};
137+
app.synth();
138+
139+
Template.fromStack(stack).hasResourceProperties("AWS::DynamoDB::Table", {
140+
OnDemandThroughput: {
141+
MaxReadRequestUnits: 50,
142+
MaxWriteRequestUnits: 50,
143+
},
144+
});
145+
});
146+
147+
it("applies DESTROY removal policy", () => {
148+
makeTable(stack, "MyTable");
149+
app.synth();
150+
151+
const resources = Template.fromStack(stack).findResources(
152+
"AWS::DynamoDB::Table"
153+
);
154+
expect(Object.values(resources)[0].DeletionPolicy).toBe("Delete");
155+
});
156+
});
157+
158+
describe("LONG duration", () => {
159+
beforeEach(() => setup("LONG"));
160+
161+
it("sets PAY_PER_REQUEST billing mode", () => {
162+
makeTable(stack, "MyTable");
163+
app.synth();
164+
165+
Template.fromStack(stack).hasResourceProperties("AWS::DynamoDB::Table", {
166+
BillingMode: "PAY_PER_REQUEST",
167+
});
168+
});
169+
170+
it("does not set on-demand throughput limits (no cap for LONG)", () => {
171+
makeTable(stack, "MyTable");
172+
app.synth();
173+
174+
const resources = Template.fromStack(stack).findResources(
175+
"AWS::DynamoDB::Table"
176+
);
177+
const props = Object.values(resources)[0].Properties;
178+
expect(props["OnDemandThroughput"]).toBeUndefined();
179+
});
180+
181+
it("applies RETAIN removal policy", () => {
182+
makeTable(stack, "MyTable");
183+
app.synth();
184+
185+
const resources = Template.fromStack(stack).findResources(
186+
"AWS::DynamoDB::Table"
187+
);
188+
expect(Object.values(resources)[0].DeletionPolicy).toBe("Retain");
189+
});
190+
});
191+
});

‎packages/cdk-aspects/lib/defaults/dynamodb.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ export class DynamoDbDefaultsAspect implements IAspect {
128128

129129
if (
130130
cfnTable.provisionedThroughput === undefined &&
131+
cfnTable.billingMode !== BillingMode.PAY_PER_REQUEST &&
131132
this.isProvisionedThroughputConfigured()
132133
) {
133134
cfnTable.provisionedThroughput = {
@@ -138,6 +139,7 @@ export class DynamoDbDefaultsAspect implements IAspect {
138139

139140
if (
140141
cfnTable.onDemandThroughput === undefined &&
142+
cfnTable.billingMode !== BillingMode.PROVISIONED &&
141143
this.isOnDemandThroughputConfigured()
142144
) {
143145
cfnTable.onDemandThroughput = {

0 commit comments

Comments
 (0)