Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(stepfunctions-tasks): checking for task token in EcsRunTask containerOverrides causes memory explosion #15187

Merged
merged 4 commits into from
Jun 18, 2021
Merged

fix(stepfunctions-tasks): checking for task token in EcsRunTask containerOverrides causes memory explosion #15187

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c66da6
fix(stepfunctions-tasks): only check for task token in ECS runTask co…
dillon-odonovan Jun 18, 2021
ac9540a
provide more specific runTask task token error message
dillon-odonovan Jun 18, 2021
e316a6d
update runTask task token tests to be more concise
dillon-odonovan Jun 18, 2021
7540a1a
Merge branch 'master' into fix/run-ecs-task-check-only-container-over…
mergify[bot] Jun 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/@aws-cdk/aws-stepfunctions-tasks/lib/ecs/run-task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,11 @@ export class EcsRunTask extends sfn.TaskStateBase implements ec2.IConnectable {

validatePatternSupported(this.integrationPattern, EcsRunTask.SUPPORTED_INTEGRATION_PATTERNS);

if (this.integrationPattern === sfn.IntegrationPattern.WAIT_FOR_TASK_TOKEN && !sfn.FieldUtils.containsTaskToken(props.containerOverrides)) {
throw new Error('Task Token is required in `containerOverrides` for callback. Use Context.taskToken to set the token.');
if (
this.integrationPattern === sfn.IntegrationPattern.WAIT_FOR_TASK_TOKEN
&& !sfn.FieldUtils.containsTaskToken(props.containerOverrides?.map(x => x.environment))
BenChaimberg marked this conversation as resolved.
Show resolved Hide resolved
) {
throw new Error('Task Token is required in at least one `containerOverrides.environment` for callback. Use JsonPath.taskToken to set the token.');
}

if (!this.props.taskDefinition.defaultContainer) {
Expand Down
206 changes: 206 additions & 0 deletions packages/@aws-cdk/aws-stepfunctions-tasks/test/ecs/run-tasks.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,209 @@ test('Running an EC2 Task with overridden number values', () => {
Type: 'Task',
});
});

test('Cannot create an EC2 task with WAIT_FOR_TASK_TOKEN if no TaskToken provided', () => {
BenChaimberg marked this conversation as resolved.
Show resolved Hide resolved
const taskDefinition = new ecs.TaskDefinition(stack, 'TaskDefinition', {
compatibility: ecs.Compatibility.EC2,
});

const containerDefinition = taskDefinition.addContainer('ContainerDefinition', {
image: ecs.ContainerImage.fromRegistry('foo/bar'),
});

expect(() =>
new tasks.EcsRunTask(stack, 'RunTask', {
cluster,
containerOverrides: [
{
containerDefinition,
environment: [
{
name: 'Foo.$',
value: 'Bar',
BenChaimberg marked this conversation as resolved.
Show resolved Hide resolved
},
],
},
],
integrationPattern: sfn.IntegrationPattern.WAIT_FOR_TASK_TOKEN,
launchTarget: new tasks.EcsEc2LaunchTarget(),
taskDefinition,
}).toStateJson(),
BenChaimberg marked this conversation as resolved.
Show resolved Hide resolved
).toThrowError(/Task Token is required in at least one `containerOverrides.environment`/);
});

test('Running an EC2 Task with WAIT_FOR_TASK_TOKEN and task token in environment', () => {
const taskDefinition = new ecs.TaskDefinition(stack, 'TaskDefinition', {
compatibility: ecs.Compatibility.EC2,
});

const primaryContainerDef = taskDefinition.addContainer('PrimaryContainerDef', {
image: ecs.ContainerImage.fromRegistry('foo/primary'),
essential: true,
});

const sidecarContainerDef = taskDefinition.addContainer('SideCarContainerDef', {
image: ecs.ContainerImage.fromRegistry('foo/sidecar'),
essential: false,
});

// WHEN task token is provided in at least one containerOverride.environment (need not be essential container)
const runTask = new tasks.EcsRunTask(stack, 'RunTask', {
cluster,
containerOverrides: [
{
containerDefinition: primaryContainerDef,
environment: [
{
name: 'Foo.$',
value: 'Bar',
},
],
},
{
containerDefinition: sidecarContainerDef,
environment: [
{
name: 'TaskToken.$',
value: sfn.JsonPath.taskToken,
},
],
},
],
integrationPattern: sfn.IntegrationPattern.WAIT_FOR_TASK_TOKEN,
launchTarget: new tasks.EcsEc2LaunchTarget(),
taskDefinition,
});

// THEN
expect(stack.resolve(runTask.toStateJson())).toEqual({
BenChaimberg marked this conversation as resolved.
Show resolved Hide resolved
End: true,
Parameters: {
Cluster: { 'Fn::GetAtt': ['ClusterEB0386A7', 'Arn'] },
LaunchType: 'EC2',
Overrides: {
ContainerOverrides: [
{
Environment: [
{
'Name': 'Foo.$',
'Value': 'Bar',
},
],
Name: 'PrimaryContainerDef',
},
{
Environment: [
{
'Name': 'TaskToken.$',
'Value.$': '$$.Task.Token',
},
],
Name: 'SideCarContainerDef',
},
],
},
TaskDefinition: 'TaskDefinition',
},
Resource: {
'Fn::Join': [
'',
[
'arn:', {
'Ref': 'AWS::Partition',
},
':states:::ecs:runTask.waitForTaskToken',
],
],
},
Type: 'Task',
});
});

test('Running an EC2 Task with WAIT_FOR_TASK_TOKEN and task token in command', () => {
BenChaimberg marked this conversation as resolved.
Show resolved Hide resolved
const taskDefinition = new ecs.TaskDefinition(stack, 'TaskDefinition', {
compatibility: ecs.Compatibility.EC2,
});

const primaryContainerDef = taskDefinition.addContainer('PrimaryContainerDef', {
image: ecs.ContainerImage.fromRegistry('foo/primary'),
essential: true,
});

const sidecarContainerDef = taskDefinition.addContainer('SideCarContainerDef', {
image: ecs.ContainerImage.fromRegistry('foo/sidecar'),
essential: false,
});

// WHEN task token is provided in at least one containerOverride.environment (need not be essential container)
const runTask = new tasks.EcsRunTask(stack, 'RunTask', {
cluster,
containerOverrides: [
{
containerDefinition: primaryContainerDef,
environment: [
{
name: 'Foo.$',
value: 'Bar',
},
],
},
{
containerDefinition: sidecarContainerDef,
environment: [
{
name: 'TaskToken.$',
value: sfn.JsonPath.taskToken,
},
],
},
],
integrationPattern: sfn.IntegrationPattern.WAIT_FOR_TASK_TOKEN,
launchTarget: new tasks.EcsEc2LaunchTarget(),
taskDefinition,
});

// THEN
expect(stack.resolve(runTask.toStateJson())).toEqual({
End: true,
Parameters: {
Cluster: { 'Fn::GetAtt': ['ClusterEB0386A7', 'Arn'] },
LaunchType: 'EC2',
Overrides: {
ContainerOverrides: [
{
Environment: [
{
'Name': 'Foo.$',
'Value': 'Bar',
},
],
Name: 'PrimaryContainerDef',
},
{
Environment: [
{
'Name': 'TaskToken.$',
'Value.$': '$$.Task.Token',
},
],
Name: 'SideCarContainerDef',
},
],
},
TaskDefinition: 'TaskDefinition',
},
Resource: {
'Fn::Join': [
'',
[
'arn:', {
'Ref': 'AWS::Partition',
},
':states:::ecs:runTask.waitForTaskToken',
],
],
},
Type: 'Task',
});
});