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.

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

fix(events): imported ECS Task Definition cannot be used as target #13293

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
108 changes: 108 additions & 0 deletions packages/@aws-cdk/aws-ecs/lib/base/_imported-task-definition.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { IRole } from '@aws-cdk/aws-iam';
import { Construct } from 'constructs';
import { IEc2TaskDefinition } from '../ec2/ec2-task-definition';
import { IFargateTaskDefinition } from '../fargate/fargate-task-definition';
import { Compatibility, NetworkMode, isEc2Compatible, isFargateCompatible } from './task-definition';
import { Resource } from '@aws-cdk/core';

/**
* The properties of ImportedTaskDefinition
*/
export interface ImportedTaskDefinitionProps {
/**
* The arn of the task definition
*/
readonly taskDefinitionArn: string;

/**
* What launch types this task definition should be compatible with.
*
* @default Compatibility.EC2_AND_FARGATE
*/
readonly compatibility?: Compatibility;

/**
* The networking mode to use for the containers in the task.
*
* @default Network mode cannot be provided to the imported task.
*/
readonly networkMode?: NetworkMode;

/**
* The name of the IAM role that grants containers in the task permission to call AWS APIs on your behalf.
*
* @default Permissions cannot be granted to the imported task.
*/
readonly taskRole?: IRole;
}

/**
* Task definition reference of an imported task
*/
export class ImportedTaskDefinition extends Resource implements IEc2TaskDefinition, IFargateTaskDefinition {
/**
* What launch types this task definition should be compatible with.
*/
readonly compatibility: Compatibility;

/**
* ARN of this task definition
*/
readonly taskDefinitionArn: string;

/**
* Execution role for this task definition
*/
readonly executionRole?: IRole = undefined;

/**
* The networking mode to use for the containers in the task.
*/
readonly _networkMode?: NetworkMode;

/**
* The name of the IAM role that grants containers in the task permission to call AWS APIs on your behalf.
*/
readonly _taskRole?: IRole;

constructor(scope: Construct, id: string, props: ImportedTaskDefinitionProps) {
super(scope, id);

this.compatibility = props.compatibility ?? Compatibility.EC2_AND_FARGATE;
this.taskDefinitionArn = props.taskDefinitionArn;
this._taskRole = props.taskRole;
this._networkMode = props.networkMode;
}

public get networkMode(): NetworkMode {
if (this._networkMode == undefined) {
throw new Error('This operation requires the networkMode in ImportedTaskDefinition to be defined. ' +
'Add the \'networkMode\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition');
} else {
return this._networkMode;
}
}

public get taskRole(): IRole {
if (this._taskRole == undefined) {
throw new Error('This operation requires the taskRole in ImportedTaskDefinition to be defined. ' +
'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition');
} else {
return this._taskRole;
}
}

/**
* Return true if the task definition can be run on an EC2 cluster
*/
public get isEc2Compatible(): boolean {
return isEc2Compatible(this.compatibility);
}

/**
* Return true if the task definition can be run on a Fargate cluster
*/
public get isFargateCompatible(): boolean {
return isFargateCompatible(this.compatibility);
}
}
47 changes: 12 additions & 35 deletions packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { FirelensLogRouter, FirelensLogRouterDefinitionOptions, FirelensLogRoute
import { AwsLogDriver } from '../log-drivers/aws-log-driver';
import { PlacementConstraint } from '../placement';
import { ProxyConfiguration } from '../proxy-configuration/proxy-configuration';
import { ImportedTaskDefinition } from './_imported-task-definition';

/**
* The interface for all task definitions.
Expand Down Expand Up @@ -197,14 +198,14 @@ export interface CommonTaskDefinitionAttributes {
/**
* The networking mode to use for the containers in the task.
*
* @default NetworkMode.BRIDGE
* @default Network mode cannot be provided to the imported task.
*/
readonly networkMode?: NetworkMode;

/**
* The name of the IAM role that grants containers in the task permission to call AWS APIs on your behalf.
*
* @default undefined.
* @default Permissions cannot be granted to the imported task.
*/
readonly taskRole?: iam.IRole;
}
Expand All @@ -213,13 +214,6 @@ export interface CommonTaskDefinitionAttributes {
* A reference to an existing task definition
*/
export interface TaskDefinitionAttributes extends CommonTaskDefinitionAttributes {
/**
* Execution role for this task definition
*
* @default: undefined
*/
readonly executionRole?: iam.IRole;

/**
* What launch types this task definition should be compatible with.
*
Expand Down Expand Up @@ -262,36 +256,19 @@ export class TaskDefinition extends TaskDefinitionBase {
* The task will have a compatibility of EC2+Fargate.
*/
public static fromTaskDefinitionArn(scope: Construct, id: string, taskDefinitionArn: string): ITaskDefinition {
return TaskDefinition.fromTaskDefinitionAttributes(scope, id, { taskDefinitionArn: taskDefinitionArn });
return new ImportedTaskDefinition(scope, id, { taskDefinitionArn: taskDefinitionArn });
}

/**
* Create a task definition from a task definition reference
*/
public static fromTaskDefinitionAttributes(scope: Construct, id: string, attrs: TaskDefinitionAttributes): ITaskDefinition {
class Import extends TaskDefinitionBase {
public readonly taskDefinitionArn = attrs.taskDefinitionArn;
public readonly compatibility = attrs.compatibility ?? Compatibility.EC2_AND_FARGATE;
public readonly executionRole = attrs.executionRole;

public get networkMode(): NetworkMode {
if (attrs.networkMode == undefined) {
throw new Error('NetworkMode is available only if it is given when importing the TaskDefinition.');
} else {
return attrs.networkMode;
}
}

public get taskRole(): iam.IRole {
if (attrs.taskRole == undefined) {
throw new Error('TaskRole is available only if it is given when importing the TaskDefinition.');
} else {
return attrs.taskRole;
}
}
}

return new Import(scope, id);
return new ImportedTaskDefinition(scope, id, {
taskDefinitionArn: attrs.taskDefinitionArn,
compatibility: attrs.compatibility,
networkMode: attrs.networkMode,
taskRole: attrs.taskRole,
});
}

/**
Expand Down Expand Up @@ -968,13 +945,13 @@ export interface ITaskDefinitionExtension {
/**
* Return true if the given task definition can be run on an EC2 cluster
*/
function isEc2Compatible(compatibility: Compatibility): boolean {
export function isEc2Compatible(compatibility: Compatibility): boolean {
return [Compatibility.EC2, Compatibility.EC2_AND_FARGATE].includes(compatibility);
}

/**
* Return true if the given task definition can be run on a Fargate cluster
*/
function isFargateCompatible(compatibility: Compatibility): boolean {
export function isFargateCompatible(compatibility: Compatibility): boolean {
return [Compatibility.FARGATE, Compatibility.EC2_AND_FARGATE].includes(compatibility);
}
39 changes: 10 additions & 29 deletions packages/@aws-cdk/aws-ecs/lib/ec2/ec2-task-definition.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import * as iam from '@aws-cdk/aws-iam';
import { Resource } from '@aws-cdk/core';
import { Construct } from 'constructs';
import {
CommonTaskDefinitionAttributes,
Expand All @@ -12,6 +10,7 @@ import {
TaskDefinition,
} from '../base/task-definition';
import { PlacementConstraint } from '../placement';
import { ImportedTaskDefinition } from '../base/_imported-task-definition';

/**
* The properties for a task definition run on an EC2 cluster.
Expand Down Expand Up @@ -79,9 +78,9 @@ export class Ec2TaskDefinition extends TaskDefinition implements IEc2TaskDefinit
* Imports a task definition from the specified task definition ARN.
*/
public static fromEc2TaskDefinitionArn(scope: Construct, id: string, ec2TaskDefinitionArn: string): IEc2TaskDefinition {
return Ec2TaskDefinition.fromEc2TaskDefinitionAttributes(
scope, id, { taskDefinitionArn: ec2TaskDefinitionArn },
);
return new ImportedTaskDefinition(scope, id, {
taskDefinitionArn: ec2TaskDefinitionArn,
});
}

/**
Expand All @@ -92,30 +91,12 @@ export class Ec2TaskDefinition extends TaskDefinition implements IEc2TaskDefinit
id: string,
attrs: Ec2TaskDefinitionAttributes,
): IEc2TaskDefinition {
class Import extends Resource implements IEc2TaskDefinition {
public readonly taskDefinitionArn = attrs.taskDefinitionArn;
public readonly compatibility = Compatibility.EC2;
public readonly isEc2Compatible = true;
public readonly isFargateCompatible = false;

public get networkMode(): NetworkMode {
if (attrs.networkMode == undefined) {
throw new Error('NetworkMode is available only if it is given when importing the Ec2 TaskDefinition.');
} else {
return attrs.networkMode;
}
}

public get taskRole(): iam.IRole {
if (attrs.taskRole == undefined) {
throw new Error('TaskRole is available only if it is given when importing the Ec2 TaskDefinition.');
} else {
return attrs.taskRole;
}
}
}

return new Import(scope, id);
return new ImportedTaskDefinition(scope, id, {
taskDefinitionArn: attrs.taskDefinitionArn,
compatibility: Compatibility.EC2,
networkMode: attrs.networkMode,
taskRole: attrs.taskRole,
});
}

/**
Expand Down
38 changes: 9 additions & 29 deletions packages/@aws-cdk/aws-ecs/lib/fargate/fargate-task-definition.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as iam from '@aws-cdk/aws-iam';
import { Resource, Tokenization } from '@aws-cdk/core';
import { Tokenization } from '@aws-cdk/core';
import { Construct } from 'constructs';
import {
CommonTaskDefinitionAttributes,
Expand All @@ -9,6 +8,7 @@ import {
NetworkMode,
TaskDefinition,
} from '../base/task-definition';
import { ImportedTaskDefinition } from '../base/_imported-task-definition';

/**
* The properties for a task definition.
Expand Down Expand Up @@ -77,9 +77,7 @@ export class FargateTaskDefinition extends TaskDefinition implements IFargateTas
* Imports a task definition from the specified task definition ARN.
*/
public static fromFargateTaskDefinitionArn(scope: Construct, id: string, fargateTaskDefinitionArn: string): IFargateTaskDefinition {
return FargateTaskDefinition.fromFargateTaskDefinitionAttributes(
scope, id, { taskDefinitionArn: fargateTaskDefinitionArn },
);
return new ImportedTaskDefinition(scope, id, { taskDefinitionArn: fargateTaskDefinitionArn });
}

/**
Expand All @@ -90,30 +88,12 @@ export class FargateTaskDefinition extends TaskDefinition implements IFargateTas
id: string,
attrs: FargateTaskDefinitionAttributes,
): IFargateTaskDefinition {
class Import extends Resource implements IFargateTaskDefinition {
public readonly taskDefinitionArn = attrs.taskDefinitionArn;
public readonly compatibility = Compatibility.FARGATE;
public readonly isEc2Compatible = false;
public readonly isFargateCompatible = true;

public get networkMode(): NetworkMode {
if (attrs.networkMode == undefined) {
throw new Error('NetworkMode is available only if it is given when importing the Fargate TaskDefinition.');
} else {
return attrs.networkMode;
}
}

public get taskRole(): iam.IRole {
if (attrs.taskRole == undefined) {
throw new Error('TaskRole is available only if it is given when importing the Fargate TaskDefinition.');
} else {
return attrs.taskRole;
}
}
}

return new Import(scope, id);
return new ImportedTaskDefinition(scope, id, {
taskDefinitionArn: attrs.taskDefinitionArn,
compatibility: Compatibility.FARGATE,
networkMode: attrs.networkMode,
taskRole: attrs.taskRole,
});
}

/**
Expand Down
10 changes: 6 additions & 4 deletions packages/@aws-cdk/aws-ecs/test/ec2/ec2-task-definition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,9 @@ describe('ec2 task definition', () => {
});

// THEN
expect(() => taskDefinition.networkMode).toThrow(/NetworkMode is available only if it is given when importing the Ec2 TaskDefinition./);
expect(() => taskDefinition.networkMode).toThrow(
'This operation requires the networkMode in ImportedTaskDefinition to be defined. ' +
'Add the \'networkMode\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition');
});

test('returns an Ec2 TaskDefinition that will throw an error when trying to access its yet to defined taskRole', () => {
Expand All @@ -1130,7 +1132,9 @@ describe('ec2 task definition', () => {
});

// THEN
expect(() => taskDefinition.taskRole).toThrow(/TaskRole is available only if it is given when importing the Ec2 TaskDefinition./);
expect(() => { taskDefinition.taskRole; }).toThrow(
'This operation requires the taskRole in ImportedTaskDefinition to be defined. ' +
'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition');
});
});

Expand All @@ -1153,7 +1157,5 @@ describe('ec2 task definition', () => {
expect(() => {
new ecs.Ec2TaskDefinition(stack, 'TaskDef', { networkMode: ecs.NetworkMode.BRIDGE, proxyConfiguration });
}).toThrow(/ProxyConfiguration can only be used with AwsVpc network mode, got: bridge/);


});
});
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ nodeunitShim({
// THEN
test.throws(() => {
taskDefinition.networkMode;
}, /NetworkMode is available only if it is given when importing the Fargate TaskDefinition./);
}, 'This operation requires the networkMode in ImportedTaskDefinition to be defined. ' +
'Add the \'networkMode\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition');

test.done();
},
Expand All @@ -193,7 +194,8 @@ nodeunitShim({
// THEN
test.throws(() => {
taskDefinition.taskRole;
}, /TaskRole is available only if it is given when importing the Fargate TaskDefinition./);
}, 'This operation requires the taskRole in ImportedTaskDefinition to be defined. ' +
'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition');

test.done();
},
Expand Down
Loading