Skip to content

Commit 0a2ed3d

Browse files
authored
fix(scaling): don't fail when using Tokens (#3758)
Numberized Tokens used to be validated, but they shouldn't be. Skip validation checks for Tokens when using AutoScalingGroups and Application AutoScaling.
1 parent 98863f9 commit 0a2ed3d

File tree

5 files changed

+110
-18
lines changed

5 files changed

+110
-18
lines changed

packages/@aws-cdk/aws-applicationautoscaling/lib/scalable-target.ts

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import iam = require('@aws-cdk/aws-iam');
2-
import { Construct, IResource, Lazy, Resource } from '@aws-cdk/core';
2+
import { Construct, IResource, Lazy, Resource, withResolved } from '@aws-cdk/core';
33
import { CfnScalableTarget } from './applicationautoscaling.generated';
44
import { Schedule } from './schedule';
55
import { BasicStepScalingPolicyProps, StepScalingPolicy } from './step-scaling-policy';
@@ -96,15 +96,23 @@ export class ScalableTarget extends Resource implements IScalableTarget {
9696
constructor(scope: Construct, id: string, props: ScalableTargetProps) {
9797
super(scope, id);
9898

99-
if (props.maxCapacity < 0) {
100-
throw new RangeError(`maxCapacity cannot be negative, got: ${props.maxCapacity}`);
101-
}
102-
if (props.minCapacity < 0) {
103-
throw new RangeError(`minCapacity cannot be negative, got: ${props.minCapacity}`);
104-
}
105-
if (props.maxCapacity < props.minCapacity) {
106-
throw new RangeError(`minCapacity (${props.minCapacity}) should be lower than maxCapacity (${props.maxCapacity})`);
107-
}
99+
withResolved(props.maxCapacity, max => {
100+
if (max < 0) {
101+
throw new RangeError(`maxCapacity cannot be negative, got: ${props.maxCapacity}`);
102+
}
103+
});
104+
105+
withResolved(props.minCapacity, min => {
106+
if (min < 0) {
107+
throw new RangeError(`minCapacity cannot be negative, got: ${props.minCapacity}`);
108+
}
109+
});
110+
111+
withResolved(props.minCapacity, props.maxCapacity, (min, max) => {
112+
if (max < min) {
113+
throw new RangeError(`minCapacity (${props.minCapacity}) should be lower than maxCapacity (${props.maxCapacity})`);
114+
}
115+
});
108116

109117
this.role = props.role || new iam.Role(this, 'Role', {
110118
assumedBy: new iam.ServicePrincipal('application-autoscaling.amazonaws.com')

packages/@aws-cdk/aws-applicationautoscaling/test/test.scalable-target.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { expect, haveResource } from '@aws-cdk/assert';
22
import cdk = require('@aws-cdk/core');
3-
import { Duration } from '@aws-cdk/core';
3+
import { Duration, Lazy } from '@aws-cdk/core';
44
import { Test } from 'nodeunit';
55
import appscaling = require('../lib');
66
import { createScalableTarget } from './util';
@@ -31,6 +31,31 @@ export = {
3131
test.done();
3232
},
3333

34+
'validation does not fail when using Tokens'(test: Test) {
35+
// GIVEN
36+
const stack = new cdk.Stack();
37+
38+
// WHEN
39+
new appscaling.ScalableTarget(stack, 'Target', {
40+
serviceNamespace: appscaling.ServiceNamespace.DYNAMODB,
41+
scalableDimension: 'test:TestCount',
42+
resourceId: 'test:this/test',
43+
minCapacity: Lazy.numberValue({ produce: () => 10 }),
44+
maxCapacity: Lazy.numberValue({ produce: () => 1 }),
45+
});
46+
47+
// THEN: no exception
48+
expect(stack).to(haveResource('AWS::ApplicationAutoScaling::ScalableTarget', {
49+
ServiceNamespace: 'dynamodb',
50+
ScalableDimension: 'test:TestCount',
51+
ResourceId: 'test:this/test',
52+
MinCapacity: 10,
53+
MaxCapacity: 1,
54+
}));
55+
56+
test.done();
57+
},
58+
3459
'add scheduled scaling'(test: Test) {
3560
// GIVEN
3661
const stack = new cdk.Stack();

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2');
55
import iam = require('@aws-cdk/aws-iam');
66
import sns = require('@aws-cdk/aws-sns');
77

8-
import { CfnAutoScalingRollingUpdate, Construct, Duration, Fn, IResource, Lazy, Resource, Stack, Tag } from '@aws-cdk/core';
8+
import { CfnAutoScalingRollingUpdate, Construct, Duration, Fn, IResource, Lazy, Resource, Stack, Tag, Token, withResolved } from '@aws-cdk/core';
99
import { CfnAutoScalingGroup, CfnAutoScalingGroupProps, CfnLaunchConfiguration } from './autoscaling.generated';
1010
import { BasicLifecycleHookProps, LifecycleHook } from './lifecycle-hook';
1111
import { BasicScheduledActionProps, ScheduledAction } from './scheduled-action';
@@ -464,16 +464,23 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
464464
const minCapacity = props.minCapacity !== undefined ? props.minCapacity : 1;
465465
const maxCapacity = props.maxCapacity !== undefined ? props.maxCapacity : desiredCapacity;
466466

467-
if (desiredCapacity < minCapacity || desiredCapacity > maxCapacity) {
468-
throw new Error(`Should have minCapacity (${minCapacity}) <= desiredCapacity (${desiredCapacity}) <= maxCapacity (${maxCapacity})`);
469-
}
467+
withResolved(desiredCapacity, minCapacity, (desired, min) => {
468+
if (desired < min) {
469+
throw new Error(`Should have minCapacity (${min}) <= desiredCapacity (${desired})`);
470+
}
471+
});
472+
withResolved(desiredCapacity, maxCapacity, (desired, max) => {
473+
if (max < desired) {
474+
throw new Error(`Should have desiredCapacity (${desired}) <= maxCapacity (${max})`);
475+
}
476+
});
470477

471478
const { subnetIds, hasPublic } = props.vpc.selectSubnets(props.vpcSubnets);
472479
const asgProps: CfnAutoScalingGroupProps = {
473480
cooldown: props.cooldown !== undefined ? props.cooldown.toSeconds().toString() : undefined,
474-
minSize: minCapacity.toString(),
475-
maxSize: maxCapacity.toString(),
476-
desiredCapacity: desiredCapacity.toString(),
481+
minSize: stringifyNumber(minCapacity),
482+
maxSize: stringifyNumber(maxCapacity),
483+
desiredCapacity: stringifyNumber(desiredCapacity),
477484
launchConfigurationName: launchConfig.ref,
478485
loadBalancerNames: Lazy.listValue({ produce: () => this.loadBalancerNames }, { omitEmpty: true }),
479486
targetGroupArns: Lazy.listValue({ produce: () => this.targetGroupArns }, { omitEmpty: true }),
@@ -1057,3 +1064,14 @@ export enum EbsDeviceVolumeType {
10571064
*/
10581065
SC1 = 'sc1',
10591066
}
1067+
1068+
/**
1069+
* Stringify a number directly or lazily if it's a Token
1070+
*/
1071+
function stringifyNumber(x: number) {
1072+
if (Token.isUnresolved(x)) {
1073+
return Lazy.stringValue({ produce: context => `${context.resolve(x)}` });
1074+
} else {
1075+
return `${x}`;
1076+
}
1077+
}

packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {expect, haveResource, haveResourceLike, InspectionFailure, ResourcePart}
22
import ec2 = require('@aws-cdk/aws-ec2');
33
import iam = require('@aws-cdk/aws-iam');
44
import cdk = require('@aws-cdk/core');
5+
import { Lazy } from '@aws-cdk/core';
56
import cxapi = require('@aws-cdk/cx-api');
67
import { Test } from 'nodeunit';
78
import autoscaling = require('../lib');
@@ -155,6 +156,30 @@ export = {
155156
test.done();
156157
},
157158

159+
'validation is not performed when using Tokens'(test: Test) {
160+
const stack = new cdk.Stack(undefined, 'MyStack', { env: { region: 'us-east-1', account: '1234' } });
161+
const vpc = mockVpc(stack);
162+
163+
new autoscaling.AutoScalingGroup(stack, 'MyFleet', {
164+
instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO),
165+
machineImage: new ec2.AmazonLinuxImage(),
166+
vpc,
167+
minCapacity: Lazy.numberValue({ produce: () => 5 }),
168+
maxCapacity: Lazy.numberValue({ produce: () => 1 }),
169+
desiredCapacity: Lazy.numberValue({ produce: () => 20 }),
170+
});
171+
172+
// THEN: no exception
173+
expect(stack).to(haveResource("AWS::AutoScaling::AutoScalingGroup", {
174+
MinSize: "5",
175+
MaxSize: "1",
176+
DesiredCapacity: "20",
177+
}
178+
));
179+
180+
test.done();
181+
},
182+
158183
'userdata can be overriden by image'(test: Test) {
159184
// GIVEN
160185
const stack = new cdk.Stack();

packages/@aws-cdk/core/lib/token.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,3 +165,19 @@ export interface EncodingOptions {
165165
export function isResolvableObject(x: any): x is IResolvable {
166166
return typeof(x) === 'object' && x !== null && typeof x.resolve === 'function';
167167
}
168+
169+
/**
170+
* Call the given function only if all given values are resolved
171+
*
172+
* Exported as a function since it will be used by TypeScript modules, but
173+
* can't be exposed via JSII because of the generics.
174+
*/
175+
export function withResolved<A>(a: A, fn: (a: A) => void): void;
176+
export function withResolved<A, B>(a: A, b: B, fn: (a: A, b: B) => void): void;
177+
export function withResolved<A, B, C>(a: A, b: B, c: C, fn: (a: A, b: B, c: C) => void): void;
178+
export function withResolved(...args: any[]) {
179+
if (args.length < 2) { return; }
180+
const argArray = args.slice(0, args.length - 1);
181+
if (argArray.some(Token.isUnresolved)) { return; }
182+
args[args.length - 1].apply(arguments, argArray);
183+
}

0 commit comments

Comments
 (0)