Skip to content

Commit

Permalink
fix(stepfunctions-tasks): EcsRunTask containerOverrides throws if con…
Browse files Browse the repository at this point in the history
…tainer name doesn't match construct ID (#15190)

 EcsRunTask, container override is now using using method `findContainer` to check for container in TaskDefinition

fixes #15171 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Cruaier authored Jun 24, 2021
1 parent d0c9602 commit 5f59787
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ export class TaskDefinition extends TaskDefinitionBase {
/**
* Returns the container that match the provided containerName.
*/
private findContainer(containerName: string): ContainerDefinition | undefined {
public findContainer(containerName: string): ContainerDefinition | undefined {
return this.containers.find(c => c.containerName === containerName);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export class EcsRunTask extends sfn.TaskStateBase implements ec2.IConnectable {
for (const override of this.props.containerOverrides ?? []) {
const name = override.containerDefinition.containerName;
if (!cdk.Token.isUnresolved(name)) {
const cont = this.props.taskDefinition.node.tryFindChild(name);
const cont = this.props.taskDefinition.findContainer(name);
if (!cont) {
throw new Error(`Overrides mention container with name '${name}', but no such container in task definition`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,80 @@ test('Cannot create a Fargate task without a default container', () => {
).toThrowError(/must have at least one essential container/);
});

test('Cannot override container definitions when container is not in task definition', () => {
const taskDefinitionA = new ecs.TaskDefinition(stack, 'TaskDefinitionA', {
memoryMiB: '512',
cpu: '256',
compatibility: ecs.Compatibility.FARGATE,
});
taskDefinitionA.addContainer('TheContainerA', {
image: ecs.ContainerImage.fromRegistry('foo/bar'),
memoryLimitMiB: 256,
});

const taskDefinitionB = new ecs.TaskDefinition(stack, 'TaskDefinitionB', {
memoryMiB: '512',
cpu: '256',
compatibility: ecs.Compatibility.FARGATE,
});
const containerDefinitionB = taskDefinitionB.addContainer('TheContainerB', {
image: ecs.ContainerImage.fromRegistry('foo/bar'),
memoryLimitMiB: 256,
});

expect(() =>
new tasks.EcsRunTask(stack, 'task', {
cluster,
taskDefinition: taskDefinitionA,
containerOverrides: [
{
containerDefinition: containerDefinitionB,
environment: [{ name: 'SOME_KEY', value: sfn.JsonPath.stringAt('$.SomeKey') }],
},
],
launchTarget: new tasks.EcsFargateLaunchTarget(),
}).toStateJson(),
).toThrowError(/no such container in task definition/);
});

test('Running a task with container override and container has explicitly set a container name', () => {
const taskDefinition = new ecs.TaskDefinition(stack, 'TD', {
memoryMiB: '512',
cpu: '256',
compatibility: ecs.Compatibility.FARGATE,
});
const containerDefinition = taskDefinition.addContainer('TheContainer', {
containerName: 'ExplicitContainerName',
image: ecs.ContainerImage.fromRegistry('foo/bar'),
memoryLimitMiB: 256,
});

expect(stack.resolve(
new tasks.EcsRunTask(stack, 'task', {
cluster,
taskDefinition,
containerOverrides: [
{
containerDefinition,
environment: [{ name: 'SOME_KEY', value: sfn.JsonPath.stringAt('$.SomeKey') }],
},
],
launchTarget: new tasks.EcsFargateLaunchTarget(),
}).toStateJson())).toHaveProperty('Parameters.Overrides', {
ContainerOverrides: [
{
Environment: [
{
Name: 'SOME_KEY',
'Value.$': '$.SomeKey',
},
],
Name: 'ExplicitContainerName',
},
],
});
});

test('Running a Fargate Task', () => {
const taskDefinition = new ecs.TaskDefinition(stack, 'TD', {
memoryMiB: '512',
Expand Down

0 comments on commit 5f59787

Please sign in to comment.