-
-
Notifications
You must be signed in to change notification settings - Fork 963
feat(client): auto-reconfigure WireGuard interface on peer IP change #4866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(client): auto-reconfigure WireGuard interface on peer IP change #4866
Conversation
WalkthroughIntroduces idempotent self-IP hot-update logic on the WireGuard interface with firewall and SSH rebinding steps; integrates into the config update path and adds tests and server Restart support. Update failures are logged and do not abort config application. Changes
Sequence Diagram(s)sequenceDiagram
participant updateConfig as updateConfig
participant updateSelf as updateSelfPeerIP
participant wgIface as wgInterface
participant fwMgr as firewallManager
participant sshSrv as sshServer
participant engine as Engine
updateConfig->>updateSelf: updateSelfPeerIP(oldAddr, newAddr)
alt oldAddr == newAddr
updateSelf-->>updateConfig: return nil (no-op)
else oldAddr != newAddr
updateSelf->>wgIface: UpdateAddr(newAddr)
alt wg update success
wgIface-->>updateSelf: nil
updateSelf->>engine: set Engine.WgAddr = newAddr
updateSelf->>fwMgr: updateFirewallForAddressChange(oldIP)
fwMgr->>sshSrv: if SSH active -> Restart(newAddr:port) (DNAT adjust)
fwMgr-->>updateSelf: firewall updated
updateSelf-->>updateConfig: return nil
else wg update failure
wgIface-->>updateSelf: error
updateSelf-->>updateConfig: return error (logged, continue)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🧰 Additional context used🧬 Code graph analysis (2)client/ssh/server/server.go (1)
client/internal/engine.go (2)
🔇 Additional comments (8)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
client/internal/engine.go (1)
906-913: Hot self-IP update helper looks correct; consider guarding future reuse and iOS hookThe new
updateSelfPeerIPflow is clean: it’s idempotent, updates the wg iface first, and only mutatesconfig.WgAddron success;updateConfigcorrectly logs and proceeds on failure so other config updates aren’t blocked. The only thing I’d consider is:
- Adding a brief comment or defensive nil-check inside
updateSelfPeerIPto make it safer if it’s ever called from elsewhere without the existingwgInterface != nilguarantee.- On iOS, we currently set
NetworkChangeListener.SetInterfaceIP(e.config.WgAddr)only at create time; if management later changes the self IP, we might also want to call that here so the mobile network listener observes the new address.Also applies to: 932-955
client/internal/engine_test.go (1)
405-461: Good, focused coverage of self-IP update behaviorThe three subtests nicely pin down the helper’s contract: idempotent no-op, successful interface+config update, and failure path with preserved config. This is sufficient for the new helper; if you later change
updateConfigbehavior around error handling, a small integration-style test there could complement these, but not required for this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/internal/engine.go(2 hunks)client/internal/engine_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
client/internal/engine_test.go (1)
client/internal/engine.go (2)
Engine(138-211)EngineConfig(81-135)
client/internal/engine.go (1)
client/iface/wgaddr/address.go (1)
Address(9-12)
|
Hi @fahrishih Unfortunately that is not enough. The IP is also used in various subsystems (DNS, Firewall, ...) that would also require updating |
|
hi @lixmal , which firewall are you referring to? if by dns you mean the netbird dns, i tested and it was able to resolve to the new machine's IP |
|
I mean the firewall implementations (userspace, nftables, iptables) and the DNS server. Pretty much everything that calls |
|
sure @lixmal , but anyways those you mentioned would still not work if the network range is changed from the console, until the user runs netbird down and netbird up again. at least on this MR, what I aim is to avoid running that command on all the peers deployed and keep the connectivity going by this autoupdate. |
|
|
hi @lixmal , i added a new commit to automatically update the nft When the management server changes a peer's IP address, the SSH server now properly restarts to bind to the new IP. Changes:
|



Added
Improved
Fixed
Notes
This enhancement significantly improves NetBird’s dynamic IP behavior and avoids downtime during network CIDR changes.
Checklist
Documentation
Select exactly one:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.