Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ecs): stack name can result in noncompliant capacity provider name #29235

Merged
merged 4 commits into from Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 16 additions & 5 deletions packages/aws-cdk-lib/aws-ecs/lib/cluster.ts
Expand Up @@ -11,7 +11,7 @@ import * as kms from '../../aws-kms';
import * as logs from '../../aws-logs';
import * as s3 from '../../aws-s3';
import * as cloudmap from '../../aws-servicediscovery';
import { Duration, IResource, Resource, Stack, Aspects, ArnFormat, IAspect, Token } from '../../core';
import { Duration, IResource, Resource, Stack, Aspects, ArnFormat, IAspect, Token, Names } from '../../core';

const CLUSTER_SYMBOL = Symbol.for('@aws-cdk/aws-ecs/lib/cluster.Cluster');

Expand Down Expand Up @@ -1168,6 +1168,8 @@ export interface AsgCapacityProviderProps extends AddAutoScalingGroupCapacityOpt
* The name of the capacity provider. If a name is specified,
* it cannot start with `aws`, `ecs`, or `fargate`. If no name is specified,
* a default name in the CFNStackName-CFNResourceName-RandomString format is used.
* If the stack name starts with `aws`, `ecs`, or `fargate`, a unique resource name
* is generated that starts with `cp-`.
*
* @default CloudFormation-generated name
*/
Expand Down Expand Up @@ -1288,6 +1290,7 @@ export class AsgCapacityProvider extends Construct {

constructor(scope: Construct, id: string, props: AsgCapacityProviderProps) {
super(scope, id);
let capacityProviderName = props.capacityProviderName;
this.autoScalingGroup = props.autoScalingGroup as autoscaling.AutoScalingGroup;
this.machineImageType = props.machineImageType ?? MachineImageType.AMAZON_LINUX_2;
this.canContainersAccessInstanceRole = props.canContainersAccessInstanceRole;
Expand All @@ -1306,9 +1309,17 @@ export class AsgCapacityProvider extends Construct {
this.autoScalingGroup.protectNewInstancesFromScaleIn();
}

if (props.capacityProviderName) {
if (!(/^(?!aws|ecs|fargate).+/gm.test(props.capacityProviderName))) {
throw new Error(`Invalid Capacity Provider Name: ${props.capacityProviderName}, If a name is specified, it cannot start with aws, ecs, or fargate.`);
const capacityProviderNameRegex = /^(?!aws|ecs|fargate).+/gm;
if (capacityProviderName) {
if (!(capacityProviderNameRegex.test(capacityProviderName))) {
throw new Error(`Invalid Capacity Provider Name: ${capacityProviderName}, If a name is specified, it cannot start with aws, ecs, or fargate.`);
}
} else {
if (!(capacityProviderNameRegex.test(Stack.of(this).stackName))) {
// name cannot start with 'aws|ecs|fargate', so append 'cp-'
// 255 is the max length, subtract 3 because of 'cp-'
// if the regex condition isn't met, CFN will name the capacity provider
capacityProviderName = 'cp-' + Names.uniqueResourceName(this, { maxLength: 252, allowedSpecialCharacters: '-_' });
}
}

Expand All @@ -1319,7 +1330,7 @@ export class AsgCapacityProvider extends Construct {
}

const capacityProvider = new CfnCapacityProvider(this, id, {
name: props.capacityProviderName,
name: capacityProviderName,
autoScalingGroupProvider: {
autoScalingGroupArn: this.autoScalingGroup.autoScalingGroupName,
managedScaling: props.enableManagedScaling === false ? undefined : {
Expand Down
27 changes: 26 additions & 1 deletion packages/aws-cdk-lib/aws-ecs/test/cluster.test.ts
Expand Up @@ -2928,7 +2928,7 @@ test('can add ASG capacity via Capacity Provider by not specifying machineImageT

});

test('throws when ASG Capacity Provider with capacityProviderName starting with aws, ecs or faragte', () => {
test('throws when ASG Capacity Provider with capacityProviderName starting with aws, ecs or fargate', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
Expand Down Expand Up @@ -2965,6 +2965,31 @@ test('throws when ASG Capacity Provider with capacityProviderName starting with
}).toThrow(/Invalid Capacity Provider Name: ecscp, If a name is specified, it cannot start with aws, ecs, or fargate./);
});

test('throws when ASG Capacity Provider with no capacityProviderName but stack name starting with aws, ecs or fargate', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'ecscp');
const vpc = new ec2.Vpc(stack, 'Vpc');
const cluster = new ecs.Cluster(stack, 'EcsCluster');

const autoScalingGroupAl2 = new autoscaling.AutoScalingGroup(stack, 'asgal2', {
vpc,
instanceType: new ec2.InstanceType('bogus'),
machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
});

expect(() => {
// WHEN Capacity Provider when stack name starts with ecs.
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'provideral2-2', {
autoScalingGroup: autoScalingGroupAl2,
enableManagedTerminationProtection: false,
});

cluster.addAsgCapacityProvider(capacityProvider);

}).not.toThrow();
});

test('throws when InstanceWarmupPeriod is less than 0', () => {
// GIVEN
const app = new cdk.App();
Expand Down