Skip to content

Commit eb2fb99

Browse files
Michae1CCMichae1CC
committed
PR feedback
1 parent 4093326 commit eb2fb99

File tree

2 files changed

+147
-12
lines changed

2 files changed

+147
-12
lines changed

‎packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/ecs/run-task.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -461,21 +461,21 @@ export class EcsRunTask extends sfn.TaskStateBase implements ec2.IConnectable {
461461
this.connections.addSecurityGroup(...this.securityGroups);
462462
}
463463

464-
private validateNoNetworkingProps(networkMode: ecs.NetworkMode | undefined) {
464+
private validateNoNetworkingProps(networkMode?: ecs.NetworkMode) {
465465
if (this.props.subnets !== undefined || this.props.securityGroups !== undefined) {
466466
// Condition on the taskDefinition to set the message to maintain backwards
467467
// compatibility
468+
const formattedNetworkMode = networkMode === undefined ? '\'networkMode\' is not defined' : `Received: ${networkMode}`;
468469
const validationErrorMsg = this.props.taskDefinition !== undefined
469470
? `Supplied TaskDefinition must have 'networkMode' of 'AWS_VPC' to use 'vpcSubnets' and 'securityGroup'. Received: ${this.props.taskDefinition.networkMode}`
470-
: `A 'networkMode' of 'AWS_VPC' is required to use 'vpcSubnets' and 'securityGroup'. Received: ${networkMode}.`;
471+
: `A 'networkMode' of 'AWS_VPC' is required to use 'vpcSubnets' and 'securityGroup'. ${formattedNetworkMode}.`;
471472
throw new ValidationError(
472473
validationErrorMsg, this,
473474
);
474475
}
475476
}
476477

477-
private makeEcsPolicyStatements(taskDefinition: ecs.TaskDefinition | undefined,
478-
taskDefinitionInput: sfn.TaskInput | undefined): iam.PolicyStatement[] {
478+
private makeEcsPolicyStatements(taskDefinition?: ecs.TaskDefinition, taskDefinitionInput?: sfn.TaskInput): iam.PolicyStatement[] {
479479
const policyStatements: Array<iam.PolicyStatement> = [];
480480

481481
if (taskDefinition !== undefined) {
@@ -512,8 +512,7 @@ export class EcsRunTask extends sfn.TaskStateBase implements ec2.IConnectable {
512512
return policyStatements;
513513
}
514514

515-
private makeIamPassRolePolicyStatements(taskDefinition: ecs.TaskDefinition | undefined,
516-
taskDefinitionInput: sfn.TaskInput | undefined): iam.PolicyStatement[] {
515+
private makeIamPassRolePolicyStatements(taskDefinition?: ecs.TaskDefinition, taskDefinitionInput?: sfn.TaskInput): iam.PolicyStatement[] {
517516
const policyStatements: Array<iam.PolicyStatement> = [];
518517

519518
if (taskDefinition !== undefined) {
@@ -578,8 +577,7 @@ export class EcsRunTask extends sfn.TaskStateBase implements ec2.IConnectable {
578577
return policyStatements;
579578
}
580579

581-
private makePolicyStatements(taskDefinition: ecs.TaskDefinition | undefined,
582-
taskDefinitionInput: sfn.TaskInput | undefined): iam.PolicyStatement[] {
580+
private makePolicyStatements(taskDefinition?: ecs.TaskDefinition, taskDefinitionInput?: sfn.TaskInput): iam.PolicyStatement[] {
583581
return [
584582
...this.makeEcsPolicyStatements(taskDefinition, taskDefinitionInput),
585583
...this.makeIamPassRolePolicyStatements(taskDefinition, taskDefinitionInput),

‎packages/aws-cdk-lib/aws-stepfunctions-tasks/test/ecs/run-tasks.test.ts‎

Lines changed: 141 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ test('Cannot supply revisionNumber when using taskDefinitionInput', () => {
125125
cluster,
126126
taskDefinitionInput: sfn.TaskInput.fromText('arn:aws:ecs:us-east-1:111122223333:task-definition/TestTaskFamilyName:*'),
127127
revisionNumber: 1,
128+
networkMode: ecs.NetworkMode.AWS_VPC,
128129
launchTarget: new tasks.EcsFargateLaunchTarget(),
129130
}).toStateJson(),
130131
).toThrow(/Cannot supply 'revisionNumber' when using 'taskDefinitionInput'./);
@@ -187,7 +188,7 @@ test('Cannot supply taskExecutionRole when using taskDefinition', () => {
187188
).toThrow(/Cannot supply 'taskExecutionRole' when using 'taskDefinition'./);
188189
});
189190

190-
test('Cannot supply subnets when networkMode is not AWS_VPC', () => {
191+
test('Cannot supply subnets when networkMode is not set', () => {
191192
expect(() =>
192193
new tasks.EcsRunTask(stack, 'task', {
193194
cluster,
@@ -197,15 +198,27 @@ test('Cannot supply subnets when networkMode is not AWS_VPC', () => {
197198
subnets: vpc.selectSubnets({ subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }),
198199
launchTarget: new tasks.EcsFargateLaunchTarget(),
199200
}).toStateJson(),
200-
).toThrow(/A 'networkMode' of 'AWS_VPC' is required to use 'vpcSubnets' and 'securityGroup'. Received: undefined./);
201+
).toThrow(/A 'networkMode' of 'AWS_VPC' is required to use 'vpcSubnets' and 'securityGroup'. 'networkMode' is not defined./);
202+
});
203+
204+
test('Cannot supply subnets when networkMode is bridge', () => {
205+
expect(() =>
206+
new tasks.EcsRunTask(stack, 'task', {
207+
cluster,
208+
taskDefinitionInput: sfn.TaskInput.fromText('arn:aws:ecs:us-east-1:111122223333:task-definition/TestTaskFamilyName:*'),
209+
networkMode: ecs.NetworkMode.BRIDGE,
210+
securityGroups: undefined,
211+
subnets: vpc.selectSubnets({ subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }),
212+
launchTarget: new tasks.EcsFargateLaunchTarget(),
213+
}).toStateJson(),
214+
).toThrow(/A 'networkMode' of 'AWS_VPC' is required to use 'vpcSubnets' and 'securityGroup'. Received: bridge./);
201215
});
202216

203217
test('Cannot supply securityGroups when networkMode is not AWS_VPC', () => {
204218
expect(() =>
205219
new tasks.EcsRunTask(stack, 'task', {
206220
cluster,
207221
taskDefinitionInput: sfn.TaskInput.fromText('arn:aws:ecs:us-east-1:111122223333:task-definition/TestTaskFamilyName:*'),
208-
networkMode: undefined,
209222
securityGroups: [new ec2.SecurityGroup(stack, 'SecurityGroup1', {
210223
allowAllOutbound: true,
211224
description: 'Test Security Group',
@@ -215,7 +228,7 @@ test('Cannot supply securityGroups when networkMode is not AWS_VPC', () => {
215228
subnets: undefined,
216229
launchTarget: new tasks.EcsFargateLaunchTarget(),
217230
}).toStateJson(),
218-
).toThrow(/A 'networkMode' of 'AWS_VPC' is required to use 'vpcSubnets' and 'securityGroup'. Received: undefined./);
231+
).toThrow(/A 'networkMode' of 'AWS_VPC' is required to use 'vpcSubnets' and 'securityGroup'. 'networkMode' is not defined./);
219232
});
220233

221234
test('Running a task with container override and container has explicitly set a container name', () => {
@@ -604,6 +617,12 @@ test('Running a Fargate Task - using JSONata', () => {
604617

605618
test('Running a Fargate Task using taskDefinitionInput', () => {
606619
const taskDefinitionInput = sfn.TaskInput.fromText('arn:aws:ecs:us-east-1:111122223333:task-definition/TestTaskFamilyName:1');
620+
const taskRole = new iam.Role(stack, 'TaskRole', {
621+
assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'),
622+
});
623+
const taskExecutionRole = new iam.Role(stack, 'ExecutionRole', {
624+
assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'),
625+
});
607626

608627
// WHEN
609628
const runTask = new tasks.EcsRunTask(stack, 'RunFargate', {
@@ -615,6 +634,8 @@ test('Running a Fargate Task using taskDefinitionInput', () => {
615634
launchTarget: new tasks.EcsFargateLaunchTarget({
616635
platformVersion: ecs.FargatePlatformVersion.VERSION1_4,
617636
}),
637+
taskRole,
638+
taskExecutionRole,
618639
});
619640

620641
new sfn.StateMachine(stack, 'SM', {
@@ -669,6 +690,19 @@ test('Running a Fargate Task using taskDefinitionInput', () => {
669690
Effect: 'Allow',
670691
Resource: '*',
671692
},
693+
{
694+
Action: 'iam:PassRole',
695+
Condition: {
696+
StringEquals: {
697+
'iam:PassedToService': 'ecs-tasks.amazonaws.com',
698+
},
699+
},
700+
Effect: 'Allow',
701+
Resource: [
702+
{ 'Fn::GetAtt': ['TaskRole30FC0FBB', 'Arn'] },
703+
{ 'Fn::GetAtt': ['ExecutionRole605A040B', 'Arn'] },
704+
],
705+
},
672706
{
673707
Action: ['events:PutTargets', 'events:PutRule', 'events:DescribeRule'],
674708
Effect: 'Allow',
@@ -750,6 +784,59 @@ test('Running a Fargate Task using JSONata for taskDefinitionInput', () => {
750784
},
751785
Type: 'Task',
752786
});
787+
788+
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
789+
PolicyDocument: {
790+
Statement: [
791+
{
792+
Action: 'ecs:RunTask',
793+
Condition: {
794+
ArnLike: {
795+
'ecs:cluster': { 'Fn::GetAtt': ['ClusterEB0386A7', 'Arn'] },
796+
},
797+
},
798+
Effect: 'Allow',
799+
Resource: '*',
800+
},
801+
{
802+
Action: ['ecs:StopTask', 'ecs:DescribeTasks'],
803+
Effect: 'Allow',
804+
Resource: '*',
805+
},
806+
{
807+
Action: 'iam:PassRole',
808+
Condition: {
809+
StringEquals: {
810+
'iam:PassedToService': 'ecs-tasks.amazonaws.com',
811+
},
812+
},
813+
Effect: 'Allow',
814+
Resource: [
815+
{ 'Fn::GetAtt': ['TaskRole30FC0FBB', 'Arn'] },
816+
{ 'Fn::GetAtt': ['ExecutionRole605A040B', 'Arn'] },
817+
],
818+
},
819+
{
820+
Action: ['events:PutTargets', 'events:PutRule', 'events:DescribeRule'],
821+
Effect: 'Allow',
822+
Resource: {
823+
'Fn::Join': [
824+
'',
825+
[
826+
'arn:',
827+
{ Ref: 'AWS::Partition' },
828+
':events:',
829+
{ Ref: 'AWS::Region' },
830+
':',
831+
{ Ref: 'AWS::AccountId' },
832+
':rule/StepFunctionsGetEventsForECSTaskRule',
833+
],
834+
],
835+
},
836+
},
837+
],
838+
},
839+
});
753840
});
754841

755842
test('Running an EC2 Task with bridge network', () => {
@@ -1013,6 +1100,56 @@ test('Running an EC2 Task with bridge network using JSONata taskDefinitionInput'
10131100
},
10141101
Type: 'Task',
10151102
});
1103+
1104+
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
1105+
PolicyDocument: {
1106+
Statement: [
1107+
{
1108+
Action: 'ecs:RunTask',
1109+
Condition: {
1110+
ArnLike: {
1111+
'ecs:cluster': { 'Fn::GetAtt': ['ClusterEB0386A7', 'Arn'] },
1112+
},
1113+
},
1114+
Effect: 'Allow',
1115+
Resource: '*',
1116+
},
1117+
{
1118+
Action: ['ecs:StopTask', 'ecs:DescribeTasks'],
1119+
Effect: 'Allow',
1120+
Resource: '*',
1121+
},
1122+
{
1123+
Action: 'iam:PassRole',
1124+
Condition: {
1125+
StringEquals: {
1126+
'iam:PassedToService': 'ecs-tasks.amazonaws.com',
1127+
},
1128+
},
1129+
Effect: 'Allow',
1130+
Resource: { 'Fn::GetAtt': ['TaskRole30FC0FBB', 'Arn'] },
1131+
},
1132+
{
1133+
Action: ['events:PutTargets', 'events:PutRule', 'events:DescribeRule'],
1134+
Effect: 'Allow',
1135+
Resource: {
1136+
'Fn::Join': [
1137+
'',
1138+
[
1139+
'arn:',
1140+
{ Ref: 'AWS::Partition' },
1141+
':events:',
1142+
{ Ref: 'AWS::Region' },
1143+
':',
1144+
{ Ref: 'AWS::AccountId' },
1145+
':rule/StepFunctionsGetEventsForECSTaskRule',
1146+
],
1147+
],
1148+
},
1149+
},
1150+
],
1151+
},
1152+
});
10161153
});
10171154

10181155
test('Running an EC2 Task with placement strategies', () => {

0 commit comments

Comments
 (0)