[PR #5945] [MERGED] [client] Add TTL-based refresh to mgmt DNS cache via handler chain #28791

Closed
opened 2026-08-05 08:06:56 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/5945
Author: @lixmal
Created: 4/21/2026
Status: Merged
Merged: 4/22/2026
Merged by: @lixmal

Base: mainHead: mgmt-cache-ttl-refresh


📝 Commits (8)

  • 7486738 Add TTL-based refresh to mgmt DNS cache via handler chain
  • e6c6241 Update client/internal/dns/handler_chain.go
  • 985e37b Merge branch 'main' into mgmt-cache-ttl-refresh
  • 236e99a Parse mgmt cache TTL env var once via sync.OnceValue
  • 4c25ac6 Clamp served TTL to remaining cache life and guard refresh by identity
  • cdc1ff8 Reject empty AddDomain results, filter chain answers by CNAME owner, dedup A/AAAA clone
  • 0b21d7c Move cache TTL to Resolver field for injection in tests
  • 19c1ba7 Demote fails=0 refresh log, clear refreshing markers on RemoveDomain

📊 Changes

6 files changed (+1107 additions, -90 deletions)

View changed files

📝 client/internal/dns/handler_chain.go (+94 -0)
📝 client/internal/dns/handler_chain_test.go (+164 -0)
📝 client/internal/dns/mgmt/mgmt.go (+385 -90)
client/internal/dns/mgmt/mgmt_refresh_test.go (+408 -0)
📝 client/internal/dns/mgmt/mgmt_test.go (+55 -0)
📝 client/internal/dns/server.go (+1 -0)

📄 Description

Describe your changes

The mgmt DNS cache was previously populated once and never refreshed, so if the mgmt/signal/relay/etc. hostname's IP changed (DDNS, cloud IP rotation, failover) the client would keep using the stale address until restart. This PR makes cache entries expire and refresh on a TTL, using stale-while-revalidate semantics so queries never block on a lookup.

The tricky part is where to send the refresh query. When NetBird has taken over the OS's DNS resolver, calling net.DefaultResolver would loop straight back into the mgmt cache itself (we'd serve our own stale answer to ourselves and "succeed"). So the refresh prefers the internal handler chain, bypassing the mgmt-cache priority and routing through the upstream / default / fallback handlers that actually know how to reach an external resolver. The OS fallback is only used when the chain has no root-zone handler at or below PriorityUpstream, which by construction means NetBird is not the system resolver and net.DefaultResolver is safe.

Pitfalls worth knowing:

  • Loop case: if the predicate ever gets it wrong (NetBird is the system resolver but no chain root handler), the OS path would loop. The flow is self-limiting via singleflight (at most one extra parked goroutine per cycle) but silently wrong: every "refresh" returns our own stale answer with a fresh timestamp. A lightweight detector arms only on the OS path and logs a single warn if a ServeDNS hit lands during an inflight OS-fallback refresh for the same question.
  • Fallback availability: the predicate depends on a root-zone handler being registered. This is true for NetBird-as-default-resolver (Primary nsgroup or addHostRootZone), and for Linux hosts where hostManagerWithOriginalNS captures the pre-takeover /etc/resolv.conf as a fallback. It does not fire on systemd-resolved Linux when no Primary nsgroup is configured (that manager doesn't expose original nameservers) — but that scenario is also the one where the OS path is safe, so it works out.
  • Persistent-failure noise: each failed refresh fires every ~30s (backoff window). The first failure logs at warn, subsequent failures at debug, so a broken upstream doesn't flood the log.
  • Goroutine fan-out: one refresh goroutine per stale hit is skipped when a refresh is already inflight for that question, keeping things quiet under bursty load.

NB_MGMT_CACHE_TTL shortens the default 5min TTL for integration testing.

https://github.com/netbirdio/netbird/issues/5113

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)

Internal behavior change; no user-facing surface or CLI/API.

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

    • In-process DNS resolution API with root-handler detection by priority.
  • Improvements

    • Priority-aware handler dispatching and captured responses for handlers that write raw bytes.
    • Management cache: per-entry timestamps, stale-while-revalidate serving, async refresh with singleflight deduplication, backoff, and separate A/AAAA handling.
    • Chain-first resolution with OS fallback.
  • Bug Fixes

    • Better context/deadline handling and clearer failure signals when no handler returns a usable response.
  • Tests

    • Expanded coverage for dispatch, priority filtering, raw-response capture, timeouts, refresh/backoff, root detection, and cache refresh behaviors.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/5945 **Author:** [@lixmal](https://github.com/lixmal) **Created:** 4/21/2026 **Status:** ✅ Merged **Merged:** 4/22/2026 **Merged by:** [@lixmal](https://github.com/lixmal) **Base:** `main` ← **Head:** `mgmt-cache-ttl-refresh` --- ### 📝 Commits (8) - [`7486738`](https://github.com/netbirdio/netbird/commit/7486738d0a4295d6c4bb1af8005b592feac737f9) Add TTL-based refresh to mgmt DNS cache via handler chain - [`e6c6241`](https://github.com/netbirdio/netbird/commit/e6c62410ea38c66dcb0bb964793fbf79a85b9a86) Update client/internal/dns/handler_chain.go - [`985e37b`](https://github.com/netbirdio/netbird/commit/985e37b4d44a0a801ee9940b94415f223e8e9c58) Merge branch 'main' into mgmt-cache-ttl-refresh - [`236e99a`](https://github.com/netbirdio/netbird/commit/236e99af630377fba4c69f9ac2739e4ae149a110) Parse mgmt cache TTL env var once via sync.OnceValue - [`4c25ac6`](https://github.com/netbirdio/netbird/commit/4c25ac674a719c944fd6b2557b479ac448efdfb2) Clamp served TTL to remaining cache life and guard refresh by identity - [`cdc1ff8`](https://github.com/netbirdio/netbird/commit/cdc1ff8fd2a6983cf2513179bd39488591a157e2) Reject empty AddDomain results, filter chain answers by CNAME owner, dedup A/AAAA clone - [`0b21d7c`](https://github.com/netbirdio/netbird/commit/0b21d7c24c80acf5a1cbc84662a8ad5d39ba8407) Move cache TTL to Resolver field for injection in tests - [`19c1ba7`](https://github.com/netbirdio/netbird/commit/19c1ba7cb780bfb6a4512970d0c4388f7633a496) Demote fails=0 refresh log, clear refreshing markers on RemoveDomain ### 📊 Changes **6 files changed** (+1107 additions, -90 deletions) <details> <summary>View changed files</summary> 📝 `client/internal/dns/handler_chain.go` (+94 -0) 📝 `client/internal/dns/handler_chain_test.go` (+164 -0) 📝 `client/internal/dns/mgmt/mgmt.go` (+385 -90) ➕ `client/internal/dns/mgmt/mgmt_refresh_test.go` (+408 -0) 📝 `client/internal/dns/mgmt/mgmt_test.go` (+55 -0) 📝 `client/internal/dns/server.go` (+1 -0) </details> ### 📄 Description ## Describe your changes The mgmt DNS cache was previously populated once and never refreshed, so if the mgmt/signal/relay/etc. hostname's IP changed (DDNS, cloud IP rotation, failover) the client would keep using the stale address until restart. This PR makes cache entries expire and refresh on a TTL, using stale-while-revalidate semantics so queries never block on a lookup. The tricky part is where to send the refresh query. When NetBird has taken over the OS's DNS resolver, calling `net.DefaultResolver` would loop straight back into the mgmt cache itself (we'd serve our own stale answer to ourselves and "succeed"). So the refresh prefers the internal handler chain, bypassing the mgmt-cache priority and routing through the upstream / default / fallback handlers that actually know how to reach an external resolver. The OS fallback is only used when the chain has no root-zone handler at or below `PriorityUpstream`, which by construction means NetBird is not the system resolver and `net.DefaultResolver` is safe. Pitfalls worth knowing: - **Loop case**: if the predicate ever gets it wrong (NetBird is the system resolver but no chain root handler), the OS path would loop. The flow is self-limiting via `singleflight` (at most one extra parked goroutine per cycle) but silently wrong: every "refresh" returns our own stale answer with a fresh timestamp. A lightweight detector arms only on the OS path and logs a single warn if a `ServeDNS` hit lands during an inflight OS-fallback refresh for the same question. - **Fallback availability**: the predicate depends on a root-zone handler being registered. This is true for NetBird-as-default-resolver (Primary nsgroup or `addHostRootZone`), and for Linux hosts where `hostManagerWithOriginalNS` captures the pre-takeover `/etc/resolv.conf` as a fallback. It does *not* fire on systemd-resolved Linux when no Primary nsgroup is configured (that manager doesn't expose original nameservers) — but that scenario is also the one where the OS path is safe, so it works out. - **Persistent-failure noise**: each failed refresh fires every ~30s (backoff window). The first failure logs at warn, subsequent failures at debug, so a broken upstream doesn't flood the log. - **Goroutine fan-out**: one refresh goroutine per stale hit is skipped when a refresh is already inflight for that question, keeping things quiet under bursty load. `NB_MGMT_CACHE_TTL` shortens the default 5min TTL for integration testing. ## Issue ticket number and link https://github.com/netbirdio/netbird/issues/5113 ## Stack <!-- branch-stack --> ### Checklist - [X] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [X] 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](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [X] Documentation is **not needed** for this change (explain why) Internal behavior change; no user-facing surface or CLI/API. ### 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/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * In-process DNS resolution API with root-handler detection by priority. * **Improvements** * Priority-aware handler dispatching and captured responses for handlers that write raw bytes. * Management cache: per-entry timestamps, stale-while-revalidate serving, async refresh with singleflight deduplication, backoff, and separate A/AAAA handling. * Chain-first resolution with OS fallback. * **Bug Fixes** * Better context/deadline handling and clearer failure signals when no handler returns a usable response. * **Tests** * Expanded coverage for dispatch, priority filtering, raw-response capture, timeouts, refresh/backoff, root detection, and cache refresh behaviors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:06:56 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#28791