Skip to content

Commit efee07d

Browse files
authored
fix(ecs): let AsgCapacityProvider use IAutoScalingGroup only when Managed Termination Protection is disable (#30335)
Let AsgCapacityProvider use IAutoScalingGroup only when Managed Termination Protection is disable. The code will throw an exception with a clear message when the user specify a self managed ASG using `AutoScalingGroup.fromAutoScalingGroupName` and let the Managed Termination Protection enabled. It will also throw a clear exception when calling `Cluster.addAsgCapacityProvider` with an `AsgCapacityProvider` created with an imported ASG. ### Issue # (if applicable) Closes #29174. ### Reason for this change As there is no clear fix to the original issue, this change's purpose it to bring clarity to the users about what is not allowed when using the L2 Constructs `AsgCapacityProvider` and `Cluster` with an imported ASG. ### Description of changes This change will replace non explicit exception, caused by missing methods, by clear error messages. ### Description of how you validated changes Added unit tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 8ebfade commit efee07d

File tree

2 files changed

+113
-32
lines changed

2 files changed

+113
-32
lines changed

Diff for: packages/aws-cdk-lib/aws-ecs/lib/cluster.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,9 @@ export class Cluster extends Resource implements ICluster {
474474
}
475475

476476
private configureAutoScalingGroup(autoScalingGroup: autoscaling.AutoScalingGroup, options: AddAutoScalingGroupCapacityOptions = {}) {
477+
if (!(autoScalingGroup instanceof autoscaling.AutoScalingGroup)) {
478+
throw new Error('Cannot configure the AutoScalingGroup because it is an imported resource.');
479+
}
477480
if (autoScalingGroup.osType === ec2.OperatingSystemType.WINDOWS) {
478481
this.configureWindowsAutoScalingGroup(autoScalingGroup, options);
479482
} else {
@@ -1177,6 +1180,10 @@ export interface AsgCapacityProviderProps extends AddAutoScalingGroupCapacityOpt
11771180

11781181
/**
11791182
* The autoscaling group to add as a Capacity Provider.
1183+
*
1184+
* Warning: When passing an imported resource using `AutoScalingGroup.fromAutoScalingGroupName` along with `enableManagedTerminationProtection: true`,
1185+
* the `AsgCapacityProvider` construct will not be able to enforce the option `newInstancesProtectedFromScaleIn` of the `AutoScalingGroup`.
1186+
* In this case the constructor of `AsgCapacityProvider` will throw an exception.
11801187
*/
11811188
readonly autoScalingGroup: autoscaling.IAutoScalingGroup;
11821189

@@ -1306,7 +1313,11 @@ export class AsgCapacityProvider extends Construct {
13061313
throw new Error('Cannot enable Managed Termination Protection on a Capacity Provider when Managed Scaling is disabled. Either enable Managed Scaling or disable Managed Termination Protection.');
13071314
}
13081315
if (this.enableManagedTerminationProtection) {
1309-
this.autoScalingGroup.protectNewInstancesFromScaleIn();
1316+
if (this.autoScalingGroup instanceof autoscaling.AutoScalingGroup) {
1317+
this.autoScalingGroup.protectNewInstancesFromScaleIn();
1318+
} else {
1319+
throw new Error('Cannot enable Managed Termination Protection on a Capacity Provider when providing an imported AutoScalingGroup.');
1320+
}
13101321
}
13111322

13121323
const capacityProviderNameRegex = /^(?!aws|ecs|fargate).+/gm;

Diff for: packages/aws-cdk-lib/aws-ecs/test/cluster.test.ts

+101-31
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import * as logs from '../../aws-logs';
88
import * as s3 from '../../aws-s3';
99
import * as cloudmap from '../../aws-servicediscovery';
1010
import * as cdk from '../../core';
11+
import { getWarnings } from '../../core/test/util';
1112
import * as cxapi from '../../cx-api';
1213
import * as ecs from '../lib';
1314

@@ -2194,36 +2195,76 @@ describe('cluster', () => {
21942195

21952196
});
21962197

2197-
test('creates ASG capacity providers with expected defaults', () => {
2198-
// GIVEN
2199-
const app = new cdk.App();
2200-
const stack = new cdk.Stack(app, 'test');
2201-
const vpc = new ec2.Vpc(stack, 'Vpc');
2202-
const autoScalingGroup = new autoscaling.AutoScalingGroup(stack, 'asg', {
2203-
vpc,
2204-
instanceType: new ec2.InstanceType('bogus'),
2205-
machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
2198+
describe('creates ASG capacity providers ', () => {
2199+
test('with expected defaults', () => {
2200+
// GIVEN
2201+
const app = new cdk.App();
2202+
const stack = new cdk.Stack(app, 'test');
2203+
const vpc = new ec2.Vpc(stack, 'Vpc');
2204+
const autoScalingGroup = new autoscaling.AutoScalingGroup(stack, 'asg', {
2205+
vpc,
2206+
instanceType: new ec2.InstanceType('bogus'),
2207+
machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
2208+
});
2209+
2210+
// WHEN
2211+
new ecs.AsgCapacityProvider(stack, 'provider', {
2212+
autoScalingGroup,
2213+
});
2214+
2215+
// THEN
2216+
Template.fromStack(stack).hasResourceProperties('AWS::ECS::CapacityProvider', {
2217+
AutoScalingGroupProvider: {
2218+
AutoScalingGroupArn: {
2219+
Ref: 'asgASG4D014670',
2220+
},
2221+
ManagedScaling: {
2222+
Status: 'ENABLED',
2223+
TargetCapacity: 100,
2224+
},
2225+
ManagedTerminationProtection: 'ENABLED',
2226+
},
2227+
});
22062228
});
22072229

2208-
// WHEN
2209-
new ecs.AsgCapacityProvider(stack, 'provider', {
2210-
autoScalingGroup,
2230+
test('with IAutoScalingGroup should throw an error if Managed Termination Protection is enabled.', () => {
2231+
// GIVEN
2232+
const app = new cdk.App();
2233+
const stack = new cdk.Stack(app, 'test');
2234+
const autoScalingGroup = autoscaling.AutoScalingGroup.fromAutoScalingGroupName(stack, 'ASG', 'my-asg');
2235+
2236+
// THEN
2237+
expect(() => {
2238+
new ecs.AsgCapacityProvider(stack, 'provider', {
2239+
autoScalingGroup,
2240+
});
2241+
}).toThrow('Cannot enable Managed Termination Protection on a Capacity Provider when providing an imported AutoScalingGroup.');
22112242
});
22122243

2213-
// THEN
2214-
Template.fromStack(stack).hasResourceProperties('AWS::ECS::CapacityProvider', {
2215-
AutoScalingGroupProvider: {
2216-
AutoScalingGroupArn: {
2217-
Ref: 'asgASG4D014670',
2218-
},
2219-
ManagedScaling: {
2220-
Status: 'ENABLED',
2221-
TargetCapacity: 100,
2244+
test('with IAutoScalingGroup should not throw an error if Managed Termination Protection is disabled.', () => {
2245+
// GIVEN
2246+
const app = new cdk.App();
2247+
const stack = new cdk.Stack(app, 'test');
2248+
const autoScalingGroup = autoscaling.AutoScalingGroup.fromAutoScalingGroupName(stack, 'ASG', 'my-asg');
2249+
2250+
// WHEN
2251+
new ecs.AsgCapacityProvider(stack, 'provider', {
2252+
autoScalingGroup,
2253+
enableManagedTerminationProtection: false,
2254+
});
2255+
2256+
// THEN
2257+
Template.fromStack(stack).hasResourceProperties('AWS::ECS::CapacityProvider', {
2258+
AutoScalingGroupProvider: {
2259+
AutoScalingGroupArn: 'my-asg',
2260+
ManagedScaling: {
2261+
Status: 'ENABLED',
2262+
TargetCapacity: 100,
2263+
},
2264+
ManagedTerminationProtection: 'DISABLED',
22222265
},
2223-
ManagedTerminationProtection: 'ENABLED',
2224-
},
2266+
});
22252267
});
2226-
22272268
});
22282269

22292270
test('can disable Managed Scaling and Managed Termination Protection for ASG capacity provider', () => {
@@ -2483,6 +2524,23 @@ describe('cluster', () => {
24832524

24842525
});
24852526

2527+
test('throws when calling Cluster.addAsgCapacityProvider with an AsgCapacityProvider created with an imported ASG', () => {
2528+
// GIVEN
2529+
const app = new cdk.App();
2530+
const stack = new cdk.Stack(app, 'test');
2531+
const importedAsg = autoscaling.AutoScalingGroup.fromAutoScalingGroupName(stack, 'ASG', 'my-asg');
2532+
const cluster = new ecs.Cluster(stack, 'EcsCluster');
2533+
2534+
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'provider', {
2535+
autoScalingGroup: importedAsg,
2536+
enableManagedTerminationProtection: false,
2537+
});
2538+
// THEN
2539+
expect(() => {
2540+
cluster.addAsgCapacityProvider(capacityProvider);
2541+
}).toThrow('Cannot configure the AutoScalingGroup because it is an imported resource.');
2542+
});
2543+
24862544
test('should throw an error if capacity provider with default strategy is not present in capacity providers', () => {
24872545
// GIVEN
24882546
const app = new cdk.App();
@@ -3042,11 +3100,17 @@ test('throws when InstanceWarmupPeriod is greater than 10000', () => {
30423100
describe('Accessing container instance role', function () {
30433101

30443102
const addUserDataMock = jest.fn();
3045-
const autoScalingGroup: autoscaling.AutoScalingGroup = {
3046-
addUserData: addUserDataMock,
3047-
addToRolePolicy: jest.fn(),
3048-
protectNewInstancesFromScaleIn: jest.fn(),
3049-
} as unknown as autoscaling.AutoScalingGroup;
3103+
3104+
function getAutoScalingGroup(stack: cdk.Stack): autoscaling.AutoScalingGroup {
3105+
const vpc = new ec2.Vpc(stack, 'Vpc');
3106+
const asg = new autoscaling.AutoScalingGroup(stack, 'asg', {
3107+
vpc,
3108+
instanceType: new ec2.InstanceType('bogus'),
3109+
machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
3110+
});
3111+
asg.addUserData = addUserDataMock;
3112+
return asg;
3113+
}
30503114

30513115
afterEach(() => {
30523116
addUserDataMock.mockClear();
@@ -3057,11 +3121,12 @@ describe('Accessing container instance role', function () {
30573121
const app = new cdk.App();
30583122
const stack = new cdk.Stack(app, 'test');
30593123
const cluster = new ecs.Cluster(stack, 'EcsCluster');
3124+
const autoScalingGroup = getAutoScalingGroup(stack);
30603125

30613126
// WHEN
30623127

30633128
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
3064-
autoScalingGroup: autoScalingGroup,
3129+
autoScalingGroup,
30653130
});
30663131

30673132
cluster.addAsgCapacityProvider(capacityProvider);
@@ -3077,10 +3142,11 @@ describe('Accessing container instance role', function () {
30773142
const app = new cdk.App();
30783143
const stack = new cdk.Stack(app, 'test');
30793144
const cluster = new ecs.Cluster(stack, 'EcsCluster');
3145+
const autoScalingGroup = getAutoScalingGroup(stack);
30803146

30813147
// WHEN
30823148
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
3083-
autoScalingGroup: autoScalingGroup,
3149+
autoScalingGroup,
30843150
});
30853151

30863152
cluster.addAsgCapacityProvider(capacityProvider, {
@@ -3098,6 +3164,7 @@ describe('Accessing container instance role', function () {
30983164
const app = new cdk.App();
30993165
const stack = new cdk.Stack(app, 'test');
31003166
const cluster = new ecs.Cluster(stack, 'EcsCluster');
3167+
const autoScalingGroup = getAutoScalingGroup(stack);
31013168

31023169
// WHEN
31033170
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
@@ -3118,6 +3185,7 @@ describe('Accessing container instance role', function () {
31183185
const app = new cdk.App();
31193186
const stack = new cdk.Stack(app, 'test');
31203187
const cluster = new ecs.Cluster(stack, 'EcsCluster');
3188+
const autoScalingGroup = getAutoScalingGroup(stack);
31213189

31223190
// WHEN
31233191
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
@@ -3140,6 +3208,7 @@ describe('Accessing container instance role', function () {
31403208
const app = new cdk.App();
31413209
const stack = new cdk.Stack(app, 'test');
31423210
const cluster = new ecs.Cluster(stack, 'EcsCluster');
3211+
const autoScalingGroup = getAutoScalingGroup(stack);
31433212

31443213
// WHEN
31453214
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
@@ -3162,6 +3231,7 @@ describe('Accessing container instance role', function () {
31623231
const app = new cdk.App();
31633232
const stack = new cdk.Stack(app, 'test');
31643233
const cluster = new ecs.Cluster(stack, 'EcsCluster');
3234+
const autoScalingGroup = getAutoScalingGroup(stack);
31653235

31663236
// WHEN
31673237
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {

0 commit comments

Comments
 (0)