Skip to content

Commit dacefd6

Browse files
authored
fix(ecs): canContainersAccessInstanceRole is ignored when passed in AsgCapacityProvider constructor (#20522)
Fixes #20293 When adding an AsgCapacityProvider the property `canContainersAccessInstanceRole` is only checked when passed in via the method `addAsgCapacityProvider`. It is ignored when passing the property via the instantiation of an AsgCapacityProvider. In this PR I added, that if either one way (method or constructor) has got the property set - it is respected in the outcome. For more details please see the issue #20293 I decided **not** to omit the property on the class level because it would bring in breaking changes. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent b7bc10c commit dacefd6

File tree

2 files changed

+152
-0
lines changed

2 files changed

+152
-0
lines changed

packages/@aws-cdk/aws-ecs/lib/cluster.ts

+10
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@ export class Cluster extends Resource implements ICluster {
370370
machineImageType: provider.machineImageType,
371371
// Don't enable the instance-draining lifecycle hook if managed termination protection is enabled
372372
taskDrainTime: provider.enableManagedTerminationProtection ? Duration.seconds(0) : options.taskDrainTime,
373+
canContainersAccessInstanceRole: options.canContainersAccessInstanceRole ?? provider.canContainersAccessInstanceRole,
373374
});
374375

375376
this._capacityProviderNames.push(provider.capacityProviderName);
@@ -1109,13 +1110,22 @@ export class AsgCapacityProvider extends CoreConstruct {
11091110
*/
11101111
readonly enableManagedTerminationProtection?: boolean;
11111112

1113+
/**
1114+
* Specifies whether the containers can access the container instance role.
1115+
*
1116+
* @default false
1117+
*/
1118+
readonly canContainersAccessInstanceRole?: boolean;
1119+
11121120
constructor(scope: Construct, id: string, props: AsgCapacityProviderProps) {
11131121
super(scope, id);
11141122

11151123
this.autoScalingGroup = props.autoScalingGroup as autoscaling.AutoScalingGroup;
11161124

11171125
this.machineImageType = props.machineImageType ?? MachineImageType.AMAZON_LINUX_2;
11181126

1127+
this.canContainersAccessInstanceRole = props.canContainersAccessInstanceRole;
1128+
11191129
this.enableManagedTerminationProtection =
11201130
props.enableManagedTerminationProtection === undefined ? true : props.enableManagedTerminationProtection;
11211131

packages/@aws-cdk/aws-ecs/test/cluster.test.ts

+142
Original file line numberDiff line numberDiff line change
@@ -2306,3 +2306,145 @@ test('throws when ASG Capacity Provider with capacityProviderName starting with
23062306
cluster.addAsgCapacityProvider(capacityProviderAl2);
23072307
}).toThrow(/Invalid Capacity Provider Name: ecscp, If a name is specified, it cannot start with aws, ecs, or fargate./);
23082308
});
2309+
2310+
describe('Accessing container instance role', function () {
2311+
2312+
const addUserDataMock = jest.fn();
2313+
const autoScalingGroup: autoscaling.AutoScalingGroup = {
2314+
addUserData: addUserDataMock,
2315+
addToRolePolicy: jest.fn(),
2316+
protectNewInstancesFromScaleIn: jest.fn(),
2317+
} as unknown as autoscaling.AutoScalingGroup;
2318+
2319+
afterEach(() => {
2320+
addUserDataMock.mockClear();
2321+
});
2322+
2323+
test('block ecs from accessing metadata service when canContainersAccessInstanceRole not set', () => {
2324+
// GIVEN
2325+
const app = new cdk.App();
2326+
const stack = new cdk.Stack(app, 'test');
2327+
const cluster = new ecs.Cluster(stack, 'EcsCluster');
2328+
2329+
// WHEN
2330+
2331+
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
2332+
autoScalingGroup: autoScalingGroup,
2333+
});
2334+
2335+
cluster.addAsgCapacityProvider(capacityProvider);
2336+
2337+
// THEN
2338+
expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP');
2339+
expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('sudo service iptables save');
2340+
expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config');
2341+
});
2342+
2343+
test('allow ecs accessing metadata service when canContainersAccessInstanceRole is set on addAsgCapacityProvider', () => {
2344+
// GIVEN
2345+
const app = new cdk.App();
2346+
const stack = new cdk.Stack(app, 'test');
2347+
const cluster = new ecs.Cluster(stack, 'EcsCluster');
2348+
2349+
// WHEN
2350+
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
2351+
autoScalingGroup: autoScalingGroup,
2352+
});
2353+
2354+
cluster.addAsgCapacityProvider(capacityProvider, {
2355+
canContainersAccessInstanceRole: true,
2356+
});
2357+
2358+
// THEN
2359+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP');
2360+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save');
2361+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config');
2362+
});
2363+
2364+
test('allow ecs accessing metadata service when canContainersAccessInstanceRole is set on AsgCapacityProvider instantiation', () => {
2365+
// GIVEN
2366+
const app = new cdk.App();
2367+
const stack = new cdk.Stack(app, 'test');
2368+
const cluster = new ecs.Cluster(stack, 'EcsCluster');
2369+
2370+
// WHEN
2371+
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
2372+
autoScalingGroup: autoScalingGroup,
2373+
canContainersAccessInstanceRole: true,
2374+
});
2375+
2376+
cluster.addAsgCapacityProvider(capacityProvider);
2377+
2378+
// THEN
2379+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP');
2380+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save');
2381+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config');
2382+
});
2383+
2384+
test('allow ecs accessing metadata service when canContainersAccessInstanceRole is set on constructor and method', () => {
2385+
// GIVEN
2386+
const app = new cdk.App();
2387+
const stack = new cdk.Stack(app, 'test');
2388+
const cluster = new ecs.Cluster(stack, 'EcsCluster');
2389+
2390+
// WHEN
2391+
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
2392+
autoScalingGroup: autoScalingGroup,
2393+
canContainersAccessInstanceRole: true,
2394+
});
2395+
2396+
cluster.addAsgCapacityProvider(capacityProvider, {
2397+
canContainersAccessInstanceRole: true,
2398+
});
2399+
2400+
// THEN
2401+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP');
2402+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save');
2403+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config');
2404+
});
2405+
2406+
test('block ecs from accessing metadata service when canContainersAccessInstanceRole set on constructor and not set on method', () => {
2407+
// GIVEN
2408+
const app = new cdk.App();
2409+
const stack = new cdk.Stack(app, 'test');
2410+
const cluster = new ecs.Cluster(stack, 'EcsCluster');
2411+
2412+
// WHEN
2413+
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
2414+
autoScalingGroup: autoScalingGroup,
2415+
canContainersAccessInstanceRole: true,
2416+
});
2417+
2418+
cluster.addAsgCapacityProvider(capacityProvider, {
2419+
canContainersAccessInstanceRole: false,
2420+
});
2421+
2422+
// THEN
2423+
expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP');
2424+
expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('sudo service iptables save');
2425+
expect(autoScalingGroup.addUserData).toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config');
2426+
});
2427+
2428+
test('allow ecs accessing metadata service when canContainersAccessInstanceRole is not set on constructor and set on method', () => {
2429+
// GIVEN
2430+
const app = new cdk.App();
2431+
const stack = new cdk.Stack(app, 'test');
2432+
const cluster = new ecs.Cluster(stack, 'EcsCluster');
2433+
2434+
// WHEN
2435+
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
2436+
autoScalingGroup: autoScalingGroup,
2437+
canContainersAccessInstanceRole: false,
2438+
});
2439+
2440+
cluster.addAsgCapacityProvider(capacityProvider, {
2441+
canContainersAccessInstanceRole: true,
2442+
});
2443+
2444+
// THEN
2445+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo iptables --insert FORWARD 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP');
2446+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('sudo service iptables save');
2447+
expect(autoScalingGroup.addUserData).not.toHaveBeenCalledWith('echo ECS_AWSVPC_BLOCK_IMDS=true >> /etc/ecs/ecs.config');
2448+
});
2449+
});
2450+

0 commit comments

Comments
 (0)