Skip to content

Commit

Permalink
fix(ecs): EC2 metadata access is blocked when using EC2 capacity prov…
Browse files Browse the repository at this point in the history
…ider for autoscaling (#28437)

### Why is this needed?
When adding a auto scaling group as a capacity provider using `Cluster.addAsgCapacityProvider` and when the task definition being run uses the AWS_VPC network mode, it results in the metadata service at `169.254.169.254` being blocked . This is a security best practice as detailed [here](https://docs.aws.amazon.com/AmazonECS/latest/bestpracticesguide/security-iam-roles.html). This practice is implemented [here](https://github.com/aws/aws-cdk/blame/2d9de189e583186f2b77386ae4fcfff42c864568/packages/aws-cdk-lib/aws-ecs/lib/cluster.ts#L502-L504). However by doing this, some applications such as those raised in #28270 as well as the aws-otel package will not be able to source for the AWS region and thus, cause the application to crash and exit. 

### What does it implement?
This PR add an override to the addContainer method when using the Ec2TaskDefinition to add in the AWS_REGION environment variable to the container if the network mode is set as AWS_VPC. The region is sourced by referencing to the stack which includes this construct at synth time.This environment variable is only required in the EC2 Capacity Provider mode and not in Fargate as this issue of not being able to source for the region on startup is only present when using the EC2 Capacity Provider with the AWS_VPC networking mode. The initial issue addresses this during the `addAsgCapacityProvider` action which targets the cluster. However, we cannot mutate the task definition at that point in time thus, this change addresses it when the task definition is actually added to a service that meets all the requirements whereby the failure to source for region will occur.

Updated the relevant integration tests to reflect the new environment variable being created alongside user-defined environment variables.

Closes #28270

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
juinquok authored Jan 18, 2024
1 parent 426d889 commit 30a0d33
Show file tree
Hide file tree
Showing 8 changed files with 196 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -919,12 +919,28 @@
"Properties": {
"ContainerDefinitions": [
{
"Environment": [
{
"Name": "AWS_REGION",
"Value": {
"Ref": "AWS::Region"
}
}
],
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"Memory": 256,
"Name": "web"
},
{
"Environment": [
{
"Name": "AWS_REGION",
"Value": {
"Ref": "AWS::Region"
}
}
],
"Essential": true,
"Image": "envoyproxy/envoy:v1.16.2",
"Memory": 256,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,14 @@
"Name": "log_router"
},
{
"Environment": [
{
"Name": "AWS_REGION",
"Value": {
"Ref": "AWS::Region"
}
}
],
"Essential": true,
"Image": "nginx",
"LogConfiguration": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,18 @@
"Properties": {
"ContainerDefinitions": [
{
"Environment": [
{
"Name": "SOME_VARIABLE",
"Value": "value"
},
{
"Name": "AWS_REGION",
"Value": {
"Ref": "AWS::Region"
}
}
],
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"Memory": 256,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef', {
const container = taskDefinition.addContainer('web', {
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
memoryLimitMiB: 256,
environment: {
SOME_VARIABLE: 'value',
},
});

container.addPortMappings({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,14 @@
"Properties": {
"ContainerDefinitions": [
{
"Environment": [
{
"Name": "AWS_REGION",
"Value": {
"Ref": "AWS::Region"
}
}
],
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"Memory": 256,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,14 @@
"Properties": {
"ContainerDefinitions": [
{
"Environment": [
{
"Name": "AWS_REGION",
"Value": {
"Ref": "AWS::Region"
}
}
],
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"Memory": 256,
Expand Down
23 changes: 21 additions & 2 deletions packages/aws-cdk-lib/aws-ecs/lib/ec2/ec2-task-definition.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import { Construct } from 'constructs';
import { Stack } from '../../../core';
import { ImportedTaskDefinition } from '../base/_imported-task-definition';
import {
CommonTaskDefinitionAttributes,
CommonTaskDefinitionProps,
Compatibility,
InferenceAccelerator,
IpcMode,
ITaskDefinition,
NetworkMode,
PidMode,
TaskDefinition,
InferenceAccelerator,
} from '../base/task-definition';
import { ContainerDefinition, ContainerDefinitionOptions } from '../container-definition';
import { PlacementConstraint } from '../placement';

/**
Expand Down Expand Up @@ -83,7 +85,6 @@ export interface Ec2TaskDefinitionAttributes extends CommonTaskDefinitionAttribu
* @resource AWS::ECS::TaskDefinition
*/
export class Ec2TaskDefinition extends TaskDefinition implements IEc2TaskDefinition {

/**
* Imports a task definition from the specified task definition ARN.
*/
Expand Down Expand Up @@ -146,4 +147,22 @@ export class Ec2TaskDefinition extends TaskDefinition implements IEc2TaskDefinit
// Validate the placement constraints
Ec2TaskDefinition.validatePlacementConstraints(props.placementConstraints ?? []);
}

/**
* Tasks running in AWSVPC networking mode requires an additional environment variable for the region to be sourced.
* This override adds in the additional environment variable as required
*/
override addContainer(id: string, props: ContainerDefinitionOptions): ContainerDefinition {
if (this.networkMode === NetworkMode.AWS_VPC) {
return super.addContainer(id, {
...props,
environment: {
...props.environment,
AWS_REGION: Stack.of(this).region,
},
});
}
// If network mode is not AWSVPC, then just add the container as normal
return super.addContainer(id, props);
}
}
120 changes: 120 additions & 0 deletions packages/aws-cdk-lib/aws-ecs/test/ec2/ec2-task-definition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,126 @@ describe('ec2 task definition', () => {
}],
});
});

test('correctly sets env variables when using EC2 capacity provider with AWSVPC mode - with no other user-defined env variables', () => {
// GIVEN AWS-VPC network mode
const stack = new cdk.Stack();
const taskDefiniton = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef', {
networkMode: ecs.NetworkMode.AWS_VPC,
});
taskDefiniton.addContainer('some-container', {
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
memoryLimitMiB: 512,
});

// THEN it should include the AWS_REGION env variable - when no user defined env variables are provided
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
NetworkMode: ecs.NetworkMode.AWS_VPC,
ContainerDefinitions: [{
Name: 'some-container',
Image: 'amazon/amazon-ecs-sample',
Memory: 512,
Environment: [{
Name: 'AWS_REGION',
Value: {
Ref: 'AWS::Region',
},
}],
}],
});
});

test('correctly sets env variables when using EC2 capacity provider with AWSVPC mode - with other user-defined env variables', () => {
// GIVEN AWS-VPC network mode
const stack = new cdk.Stack();
const taskDefiniton = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef', {
networkMode: ecs.NetworkMode.AWS_VPC,
});
taskDefiniton.addContainer('some-container', {
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
memoryLimitMiB: 512,
environment: {
SOME_VARIABLE: 'some-value',
},
});

// THEN it should include the AWS_REGION env variable
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
NetworkMode: ecs.NetworkMode.AWS_VPC,
ContainerDefinitions: [{
Name: 'some-container',
Image: 'amazon/amazon-ecs-sample',
Memory: 512,
Environment: [{
Name: 'SOME_VARIABLE',
Value: 'some-value',
}, {
Name: 'AWS_REGION',
Value: {
Ref: 'AWS::Region',
},
}],
}],
});
});

test('correctly sets env variables when using EC2 capacity provider with HOST mode', () => {
// GIVEN HOST network mode
const stack = new cdk.Stack();
const taskDefiniton = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef', {
networkMode: ecs.NetworkMode.HOST,
});
taskDefiniton.addContainer('some-container', {
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
memoryLimitMiB: 512,
environment: {
SOME_VARIABLE: 'some-value',
},
});

// THEN it should not include the AWS_REGION env variable
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
NetworkMode: ecs.NetworkMode.HOST,
ContainerDefinitions: [{
Name: 'some-container',
Image: 'amazon/amazon-ecs-sample',
Memory: 512,
Environment: [{
Name: 'SOME_VARIABLE',
Value: 'some-value',
}],
}],
});
});

test('correctly sets env variables when using EC2 capacity provider with BRIDGE mode', () => {
// GIVEN HOST network mode
const stack = new cdk.Stack();
const taskDefiniton = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef', {
networkMode: ecs.NetworkMode.BRIDGE,
});
taskDefiniton.addContainer('some-container', {
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
memoryLimitMiB: 512,
environment: {
SOME_VARIABLE: 'some-value',
},
});

// THEN it should not include the AWS_REGION env variable
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
NetworkMode: ecs.NetworkMode.BRIDGE,
ContainerDefinitions: [{
Name: 'some-container',
Image: 'amazon/amazon-ecs-sample',
Memory: 512,
Environment: [{
Name: 'SOME_VARIABLE',
Value: 'some-value',
}],
}],
});
});
});

describe('setting inferenceAccelerators', () => {
Expand Down

0 comments on commit 30a0d33

Please sign in to comment.