Skip to content

Conversation

@lixmal
Copy link
Collaborator

@lixmal lixmal commented Oct 28, 2025

Describe your changes

  • Makes routing client respect management-provided dns forwarder port to keep compatibility with older routing peers

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

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/__

Copilot AI review requested due to automatic review settings October 28, 2025 20:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR makes the DNS forwarder port configurable and dynamic based on peer versions, replacing the hardcoded ForwarderClientPort constant. The management server now computes an appropriate forwarder port based on peer capabilities and sends it to clients via the DNS configuration.

  • Adds ForwarderPort field to DNS configuration structures for transmitting the port from management server to clients
  • Implements dynamic port computation in the route manager using atomic operations for thread-safe updates
  • Updates DNS interceptor to use the dynamic forwarder port instead of the hardcoded constant

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
dns/dns.go Adds ForwarderPort field to the DNS Config struct
client/internal/routemanager/manager.go Adds atomic dnsForwarderPort field and SetDNSForwarderPort method to manage the port dynamically
client/internal/routemanager/common/params.go Adds ForwarderPort field to handler parameters for passing the atomic port reference
client/internal/routemanager/dnsinterceptor/handler.go Updates DNS interceptor to use the dynamic forwarder port from atomic storage instead of the hardcoded constant
client/internal/engine.go Extracts DNS config to call SetDNSForwarderPort and sets default fallback when forwarder port is 0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lixmal lixmal force-pushed the fix/client-forwarder-port branch from 1315c63 to 2f46963 Compare October 28, 2025 21:03
@sonarqubecloud
Copy link

@lixmal lixmal requested a review from Copilot October 28, 2025 21:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lixmal lixmal merged commit 1ee575b into main Oct 28, 2025
42 checks passed
@lixmal lixmal deleted the fix/client-forwarder-port branch October 28, 2025 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants