Skip to content

Commit 5446ded

Browse files
fix(ecs): remove unnecessary error when adding volume to external task definition (#19774)
fixes #19259 ---- ### All Submissions: * [ ] 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 * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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 2550589 commit 5446ded

File tree

2 files changed

+17
-12
lines changed

2 files changed

+17
-12
lines changed

packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts

-8
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
ITaskDefinition,
99
NetworkMode,
1010
TaskDefinition,
11-
Volume,
1211
} from '../base/task-definition';
1312

1413
/**
@@ -75,13 +74,6 @@ export class ExternalTaskDefinition extends TaskDefinition implements IExternalT
7574
});
7675
}
7776

78-
/**
79-
* Overridden method to throw error, as volumes are not supported for external task definitions
80-
*/
81-
public addVolume(_volume: Volume) {
82-
throw new Error('External task definitions doesnt support volumes');
83-
}
84-
8577
/**
8678
* Overriden method to throw error as interface accelerators are not supported for external tasks
8779
*/

packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts

+17-4
Original file line numberDiff line numberDiff line change
@@ -601,17 +601,30 @@ describe('external task definition', () => {
601601
Annotations.fromStack(stack).hasWarning('/Default/ExternalTaskDef/web', "Proper policies need to be attached before pulling from ECR repository, or use 'fromEcrRepository'.");
602602
});
603603

604-
test('correctly sets volumes from', () => {
604+
test('correctly sets volumes', () => {
605+
// GIVEN
605606
const stack = new cdk.Stack();
606607
const taskDefinition = new ecs.ExternalTaskDefinition(stack, 'ExternalTaskDef', {});
607608

608-
// THEN
609-
expect(() => taskDefinition.addVolume({
609+
// WHEN
610+
taskDefinition.addVolume({
610611
host: {
611612
sourcePath: '/tmp/cache',
612613
},
613614
name: 'scratch',
614-
})).toThrow('External task definitions doesnt support volumes' );
615+
});
616+
617+
// THEN
618+
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
619+
Volumes: [
620+
{
621+
Host: {
622+
SourcePath: '/tmp/cache',
623+
},
624+
Name: 'scratch',
625+
},
626+
],
627+
});
615628
});
616629

617630
test('error when interferenceAccelerators set', () => {

0 commit comments

Comments
 (0)