Fix IPv6 DNAT/SNAT rule formatting in iptables and nftables backends#11960
Conversation
Both ip6tables and nftables require bracketed IPv6 addresses when a port is present (e.g., [2001:db8::1]:80). The DNAT and SNAT actions were using plain string formatting which produces invalid rules for IPv6. Use net.JoinHostPort() which correctly brackets IPv6 addresses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
marvin-tigera
added
release-note-required
hjiawei
added
docs-not-required
There was a problem hiding this comment.
Pull request overview
Fixes NAT rule rendering so IPv6 addresses are correctly bracketed when combined with ports/port-ranges, which is required by ip6tables and nftables.
Changes:
- Use
net.JoinHostPort()when rendering SNAT target with a port range inNATOutgoingChain. - Use
net.JoinHostPort()for DNAT rule fragments in both iptables and nftables backends.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| felix/rules/nat.go | Formats SNAT address+port-range using net.JoinHostPort() to ensure valid IPv6 output. |
| felix/nftables/actions.go | Formats DNAT addr:port using net.JoinHostPort() to ensure valid IPv6 output. |
| felix/iptables/actions.go | Formats DNAT addr:port using net.JoinHostPort() to ensure valid IPv6 output. |
| if r.NATPortRange.MaxPort > 0 { | ||
| toPorts := fmt.Sprintf("%d-%d", r.NATPortRange.MinPort, r.NATPortRange.MaxPort) | ||
| portRangeSnatRule := r.Masq(toPorts) | ||
| if r.NATOutgoingAddress != nil { | ||
| toAddress := fmt.Sprintf("%s:%s", r.NATOutgoingAddress.String(), toPorts) | ||
| toAddress := net.JoinHostPort(r.NATOutgoingAddress.String(), toPorts) | ||
| portRangeSnatRule = r.SNAT(toAddress) | ||
| } |
There was a problem hiding this comment.
The new JoinHostPort-based formatting is specifically to fix IPv6 when a port range is present, but the existing NATOutgoingChain unit tests only cover IPv4. Please add an IPv6 test case that sets NATOutgoingAddress to an IPv6 literal and NATPortRange to a non-zero range, and assert the rendered SNAT address is bracketed (e.g., "[v6addr]:min-max").
| func (g DNATAction) ToFragment(features *environment.Features) string { | ||
| if g.DestPort == 0 { | ||
| return fmt.Sprintf("--jump DNAT --to-destination %s", g.DestAddr) | ||
| } else { | ||
| return fmt.Sprintf("--jump DNAT --to-destination %s:%d", g.DestAddr, g.DestPort) | ||
| return fmt.Sprintf("--jump DNAT --to-destination %s", net.JoinHostPort(g.DestAddr, strconv.Itoa(int(g.DestPort)))) | ||
| } |
There was a problem hiding this comment.
There are existing action rendering tests in this package, but none validate DNAT rendering for IPv6 addresses with a port. Please add a test case using an IPv6 DestAddr and non-zero DestPort to assert the fragment uses bracketed IPv6 hostport formatting ("[v6addr]:port").
| func (g DNATAction) ToFragment(features *environment.Features) string { | ||
| if g.DestPort == 0 { | ||
| return fmt.Sprintf("dnat to %s", g.DestAddr) | ||
| } else { | ||
| return fmt.Sprintf("dnat to %s:%d", g.DestAddr, g.DestPort) | ||
| return fmt.Sprintf("dnat to %s", net.JoinHostPort(g.DestAddr, strconv.Itoa(int(g.DestPort)))) | ||
| } |
There was a problem hiding this comment.
There are existing action rendering tests in this package, but none validate DNAT rendering for IPv6 addresses with a port. Please add a test case using an IPv6 DestAddr and non-zero DestPort to assert the fragment uses bracketed IPv6 hostport formatting ("[v6addr]:port").
Both ip6tables and nftables require bracketed IPv6 addresses when a port is present (e.g., [2001:db8::1]:80). The DNAT and SNAT actions were using plain string formatting which produces invalid rules for IPv6. Use net.JoinHostPort() which correctly brackets IPv6 addresses.
Release note: