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

The port_mappings variable needs to include the "name" parameter to support Service Connect integration. #162

Closed
ckonefke opened this issue May 24, 2023 · 0 comments · Fixed by #166

Comments

@ckonefke
Copy link

Describe the Feature

I was trying to use a container definition created with this module with an ECS fargate service that includes Service Connect for cross service communication. The Service Connect setup requires a reference to a named port mapping from the container that will be used. From what I can see, there's currently no way to define the name parameter for a containers port mapping.

Expected Behavior

I attempted to add the "name" parameter to the port mapping with a block like this...
port_mappings = [ { name = "http-80-tcp" containerPort = 80 hostPort = 80 protocol = "tcp" }, ]

The mapping was successfully setup, but the "name" parameter was ignored since it's missing from the variable definition.
https://github.com/cloudposse/terraform-aws-ecs-container-definition/blob/main/variables.tf#L29

Use Case

I need a named port mapping to successfully configure Service Connect in our environment. I'm currently trying to find a workaround, but this should be a fairly easy fix.

Describe Ideal Solution

The "name" parameter would be added as an option entry in the port_mappings setting. That way the container would have a properly named port map that could be referred to by the Service Connect setup.

Alternatives Considered

I believe that it's just a matter of adding the "name" parameter as an optional setting for the port_mappings variable
variable "port_mappings" { type = list(object({ name = optional(string) containerPort = number hostPort = number protocol = string }))

Additional Context

No response

max-lobur pushed a commit that referenced this issue Sep 26, 2023
* fix: add port mapping name

* docs: update documentation

---------

Co-authored-by: Dan Miller <miller0daniel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant