From c6504e6433d540414a417b9fb23fb9950a44eb5c Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Tue, 12 May 2020 17:26:08 -0700 Subject: [PATCH] feat(events-targets): support multiple security groups for an ECS task (#7857) * feat(aws-events-targets): support multiple security groups for an ECS task Resolves #3312 * chore: expose generated security group in securityGroup field Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../aws-events-targets/lib/ecs-task.ts | 48 ++++++++- .../test/ecs/event-rule-target.test.ts | 99 ++++++++++++++++++- 2 files changed, 141 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts index 68ef60739cc09..57cf46b6d4aeb 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts @@ -49,28 +49,68 @@ export interface EcsTaskProps { * (Only applicable in case the TaskDefinition is configured for AwsVpc networking) * * @default A new security group is created + * @deprecated use securityGroups instead */ readonly securityGroup?: ec2.ISecurityGroup; + + /** + * Existing security groups to use for the task's ENIs + * + * (Only applicable in case the TaskDefinition is configured for AwsVpc networking) + * + * @default A new security group is created + */ + readonly securityGroups?: ec2.ISecurityGroup[]; } /** * Start a task on an ECS cluster */ export class EcsTask implements events.IRuleTarget { + // Security group fields are public because we can generate a new security group if none is provided. + + /** + * The security group associated with the task. Only applicable with awsvpc network mode. + * + * @default - A new security group is created. + * @deprecated use securityGroups instead. + */ public readonly securityGroup?: ec2.ISecurityGroup; + + /** + * The security groups associated with the task. Only applicable with awsvpc network mode. + * + * @default - A new security group is created. + */ + public readonly securityGroups?: ec2.ISecurityGroup[]; private readonly cluster: ecs.ICluster; private readonly taskDefinition: ecs.TaskDefinition; private readonly taskCount: number; constructor(private readonly props: EcsTaskProps) { + if (props.securityGroup !== undefined && props.securityGroups !== undefined) { + throw new Error('Only one of SecurityGroup or SecurityGroups can be populated.'); + } + this.cluster = props.cluster; this.taskDefinition = props.taskDefinition; this.taskCount = props.taskCount !== undefined ? props.taskCount : 1; - if (this.taskDefinition.networkMode === ecs.NetworkMode.AWS_VPC) { - const securityGroup = props.securityGroup || this.taskDefinition.node.tryFindChild('SecurityGroup') as ec2.ISecurityGroup; - this.securityGroup = securityGroup || new ec2.SecurityGroup(this.taskDefinition, 'SecurityGroup', { vpc: this.props.cluster.vpc }); + // Security groups are only configurable with the "awsvpc" network mode. + if (this.taskDefinition.networkMode !== ecs.NetworkMode.AWS_VPC) { + if (props.securityGroup !== undefined || props.securityGroups !== undefined) { + this.taskDefinition.node.addWarning('security groups are ignored when network mode is not awsvpc'); + } + return; + } + if (props.securityGroups) { + this.securityGroups = props.securityGroups; + return; } + let securityGroup = props.securityGroup || this.taskDefinition.node.tryFindChild('SecurityGroup') as ec2.ISecurityGroup; + securityGroup = securityGroup || new ec2.SecurityGroup(this.taskDefinition, 'SecurityGroup', { vpc: this.props.cluster.vpc }); + this.securityGroup = securityGroup; // Maintain backwards-compatibility for customers that read the generated security group. + this.securityGroups = [securityGroup]; } /** @@ -123,7 +163,7 @@ export class EcsTask implements events.IRuleTarget { awsVpcConfiguration: { subnets: this.props.cluster.vpc.selectSubnets(subnetSelection).subnetIds, assignPublicIp, - securityGroups: this.securityGroup && [this.securityGroup.securityGroupId], + securityGroups: this.securityGroups && this.securityGroups.map(sg => sg.securityGroupId), }, }, } diff --git a/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts b/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts index 3b991c4725dd3..66d7b2aced7fb 100644 --- a/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts @@ -73,7 +73,7 @@ test('Can use Fargate taskdef as EventRule target', () => { }); // WHEN - rule.addTarget(new targets.EcsTask({ + const target = new targets.EcsTask({ cluster, taskDefinition, taskCount: 1, @@ -81,9 +81,11 @@ test('Can use Fargate taskdef as EventRule target', () => { containerName: 'TheContainer', command: ['echo', events.EventField.fromPath('$.detail.event')], }], - })); + }); + rule.addTarget(target); // THEN + expect(target.securityGroup).toBeDefined(); // Generated security groups should be accessible. expect(stack).toHaveResourceLike('AWS::Events::Rule', { Targets: [ { @@ -258,3 +260,96 @@ test('Isolated subnet does not have AssignPublicIp=true', () => { ], }); }); + +test('throws an error if both securityGroup and securityGroups is specified', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 1 }); + const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc }); + + const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef'); + taskDefinition.addContainer('TheContainer', { + image: ecs.ContainerImage.fromRegistry('henk'), + }); + + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.expression('rate(1 min)'), + }); + const securityGroup = new ec2.SecurityGroup(stack, 'SecurityGroup', { vpc }); + + // THEN + expect(() => { + rule.addTarget(new targets.EcsTask({ + cluster, + taskDefinition, + taskCount: 1, + securityGroup, + securityGroups: [securityGroup], + containerOverrides: [{ + containerName: 'TheContainer', + command: ['echo', 'yay'], + }], + })); + }).toThrow(/Only one of SecurityGroup or SecurityGroups can be populated./); +}); + +test('uses multiple security groups', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 1 }); + const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc }); + + const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef'); + taskDefinition.addContainer('TheContainer', { + image: ecs.ContainerImage.fromRegistry('henk'), + }); + + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.expression('rate(1 min)'), + }); + const securityGroups = [ + new ec2.SecurityGroup(stack, 'SecurityGroupA', { vpc }), + new ec2.SecurityGroup(stack, 'SecurityGroupB', { vpc }), + ]; + + // WHEN + rule.addTarget(new targets.EcsTask({ + cluster, + taskDefinition, + taskCount: 1, + securityGroups, + containerOverrides: [{ + containerName: 'TheContainer', + command: ['echo', 'yay'], + }], + })); + + // THEN + expect(stack).toHaveResourceLike('AWS::Events::Rule', { + Targets: [ + { + Arn: { 'Fn::GetAtt': ['EcsCluster97242B84', 'Arn'] }, + EcsParameters: { + LaunchType: 'FARGATE', + NetworkConfiguration: { + AwsVpcConfiguration: { + AssignPublicIp: 'DISABLED', + SecurityGroups: [ + { 'Fn::GetAtt': ['SecurityGroupAED40ADC5', 'GroupId']}, + {'Fn::GetAtt': ['SecurityGroupB04591F90', 'GroupId']}, + ], + Subnets: [{ Ref: 'VpcPrivateSubnet1Subnet536B997A'}], + }, + }, + TaskCount: 1, + TaskDefinitionArn: { + Ref: 'TaskDef54694570', + }, + }, + Id: 'Target0', + Input: '{"containerOverrides":[{"name":"TheContainer","command":["echo","yay"]}]}', + RoleArn: { 'Fn::GetAtt': ['TaskDefEventsRoleFB3B67B8', 'Arn'] }, + }, + ], + }); +}); \ No newline at end of file