Skip to content
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

[client] Support dns upstream failover for nameserver groups with same match domain #3178

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lixmal
Copy link
Contributor

@lixmal lixmal commented Jan 13, 2025

Describe your changes

  1. Failover between nameserver groups handling the same match domain. The old behavior was non-deterministic about which one was picked.
    This is achieved by creating an upstream handler for each domain per nameserver group with decreasing priorities in the handler chain.

    Example:

    Received network map with 2 groups:

    #1: [1.1.1.1, 1.0.0.1] for [example.org]
    #2: [8.8.8.8, 8.8.4.4] for [example.org, example.com]

    Results in the following handlers:

    #1: domain=example.org upstream=[1.1.1.1, 1.0.0.1] priority=50
    #2: domain=example.org upstream=[8.8.8.8, 8.8.4.4] priority=49
    #3: domain=example.com upstream=[8.8.8.8, 8.8.4.4] priority=50

    Where priority/domain pairs are unique

    Should one of the handlers detect failure (failed attempts >=5), the original server group (handling all the domains) will be marked unavailable and the handler will be deregistered.

  2. Return a DNS error when all upstream handlers fail instead of silently dropping the packet (here)

  3. Handler chain cleanup

  4. Improved log messages

Issue ticket number and link

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

@lixmal lixmal force-pushed the match-domain-failover branch from c728b16 to 9b89841 Compare January 13, 2025 17:44
Copy link

sonarqubecloud bot commented Jan 13, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@mlsmaycon mlsmaycon requested a review from Copilot January 14, 2025 09:47

Choose a reason for hiding this comment

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

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

Files not reviewed (5)
  • client/internal/dns/upstream_android.go: Evaluated as low risk
  • client/internal/dns/upstream_ios.go: Evaluated as low risk
  • client/internal/dns/upstream_general.go: Evaluated as low risk
  • client/internal/peer/status.go: Evaluated as low risk
  • client/internal/dns/local.go: Evaluated as low risk
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.

1 participant