aws / aws/aws-cdk

(ecs-patterns): NetworkLoadBalancedServiceBase does not support container port mapping using taskDefinition

Open
#29,041 1 comment 1 reaction 0 assignees View on GitHub
@aws-cdk/aws-ecs-patterns effort/medium feature-request p2
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
2d 3h
Merged PRs (30d)
83

Description

### Describe the bug

When initializing the NetworkBalancedServiceBase with a [`taskDefinition`](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-ecs-patterns/lib/base/fargate-service-base.ts#L4-L11), there is no way to set the target group port to a value other than 80.

### Expected Behavior

I expect the target group port of the load balancer to be set to the `taskDefinition`'s default container container port.

### Current Behavior

The target group port is 80, different from the task definition's default container port mapping configuration.

### Reproduction Steps

```ts
import { Stack, StackProps } from 'aws-cdk-lib';
import { Vpc } from 'aws-cdk-lib/aws-ec2';
import { Cluster, ContainerImage } from 'aws-cdk-lib/aws-ecs';
import { NetworkLoadBalancedFargateService } from 'aws-cdk-lib/aws-ecs-patterns';
import { Construct } from 'constructs';

export class ElbTestStack extends Stack {
constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);
const vpc = new Vpc(this, 'vpc', {});

const fargateCluster = new Cluster(this, 'Cluster', {
clusterName: 'test',
vpc: vpc,
enableFargateCapacityProviders: true,
containerInsights: true
})

this.taskDefinition = new FargateTaskDefinition(this, 'TaskDefinition', {
cpu: 0.5,
memoryLimitMiB: 1,
});
this.taskDefinition.addContainer('service', {
image: ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
portMappings: [{containerPort: 81}]
})

const loadBalancedFargateService = new NetworkLoadBalancedFargateService(this, 'NLBService', {
cluster: fargateCluster,
memoryLimitMiB: 1024,
cpu: 512,
taskDefinition: this.taskDefinition,
listenerPort: 8181,
});
}
}
```

### Possible Solution

Currently, the logic to set the target port is as follows
```ts
const targetProps = {
port: props.taskImageOptions?.containerPort ?? 80,
};
```
https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts#L361

It needs to be changed to something like this:
```ts
const targetProps = {
port: props.taskImageOptions?.containerPort ?? props.taskDefinition.defaultContainer.portMappings[0].containerPort ?? 80
}
```

### Additional Information/Context

_No response_

### CDK CLI Version

2.100.0

### Framework Version

_No response_

### Node.js Version

18

### OS

AL2

### Language

TypeScript

### Language Version

_No response_

### Other information

_No response_

Contributor guide

Open the contributing guide

Research direction

Start in packages/aws-cdk-lib/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts around line 361, where the target port is selected. Reproduce the issue with a task definition whose container mapping uses port 81 and no taskImageOptions containerPort. Done means the target group uses the task definition's default container port instead of 80 in that case.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
cloud, infrastructure
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.