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

feat(ecs): container port ranges in port mappings #26692

Merged
merged 23 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
aa97179
Add support for container port ranges for ECS
ste93cry Jul 11, 2023
bbdea4b
Update the README
ste93cry Aug 9, 2023
21315e3
Fix unit tests and rework port mapping validation logic
ste93cry Aug 10, 2023
38d3a39
Fix wrong expectations in unit tests
ste93cry Aug 10, 2023
ea90bd8
Add integration test
ste93cry Aug 10, 2023
2251c5d
Update the README
ste93cry Aug 14, 2023
d64f317
Reword the error message and add missing unit test
ste93cry Aug 14, 2023
9e234ca
Add validation of string format for `containerPortRange` prop
ste93cry Aug 14, 2023
84f0755
Make `containerPort` `non-nullable` again and handle BC
ste93cry Aug 17, 2023
e0e9044
Improve the docblock for `containerPort` and `containerPortRange`
ste93cry Aug 17, 2023
2406904
Add test for case where `containerPortRange` value is not concrete
ste93cry Aug 17, 2023
078eec3
Fix rendering of L1 construct for port mapping
ste93cry Aug 17, 2023
b6ea1a4
Fix typehint typo in README
ste93cry Aug 18, 2023
77d073e
Inline class property to local scoped variable
ste93cry Aug 18, 2023
539f810
Use `test()` instead of `describe()` when possible
ste93cry Aug 18, 2023
c4e3278
Improve usage documentation for `containerPort` and `containerPortRange`
ste93cry Aug 18, 2023
24fb02f
Rename the `CONTAINER_PORT_UNSET_VALUE` and move it
ste93cry Aug 18, 2023
f5089a5
Update typo in README (again)
ste93cry Aug 18, 2023
236bb9f
Merge branch 'main' into add-container-port-range-support-for-ecs
rix0rrr Aug 18, 2023
5544967
Update aws-ecs-container-port-range.template.json
rix0rrr Aug 21, 2023
2b4aadd
Merge branch 'main' into add-container-port-range-support-for-ecs
rix0rrr Aug 22, 2023
895afcc
Merge branch 'main' into add-container-port-range-support-for-ecs
rix0rrr Aug 22, 2023
aa7cb6d
Merge branch 'main' into add-container-port-range-support-for-ecs
mergify[bot] Aug 22, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,15 @@ export class NetworkMultipleTargetGroupsEc2Service extends NetworkMultipleTarget
this.addPortMappingForTargets(this.taskDefinition.defaultContainer, props.targetGroups);
this.targetGroup = this.registerECSTargets(this.service, this.taskDefinition.defaultContainer, props.targetGroups);
} else {
const containerPort = this.taskDefinition.defaultContainer.portMappings[0].containerPort;

if (!containerPort) {
throw new Error('You must specify a containerPort');
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
}

this.targetGroup = this.listener.addTargets('ECS', {
targets: [this.service],
port: this.taskDefinition.defaultContainer.portMappings[0].containerPort,
port: containerPort,
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,15 @@ export class NetworkMultipleTargetGroupsFargateService extends NetworkMultipleTa
this.addPortMappingForTargets(this.taskDefinition.defaultContainer, props.targetGroups);
this.targetGroup = this.registerECSTargets(this.service, this.taskDefinition.defaultContainer, props.targetGroups);
} else {
const containerPort = this.taskDefinition.defaultContainer.portMappings[0].containerPort;

if (!containerPort) {
throw new Error('The first port mapping added to the essential container must expose a single port');
}

this.targetGroup = this.listener.addTargets('ECS', {
targets: [this.service],
port: this.taskDefinition.defaultContainer.portMappings[0].containerPort,
port: containerPort,
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -837,4 +837,27 @@ describe('When Network Load Balancer', () => {
},
});
});

test('Fargate multinetworkloadbalanced construct errors when container port range is set for essential container', () => {
// GIVEN
const stack = new Stack();
const vpc = new Vpc(stack, 'VPC');
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
const taskDefinition = new ecs.FargateTaskDefinition(stack, 'FargateTaskDef');

taskDefinition.addContainer('MainContainer', {
image: ecs.ContainerImage.fromRegistry('test'),
portMappings: [{
containerPortRange: '8080-8081',
}],
});

// THEN
expect(() => {
new NetworkMultipleTargetGroupsFargateService(stack, 'Service', {
cluster,
taskDefinition,
});
}).toThrow('The first port mapping added to the essential container must expose a single port');
});
});
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-ecs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ declare const taskDefinition: ecs.TaskDefinition;
taskDefinition.addContainer("WebContainer", {
image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
memoryLimitMiB: 1024,
portMappings: [{ containerPort: 3000 }],
portMappings: [{ containerPort: 3000 }, { containerPortRange: '3000-4000' }],
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
});
```

Expand Down
6 changes: 3 additions & 3 deletions packages/aws-cdk-lib/aws-ecs/lib/base/base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1029,14 +1029,14 @@ export abstract class BaseService extends Resource
return {
attachToApplicationTargetGroup(targetGroup: elbv2.ApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
targetGroup.registerConnectable(self, self.taskDefinition._portRangeFromPortMapping(target.portMapping));
return self.attachToELBv2(targetGroup, target.containerName, target.portMapping.containerPort);
return self.attachToELBv2(targetGroup, target.containerName, target.portMapping.containerPort!);
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
},
attachToNetworkTargetGroup(targetGroup: elbv2.NetworkTargetGroup): elbv2.LoadBalancerTargetProps {
return self.attachToELBv2(targetGroup, target.containerName, target.portMapping.containerPort);
return self.attachToELBv2(targetGroup, target.containerName, target.portMapping.containerPort!);
},
connections,
attachToClassicLB(loadBalancer: elb.LoadBalancer): void {
return self.attachToELB(loadBalancer, target.containerName, target.portMapping.containerPort);
return self.attachToELB(loadBalancer, target.containerName, target.portMapping.containerPort!);
},
};
}
Expand Down
6 changes: 5 additions & 1 deletion packages/aws-cdk-lib/aws-ecs/lib/base/task-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,11 @@ export class TaskDefinition extends TaskDefinitionBase {
if (this.networkMode === NetworkMode.BRIDGE || this.networkMode === NetworkMode.NAT) {
return EPHEMERAL_PORT_RANGE;
}
return portMapping.protocol === Protocol.UDP ? ec2.Port.udp(portMapping.containerPort) : ec2.Port.tcp(portMapping.containerPort);
if (portMapping.containerPort !== undefined) {
return portMapping.protocol === Protocol.UDP ? ec2.Port.udp(portMapping.containerPort) : ec2.Port.tcp(portMapping.containerPort);
}
const [startPort, endPort] = portMapping.containerPortRange!.split('-', 2).map(v => +v);
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
return portMapping.protocol === Protocol.UDP ? ec2.Port.udpRange(startPort, endPort) : ec2.Port.tcpRange(startPort, endPort);
}

/**
Expand Down
55 changes: 48 additions & 7 deletions packages/aws-cdk-lib/aws-ecs/lib/container-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -707,16 +707,18 @@ export class ContainerDefinition extends Construct {
}

/**
* Set HostPort to 0 When netowork mode is Brdige
* This method sets the host port to 0 if the network mode is Bridge and neither
* the host port nor the container port range is already set.
*/
private addHostPortIfNeeded(pm: PortMapping) :PortMapping {
const newPM = {
if (this.taskDefinition.networkMode !== NetworkMode.BRIDGE || pm.hostPort !== undefined || pm.containerPortRange !== undefined) {
return pm;
}

return {
...pm,
hostPort: 0,
};
if (this.taskDefinition.networkMode !== NetworkMode.BRIDGE) return newPM;
if (pm.hostPort !== undefined) return newPM;
newPM.hostPort = 0;
return newPM;
}

/**
Expand Down Expand Up @@ -750,6 +752,11 @@ export class ContainerDefinition extends Construct {
if (this.taskDefinition.networkMode === NetworkMode.BRIDGE) {
return 0;
}

if (defaultPortMapping.containerPort === undefined) {
throw new Error(`The first port mapping of the container ${this.containerName} must expose a single port.`);
}

return defaultPortMapping.containerPort;
}

Expand All @@ -761,6 +768,11 @@ export class ContainerDefinition extends Construct {
throw new Error(`Container ${this.containerName} hasn't defined any ports. Call addPortMappings().`);
}
const defaultPortMapping = this.portMappings[0];

if (defaultPortMapping.containerPort === undefined) {
throw new Error(`The first port mapping of the container ${this.containerName} must expose a single port.`);
}

return defaultPortMapping.containerPort;
}

Expand Down Expand Up @@ -1073,7 +1085,21 @@ export interface PortMapping {
* For more information, see hostPort.
* Port mappings that are automatically assigned in this way do not count toward the 100 reserved ports limit of a container instance.
*/
readonly containerPort: number;
readonly containerPort?: number;

/**
* The port number range on the container that's bound to the dynamically mapped host port range.
*
* The following rules apply when you specify a `containerPortRange`:
*
* - You must use either the `bridge` network mode or the `awsvpc` network mode.
* - The container instance must have at least version 1.67.0 of the container agent and at least version 1.67.0-1 of the `ecs-init` package
* - You can specify a maximum of 100 port ranges per container.
* - A port can only be included in one port mapping per container.
* - You cannot specify overlapping port ranges.
* - The first port in the range must be less than last port in the range.
*/
readonly containerPortRange?: string;

/**
* The port number on the container instance to reserve for your container.
Expand Down Expand Up @@ -1150,6 +1176,20 @@ export class PortMap {
const pm = this.portmapping;
throw new Error(`Host port (${pm.hostPort}) must be left out or equal to container port ${pm.containerPort} for network mode ${this.networkmode}`);
}

if (this.portmapping.containerPort === undefined && this.portmapping.containerPortRange === undefined) {
throw new Error('Either "containerPort" or "containerPortRange" must be set.');
}

if (this.portmapping.containerPortRange !== undefined) {
if (this.portmapping.hostPort !== undefined || this.portmapping.containerPort !== undefined) {
throw new Error('Cannot set "hostPort" or "containerPort" while using the port range for the container.');
}

if (this.networkmode !== NetworkMode.BRIDGE && this.networkmode !== NetworkMode.AWS_VPC) {
throw new Error('Either AwsVpc or Bridge network mode is required to set a port range for the container.');
}
}
}

private isvalidPortName(): boolean {
Expand Down Expand Up @@ -1272,6 +1312,7 @@ export class AppProtocol {
function renderPortMapping(pm: PortMapping): CfnTaskDefinition.PortMappingProperty {
return {
containerPort: pm.containerPort,
containerPortRange: pm.containerPortRange,
hostPort: pm.hostPort,
protocol: pm.protocol || Protocol.TCP,
appProtocol: pm.appProtocol?.value,
Expand Down
Loading