Support exposing UDP ports when using Network Load Balancer

[dannyrandall]

From #1783 (comment).

Copilot currently supports exposing TCP ports through an NLB, and it would be nice to support UDP ports. Additionally, it would be nice to specify a range of UDP ports to expose through the NLB instead of listing a range one-by-one.

[h5aaimtron]

My use cases is specific to a voice chat application (WebRTC) that requires a range of UDP ports to be opened to facilitate the SFU (Selective Forwarding Unit) running. I could see this being true beyond my specific use-case as other chat systems, games, etc. could utilize port ranges.

[tjhorner]

I am also interested in this functionality, so I went ahead and added udp to the list of valid NLB protocols in this file and passed port: 80/udp to nlb in my service manifest.

The deployment appears to have worked:

However, I haven't written any tests to determine if there are side effects I should be aware of (i.e., health check fail?). But it does work in this limited scenario.

[dannyrandall]

@tjhorner thanks for that!☺️ I think you're pretty much right that the health check is the only thing missing - We'd love a PR if you're interested! I think the only other things you'll have to update are:

• Manifest configuration changes:
  • Change NLBHealthCheckArgs.Port to a string https://github.com/aws/copilot-cli/blob/mainline/internal/pkg/manifest/svc.go#L378-L389
  • Update the validation logic for NLBHealthCheckArgs.Port to call validateNLBPort() for the Port field https://github.com/aws/copilot-cli/blob/mainline/internal/pkg/manifest/validate.go#L907-L912
• CFN Template data changes:
  • Add a Protocol field to NLBHealthCheck https://github.com/aws/copilot-cli/blob/mainline/internal/pkg/template/workload.go#L418-L425
  • Update the NLBHealthCheck transformer https://github.com/aws/copilot-cli/blob/mainline/internal/pkg/deploy/cloudformation/stack/transformers.go#L397-L416 to set the Protocol field using manifest.ParsePortMapping(). Similar logic here, for the NLB port.
• CFN Template changes:
  • The security group protocol to be based on the Protocol field: https://github.com/aws/copilot-cli/blob/mainline/internal/pkg/template/templates/workloads/partials/cf/nlb.yml#L86. Also need to update the logic to add a rule if the target protocol and health check protocol differ.

[tjhorner]

Thank you for the pointers! I'm not super familiar with this codebase so they are very helpful. I'll take a crack at a PR for this sometime next week, would love to help get this out the door.

[dannyrandall]

Awesome! Feel free to ask any questions here😊.

[dannyrandall]

@tjhorner I'm so sorry!😅 I just realized I didn't think enough about UDP health checks - NLB doesn't support UDP health checks, which makes total sense haha. So I think all of the changes I mentioned to support UDP health checks are totally unnecessary!

[tjhorner]

I made some progress on this today; it turns out most everything works out of the box with the one change I made, except that I also needed to update the exposed port on the ECS task to listen on the correct protocol. It was hardcoded to tcp previously:

copilot-cli/internal/pkg/manifest/svc.go

Lines 525 to 531 in 65dca5f

	return []ExposedPort{
	{
	Port: targetPort,
	Protocol: "tcp",
	ContainerName: targetContainer,
	},
	}, nil

I'll do some more manual testing then try to write some automated tests then open the PR!

[tjhorner]

It also looks like some additional security group rules will be required in order for UDP traffic to reach the task containers since NLB UDP listeners preserve the source IP (this was a pleasant surprise!), so the existing NLB->Task ingress rules don't work as expected.

How would you suggest this be done? Correct me if I'm wrong, but I don't see any existing parameters to take inspiration/best practice from. I think providing a CIDR block of allowed source IPs alongside the port definition would work well. Something like this:

nlb:
  port: 8080/udp
  allow_from: 0.0.0.0/0

It can alternatively be provided via a YAML patch or addon, but this doesn't seem ideal.

Since the NLB listens on the public Internet I think 0.0.0.0/0 would actually be a reasonable default here. It's safe enough since the task containers don't have a public IP, so they can't be accessed directly anyway, only through the NLB. (I just double-checked and "Auto-assign public IP" actually enabled with no way to turn it off, oops. So not sure what to do in this case 🤔)

For completeness, here are the YAML patches I'm using to add Internet access to the security group:

- op: add
  path: /Resources/NLBSecurityGroup/Properties/SecurityGroupIngress/-
  value:
    CidrIp: 0.0.0.0/0
    Description: Ingress to allow all RADIUS UDP ports from Internet
    FromPort: 1812
    ToPort: 1812
    IpProtocol: UDP

Previously I was also disabling AssignPublicIp with a YAML patch but since I don't have NAT Gateway enabled in my configuration (I am importing an existing VPC/subnets) it failed to retrieve the image etc.

[Lou1415926]

Related: #3281, #2918 (comment)

[Lou1415926]

Hey @tjhorner 👋🏼 !! Sorry for jumping in late! I've seen your PR - thanks a bunch for the awesome contribution. However before we review your PR, here are some information that I'd like to gather from you. Then, we can work towards a design for this feature, and finally finish up the PR. Does that sound good to you 👀 ?

We removed UDP from our NLB release mostly because of the issue that you've found - source IP preservation + auto assigned public IPs which potentially exposes your tasks' public IPs.

This prompted us the unanswered question of how users like to use UDP - do you need UDP for your public-facing service (Load-Balanced Web Service) or for your internal service (Backend Service)?

From what you've said here, it looks like your UDP is for public usage. Can I have a high-level understanding of what that service do?

In addition, is UDP the only type of traffic that your service is listening to?