[client] Support disable search domain for custom zones #4826

mlsmaycon · 2025-11-20T10:59:03Z

Describe your changes

Add support to disable the search domain for custom zones. This is part of the required changes for custom zones management that will be added soon.

CodeRabbit description:
A new SearchDomainDisabled boolean flag is introduced across the DNS subsystem, propagating from the protobuf management configuration through the engine to the host resolver, replacing previous suffix-based logic for determining domain match-only behavior.

Issue ticket number and link

Stack

Checklist

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

Summary by CodeRabbit

New Features
- Added an option to disable search-domain behavior for custom DNS zones.
- Added an option to exclude specific zones from reverse (PTR) record generation.
- Custom DNS zone settings now include and honor these two new flags for more granular DNS behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-11-20T10:59:14Z

Walkthrough

Two new boolean flags, SearchDomainDisabled and SkipPTRProcess, are added to CustomZone and its protobuf; they are propagated through the engine to DNS host logic. Host matching now uses SearchDomainDisabled directly, and PTR collection skips zones with SkipPTRProcess; reverse zones are initialized with SearchDomainDisabled: true.

Changes

Cohort / File(s)	Summary
Proto & Data model `shared/management/proto/management.proto`, `dns/dns.go`	Added `bool SearchDomainDisabled` and `bool SkipPTRProcess` to the `CustomZone` protobuf message and `CustomZone` struct.
Engine — config propagation `client/internal/engine.go`	Propagates `SearchDomainDisabled` and `SkipPTRProcess` from proto `CustomZone` into in-memory DNS config when building `DNSConfig`.
Host / resolution logic `client/internal/dns/host.go`	Removed IPv4/IPv6 reverse-zone constants; changed `DomainConfig.MatchOnly` determination to use `customZone.SearchDomainDisabled` directly.
PTR and reverse-zone handling `client/internal/dns.go`	`collectPTRRecords` now skips zones with `SkipPTRProcess == true`; `addReverseZone` initializes reverse zones with `SearchDomainDisabled: true`.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Proto as Proto (management.proto)
participant Engine as Engine (toDNSConfig)
participant DNS as DNS Config
participant Host as Host Resolver
participant PTR as PTR Collector

Note over Proto,Engine: config propagation
Proto->>Engine: CustomZone { Domain, SearchDomainDisabled, SkipPTRProcess }
Engine->>DNS: build CustomZone entries (include flags)

Note over DNS,Host: resolution decision
DNS->>Host: resolve request
Host->>Host: use CustomZone.SearchDomainDisabled to set MatchOnly

Note over DNS,PTR: PTR processing
DNS->>PTR: collectPTRRecords for zones
alt SkipPTRProcess == true
    PTR->>PTR: skip zone (no PTR generation)
else
    PTR->>PTR: generate PTR records
end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pay attention to client/internal/dns/host.go to confirm MatchOnly semantics match intended behavior across edge cases.
Verify SkipPTRProcess is respected in all PTR-generation paths and that reverse zones initialized with SearchDomainDisabled: true do not regress lookup behavior.
Confirm protobuf field numbering and generated code compatibility where applicable.

Poem

🐇 I hopped through proto, flags in tow,
Search and PTRs now tell me where to go,
Zones that say "no PTR" I gently skip,
Reverse zones set their quiet little ship,
A tidy hop — configuration snug and slow.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: adding support to disable search domain for custom zones, which aligns with the core functionality introduced across the DNS subsystem.
Description check	✅ Passed	The description explains the feature and includes the required documentation selection, but lacks issue ticket number, stack details, and unchecked checklist items indicating incomplete metadata.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch support-disable-search-domain

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8207260 and 61070b0.

⛔ Files ignored due to path filters (1)

shared/management/proto/management.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (4)

client/internal/dns.go (2 hunks)
client/internal/engine.go (1 hunks)
dns/dns.go (1 hunks)
shared/management/proto/management.proto (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

shared/management/proto/management.proto
client/internal/dns.go
client/internal/engine.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

GitHub Check: Relay / Unit (386)
GitHub Check: Management / Unit (amd64, mysql)
GitHub Check: Management / Unit (amd64, postgres)
GitHub Check: Client / Unit (386)
GitHub Check: Relay / Unit (amd64, -race)
GitHub Check: Client / Unit (amd64)
GitHub Check: Client (Docker) / Unit
GitHub Check: Management / Unit (amd64, sqlite)
GitHub Check: Linux
GitHub Check: Windows
GitHub Check: iOS / Build
GitHub Check: Darwin
GitHub Check: JS / Lint
GitHub Check: Client / Unit
GitHub Check: Client / Unit
GitHub Check: Android / Build
GitHub Check: release_ui_darwin
GitHub Check: release
GitHub Check: release_ui
GitHub Check: Client / Unit

🔇 Additional comments (1)

dns/dns.go (1)

48-51: LGTM. Code changes verified and approved.

Verification confirms both new boolean fields are properly initialized and used throughout the codebase:

SearchDomainDisabled: Initialized in client/internal/dns.go and properly populated from protobuf in client/internal/engine.go; used correctly in client/internal/dns/host.go

SkipPTRProcess: Properly populated from protobuf in client/internal/engine.go; correctly evaluated in client/internal/dns.go to conditionally skip PTR processing

Default values (false) are semantically appropriate for both fields.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (4)

client/internal/dns.go (1)

108-112: Reverse zones correctly marked as match-only; consider documenting/testing this invariant

Hard‑coding SearchDomainDisabled: true for the reverse zone makes its behavior explicit and avoids polluting system search domains with .in-addr.arpa suffixes, which is desirable.

It might be worth adding a brief comment here (or a unit test around addReverseZone + dnsConfigToHostDNSConfig) to lock in the expectation that reverse zones are always configured as match‑only on the host side.

dns/dns.go (1)

42-50: Clarify SearchDomainDisabled comment to spell out true/false semantics

The new field fits well with how DomainConfig.MatchOnly is derived, but the comment is a bit ambiguous (“add match domains to search domains list or not”).

Consider tightening it to something like:

// SearchDomainDisabled, when true, prevents this custom zone's domain from being added to the system resolver's search-domain list (i.e. it is treated as match-only).

This makes the flag’s effect obvious without having to jump to client/internal/dns/host.go.

client/internal/dns/host.go (1)

107-112: MatchOnly now driven purely by config flag; confirm this behavior under version skew is intended

Switching MatchOnly for custom zones to customZone.SearchDomainDisabled is a nice cleanup compared to suffix-based heuristics and lines up with the new CustomZone flag.

It does mean that on new clients talking to an older management server (which won’t populate SearchDomainDisabled), all management-defined custom zones will be treated as search-domain-enabled unless the client itself injected them (e.g. reverse zones via addReverseZone, which now explicitly set the flag). If you expect mixed versions in the field, consider whether that change is acceptable or whether you want a temporary OR with a suffix check as a compatibility fallback.

client/internal/engine.go (1)

1208-1224: SearchDomainDisabled propagation is correct; think about server-sourced reverse zones

Mapping zone.GetDomain() and zone.GetSearchDomainDisabled() into nbdns.CustomZone is the right way to thread the new flag through to the runtime DNS config; combined with dnsConfigToHostDNSConfig, this fully drives DomainConfig.MatchOnly from configuration.

One subtle point: if the management server is older and never sets SearchDomainDisabled, this will leave all server-defined custom zones search-domain-enabled by default. That’s fine for most zones, but if you currently define any reverse-style custom zones on the server (rather than relying on addReverseZone), their behavior will change on new clients unless the server starts setting this flag. If that deployment pattern exists, you may want to either (a) set the flag server-side for those zones, or (b) introduce a temporary suffix-based fallback here for such domains.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68f56b7 and 8207260.

⛔ Files ignored due to path filters (1)

shared/management/proto/management.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (5)

client/internal/dns.go (1 hunks)
client/internal/dns/host.go (1 hunks)
client/internal/engine.go (1 hunks)
dns/dns.go (1 hunks)
shared/management/proto/management.proto (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

GitHub Check: Management / Unit (amd64, mysql)
GitHub Check: Management / Unit (amd64, sqlite)
GitHub Check: Management / Unit (amd64, postgres)
GitHub Check: Relay / Unit (amd64, -race)
GitHub Check: Relay / Unit (386)
GitHub Check: Client / Unit (386)
GitHub Check: Client / Unit (amd64)
GitHub Check: Client (Docker) / Unit
GitHub Check: iOS / Build
GitHub Check: Android / Build
GitHub Check: Client / Unit
GitHub Check: Darwin
GitHub Check: Client / Unit
GitHub Check: Linux
GitHub Check: Windows
GitHub Check: Client / Unit
GitHub Check: release_ui_darwin
GitHub Check: release
GitHub Check: release_ui
GitHub Check: JS / Lint

🔇 Additional comments (1)

shared/management/proto/management.proto (1)

432-437: Proto field addition looks correct; be aware of mixed-version defaults

Adding bool SearchDomainDisabled = 3; to CustomZone cleanly mirrors the new field in dns.CustomZone and is wire‑compatible (new field, new tag).

Just be aware that on mixed‑version deployments where the management service hasn’t been updated to populate this field yet, clients will see SearchDomainDisabled == false for all custom zones and will treat them as search‑domain‑enabled. If you still rely on any implicit “match‑only by suffix” behavior on the server side for certain zones, decide whether that behavior change on new clients is acceptable or if you want a fallback strategy during rollout.

Introduced a `SkipPTRProcess` flag to DNS zones, allowing users to skip processing PTR records explicitly. Updated related protobuf definitions, client internal logic, and DNS handling to respect this new configuration.

sonarqubecloud · 2025-11-22T01:08:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Support disable search domain for custom zones

8207260

mlsmaycon requested a review from lixmal November 20, 2025 10:59

coderabbitai bot reviewed Nov 20, 2025

View reviewed changes

Add SkipPTRProcess functionality to custom DNS zones

61070b0

Introduced a `SkipPTRProcess` flag to DNS zones, allowing users to skip processing PTR records explicitly. Updated related protobuf definitions, client internal logic, and DNS handling to respect this new configuration.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[client] Support disable search domain for custom zones #4826

[client] Support disable search domain for custom zones #4826

Uh oh!

mlsmaycon commented Nov 20, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 20, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

sonarqubecloud bot commented Nov 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

[client] Support disable search domain for custom zones #4826

Are you sure you want to change the base?

[client] Support disable search domain for custom zones #4826

Uh oh!

Conversation

mlsmaycon commented Nov 20, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Issue ticket number and link

Stack

Checklist

Documentation

Docs PR URL (required if "docs added" is checked)

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 22, 2025

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mlsmaycon commented Nov 20, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 20, 2025 •

edited

Loading