Skip to content

Commit d115b47

Browse files
authored
fix(autoscaling): update validation on maxInstanceLifetime (#19584)
### Summary This patch fixes the following bugs in `maxInstanceLifetime` validation by aligning with [CFN doc](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-as-group.html#cfn-as-group-maxinstancelifetime): - A lower bound of `maxInstanceLifetime` is 1 day, not 7-days. - `maxInstanceLifetime` can have 0 which is used to clear a previously set value. ### Test - Run unit test. ``` PASS test/auto-scaling-group.test.js =============================== Coverage summary =============================== Statements : 93.11% ( 419/450 ) Branches : 86.93% ( 286/329 ) Functions : 91.81% ( 101/110 ) Lines : 92.84% ( 402/433 ) ================================================================================ ``` - Deploy CFN template having `maxInstanceLifetime` as 86,400 and 0. ### Notes Originally, this issue was reported by #12588. ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent b212cee commit d115b47

File tree

2 files changed

+26
-7
lines changed

2 files changed

+26
-7
lines changed

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

+5-3
Original file line numberDiff line numberDiff line change
@@ -1035,9 +1035,11 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
10351035
}
10361036

10371037
this.maxInstanceLifetime = props.maxInstanceLifetime;
1038-
if (this.maxInstanceLifetime &&
1039-
(this.maxInstanceLifetime.toSeconds() < 604800 || this.maxInstanceLifetime.toSeconds() > 31536000)) {
1040-
throw new Error('maxInstanceLifetime must be between 7 and 365 days (inclusive)');
1038+
// See https://docs.aws.amazon.com/autoscaling/ec2/userguide/asg-max-instance-lifetime.html for details on max instance lifetime.
1039+
if (this.maxInstanceLifetime && !this.maxInstanceLifetime.isUnresolved() &&
1040+
(this.maxInstanceLifetime.toSeconds() !== 0) &&
1041+
(this.maxInstanceLifetime.toSeconds() < 86400 || this.maxInstanceLifetime.toSeconds() > 31536000)) {
1042+
throw new Error('maxInstanceLifetime must be between 1 and 365 days (inclusive)');
10411043
}
10421044

10431045
if (props.notificationsTopic && props.notifications) {

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

+21-4
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,24 @@ describe('auto scaling group', () => {
758758
});
759759
});
760760

761-
test('throws if maxInstanceLifetime < 7 days', () => {
761+
test('can configure maxInstanceLifetime with 0', () => {
762+
// GIVEN
763+
const stack = new cdk.Stack();
764+
const vpc = mockVpc(stack);
765+
new autoscaling.AutoScalingGroup(stack, 'MyStack', {
766+
instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO),
767+
machineImage: new ec2.AmazonLinuxImage(),
768+
vpc,
769+
maxInstanceLifetime: cdk.Duration.days(0),
770+
});
771+
772+
// THEN
773+
Template.fromStack(stack).hasResourceProperties('AWS::AutoScaling::AutoScalingGroup', {
774+
'MaxInstanceLifetime': 0,
775+
});
776+
});
777+
778+
test('throws if maxInstanceLifetime < 1 day', () => {
762779
// GIVEN
763780
const stack = new cdk.Stack();
764781
const vpc = mockVpc(stack);
@@ -769,9 +786,9 @@ describe('auto scaling group', () => {
769786
instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO),
770787
machineImage: new ec2.AmazonLinuxImage(),
771788
vpc,
772-
maxInstanceLifetime: cdk.Duration.days(6),
789+
maxInstanceLifetime: cdk.Duration.hours(23),
773790
});
774-
}).toThrow(/maxInstanceLifetime must be between 7 and 365 days \(inclusive\)/);
791+
}).toThrow(/maxInstanceLifetime must be between 1 and 365 days \(inclusive\)/);
775792
});
776793

777794
test('throws if maxInstanceLifetime > 365 days', () => {
@@ -787,7 +804,7 @@ describe('auto scaling group', () => {
787804
vpc,
788805
maxInstanceLifetime: cdk.Duration.days(366),
789806
});
790-
}).toThrow(/maxInstanceLifetime must be between 7 and 365 days \(inclusive\)/);
807+
}).toThrow(/maxInstanceLifetime must be between 1 and 365 days \(inclusive\)/);
791808
});
792809

793810
test('can configure instance monitoring', () => {

0 commit comments

Comments
 (0)