Skip to content

[client] Apply routes right away instead of on peer connection #3907

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

Merged
merged 17 commits into from
Jun 3, 2025

Conversation

lixmal
Copy link
Contributor

@lixmal lixmal commented Jun 1, 2025

Describe your changes

  • Moves the apply routes logic from the route client watcher to the network map update
  • Moves routes client and server to own packages
  • Applies DNS settings before routes to satisfy DNS route dependencies

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)
  • Extended the README / documentation, if necessary

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

@Copilot Copilot AI review requested due to automatic review settings June 1, 2025 16:18
Copy link
Contributor

@Copilot 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 refactors routing logic by immediately applying routes instead of waiting on peer connection, while also renaming and reorganizing several internal types and functions for consistency. Key changes include:

  • Updating dialog button labels in the UI for route-related error dialogs.
  • Refactoring the server router and client watcher components with renamed types and methods.
  • Adjusting DNS configuration updates and streamlining route updates in the engine.

Reviewed Changes

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

Show a summary per file
File Description
client/ui/debug.go Updated dialog button labels from “Cancel” to “cancel”.
client/internal/routemanager/static/route.go Switched to inline error checking and added explicit return statements.
client/internal/routemanager/server/server.go Renamed internal router type from serverRouter to Router and updated method names.
client/internal/routemanager/manager.go Refactored client network watcher integration with new activeRoutes and update flow.
client/internal/routemanager/client/client.go Renamed and refactored client network (Watcher) functions and update handling.
client/internal/engine.go Moved and streamlined DNS server update logic.
client/internal/dns/server.go Modified debug logging to show extra domain keys using maps.Keys.

lixmal and others added 5 commits June 2, 2025 12:14
Co-authored-by: hakansa <[email protected]>
Because the code generated new channel for every single event, was easy to miss notification.
Use single channel.
hakansa
hakansa previously approved these changes Jun 3, 2025
Copy link

sonarqubecloud bot commented Jun 3, 2025

@lixmal lixmal merged commit 06980e7 into main Jun 3, 2025
48 of 50 checks passed
@lixmal lixmal deleted the apply-routes-early branch June 3, 2025 08:53
@lixmal lixmal restored the apply-routes-early branch June 5, 2025 10:21
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