-
-
Notifications
You must be signed in to change notification settings - Fork 614
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, management] Support DNS Labels for Peer Addressing #3252
Merged
+1,500
−1,084
Merged
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
1147702
Add DNS labels support to login request and configuration
hakansa d1025b2
[client] Update Login method to include DNS labels parameter
hakansa 89b1e5a
move validateDomains to management.domain pkg to reuse
hakansa fad86ab
Add cleanDNSLabels field to LoginRequest and update validation logic
hakansa d943431
Rename dnsLabelsFlag to extra-dns-labels for clarity in command usage
hakansa 1c34883
Merge branch 'main' into feature/dns-labels
hakansa 12979f2
Refactor localResolver to improve multiple DNS record handling
hakansa 5d87c66
Refactor local handler update to support multiple DNS records per domain
hakansa 1682c53
Merge branch 'main' into feature/dns-labels
hakansa 8dbd6d1
Remove unused import from routes_handler_test.go
hakansa 940e8b3
Update management/domain/validate.go
hakansa c615337
Use slices.Equal for DNS label comparison and add error logging for r…
hakansa 2c38be7
Update protoc version in generated proto files
hakansa 5afa933
Fix DNS labels comparison logic
hakansa ceb4280
Merge branch 'main' into feature/dns-labels
hakansa 6fc6c00
fix proto merge conflict
hakansa 4946a7b
Replace assert library
hakansa ffdfa6b
use domain.FromPunycodeList on client login
hakansa 74d4bc7
Merge branch 'main' into feature/dns-labels
hakansa 64e1433
client grpc proto fix
hakansa f2a749f
[client] Update DNS labels flag description to specify maximum limit
hakansa b375671
Implement round-robin rotation for DNS records when multiple records …
hakansa 94c8f84
Refactor DNS label handling in login and up commands for improved val…
hakansa f041b0f
Refactor DNS label validation in up command to improve error handling
hakansa d536f8f
Merge branch 'main' into feature/dns-labels
hakansa 8ebb26a
Merge branch 'main' into feature/dns-labels
hakansa f2c867c
[management] Support Extra DNS Labels for Peer Addressing (#3291)
hakansa cbd9272
Merge branch 'main' into feature/dns-labels
hakansa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This change has broken DNS for me.
I don't have DNS server added to netbird, thus netbird can only resolve subdomains of
netbird.local
However, netbird sets search domain to
netbird.local
, thus any search first comes through netbird embedded DNSE.g, when I do
resolvectl query google.com
, resolved first tries to ask netbird forgoogle.com.netbird.local
.And there is a bug: in my case netbird can't lookup google.com, and thus returns
NXDOMAIN
(RcodeNameError
) error, makingsystemd-resolved
skip all other DNS servers and return the answer directly:google.com: resolve call failed: Could not resolve 'google.com', server or network returned error: SERVFAIL
If netbird DNS server is not authoritative (i.e it has no DNS server defined in dashboard), it should instead return
RcodeSuccess
with zero records, so thatresolved
can continue the search using system DNS server.The previous code was working, because it was responding with zero records in that case.
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.
...Or I misunderstood the DNS specification?
For now, I have added DNS in netbird dashboard and it works, but it doesn't seem right that system DNS is skipped now
Edit: In systemd-resolved, FallbackDNS only works when there is no other DNS server defined
Adding 1.1.1.1 as global DNS server and removing DNS servers from netbird dashboard have fixed the issue for me, I'm not sure why did it work out of the box on previous netbird version, but it doesn't seem right that netbird tries to handle all DNS by itself when no DNS server is defined in dashboard.
Looks like I have bisected it to the wrong place, but it definitely broken after merging this PR.