From 9149ccb5e010b45e13ecc996adda9139ac176f3d Mon Sep 17 00:00:00 2001 From: David Killmon Date: Wed, 3 Jul 2019 18:41:46 -0700 Subject: [PATCH] fix(ecs): grant drain-hook policy container-instance permissions UpdateContainerInstanceState and ListTask APIs require permissions on a container-instance resource, rather than a cluster resource. This change updates the policy to: 1. remove the cluster as the resource restriction 2. add the cluster as a resource condition More info on ECS Resource-Level permissions can be found here: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-supported-iam-actions-resources.html Fixes #3190 --- .../lib/drain-hook/instance-drain-hook.ts | 16 +++++-- .../test/ec2/integ.lb-awsvpc-nw.expected.json | 22 +++++++-- .../test/ec2/integ.lb-bridge-nw.expected.json | 22 +++++++-- .../test/ec2/integ.sd-awsvpc-nw.expected.json | 22 +++++++-- .../test/ec2/integ.sd-bridge-nw.expected.json | 22 +++++++-- .../integ.event-ec2-task.lit.expected.json | 46 +++++++++++++------ .../test/integ.ec2-task.expected.json | 24 ++++++++-- 7 files changed, 140 insertions(+), 34 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..1c80f86f1b560 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,21 @@ export class InstanceDrainHook extends cdk.Construct { actions: [ 'ecs:ListContainerInstances', 'ecs:SubmitContainerStateChange', - 'ecs:SubmitTaskStateChange', - 'ecs:UpdateContainerInstancesState', - 'ecs:ListTasks' + 'ecs:SubmitTaskStateChange' ], resources: [props.cluster.clusterArn] })); + + // Restrict to the container-instance operations to the ECS Cluster + fn.addToRolePolicy(new iam.PolicyStatement({ + actions: [ + 'ecs:UpdateContainerInstancesState', + 'ecs:ListTasks' + ], + conditions: { + ArnEquals: {'ecs:cluster': props.cluster.clusterArn} + }, + resources: ['*'] + })); } } 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..27a716eab42c5 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,24 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + }, + "Effect": "Allow", + "Resource": "*" } ], "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..599d4d566c8c7 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,24 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + }, + "Effect": "Allow", + "Resource": "*" } ], "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..e404d1fa015ac 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,24 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + }, + "Effect": "Allow", + "Resource": "*" } ], "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..3310f3c9bc329 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,24 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + }, + "Effect": "Allow", + "Resource": "*" } ], "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..3a51383a460e8 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 @@ -446,21 +446,37 @@ "Resource": "*" }, { - "Action": [ - "ecs:ListContainerInstances", - "ecs:SubmitContainerStateChange", - "ecs:SubmitTaskStateChange", - "ecs:UpdateContainerInstancesState", - "ecs:ListTasks" - ], - "Effect": "Allow", - "Resource": { - "Fn::GetAtt": [ - "EcsCluster97242B84", - "Arn" - ] + "Action": [ + "ecs:ListContainerInstances", + "ecs:SubmitContainerStateChange", + "ecs:SubmitTaskStateChange" + ], + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + }, + "Effect": "Allow", + "Resource": "*" } - } ], "Version": "2012-10-17" }, @@ -1197,4 +1213,4 @@ "Description": "Artifact hash for asset \"aws-ecs-integ-ecs/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\"" } } -} \ No newline at end of file +} 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..300bead009820 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,24 @@ "Arn" ] } + }, + { + "Action": [ + "ecs:UpdateContainerInstancesState", + "ecs:ListTasks" + ], + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "FargateCluster7CCD5F93", + "Arn" + ] + } + } + }, + "Effect": "Allow", + "Resource": "*" } ], "Version": "2012-10-17" @@ -978,4 +994,4 @@ "Description": "Artifact hash for asset \"aws-ecs-integ2/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\"" } } -} \ No newline at end of file +}