[client] fix dns forwarder port update #4613
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with DNS forwarder port updates by changing how the port is configured. Instead of passing the port through the constructor and setting it during Start(), the port is now set directly via a new SetListenPort() function before creating the manager.
- Removed port parameter from NewManager constructor
- Added SetListenPort() function to set the port globally before manager creation
- Updated port setting to occur before manager instantiation rather than during Start()
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| client/internal/engine.go | Updated to call SetListenPort() before creating DNS manager and removed port parameter from NewManager calls |
| client/internal/dnsfwd/manager.go | Removed port field from Manager struct, added SetListenPort() function, and removed port setting logic from Start() method |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| func SetListenPort(port uint16) { | ||
| listenPortMu.Lock() | ||
| listenPort = port | ||
| listenPortMu.Unlock() | ||
| } |
There was a problem hiding this comment.
The SetListenPort function modifies global state without considering if a DNS forwarder is already running. This could cause race conditions if called while the forwarder is active, potentially leading to the forwarder listening on one port while the global listenPort variable indicates another.
Copilot uses AI. Check for mistakes.
2 participants
Describe your changes
[client] fix dns forwarder port update
Issue 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:
https://github.com/netbirdio/docs/pull/__