Skip to content

Commit

Permalink
fix(ecs): Scope-down IAM permissions on Cluster ASG (#9493)
Browse files Browse the repository at this point in the history 
This fixes #9492 by down-scoping some IAM permissions granted to the ASG that is created for an ECS cluster, and removing some unneccessary permissions.

### Testing

This was tested by deploying a simple app that was basically the sample from the ECS module readme, and verifying that: (a) the cluster is operational (i.e. tasks are running), and (b) those tasks are able to write to logs.

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*
  • Loading branch information
@ddneilson
ddneilson authored Aug 7, 2020
1 parent 1de9dc8 commit 1670289
Show file tree
Hide file tree
Showing 15 changed files with 554 additions and 88 deletions.
36 changes: 31 additions & 5 deletions ...-ecs-patterns/test/ec2/integ.multiple-application-load-balanced-ecs-service.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,13 +441,39 @@
"Statement": [
{
"Action": [
"ecs:CreateCluster",
"ecs:DeregisterContainerInstance",
"ecs:DiscoverPollEndpoint",
"ecs:Poll",
"ecs:RegisterContainerInstance",
"ecs:StartTelemetrySession",
"ecs:Submit*",
"ecs:Submit*"
],
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"ClusterEB0386A7",
"Arn"
]
}
},
{
"Action": [
"ecs:Poll",
"ecs:StartTelemetrySession"
],
"Effect": "Allow",
"Resource": "*",
"Condition": {
"ArnEquals": {
"ecs:cluster": {
"Fn::GetAtt": [
"ClusterEB0386A7",
"Arn"
]
}
}
}
},
{
"Action": [
"ecs:DiscoverPollEndpoint",
"ecr:GetAuthorizationToken",
"logs:CreateLogStream",
"logs:PutLogEvents"
Expand Down
39 changes: 32 additions & 7 deletions packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,21 +261,46 @@
"Statement": [
{
"Action": [
"ecs:CreateCluster",
"ecs:DeregisterContainerInstance",
"ecs:DiscoverPollEndpoint",
"ecs:Poll",
"ecs:RegisterContainerInstance",
"ecs:StartTelemetrySession",
"ecs:Submit*",
"ecs:Submit*"
],
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
},
{
"Action": [
"ecs:Poll",
"ecs:StartTelemetrySession"
],
"Effect": "Allow",
"Resource": "*",
"Condition": {
"ArnEquals": {
"ecs:cluster": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
}
}
},
{
"Action": [
"ecs:DiscoverPollEndpoint",
"ecr:GetAuthorizationToken",
"logs:CreateLogStream",
"logs:PutLogEvents"
],
"Effect": "Allow",
"Resource": "*"
}
],
} ],
"Version": "2012-10-17"
},
"PolicyName": "EcsClusterDefaultAutoScalingGroupInstanceRoleDefaultPolicy04DC6C80",
Expand Down
33 changes: 29 additions & 4 deletions packages/@aws-cdk/aws-ecs/lib/cluster.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,16 +212,41 @@ export class Cluster extends Resource implements ICluster {

// ECS instances must be able to do these things
// Source: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/instance_IAM_role.html
// But, scoped down to minimal permissions required.
// Notes:
// - 'ecs:CreateCluster' removed. The cluster already exists.
autoScalingGroup.addToRolePolicy(new iam.PolicyStatement({
actions: [
'ecs:CreateCluster',
'ecs:DeregisterContainerInstance',
'ecs:DiscoverPollEndpoint',
'ecs:Poll',
'ecs:RegisterContainerInstance',
'ecs:StartTelemetrySession',
'ecs:Submit*',
],
resources: [
this.clusterArn,
],
}));
autoScalingGroup.addToRolePolicy(new iam.PolicyStatement({
actions: [
// These act on a cluster instance, and the instance doesn't exist until the service starts.
// Thus, scope to the cluster using a condition.
// See: https://docs.aws.amazon.com/IAM/latest/UserGuide/list_amazonelasticcontainerservice.html
'ecs:Poll',
'ecs:StartTelemetrySession',
],
resources: ['*'],
conditions: {
ArnEquals: { 'ecs:cluster': this.clusterArn },
},
}));
autoScalingGroup.addToRolePolicy(new iam.PolicyStatement({
actions: [
// These do not support resource constraints, and must be resource '*'
'ecs:DiscoverPollEndpoint',
'ecr:GetAuthorizationToken',
// Preserved for backwards compatibility.
// Users are able to enable cloudwatch agent using CDK. Existing
// customers might be installing CW agent as part of user-data so if we
// remove these permissions we will break that customer use cases.
'logs:CreateLogStream',
'logs:PutLogEvents',
],
Expand Down
36 changes: 31 additions & 5 deletions packages/@aws-cdk/aws-ecs/test/ec2/integ.app-mesh-proxy-config.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,39 @@
"Statement": [
{
"Action": [
"ecs:CreateCluster",
"ecs:DeregisterContainerInstance",
"ecs:DiscoverPollEndpoint",
"ecs:Poll",
"ecs:RegisterContainerInstance",
"ecs:StartTelemetrySession",
"ecs:Submit*",
"ecs:Submit*"
],
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
},
{
"Action": [
"ecs:Poll",
"ecs:StartTelemetrySession"
],
"Effect": "Allow",
"Resource": "*",
"Condition": {
"ArnEquals": {
"ecs:cluster": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
}
}
},
{
"Action": [
"ecs:DiscoverPollEndpoint",
"ecr:GetAuthorizationToken",
"logs:CreateLogStream",
"logs:PutLogEvents"
Expand Down
36 changes: 31 additions & 5 deletions packages/@aws-cdk/aws-ecs/test/ec2/integ.clb-host-nw.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,13 +441,39 @@
"Statement": [
{
"Action": [
"ecs:CreateCluster",
"ecs:DeregisterContainerInstance",
"ecs:DiscoverPollEndpoint",
"ecs:Poll",
"ecs:RegisterContainerInstance",
"ecs:StartTelemetrySession",
"ecs:Submit*",
"ecs:Submit*"
],
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
},
{
"Action": [
"ecs:Poll",
"ecs:StartTelemetrySession"
],
"Effect": "Allow",
"Resource": "*",
"Condition": {
"ArnEquals": {
"ecs:cluster": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
}
}
},
{
"Action": [
"ecs:DiscoverPollEndpoint",
"ecr:GetAuthorizationToken",
"logs:CreateLogStream",
"logs:PutLogEvents"
Expand Down
36 changes: 31 additions & 5 deletions packages/@aws-cdk/aws-ecs/test/ec2/integ.firelens-s3-config.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,39 @@
"Statement": [
{
"Action": [
"ecs:CreateCluster",
"ecs:DeregisterContainerInstance",
"ecs:DiscoverPollEndpoint",
"ecs:Poll",
"ecs:RegisterContainerInstance",
"ecs:StartTelemetrySession",
"ecs:Submit*",
"ecs:Submit*"
],
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
},
{
"Action": [
"ecs:Poll",
"ecs:StartTelemetrySession"
],
"Effect": "Allow",
"Resource": "*",
"Condition": {
"ArnEquals": {
"ecs:cluster": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
}
}
},
{
"Action": [
"ecs:DiscoverPollEndpoint",
"ecr:GetAuthorizationToken",
"logs:CreateLogStream",
"logs:PutLogEvents"
Expand Down
36 changes: 31 additions & 5 deletions packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,39 @@
"Statement": [
{
"Action": [
"ecs:CreateCluster",
"ecs:DeregisterContainerInstance",
"ecs:DiscoverPollEndpoint",
"ecs:Poll",
"ecs:RegisterContainerInstance",
"ecs:StartTelemetrySession",
"ecs:Submit*",
"ecs:Submit*"
],
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
},
{
"Action": [
"ecs:Poll",
"ecs:StartTelemetrySession"
],
"Effect": "Allow",
"Resource": "*",
"Condition": {
"ArnEquals": {
"ecs:cluster": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
}
}
},
{
"Action": [
"ecs:DiscoverPollEndpoint",
"ecr:GetAuthorizationToken",
"logs:CreateLogStream",
"logs:PutLogEvents"
Expand Down
36 changes: 31 additions & 5 deletions packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,13 +441,39 @@
"Statement": [
{
"Action": [
"ecs:CreateCluster",
"ecs:DeregisterContainerInstance",
"ecs:DiscoverPollEndpoint",
"ecs:Poll",
"ecs:RegisterContainerInstance",
"ecs:StartTelemetrySession",
"ecs:Submit*",
"ecs:Submit*"
],
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
},
{
"Action": [
"ecs:Poll",
"ecs:StartTelemetrySession"
],
"Effect": "Allow",
"Resource": "*",
"Condition": {
"ArnEquals": {
"ecs:cluster": {
"Fn::GetAtt": [
"EcsCluster97242B84",
"Arn"
]
}
}
}
},
{
"Action": [
"ecs:DiscoverPollEndpoint",
"ecr:GetAuthorizationToken",
"logs:CreateLogStream",
"logs:PutLogEvents"
Expand Down
Loading

0 comments on commit 1670289

Please sign in to comment.