fix: no extra port mapping in main container when nlb target is a sidecar

[Lou1415926]

Fix #3768

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

[efekarakus]

:ack: this looks good, what about this feedback from the issue:

It appears that the cf partial below is not sidecar aware, and assumes that the nlb target is always the main workload container.

To me it sounded like the NLB target group is pointing to the wrong port?

[Lou1415926]

With a manifest like this:

nlb:
  port: 80/tcp
  target_container: envoy

image:
  location: blah
  port: 10000

sidecars:
  envoy:
    # envoy admin port
    port: 9901
    image: public.ecr.aws/hashicorp/envoy-alpine:latest

The NLB target group's target port will be 9901, not 10000, with the current implementation and as expected!

My understanding for the quoted comment was specifically for the code:

copilot-cli/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml

Line 33 in 2ac6b01

{{if ne .NLB.Listener.TargetPort .NLB.MainContainerPort}} {{/*No need to add additional port if the target port is the same as image port*/}}

The (bugged) code assume the target container is main container, and add the target port to main container's port mapping if it's not the same as the container port. It fails to take into consideration the possibility that the target container could be a sidecar, in which case, it does not need to add the extra port mapping to the main container.

[efekarakus]

Thanks for the explanation, totally makes sense!