Fix A2A agent card advertising unroutable wildcard address#1685
Fix A2A agent card advertising unroutable wildcard address#1685dgageot merged 1 commit intodocker:mainfrom
Conversation
Replace wildcard listen addresses (::, 0.0.0.0) with localhost in the agent card URL so clients can actually reach the server. Also switch TCP listeners to tcp4 to avoid binding to IPv6 on dual-stack systems. Fixes docker#1679 Assisted-By: cagent
![docker-agent[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
The routableAddr() function is well-implemented with good test coverage. However, there's a concern about the tcp4 change in listen.go that removes IPv6 support, potentially breaking existing users without clear error messages.
See inline comment for details.
| func listenTCP(ctx context.Context, addr string) (net.Listener, error) { | ||
| var lc net.ListenConfig | ||
| return lc.Listen(ctx, "tcp", addr) | ||
| return lc.Listen(ctx, "tcp4", addr) |
There was a problem hiding this comment.
Potential Breaking Change: IPv6 Support Removed
Changing from "tcp" to "tcp4" restricts socket binding to IPv4 only. While this aligns with the PR description ("avoid binding to IPv6 on dual-stack systems"), it introduces a breaking change:
- Impact: If users pass IPv6 addresses like
[::1]:8080or[2001:db8::1]:8080, binding will fail with a cryptic error:"listen tcp4: address ::1: no suitable address found" - No validation: There's no check to prevent IPv6 addresses from being passed or to provide a clear error message
- Tests consider IPv6: The
routableAddr()tests include IPv6 cases ("[::]:8080"), suggesting IPv6 was considered in the design
Alternative approach: Consider keeping "tcp" for backward compatibility and let routableAddr() handle the agent card URL transformation. This would:
- Fix the wildcard address advertising issue (the original bug)
- Maintain IPv6 support for explicit addresses
- Avoid a breaking change
If tcp4 is the desired approach: Consider adding validation to detect IPv6 addresses and return a clear error message like "IPv6 addresses are not supported, please use IPv4".
Thoughts?
2 participants
Replace wildcard listen addresses (::, 0.0.0.0) with localhost in the agent card URL so clients can actually reach the server. Also switch TCP listeners to tcp4 to avoid binding to IPv6 on dual-stack systems.
Fixes #1679
Assisted-By: cagent