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

aws-ecs-pattern: containerPort should be PortMapping[] instead of just number #28202

Open
1 of 2 tasks
jolo-dev opened this issue Nov 30, 2023 · 3 comments
Open
1 of 2 tasks
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@jolo-dev
Copy link
Contributor

Describe the feature

When using for example ApplicationLoadBalancedFargateService you can pass in taskImageOptions.
However, the containerPort is from type number but should be a PortMapping[] instead.
I ran into the issue that I needed that because my container exposed multiple Ports thus I had to create a taskDefinition due to that.

Use Case

Container, which exposes multiple ports. This is possible through taskDefinition but would bloat the codebase.

Proposed Solution

Instead of containerPort: number, it should be containerPorts: PortMapping[].

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.110.0

Environment details (OS name and version, etc.)

MacOS 14.1

@jolo-dev jolo-dev added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 30, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Nov 30, 2023
@khushail khushail self-assigned this Dec 1, 2023
@khushail khushail added needs-review p2 and removed needs-triage This issue or PR still needs to be triaged. needs-review labels Dec 2, 2023
@khushail
Copy link
Contributor

khushail commented Dec 6, 2023

Thanks @jolo-dev for the inputs and pull request.

@khushail khushail added the effort/small Small work item – less than a day of effort label Dec 6, 2023
@khushail khushail removed their assignment Dec 6, 2023
@paulosergio-jnr
Copy link

Wouldn't this occur in a breaking change as we will have a parameter type changed?

Could we instead add another property as @jolo-dev mentioned, containerPorts: PortMapping[], and evaluating which will be passed to Cfn on building time? (both fields provided = error)

Another alternative would be marking the current containerPort as Deprecated and create the new field as mentioned.

WDYT?

@jolo-dev
Copy link
Contributor Author

Hey @paulosergio-jnr,
Yeah you're right. It would be a breaking change. I think making it Deprecated and creating a new field would be the cleaner approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

3 participants