Skip to content

Commit 58b004e

Browse files
authored
fix(autoscaling): StepScalingPolicy intervals not checked for going over allowable maximum (#26490)
`StepScalingPolicy` did not have a validation that the number of intervals in `scalingSteps` was in the allowable range. The [Autoscaling documentation](https://docs.aws.amazon.com/autoscaling/ec2/userguide/ec2-auto-scaling-quotas.html) states 20 is the maximum step changes in a policy, and since autoscaling creates 2 policies (an [UpperPolicy](https://github.com/aws/aws-cdk/blob/bc029fe5ac69a8b7fd2dfdbcd8834e9a2cf8e000/packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts#L136-L166) and a [LowerPolicy](https://github.com/aws/aws-cdk/blob/bc029fe5ac69a8b7fd2dfdbcd8834e9a2cf8e000/packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts#L105-L134)), our cap is 40. There is an identical change in Application Autoscaling. Closes #26215. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent dc287e5 commit 58b004e

File tree

4 files changed

+107
-0
lines changed

4 files changed

+107
-0
lines changed

packages/aws-cdk-lib/aws-applicationautoscaling/lib/step-scaling-policy.ts

+6
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ export interface BasicStepScalingPolicyProps {
1515
* The intervals for scaling.
1616
*
1717
* Maps a range of metric values to a particular scaling behavior.
18+
*
19+
* Must be between 2 and 40 steps.
1820
*/
1921
readonly scalingSteps: ScalingInterval[];
2022

@@ -111,6 +113,10 @@ export class StepScalingPolicy extends Construct {
111113
throw new Error('You must supply at least 2 intervals for autoscaling');
112114
}
113115

116+
if (props.scalingSteps.length > 40) {
117+
throw new Error(`'scalingSteps' can have at most 40 steps, got ${props.scalingSteps.length}`);
118+
}
119+
114120
if (props.datapointsToAlarm !== undefined && props.datapointsToAlarm < 1) {
115121
throw new RangeError(`datapointsToAlarm cannot be less than 1, got: ${props.datapointsToAlarm}`);
116122
}

packages/aws-cdk-lib/aws-applicationautoscaling/test/step-scaling-policy.test.ts

+41
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,47 @@ describe('step scaling policy', () => {
273273
});
274274
}).toThrow('datapointsToAlarm cannot be less than 1, got: 0');
275275
});
276+
277+
test('scalingSteps must have at least 2 steps', () => {
278+
// GIVEN
279+
const stack = new cdk.Stack();
280+
const target = createScalableTarget(stack);
281+
282+
expect(() => {
283+
target.scaleOnMetric('Tracking', {
284+
metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric' }),
285+
scalingSteps: [
286+
{ lower: 0, upper: 2, change: +1 },
287+
],
288+
});
289+
}).toThrow(/must supply at least 2/);
290+
});
291+
292+
test('scalingSteps has a maximum of 40 steps', () => {
293+
// GIVEN
294+
const stack = new cdk.Stack();
295+
const target = createScalableTarget(stack);
296+
297+
const numSteps = 41;
298+
const messagesPerTask = 20;
299+
let steps: appscaling.ScalingInterval[] = [];
300+
301+
for (let i = 0; i < numSteps; ++i) {
302+
const step: appscaling.ScalingInterval = {
303+
lower: i * messagesPerTask,
304+
upper: i * (messagesPerTask + 1) - 1,
305+
change: i + 1,
306+
};
307+
steps.push(step);
308+
}
309+
310+
expect(() => {
311+
target.scaleOnMetric('Tracking', {
312+
metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric' }),
313+
scalingSteps: steps,
314+
});
315+
}).toThrow('\'scalingSteps\' can have at most 40 steps, got 41');
316+
});
276317
});
277318

278319
/**

packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts

+6
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ export interface BasicStepScalingPolicyProps {
1515
* The intervals for scaling.
1616
*
1717
* Maps a range of metric values to a particular scaling behavior.
18+
*
19+
* Must be between 2 and 40 steps.
1820
*/
1921
readonly scalingSteps: ScalingInterval[];
2022

@@ -96,6 +98,10 @@ export class StepScalingPolicy extends Construct {
9698
throw new Error('You must supply at least 2 intervals for autoscaling');
9799
}
98100

101+
if (props.scalingSteps.length > 40) {
102+
throw new Error(`'scalingSteps' can have at most 40 steps, got ${props.scalingSteps.length}`);
103+
}
104+
99105
const adjustmentType = props.adjustmentType || AdjustmentType.CHANGE_IN_CAPACITY;
100106
const changesAreAbsolute = adjustmentType === AdjustmentType.EXACT_CAPACITY;
101107

packages/aws-cdk-lib/aws-autoscaling/test/scaling.test.ts

+54
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,60 @@ test('step scaling with evaluation period configured', () => {
327327
});
328328
});
329329

330+
describe('step-scaling-policy scalingSteps length validation checks', () => {
331+
test('scalingSteps must have at least 2 steps', () => {
332+
// GIVEN
333+
const stack = new cdk.Stack();
334+
const fixture = new ASGFixture(stack, 'Fixture');
335+
336+
expect(() => {
337+
fixture.asg.scaleOnMetric('Metric', {
338+
metric: new cloudwatch.Metric({
339+
metricName: 'Legs',
340+
namespace: 'Henk',
341+
dimensionsMap: { Mustache: 'Bushy' },
342+
}),
343+
estimatedInstanceWarmup: cdk.Duration.seconds(150),
344+
// only one scaling step throws an error.
345+
scalingSteps: [
346+
{ lower: 0, upper: 2, change: +1 },
347+
],
348+
});
349+
}).toThrow(/must supply at least 2/);
350+
});
351+
352+
test('scalingSteps has a maximum of 40 steps', () => {
353+
// GIVEN
354+
const stack = new cdk.Stack();
355+
const fixture = new ASGFixture(stack, 'Fixture');
356+
357+
const numSteps = 41;
358+
const messagesPerTask = 20;
359+
let steps: autoscaling.ScalingInterval[] = [];
360+
361+
for (let i = 0; i < numSteps; ++i) {
362+
const step: autoscaling.ScalingInterval = {
363+
lower: i * messagesPerTask,
364+
upper: i * (messagesPerTask + 1) - 1,
365+
change: i + 1,
366+
};
367+
steps.push(step);
368+
}
369+
370+
expect(() => {
371+
fixture.asg.scaleOnMetric('Metric', {
372+
metric: new cloudwatch.Metric({
373+
metricName: 'Legs',
374+
namespace: 'Henk',
375+
dimensionsMap: { Mustache: 'Bushy' },
376+
}),
377+
estimatedInstanceWarmup: cdk.Duration.seconds(150),
378+
scalingSteps: steps,
379+
});
380+
}).toThrow('\'scalingSteps\' can have at most 40 steps, got 41');
381+
});
382+
});
383+
330384
class ASGFixture extends Construct {
331385
public readonly vpc: ec2.Vpc;
332386
public readonly asg: autoscaling.AutoScalingGroup;

0 commit comments

Comments
 (0)