fix(aws-ecs): downscope instance drain hook permissions for ASG and E…

…CS Cluster actions
aws · Jun 11, 2019 · d064196 · d064196
1 parent 1378e2d
commit d064196
Show file tree

Hide file tree

Showing 7 changed files with 211 additions and 38 deletions.
diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
@@ -5,7 +5,7 @@ import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2');
 import iam = require('@aws-cdk/aws-iam');
 import sns = require('@aws-cdk/aws-sns');
 
-import { AutoScalingRollingUpdate, Construct, Fn, IResource, Resource, Tag, Token } from '@aws-cdk/cdk';
+import { AutoScalingRollingUpdate, Construct, Fn, IResource, Resource, Stack, Tag, Token } from '@aws-cdk/cdk';
 import { CfnAutoScalingGroup, CfnAutoScalingGroupProps, CfnLaunchConfiguration } from './autoscaling.generated';
 import { BasicLifecycleHookProps, LifecycleHook } from './lifecycle-hook';
 import { BasicScheduledActionProps, ScheduledAction } from './scheduled-action';
@@ -196,6 +196,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps {
 abstract class AutoScalingGroupBase extends Resource implements IAutoScalingGroup {
 
   public abstract autoScalingGroupName: string;
+  public abstract autoScalingGroupArn: string;
   protected albTargetGroup?: elbv2.ApplicationTargetGroup;
 
   /**
@@ -318,6 +319,11 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
   public static fromAutoScalingGroupName(scope: Construct, id: string, autoScalingGroupName: string): IAutoScalingGroup {
     class Import extends AutoScalingGroupBase {
       public autoScalingGroupName = autoScalingGroupName;
+      public autoScalingGroupArn = Stack.of(this).formatArn({
+        service: 'autoscaling',
+        resource: 'autoScalingGroup:*:autoScalingGroupName',
+        resourceName: this.autoScalingGroupName
+      });
     }
 
     return new Import(scope, id);
@@ -343,6 +349,11 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
    */
   public readonly autoScalingGroupName: string;
 
+  /**
+   * Arn of the AutoScalingGroup
+   */
+  public readonly autoScalingGroupArn: string;
+
   private readonly userDataLines = new Array<string>();
   private readonly autoScalingGroup: CfnAutoScalingGroup;
   private readonly securityGroup: ec2.ISecurityGroup;
@@ -432,6 +443,11 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
     this.autoScalingGroup = new CfnAutoScalingGroup(this, 'ASG', asgProps);
     this.osType = machineImage.os.type;
     this.autoScalingGroupName = this.autoScalingGroup.autoScalingGroupName;
+    this.autoScalingGroupArn = Stack.of(this).formatArn({
+      service: 'autoscaling',
+      resource: 'autoScalingGroup:*:autoScalingGroupName',
+      resourceName: this.autoScalingGroupName
+    });
 
     this.applyUpdatePolicies(props);
   }
@@ -707,6 +723,12 @@ export interface IAutoScalingGroup extends IResource {
    */
   readonly autoScalingGroupName: string;
 
+  /**
+   * The arn of the AutoScalingGroup
+   * @attribute
+   */
+  readonly autoScalingGroupArn: string;
+
   /**
    * Send a message to either an SQS queue or SNS topic when instances launch or terminate
    */

diff --git a/packages/@aws-cdk/aws-ecs/lib/cluster.ts b/packages/@aws-cdk/aws-ecs/lib/cluster.ts
@@ -401,13 +401,7 @@ class ImportedCluster extends Resource implements ICluster {
     this.clusterArn = props.clusterArn !== undefined ? props.clusterArn : Stack.of(this).formatArn({
       service: 'ecs',
       resource: 'cluster',
-      resourceName: props.clusterName,
-    });
-
-    this.clusterArn = props.clusterArn !== undefined ? props.clusterArn : Stack.of(this).formatArn({
-      service: 'ecs',
-      resource: 'cluster',
-      resourceName: props.clusterName,
+      resourceName: props.clusterName
     });
 
     let i = 1;

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
@@ -69,29 +69,38 @@ export class InstanceDrainHook extends cdk.Construct {
       heartbeatTimeoutSec: drainTimeSeconds,
     });
 
-    // FIXME: These should probably be restricted usefully in some way, but I don't exactly
-    // know how.
+    // Describe actions cannot be restricted and restrict the CompleteLifecycleAction to the ASG arn
+    // https://docs.aws.amazon.com/autoscaling/ec2/userguide/control-access-using-iam.html
     fn.addToRolePolicy(new iam.PolicyStatement()
       .addActions(
-        'autoscaling:CompleteLifecycleAction',
         'ec2:DescribeInstances',
         'ec2:DescribeInstanceAttribute',
         'ec2:DescribeInstanceStatus',
-        'ec2:DescribeHosts',
+        'ec2:DescribeHosts'
       )
       .addAllResources());
 
-    // FIXME: These should be restricted to the ECS cluster probably, but I don't exactly
-    // know how.
+    // Restrict to the ASG
+    fn.addToRolePolicy(new iam.PolicyStatement()
+      .addActions(
+        'autoscaling:CompleteLifecycleAction'
+      )
+      .addResource(props.autoScalingGroup.autoScalingGroupArn));
+
+    fn.addToRolePolicy(new iam.PolicyStatement()
+      .addActions(
+        'ecs:DescribeContainerInstances',
+        'ecs:DescribeTasks')
+      .addAllResources());
+
+    // Restrict to the ECS Cluster
     fn.addToRolePolicy(new iam.PolicyStatement()
       .addActions(
         'ecs:ListContainerInstances',
         'ecs:SubmitContainerStateChange',
         'ecs:SubmitTaskStateChange',
-        'ecs:DescribeContainerInstances',
         'ecs:UpdateContainerInstancesState',
-        'ecs:ListTasks',
-        'ecs:DescribeTasks')
-      .addAllResources());
+        'ecs:ListTasks')
+      .addResource(props.cluster.clusterArn));
   }
 }
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
@@ -559,7 +559,6 @@
           "Statement": [
             {
               "Action": [
-                "autoscaling:CompleteLifecycleAction",
                 "ec2:DescribeInstances",
                 "ec2:DescribeInstanceAttribute",
                 "ec2:DescribeInstanceStatus",
@@ -568,18 +567,56 @@
               "Effect": "Allow",
               "Resource": "*"
             },
+            {
+              "Action": "autoscaling:CompleteLifecycleAction",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::Join": [
+                  "",
+                  [
+                    "arn:",
+                    {
+                      "Ref": "AWS::Partition"
+                    },
+                    ":autoscaling:",
+                    {
+                      "Ref": "AWS::Region"
+                    },
+                    ":",
+                    {
+                      "Ref": "AWS::AccountId"
+                    },
+                    ":autoScalingGroup:*:autoScalingGroupName/",
+                    {
+                      "Ref": "EcsClusterDefaultAutoScalingGroupASGC1A785DB"
+                    }
+                  ]
+                ]
+              }
+            },
+            {
+              "Action": [
+                "ecs:DescribeContainerInstances",
+                "ecs:DescribeTasks"
+              ],
+              "Effect": "Allow",
+              "Resource": "*"
+            },
             {
               "Action": [
                 "ecs:ListContainerInstances",
                 "ecs:SubmitContainerStateChange",
                 "ecs:SubmitTaskStateChange",
-                "ecs:DescribeContainerInstances",
                 "ecs:UpdateContainerInstancesState",
-                "ecs:ListTasks",
-                "ecs:DescribeTasks"
+                "ecs:ListTasks"
               ],
               "Effect": "Allow",
-              "Resource": "*"
+              "Resource": {
+                "Fn::GetAtt": [
+                  "EcsCluster97242B84",
+                  "Arn"
+                ]
+              }
             }
           ],
           "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
@@ -580,7 +580,6 @@
           "Statement": [
             {
               "Action": [
-                "autoscaling:CompleteLifecycleAction",
                 "ec2:DescribeInstances",
                 "ec2:DescribeInstanceAttribute",
                 "ec2:DescribeInstanceStatus",
@@ -589,18 +588,56 @@
               "Effect": "Allow",
               "Resource": "*"
             },
+            {
+              "Action": "autoscaling:CompleteLifecycleAction",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::Join": [
+                  "",
+                  [
+                    "arn:",
+                    {
+                      "Ref": "AWS::Partition"
+                    },
+                    ":autoscaling:",
+                    {
+                      "Ref": "AWS::Region"
+                    },
+                    ":",
+                    {
+                      "Ref": "AWS::AccountId"
+                    },
+                    ":autoScalingGroup:*:autoScalingGroupName/",
+                    {
+                      "Ref": "EcsClusterDefaultAutoScalingGroupASGC1A785DB"
+                    }
+                  ]
+                ]
+              }
+            },
+            {
+              "Action": [
+                "ecs:DescribeContainerInstances",
+                "ecs:DescribeTasks"
+              ],
+              "Effect": "Allow",
+              "Resource": "*"
+            },
             {
               "Action": [
                 "ecs:ListContainerInstances",
                 "ecs:SubmitContainerStateChange",
                 "ecs:SubmitTaskStateChange",
-                "ecs:DescribeContainerInstances",
                 "ecs:UpdateContainerInstancesState",
-                "ecs:ListTasks",
-                "ecs:DescribeTasks"
+                "ecs:ListTasks"
               ],
               "Effect": "Allow",
-              "Resource": "*"
+              "Resource": {
+                "Fn::GetAtt": [
+                  "EcsCluster97242B84",
+                  "Arn"
+                ]
+              }
             }
           ],
           "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
@@ -559,7 +559,6 @@
           "Statement": [
             {
               "Action": [
-                "autoscaling:CompleteLifecycleAction",
                 "ec2:DescribeInstances",
                 "ec2:DescribeInstanceAttribute",
                 "ec2:DescribeInstanceStatus",
@@ -568,18 +567,56 @@
               "Effect": "Allow",
               "Resource": "*"
             },
+            {
+              "Action": "autoscaling:CompleteLifecycleAction",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::Join": [
+                  "",
+                  [
+                    "arn:",
+                    {
+                      "Ref": "AWS::Partition"
+                    },
+                    ":autoscaling:",
+                    {
+                      "Ref": "AWS::Region"
+                    },
+                    ":",
+                    {
+                      "Ref": "AWS::AccountId"
+                    },
+                    ":autoScalingGroup:*:autoScalingGroupName/",
+                    {
+                      "Ref": "EcsClusterDefaultAutoScalingGroupASGC1A785DB"
+                    }
+                  ]
+                ]
+              }
+            },
+            {
+              "Action": [
+                "ecs:DescribeContainerInstances",
+                "ecs:DescribeTasks"
+              ],
+              "Effect": "Allow",
+              "Resource": "*"
+            },
             {
               "Action": [
                 "ecs:ListContainerInstances",
                 "ecs:SubmitContainerStateChange",
                 "ecs:SubmitTaskStateChange",
-                "ecs:DescribeContainerInstances",
                 "ecs:UpdateContainerInstancesState",
-                "ecs:ListTasks",
-                "ecs:DescribeTasks"
+                "ecs:ListTasks"
               ],
               "Effect": "Allow",
-              "Resource": "*"
+              "Resource": {
+                "Fn::GetAtt": [
+                  "EcsCluster97242B84",
+                  "Arn"
+                ]
+              }
             }
           ],
           "Version": "2012-10-17"