feat: add appProtocol and name to var.port_mappings

cloudposse / terraform-aws-ecs-container-definition

Terraform module to generate well-formed JSON documents (container definitions) that are passed to the aws_ecs_task_definition Terraform resource

https://cloudposse.com/accelerate

Apache License 2.0

339 stars 244 forks source link

feat: add appProtocol and name to var.port_mappings #169

Closed jtribble closed 11 months ago

jtribble commented 12 months ago

what

Adding missing fields to var.port_mappings (appProtocol, name, containerPortRange)

why

So that devs have access to all of the available fields: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_PortMapping.html

references

jamengual commented 12 months ago

/terratest

max-lobur commented 11 months ago

/terratest

max-lobur commented 11 months ago

Just merged earlier PR with the same feature https://github.com/cloudposse/terraform-aws-ecs-container-definition/pull/168

dudymas commented 11 months ago

the other PR had the additional requirement of setting the name field, which is used in the ECS Service Connect api

err... I see this PR also adds name now. Regardless, I hope that #168 accomplishes what folks needed. I saw some issues regarding ordering of the port mapping config. If that's still an issue we should use this PR to re-order them.

jtribble commented 11 months ago

Hey @max-lobur @dudymas! #168 added name and appProtocol to var.port_mappings, which is the main change I needed! In this PR I also added containerPortRange since I noticed it was missing.

Would you like me to refactor this PR to just contain the port range change? We would also want to make containerPort optional since it's optional in the AWS config.