[client] Add IPv6 reverse DNS and host configurator support#5686
[client] Add IPv6 reverse DNS and host configurator support#5686lixmal wants to merge 7 commits intoclient-ipv6-ifacefrom
Conversation
📝 WalkthroughWalkthroughThis PR adds comprehensive IPv6 support to DNS PTR record generation and reverse zone management. Changes include IPv6-capable PTR creation using RFC 3596 nibble expansion (ip6.arpa), IPv6 address family detection across DNS backends (systemd, NetworkManager), and IPv6 upstream resolver support for iOS. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Engine as DNS Engine
participant CoreDNS as PTR/Zone Logic
participant Platform as Platform Backend
participant System as System DNS
App->>Engine: Generate DNS config
Engine->>CoreDNS: toDNSConfig (IPv4 + IPv6 networks)
alt IPv4 Address
CoreDNS->>CoreDNS: Create PTR with in-addr.arpa
Note over CoreDNS: Byte-based reverse
else IPv6 Address
CoreDNS->>CoreDNS: Create PTR with ip6.arpa<br/>(nibble-expanded)
Note over CoreDNS: Nibble-based reverse (RFC 3596)
end
CoreDNS-->>Engine: Config with reverse zones
Engine->>Platform: applyDNSConfig
alt systemd Backend
Platform->>Platform: Detect AF_INET vs AF_INET6
Platform->>System: SetDNS (with address family)
else NetworkManager Backend
Platform->>Platform: Select ipv4 vs ipv6 key
Platform->>System: Update D-Bus settings
end
System-->>Platform: DNS Updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
client/internal/dns/upstream_ios.go (1)
72-88: Consider edge case with private IPv6 upstream but no local IPv6 address.The
needsPrivatecheck at line 76-77 can be true for an IPv6 upstream viaupstreamIP.IsPrivate()even whenlIPv6is invalid. In this case,bindIPwould incorrectly remain as the IPv4lIP(line 79), causing a protocol mismatch when binding the socket.Consider adding a guard to skip private-client usage when the address families don't match:
needsPrivate := u.lNet.Contains(upstreamIP) || upstreamIP.IsPrivate() || (u.lNetV6.IsValid() && u.lNetV6.Contains(upstreamIP)) if needsPrivate { bindIP := u.lIP if upstreamIP.Is6() && u.lIPv6.IsValid() { bindIP = u.lIPv6 + } else if upstreamIP.Is6() { + // No valid IPv6 local address, fall back to regular client + log.Debugf("skipping private client for IPv6 upstream %s: no local IPv6 address", upstream) + goto regularClient }Alternatively, this may be an acceptable limitation given the existing TODO comment acknowledges the heuristic's limitations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/internal/dns/upstream_ios.go` around lines 72 - 88, The current needsPrivate logic can select private-client handling for an IPv6 upstream (upstreamIP.IsPrivate()) while lIPv6 is invalid, leaving bindIP set to the IPv4 lIP and causing an address-family mismatch when calling GetClientPrivate; update the branch in which needsPrivate is handled (the block referencing bindIP, lIP, lIPv6, upstreamIP.Is6(), interfaceName, and GetClientPrivate) to guard that you only use the private-client path when the local IP family matches the upstream family — i.e., if upstreamIP.Is6() require lIPv6.IsValid() (skip or fall back otherwise), and similarly ensure IPv4 upstreams use lIP.IsValid() — so you never bind an IPv4 local IP for an IPv6 upstream (or vice versa).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@client/internal/dns/upstream_ios.go`:
- Around line 72-88: The current needsPrivate logic can select private-client
handling for an IPv6 upstream (upstreamIP.IsPrivate()) while lIPv6 is invalid,
leaving bindIP set to the IPv4 lIP and causing an address-family mismatch when
calling GetClientPrivate; update the branch in which needsPrivate is handled
(the block referencing bindIP, lIP, lIPv6, upstreamIP.Is6(), interfaceName, and
GetClientPrivate) to guard that you only use the private-client path when the
local IP family matches the upstream family — i.e., if upstreamIP.Is6() require
lIPv6.IsValid() (skip or fall back otherwise), and similarly ensure IPv4
upstreams use lIP.IsValid() — so you never bind an IPv4 local IP for an IPv6
upstream (or vice versa).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c355c27-698d-4f8b-bf84-3e9980de8268
📒 Files selected for processing (7)
client/internal/dns.goclient/internal/dns/host_darwin.goclient/internal/dns/network_manager_unix.goclient/internal/dns/systemd_linux.goclient/internal/dns/upstream_ios.goclient/internal/dns_test.goclient/internal/engine.go
…anic, v6 validation
lixmal
force-pushed
the
client-ipv6-iface
branch
from
33628b0 to
1a7e835
Compare
|
❌ The last analysis has failed. |
…anic, v6 validation
lixmal
force-pushed
the
client-ipv6-iface
branch
from
1a7e835 to
3be5a5f
Compare
# Conflicts: # client/iface/wgaddr/address.go
lixmal
force-pushed
the
client-ipv6-iface
branch
from
3be5a5f to
baf2c03
Compare
# Conflicts: # client/iface/wgaddr/address.go
# Conflicts: # client/internal/dns/upstream_ios.go
|
| bindIP = u.lIP | ||
| } | ||
|
|
||
| if bindIP.IsValid() { |
There was a problem hiding this comment.
If have validation issue with lIPv6 or lIP then bindIP will be invalid.
The old code always used the private client for matching upstreams. This is a behavior change. If this i the expected then fine.
There was a problem hiding this comment.
Yes, this is a subtle intentional change. If neither lIP nor lIPv6 is valid for the upstream's address family, we fall through to the default (non-private) client instead of binding to an invalid address. In practice, if needsPrivate is true but the corresponding local IP is invalid, the configuration is broken and there's nothing useful to bind to.
2 participants
Describe your changes
ip6.arpazones, alongside existingin-addr.arpafor v4Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
netbirdio/docs#667