fix(udp): map udp ports correctly#3484
fix(udp): map udp ports correctly#3484blueprismo wants to merge 1 commit intotestcontainers:mainfrom
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbitBug Fixes
Tests
WalkthroughThe pull request adds UDP port binding support by normalizing empty HostPort values to "0" in the mergePortBindings function, triggering Docker to allocate random ports. A new test file validates UDP port allocation, connectivity, and the internal port binding logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lifecycle.go(1 hunks)udp_port_binding_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-29T15:08:18.694Z
Learnt from: mdelapenya
Repo: testcontainers/testcontainers-go PR: 3320
File: modules/artemis/artemis.go:98-103
Timestamp: 2025-09-29T15:08:18.694Z
Learning: In testcontainers-go, nat.Port is a type alias for string, so untyped string constants can be passed directly to functions expecting nat.Port (like wait.ForListeningPort) without explicit type conversion - the Go compiler handles the implicit conversion automatically.
Applied to files:
udp_port_binding_test.golifecycle.go
🧬 Code graph analysis (1)
udp_port_binding_test.go (2)
container.go (1)
ContainerRequest(131-171)generic.go (2)
GenericContainer(52-98)GenericContainerRequest(21-27)
🔇 Additional comments (1)
lifecycle.go (1)
617-625: HostPort normalization aligns with Docker expectations.Converting empty bindings to
"0"ensures Docker actually allocates a host port (especially for UDP) and keeps existing explicit mappings untouched. Nice catch.
| address := fmt.Sprintf("%s:%s", hostIP, mappedPort.Port()) | ||
| conn, err := net.DialTimeout("udp", address, 2*time.Second) |
There was a problem hiding this comment.
Use net.JoinHostPort so IPv6 hosts don’t break this test.
fmt.Sprintf("%s:%s", …) produces an invalid endpoint like ::1:55051 when Docker reports an IPv6 host, causing net.DialTimeout to fail. Please switch to net.JoinHostPort to generate a valid host:port string across IPv4 and IPv6.
Apply this diff:
- address := fmt.Sprintf("%s:%s", hostIP, mappedPort.Port())
+ address := net.JoinHostPort(hostIP, mappedPort.Port())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| address := fmt.Sprintf("%s:%s", hostIP, mappedPort.Port()) | |
| conn, err := net.DialTimeout("udp", address, 2*time.Second) | |
| address := net.JoinHostPort(hostIP, mappedPort.Port()) | |
| conn, err := net.DialTimeout("udp", address, 2*time.Second) |
🤖 Prompt for AI Agents
In udp_port_binding_test.go around lines 65 to 66, the test builds the address
with fmt.Sprintf("%s:%s", hostIP, mappedPort.Port()) which breaks for IPv6 hosts
(e.g., ::1:55051); replace that construction with net.JoinHostPort(hostIP,
mappedPort.Port()) so the host:port string is valid for both IPv4 and IPv6
before calling net.DialTimeout.
|
closing in favour of a new PR from a new branch != main |
2 participants
What does this PR do?
Fixes UDP port exposure.
Problem:
When using ExposedPorts: []string{"8080/udp"} in testcontainers-go, the MappedPort() function always returned "0/udp" instead of the actual mapped host port (e.g., "55051/udp"). This made it impossible to connect to UDP services running in containers.
Root Cause:
The issue was in the mergePortBindings() function in lifecycle.go. When nat.ParsePortSpecs() processes exposed port specifications like
8080/udp, it creates PortBinding structs with empty HostPort fields({HostIP: "", HostPort: ""})Docker interprets these differently:
Solution:
Modified mergePortBindings() to convert empty HostPort values to "0" for automatic port allocation:
Why is it important?
To support UDP port bindings in testcontainers.
Related issues
n/a
How to test this PR
I added a test scenario for udp testing