From 9fbeec3d7fe73ec870fe2de0e122b7714165f70e Mon Sep 17 00:00:00 2001 From: Daniel Neilson <53624638+ddneilson@users.noreply.github.com> Date: Fri, 7 Aug 2020 12:17:32 -0500 Subject: [PATCH] fix(ecs): Scope-down IAM permissions for ECS drain (#9502) Fixes https://github.com/aws/aws-cdk/issues/9501 ### Testing This was tested by deploying a simple app that was basically the sample from the ECS module readme, and then manually killing off instances from the ECS cluster's ASG. When I killed off an instance I then verified, from the lambda logs, that the task-draining lambda was able to complete its work with no errors. The essentials of the app are: ```ts const app = new cdk.App(); const env = { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION } const stack = new cdk.Stack(app, 'Testing', { env }); const vpc = new ec2.Vpc(stack, 'Vpc'); // Create an ECS cluster const cluster = new ecs.Cluster(stack, 'Cluster', { vpc, }); // Add capacity to it cluster.addCapacity('DefaultAutoScalingGroupCapacity', { instanceType: new ec2.InstanceType("t2.xlarge"), desiredCapacity: 2, }); const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef'); taskDefinition.addContainer('DefaultContainer', { image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"), memoryLimitMiB: 512, logging: ecs.LogDriver.awsLogs({ logGroup: new logs.LogGroup(stack, 'LogGroup', { logGroupName: '/test-group/', removalPolicy: cdk.RemovalPolicy.DESTROY, retention: logs.RetentionDays.ONE_DAY, }), streamPrefix: 'testing-', }), }); // Instantiate an Amazon ECS Service const ecsService = new ecs.Ec2Service(stack, 'Service', { cluster, taskDefinition, desiredCount: 2, }); ``` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- ...on-load-balanced-ecs-service.expected.json | 12 +++++++++- ...integ.scheduled-ecs-task.lit.expected.json | 12 +++++++++- .../lib/drain-hook/instance-drain-hook.ts | 3 +++ .../integ.app-mesh-proxy-config.expected.json | 12 +++++++++- .../test/ec2/integ.clb-host-nw.expected.json | 12 +++++++++- .../integ.firelens-s3-config.expected.json | 12 +++++++++- .../test/ec2/integ.lb-awsvpc-nw.expected.json | 12 +++++++++- .../test/ec2/integ.lb-bridge-nw.expected.json | 12 +++++++++- .../test/ec2/integ.sd-awsvpc-nw.expected.json | 12 +++++++++- .../test/ec2/integ.sd-bridge-nw.expected.json | 12 +++++++++- .../test/ec2/integ.spot-drain.expected.json | 24 +++++++++++++++++-- .../@aws-cdk/aws-ecs/test/test.ecs-cluster.ts | 10 ++++++++ .../integ.event-ec2-task.lit.expected.json | 12 +++++++++- .../test/ecs/integ.ec2-run-task.expected.json | 12 +++++++++- .../test/ecs/integ.ec2-task.expected.json | 12 +++++++++- 15 files changed, 167 insertions(+), 14 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.multiple-application-load-balanced-ecs-service.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.multiple-application-load-balanced-ecs-service.expected.json index 8765123f05aa9..7afecdb128746 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.multiple-application-load-balanced-ecs-service.expected.json +++ b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.multiple-application-load-balanced-ecs-service.expected.json @@ -658,7 +658,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "ClusterEB0386A7", + "Arn" + ] + } + } + } }, { "Action": [ 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 e9cdab87a08f6..e9b8c11f8754a 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 @@ -474,7 +474,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ 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 34f402fb97ab1..f8062befb60d4 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 @@ -90,6 +90,9 @@ export class InstanceDrainHook extends cdk.Construct { fn.addToRolePolicy(new iam.PolicyStatement({ actions: ['ecs:DescribeContainerInstances', 'ecs:DescribeTasks'], resources: ['*'], + conditions: { + ArnEquals: { 'ecs:cluster': props.cluster.clusterArn }, + }, })); // Restrict to the ECS Cluster diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.app-mesh-proxy-config.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.app-mesh-proxy-config.expected.json index ec532abc2e2ec..24784b1521a45 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.app-mesh-proxy-config.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.app-mesh-proxy-config.expected.json @@ -637,7 +637,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.clb-host-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.clb-host-nw.expected.json index 124f820c1e376..80111c4451470 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.clb-host-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.clb-host-nw.expected.json @@ -658,7 +658,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.firelens-s3-config.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.firelens-s3-config.expected.json index 25eb03aecdefc..ba3354e59a7ef 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.firelens-s3-config.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.firelens-s3-config.expected.json @@ -637,7 +637,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ 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 39aed8c584a72..20b2143e7ebfa 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 @@ -637,7 +637,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ 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 013adf12a5a82..ae18cb651155e 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 @@ -658,7 +658,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ 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 e67a15948431f..66d961bceb04b 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 @@ -637,7 +637,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ 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 23ad8a29601a6..2214c69632d98 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 @@ -637,7 +637,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.spot-drain.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.spot-drain.expected.json index 502fe1508bb78..3c78f85f86425 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.spot-drain.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.spot-drain.expected.json @@ -639,7 +639,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ @@ -1110,7 +1120,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ diff --git a/packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts b/packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts index 62ff827cd3c77..9eb4ca0ed3bc0 100644 --- a/packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts +++ b/packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts @@ -444,6 +444,16 @@ export = { ], Effect: 'Allow', Resource: '*', + Condition: { + ArnEquals: { + 'ecs:cluster': { + 'Fn::GetAtt': [ + 'EcsCluster97242B84', + 'Arn', + ], + }, + }, + }, }, { Action: [ 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 7d675a2bb2141..5fb1bb925e0bf 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 @@ -475,7 +475,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "EcsCluster97242B84", + "Arn" + ] + } + } + } }, { "Action": [ diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/integ.ec2-run-task.expected.json b/packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/integ.ec2-run-task.expected.json index af47fa1516cbd..bf007c4e55c32 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/integ.ec2-run-task.expected.json +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/integ.ec2-run-task.expected.json @@ -269,7 +269,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "Ec2ClusterEE43E89D", + "Arn" + ] + } + } + } }, { "Action": [ diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/integ.ec2-task.expected.json b/packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/integ.ec2-task.expected.json index 3493dd636ddb5..09e4369720880 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/integ.ec2-task.expected.json +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/integ.ec2-task.expected.json @@ -269,7 +269,17 @@ "ecs:DescribeTasks" ], "Effect": "Allow", - "Resource": "*" + "Resource": "*", + "Condition": { + "ArnEquals": { + "ecs:cluster": { + "Fn::GetAtt": [ + "FargateCluster7CCD5F93", + "Arn" + ] + } + } + } }, { "Action": [