From 13645122aa595848d891a53f36c2cde346e50e60 Mon Sep 17 00:00:00 2001 From: daschaa Date: Fri, 27 May 2022 14:05:14 +0200 Subject: [PATCH 1/5] :adhesive_bandage: Adds functionality for canContainersAccessInstanceRole --- packages/@aws-cdk/aws-ecs/lib/cluster.ts | 14 +++- .../@aws-cdk/aws-ecs/test/cluster.test.ts | 76 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/cluster.ts b/packages/@aws-cdk/aws-ecs/lib/cluster.ts index 8e48e2be59cec..f13c5ecaddb0d 100644 --- a/packages/@aws-cdk/aws-ecs/lib/cluster.ts +++ b/packages/@aws-cdk/aws-ecs/lib/cluster.ts @@ -366,7 +366,10 @@ export class Cluster extends Resource implements ICluster { } this._hasEc2Capacity = true; this.configureAutoScalingGroup(provider.autoScalingGroup, { - ...options, + ...{ + ...options, + canContainersAccessInstanceRole: provider.canContainersAccessInstanceRole || options.canContainersAccessInstanceRole, + }, machineImageType: provider.machineImageType, // Don't enable the instance-draining lifecycle hook if managed termination protection is enabled taskDrainTime: provider.enableManagedTerminationProtection ? Duration.seconds(0) : options.taskDrainTime, @@ -1109,6 +1112,13 @@ export class AsgCapacityProvider extends CoreConstruct { */ readonly enableManagedTerminationProtection?: boolean; + /** + * Specifies whether the containers can access the container instance role. + * + * @default false + */ + readonly canContainersAccessInstanceRole?: boolean; + constructor(scope: Construct, id: string, props: AsgCapacityProviderProps) { super(scope, id); @@ -1116,6 +1126,8 @@ export class AsgCapacityProvider extends CoreConstruct { this.machineImageType = props.machineImageType ?? MachineImageType.AMAZON_LINUX_2; + this.canContainersAccessInstanceRole = props.canContainersAccessInstanceRole; + this.enableManagedTerminationProtection = props.enableManagedTerminationProtection === undefined ? true : props.enableManagedTerminationProtection; diff --git a/packages/@aws-cdk/aws-ecs/test/cluster.test.ts b/packages/@aws-cdk/aws-ecs/test/cluster.test.ts index d167c30989ded..c5fe19c504f83 100644 --- a/packages/@aws-cdk/aws-ecs/test/cluster.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/cluster.test.ts @@ -2306,3 +2306,79 @@ test('throws when ASG Capacity Provider with capacityProviderName starting with cluster.addAsgCapacityProvider(capacityProviderAl2); }).toThrow(/Invalid Capacity Provider Name: ecscp, If a name is specified, it cannot start with aws, ecs, or fargate./); }); + +describe('Accessing container instance role', function () { + + const addUserDataMock = jest.fn(); + const autoScalingGroup: autoscaling.AutoScalingGroup = { + addUserData: addUserDataMock, + addToRolePolicy: jest.fn(), + protectNewInstancesFromScaleIn: jest.fn(), + } as unknown as autoscaling.AutoScalingGroup; + + afterEach(() => { + addUserDataMock.mockClear(); + }); + + test('block ecs from accessing metadata service when canContainersAccessInstanceRole not set', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'test'); + const cluster = new ecs.Cluster(stack, 'EcsCluster'); + + // WHEN + + const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', { + autoScalingGroup: autoScalingGroup, + }); + + cluster.addAsgCapacityProvider(capacityProvider); + + // THEN + expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP'); + expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('sudo service iptables save'); + expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config'); + }); + + test('allow ecs accessing metadata service when canContainersAccessInstanceRole is set on addAsgCapacityProvider', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'test'); + const cluster = new ecs.Cluster(stack, 'EcsCluster'); + + // WHEN + const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', { + autoScalingGroup: autoScalingGroup, + }); + + cluster.addAsgCapacityProvider(capacityProvider, { + canContainersAccessInstanceRole: true, + }); + + // THEN + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP'); + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save'); + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config'); + }); + + test('allow ecs accessing metadata service when canContainersAccessInstanceRole is set on AsgCapacityProvider instantiation', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'test'); + const cluster = new ecs.Cluster(stack, 'EcsCluster'); + + // WHEN + const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', { + autoScalingGroup: autoScalingGroup, + canContainersAccessInstanceRole: true, + }); + + cluster.addAsgCapacityProvider(capacityProvider); + + // THEN + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP'); + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save'); + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config'); + }); +}); + From 990151440ad2f3c43da06b8af77d62712bce66c8 Mon Sep 17 00:00:00 2001 From: daschaa Date: Fri, 27 May 2022 14:14:08 +0200 Subject: [PATCH 2/5] :white_check_mark: Adds test for missing case --- .../@aws-cdk/aws-ecs/test/cluster.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/@aws-cdk/aws-ecs/test/cluster.test.ts b/packages/@aws-cdk/aws-ecs/test/cluster.test.ts index c5fe19c504f83..ec67dd7c5b0dc 100644 --- a/packages/@aws-cdk/aws-ecs/test/cluster.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/cluster.test.ts @@ -2380,5 +2380,27 @@ describe('Accessing container instance role', function () { expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save'); expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config'); }); + + test('allow ecs accessing metadata service when canContainersAccessInstanceRole is set on constructor and method', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'test'); + const cluster = new ecs.Cluster(stack, 'EcsCluster'); + + // WHEN + const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', { + autoScalingGroup: autoScalingGroup, + canContainersAccessInstanceRole: true, + }); + + cluster.addAsgCapacityProvider(capacityProvider, { + canContainersAccessInstanceRole: true, + }); + + // THEN + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP'); + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save'); + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config'); + }); }); From 0074a0b621408d9f9cc7c6231764d3e542827cc9 Mon Sep 17 00:00:00 2001 From: Joshua Weber <57131123+daschaa@users.noreply.github.com> Date: Fri, 27 May 2022 16:05:56 +0200 Subject: [PATCH 3/5] Update packages/@aws-cdk/aws-ecs/lib/cluster.ts Co-authored-by: Cory Hall <43035978+corymhall@users.noreply.github.com> --- packages/@aws-cdk/aws-ecs/lib/cluster.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/cluster.ts b/packages/@aws-cdk/aws-ecs/lib/cluster.ts index f13c5ecaddb0d..35579c992a662 100644 --- a/packages/@aws-cdk/aws-ecs/lib/cluster.ts +++ b/packages/@aws-cdk/aws-ecs/lib/cluster.ts @@ -368,7 +368,7 @@ export class Cluster extends Resource implements ICluster { this.configureAutoScalingGroup(provider.autoScalingGroup, { ...{ ...options, - canContainersAccessInstanceRole: provider.canContainersAccessInstanceRole || options.canContainersAccessInstanceRole, + canContainersAccessInstanceRole: provider.canContainersAccessInstanceRole ?? options.canContainersAccessInstanceRole, }, machineImageType: provider.machineImageType, // Don't enable the instance-draining lifecycle hook if managed termination protection is enabled From 809f85145bc346afb725a7928ddd46e6dc6b9e52 Mon Sep 17 00:00:00 2001 From: daschaa Date: Fri, 27 May 2022 16:18:13 +0200 Subject: [PATCH 4/5] :recycle: Adds changes due to review --- packages/@aws-cdk/aws-ecs/lib/cluster.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/cluster.ts b/packages/@aws-cdk/aws-ecs/lib/cluster.ts index 35579c992a662..188fa661944d3 100644 --- a/packages/@aws-cdk/aws-ecs/lib/cluster.ts +++ b/packages/@aws-cdk/aws-ecs/lib/cluster.ts @@ -366,13 +366,11 @@ export class Cluster extends Resource implements ICluster { } this._hasEc2Capacity = true; this.configureAutoScalingGroup(provider.autoScalingGroup, { - ...{ - ...options, - canContainersAccessInstanceRole: provider.canContainersAccessInstanceRole ?? options.canContainersAccessInstanceRole, - }, + ...options, machineImageType: provider.machineImageType, // Don't enable the instance-draining lifecycle hook if managed termination protection is enabled taskDrainTime: provider.enableManagedTerminationProtection ? Duration.seconds(0) : options.taskDrainTime, + canContainersAccessInstanceRole: options.canContainersAccessInstanceRole ?? provider.canContainersAccessInstanceRole, }); this._capacityProviderNames.push(provider.capacityProviderName); From 6c8c11cb7dab130d2ab58ed51d91fefee7766d73 Mon Sep 17 00:00:00 2001 From: daschaa Date: Fri, 27 May 2022 16:36:55 +0200 Subject: [PATCH 5/5] :white_check_mark: Adds test for missing cases --- .../@aws-cdk/aws-ecs/test/cluster.test.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/@aws-cdk/aws-ecs/test/cluster.test.ts b/packages/@aws-cdk/aws-ecs/test/cluster.test.ts index ec67dd7c5b0dc..45f9601728ef7 100644 --- a/packages/@aws-cdk/aws-ecs/test/cluster.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/cluster.test.ts @@ -2402,5 +2402,49 @@ describe('Accessing container instance role', function () { expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save'); expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config'); }); + + test('block ecs from accessing metadata service when canContainersAccessInstanceRole set on constructor and not set on method', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'test'); + const cluster = new ecs.Cluster(stack, 'EcsCluster'); + + // WHEN + const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', { + autoScalingGroup: autoScalingGroup, + canContainersAccessInstanceRole: true, + }); + + cluster.addAsgCapacityProvider(capacityProvider, { + canContainersAccessInstanceRole: false, + }); + + // THEN + expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP'); + expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('sudo service iptables save'); + expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config'); + }); + + test('allow ecs accessing metadata service when canContainersAccessInstanceRole is not set on constructor and set on method', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'test'); + const cluster = new ecs.Cluster(stack, 'EcsCluster'); + + // WHEN + const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', { + autoScalingGroup: autoScalingGroup, + canContainersAccessInstanceRole: false, + }); + + cluster.addAsgCapacityProvider(capacityProvider, { + canContainersAccessInstanceRole: true, + }); + + // THEN + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP'); + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save'); + expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config'); + }); });