From caaee9eccbaa97783f0d86136322e9ce6df84df5 Mon Sep 17 00:00:00 2001 From: Mate GABRI Date: Thu, 4 Jul 2019 10:27:46 +1000 Subject: [PATCH 1/2] fix(ecs): ECS drain hook can't change instance state to draining (#3190) The UpdateContainerInstancesState permission was scoped to the ECS cluster while it should be scoped to the container instance. fixes #3190 --- .../aws-ecs/lib/drain-hook/instance-drain-hook.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/drain-hook/instance-drain-hook.ts b/packages/@aws-cdk/aws-ecs/lib/drain-hook/instance-drain-hook.ts index 8553aefe6de0e..e938b7d68bb15 100644 --- a/packages/@aws-cdk/aws-ecs/lib/drain-hook/instance-drain-hook.ts +++ b/packages/@aws-cdk/aws-ecs/lib/drain-hook/instance-drain-hook.ts @@ -97,11 +97,17 @@ export class InstanceDrainHook extends cdk.Construct { actions: [ 'ecs:ListContainerInstances', 'ecs:SubmitContainerStateChange', - 'ecs:SubmitTaskStateChange', + 'ecs:SubmitTaskStateChange' + ], + resources: [props.cluster.clusterArn] + })); + + fn.addToRolePolicy(new iam.PolicyStatement({ + actions: [ 'ecs:UpdateContainerInstancesState', 'ecs:ListTasks' ], - resources: [props.cluster.clusterArn] + resources: [`arn:aws:ecs:${cdk.Aws.REGION}:${cdk.Aws.ACCOUNT_ID}:container-instance/*`] })); } } From 150e3a82d9b27434d469111f3a277df6b49b3d70 Mon Sep 17 00:00:00 2001 From: Mate GABRI Date: Thu, 4 Jul 2019 12:39:14 +1000 Subject: [PATCH 2/2] fix(ecs): ECS drain hook can't change instance state to draining (#3190) update integration test data fixes #3190 --- ...integ.scheduled-ecs-task.lit.expected.json | 89 ++++++++++++------- .../test/ec2/integ.lb-awsvpc-nw.expected.json | 27 +++++- .../test/ec2/integ.lb-bridge-nw.expected.json | 27 +++++- .../test/ec2/integ.sd-awsvpc-nw.expected.json | 27 +++++- .../test/ec2/integ.sd-bridge-nw.expected.json | 27 +++++- .../integ.event-ec2-task.lit.expected.json | 27 +++++- .../test/integ.ec2-task.expected.json | 27 +++++- 7 files changed, 199 insertions(+), 52 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json index d344a7f79e42c..111cda251e137 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json +++ b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json @@ -449,9 +449,7 @@ "Action": [ "ecs:ListContainerInstances", "ecs:SubmitContainerStateChange", - "ecs:SubmitTaskStateChange", - "ecs:UpdateContainerInstancesState", - "ecs:ListTasks" + "ecs:SubmitTaskStateChange" ], "Effect": "Allow", "Resource": { @@ -460,6 +458,29 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:aws:ecs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":container-instance/*" + ] + ] + } } ], "Version": "2012-10-17" @@ -614,6 +635,37 @@ "EcsClusterDefaultAutoScalingGroupLifecycleHookDrainHookRoleA38EC83B" ] }, + "ScheduledEc2TaskScheduledEventRuleFE2376A2": { + "Type": "AWS::Events::Rule", + "Properties": { + "ScheduleExpression": "rate(1 minute)", + "State": "ENABLED", + "Targets": [ + { + "Arn": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + }, + "EcsParameters": { + "TaskCount": 2, + "TaskDefinitionArn": { + "Ref": "ScheduledEc2TaskScheduledTaskDef56328BA4" + } + }, + "Id": "awsecsintegecsScheduledEc2TaskScheduledTaskDef18FB4348", + "Input": "{}", + "RoleArn": { + "Fn::GetAtt": [ + "ScheduledEc2TaskScheduledTaskDefEventsRole64113C5F", + "Arn" + ] + } + } + ] + } + }, "ScheduledEc2TaskScheduledTaskDefTaskRoleC3FA127C": { "Type": "AWS::IAM::Role", "Properties": { @@ -829,37 +881,6 @@ } ] } - }, - "ScheduledEc2TaskScheduledEventRuleFE2376A2": { - "Type": "AWS::Events::Rule", - "Properties": { - "ScheduleExpression": "rate(1 minute)", - "State": "ENABLED", - "Targets": [ - { - "Arn": { - "Fn::GetAtt": [ - "EcsCluster97242B84", - "Arn" - ] - }, - "EcsParameters": { - "TaskCount": 2, - "TaskDefinitionArn": { - "Ref": "ScheduledEc2TaskScheduledTaskDef56328BA4" - } - }, - "Id": "awsecsintegecsScheduledEc2TaskScheduledTaskDef18FB4348", - "Input": "{}", - "RoleArn": { - "Fn::GetAtt": [ - "ScheduledEc2TaskScheduledTaskDefEventsRole64113C5F", - "Arn" - ] - } - } - ] - } } }, "Parameters": { diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json index 692b586e9ccf0..ee4f2df7ecdb4 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json @@ -605,9 +605,7 @@ "Action": [ "ecs:ListContainerInstances", "ecs:SubmitContainerStateChange", - "ecs:SubmitTaskStateChange", - "ecs:UpdateContainerInstancesState", - "ecs:ListTasks" + "ecs:SubmitTaskStateChange" ], "Effect": "Allow", "Resource": { @@ -616,6 +614,29 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:aws:ecs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":container-instance/*" + ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json index 98e43f40c4912..ab85830609c05 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json @@ -626,9 +626,7 @@ "Action": [ "ecs:ListContainerInstances", "ecs:SubmitContainerStateChange", - "ecs:SubmitTaskStateChange", - "ecs:UpdateContainerInstancesState", - "ecs:ListTasks" + "ecs:SubmitTaskStateChange" ], "Effect": "Allow", "Resource": { @@ -637,6 +635,29 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:aws:ecs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":container-instance/*" + ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-awsvpc-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-awsvpc-nw.expected.json index f341974270a3e..b2efd45499ee1 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-awsvpc-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-awsvpc-nw.expected.json @@ -605,9 +605,7 @@ "Action": [ "ecs:ListContainerInstances", "ecs:SubmitContainerStateChange", - "ecs:SubmitTaskStateChange", - "ecs:UpdateContainerInstancesState", - "ecs:ListTasks" + "ecs:SubmitTaskStateChange" ], "Effect": "Allow", "Resource": { @@ -616,6 +614,29 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:aws:ecs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":container-instance/*" + ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-bridge-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-bridge-nw.expected.json index bb423ca150ddd..c457923c78cae 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-bridge-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-bridge-nw.expected.json @@ -605,9 +605,7 @@ "Action": [ "ecs:ListContainerInstances", "ecs:SubmitContainerStateChange", - "ecs:SubmitTaskStateChange", - "ecs:UpdateContainerInstancesState", - "ecs:ListTasks" + "ecs:SubmitTaskStateChange" ], "Effect": "Allow", "Resource": { @@ -616,6 +614,29 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:aws:ecs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":container-instance/*" + ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json b/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json index 7816a7c459ca5..f54cf6544f995 100644 --- a/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json @@ -449,9 +449,7 @@ "Action": [ "ecs:ListContainerInstances", "ecs:SubmitContainerStateChange", - "ecs:SubmitTaskStateChange", - "ecs:UpdateContainerInstancesState", - "ecs:ListTasks" + "ecs:SubmitTaskStateChange" ], "Effect": "Allow", "Resource": { @@ -460,6 +458,29 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:aws:ecs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":container-instance/*" + ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.ec2-task.expected.json b/packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.ec2-task.expected.json index 4d03787417868..81c21ad370623 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.ec2-task.expected.json +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.ec2-task.expected.json @@ -249,9 +249,7 @@ "Action": [ "ecs:ListContainerInstances", "ecs:SubmitContainerStateChange", - "ecs:SubmitTaskStateChange", - "ecs:UpdateContainerInstancesState", - "ecs:ListTasks" + "ecs:SubmitTaskStateChange" ], "Effect": "Allow", "Resource": { @@ -260,6 +258,29 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:aws:ecs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":container-instance/*" + ] + ] + } } ], "Version": "2012-10-17"