Skip to content

Release ctrld-client - #266

Open
cuonglm wants to merge 219 commits into
mainfrom
release-branch-v2.0.0
Open

cuonglm wants to merge 219 commits into
mainfrom
release-branch-v2.0.0

Conversation

@cuonglm

@cuonglm cuonglm commented Oct 9, 2025 •

Copy link
Copy Markdown
Collaborator

Major Release

This is the first release of ctrld-client.

Added

  • Rule Matching Engine: Implemented new modular rule matching engine infrastructure with support for configurable rule evaluation order (infrastructure ready, not yet exposed to users)
  • Modular CLI Architecture: Split monolithic CLI command structure into focused, maintainable command files
  • Context Support: Added context.Context support throughout configuration methods for better cancellation and timeout handling

Improvements

  • Logging System: Migrated from zerolog to uber zap logging package with improved performance, better structured logging, and enhanced extensibility
  • CLI Architecture: Refactored monolithic commands.go (1,397 lines) into 13 focused command files, improving maintainability and testability
  • DNS Proxy: Major refactoring of DNS proxy implementation with better code organization, improved error handling, and enhanced separation of concerns
  • Client Information System: Enhanced client discovery and information management with improved DHCP, mDNS, ARP, NDP, and hosts file parsing
  • Service Management: Improved service lifecycle management, enhanced reload functionality, and better status reporting across all platforms
  • Network/OS Abstractions: Improved network and OS abstraction layers with reduced code duplication and more consistent behavior across platforms
  • Configuration System: Added context support and improved logging throughout configuration initialization and bootstrap operations
  • Code Organization: Removed ~3,000 lines of deprecated router-specific code, focusing development on core DNS proxy functionality

Fixes

  • Improved error handling and recovery mechanisms throughout the codebase
  • Enhanced service state management and lifecycle handling

Breaking Changes

⚠️ Server and Router-Specific Integrations Removed: All server platforms and router-specific integration code has been removed, including support for:

  • DD-WRT, dnsmasq, EdgeOS, Firewalla, AsusWRT-Merlin, Netgear/Orbi/Voxel, OpenWRT, Synology, Tomato, and Ubiquiti UniFi OS routers
  • Windows Server

If you were using ctrld with any of these router platforms, you will need to use alternative deployment methods. See the migration guide for details.

Note: All other functionality remains backward compatible. Existing configuration files and CLI commands continue to work without changes.

@cuonglm
cuonglm force-pushed the release-branch-v2.0.0 branch 2 times, most recently from f38c9ae to 90eddb8 Compare October 9, 2025 13:51
@cuonglm
cuonglm force-pushed the release-branch-v2.0.0 branch 2 times, most recently from 1c74fc4 to f9d0263 Compare November 12, 2025 08:21
@cuonglm cuonglm changed the title [WIP] Release branch v2.0.0 Release branch v2.0.0 Nov 12, 2025
@cuonglm
cuonglm force-pushed the release-branch-v2.0.0 branch 4 times, most recently from 0d4b697 to d0e66b8 Compare December 17, 2025 08:28
@cuonglm
cuonglm force-pushed the release-branch-v2.0.0 branch 4 times, most recently from de415df to 1fbbb14 Compare March 10, 2026 10:42
cuonglm added 17 commits April 30, 2026 19:19
This commit reverts changes from v1.4.5 to v1.4.7, to prepare for v2.0.0
branch codes.

Changes includes in these releases have been included in v2.0.0 branch
already.

Details:

Revert "feat: add --rfc1918 flag for explicit LAN client support"

This reverts commit 0e3f764.

Revert "Upgrade quic-go to v0.54.0"

This reverts commit e52402e.

Revert "docs: add known issues documentation for Darwin 15.5 upgrade issue"

This reverts commit 2133f31.

Revert "start mobile library with provision id and custom hostname."

This reverts commit a198a5c.

Revert "Add OPNsense new lease file"

This reverts commit 7af29cf.

Revert ".github/workflows: bump go version to 1.24.x"

This reverts commit ce1a165.

Revert "fix: ensure upstream health checks can handle large DNS responses"

This reverts commit fd48e6d.

Revert "refactor(prog): move network monitoring outside listener loop"

This reverts commit d71d134.

Revert "fix: correct Windows API constants to fix domain join detection"

This reverts commit 21855df.

Revert "refactor: move network monitoring to separate goroutine"

This reverts commit 66e2d3a.

Revert "refactor: extract empty string filtering to reusable function"

This reverts commit 36a7423.

Revert "cmd/cli: ignore empty positional argument for start command"

This reverts commit e616091.

Revert "Avoiding Windows runners file locking issue"

This reverts commit 0948161.

Revert "refactor: split selfUpgradeCheck into version check and upgrade execution"

This reverts commit ce29b5d.

Revert "internal/router: support Ubios 4.3+"

This reverts commit de24fa2.

Revert "internal/router: support Merlin Guest Network Pro VLAN"

This reverts commit 6663925.
So setting up logging for ctrld binary and ctrld packages could be done
more easily, decouple the required setup for interactive vs daemon
running.

This is the first step toward replacing rs/zerolog libary with a
different logging library.
By adding a logger field to "prog" struct, and use this field inside its
method instead of always accessing global mainLog variable. This at
least ensure more consistent usage of the logger during ctrld prog
runtime, and also help refactoring the code more easily in the future
(like replacing the logger library).
Make nameserver resolution functions more consistent and accessible:
- Rename currentNameserversFromResolvconf to CurrentNameserversFromResolvconf
- Move function to public API for better reusability
- Update all internal references to use the new public API
- Add comprehensive godoc comments for nameserver functions
- Improve code organization by centralizing DNS resolution logic

This change makes the nameserver resolution functionality more maintainable
and easier to use across different parts of the codebase.
- Add timeouts and proper cleanup in Test_osResolver_Singleflight:
  * Implement context timeout
  * Add proper PacketConn cleanup
  * Fix race conditions in error handling
  * Improve atomic value reporting

- Enhance Test_osResolver_HotCache:
  * Add proper timeout context
  * Implement more reliable cache verification
  * Fix potential resource leaks
  * Add deterministic polling intervals

- Add thread safety to Test_Edns0_CacheReply:
  * Implement proper timeout context
  * Add proper resource cleanup
  * Fix concurrent operations handling

The changes improve overall test suite reliability by addressing resource
management, timeout handling, and thread safety concerns across multiple DNS
resolver test cases.
Move client information related functions from client_info_*.go to desktop_*.go files
to better organize platform-specific code and separate desktop functionality from
shared code.

No functional changes.
Improve documentation for Test_prog_parseResolvConfNameservers to clarify that
the old implementation was removed as part of code deduplication effort. The code
for handling resolv.conf was unified into the resolvconffile package to provide
a consistent interface across the codebase.

This change provides better context for future developers about why the
refactoring was done and what benefits it brings.
Add context parameter to validInterfacesMap for better error handling and
logging. Move Windows-specific network adapter validation logic to the
ctrld package. Key changes include:

- Add context parameter to validInterfacesMap across all platforms
- Move Windows validInterfaces to ctrld.ValidInterfaces
- Improve error handling for virtual interface detection on Linux
- Update all callers to pass appropriate context

This change improves error reporting and makes the interface validation
code more maintainable across different platforms.
Move getDNS type definition from dns.go to os_linux.go where it is used.
Remove the now-empty dns.go file. This change improves code organization
by keeping platform-specific types with their implementations.
Break down the large DNS handling function into smaller, focused functions
with clear responsibilities:

- Extract handleDNSQuery from serveDNS handler function
- Create dedicated startListeners function for listener management
- Add standardQueryRequest struct to encapsulate query parameters
- Split special domain handling into separate function
- Add descriptive comments for each new function
- Improve variable names for better clarity (e.g., startTime vs t)

This refactoring improves code maintainability and readability without
changing the core DNS proxy functionality.
By looking for any additional dnsmasq configuration files under
/tmp/etc, and handling them like default one.
This change improves compatibility with newer UniFi OS versions while
maintaining backward compatibility with UniFi OS 4.2 and earlier.
The refactoring also reduces code duplication and improves maintainability
by centralizing dnsmasq configuration path logic.
cuonglm and others added 4 commits September 22, 2026 21:06
Running with --silent still created and grew log files in cd mode.
needInternalLogging() only checked cdUID and Service.LogPath, so a
--silent flag enabled internal logging, persisted it to disk, and
reset the global log level back to debug, overriding the NoLevel that
--silent had set.

Return false from needInternalLogging() when silent is set, so ctrld
neither creates the internal log file nor writes debug logs. Add
regression tests asserting needInternalLogging() is false in silent mode
and that initInternalLogging() creates no log file.
Keep missing interfaces as debug skips while preserving enumeration
causes and error-level failures restoring DNS on existing interfaces.
Adapt the logger and NetworkManager method to master. Stub saved DNS
lookup in tests because master stores it through ctrld rather than the
CLI homedir; tests must not touch the service's saved DNS files.
NextDNS serves alternative DoH endpoints beside dns.nextdns.io: the
ultralow and anycast variants of dns, dns1 and dns2. They are the same
service and take the same client info headers, but isNextDNS matched
the one host, so an upstream pointed at any of them was treated as a
third party and sent no client info.

Recognition now follows the parent domain through dns.IsSubDomain, as
IsControlD already does for the ControlD domains. The label by label
comparison is case insensitive and keeps lookalikes such as
notnextdns.io and nextdns.io.example.com out.

Based on the change proposed by Mike (Github username @mike406).
See: #335.
@cuonglm
cuonglm force-pushed the release-branch-v2.0.0 branch from da80a0f to 682c6c8 Compare September 22, 2026 14:08
cuonglm and others added 25 commits September 22, 2026 21:18
Remove empty and root domain (".") entries from search domains list
to prevent systemd-resolved errors. This addresses the issue where
systemd doesn't allow root domain in search domains configuration.

The filtering ensures only valid search domains are passed to
systemd-resolved, preventing DNS operation failures.
checkDnsLoop resolved with context.Background() and no dns.Client
timeout, so the DNS client fell back to its own 2s default and the
configured upstream timeout was ignored. The probes run serially on a
one-minute ticker, so each unreachable local upstream stalled the loop
for a fixed 2s no matter what the config asked for.

This is most visible on Windows, where UDP to a closed loopback port
does not surface an immediate refusal the way it does on Linux, so the
probe waits out the full default.

Apply the upstream timeout the way checkUpstreamOnce already does, and
keep 2s for an upstream that configures none so the unconfigured case
is unchanged.

The probe context deliberately carries no logger. A resolver logs its
own failure at error level with the endpoint in the message, which
would place an Internal Domain resolver address in the retained
journal; logUpstreamProbeFailure remains the reporter and bounds what
the line may hold. TestInternalDomainsLoopCheckHidesResolverAddress
covers this.
captureDebugMainLog swaps the process-wide mainLog, so every
mainLog.Load().Warn()/.Error() site in the package writes into the
buffer while it is installed, including goroutines left running by
other tests. retainedProbeLines returned every retained line and the
tests asserted field values over all of them, so one unrelated warn
failed the run.

Test_checkDnsLoopKeepsACustomKeyOutOfTheJournal hit this on Windows,
where the loop probe held the buffer open for two seconds:

    upstream_probe_journal_test.go: field "upstream": got , want upstream.custom

The sibling tests were exposed the same way; they index retained[0]
after a strict length check, so a stray line fails them as got 2, want 1.

Let retainedProbeLines take the messages a caller owns and return only
those lines. wantNoProbeSecret keeps scanning every retained line,
which is what that leak check is for.
Decode networksetup entries once for interface lookup and DNS-target
selection. Preserve first-enabled lookup and fail-closed DNS uniqueness
as separate policies. Treat only the (*) index as disabled, preserving
asterisks and whitespace in enabled service names.
Retain master's context-aware VPN and resolver APIs, per-program logger,
shutdown fencing, native CLAT service identity and firewall behavior.
Keep saved-DNS paths owned by the root ctrld package; isolate the added
ownership/sweep tests with a shared path seam and temporary backups.

Preserve the source DNS64 DNSSEC/cache gates, longest-suffix VPN routing,
DNS-journal convergence, scoped IPv6 handling, target ownership checks,
custom-listener regressions and documented native QA limits.
The auto-added AD split rules route corp names to upstream.os, but a rule
match returned before the LAN-query marking, so osResolver raced the domain
controller against 76.76.2.0 and sent every AD lookup (DC hostnames,
DC-locator SRV names carrying the machine name, WPAD) there in plaintext.
Mark a matched query under the machine's AD domain whose upstreams are the
OS resolver alone as a LAN query, which drops 76.76.2.0 for it.
The previous change marked a rule-matched AD query as a LAN query, but
two paths still reach the OS resolver without that mark, so osResolver
raced the domain controller against 76.76.2.0 in plaintext:

  - the DNS-intercept recovery bypass, which forwards queries before
    rule handling runs
  - the OS-resolver retry catch-all after an explicit upstream fails
    with leak_on_upstream_failure enabled

Mark a query under the detected AD domain on both paths. Other names
keep the public fallback, and an explicit upstream is still tried first.

Build resolvers in queryUpstream() through newResolverFn, so tests drive proxy()
and assert the LAN mark that reaches each resolver, instead of testing
the predicate alone.
During the reported incident the API answered "Maintenance is in
progress. Please try again later." after a successful DNS resolution
and TCP connection, and ctrld reported it as API_UNREACHABLE. Nothing
in the client could tell that answer apart from the API refusing the
request, so callers had to guess from an HTTP status that does not
carry the distinction.

Add IsMaintenance. The API answers every public request in hard
maintenance mode with HTTP 503 and error.code 50302
(Application::ERROR_HARD_MAINTENANCE), exported as MaintenanceCode, and
IsMaintenance classifies on that code. It falls back to the message for
deployments that answer without the code, which a test covers so that
path cannot silently drop. The answer's "body" is an empty JSON array;
a test pins that the error decode does not depend on it.

50301 (service unavailable) and 50003 (read-only mode) are deliberately
not maintenance: the first is a failure, and read-only only gates
authenticated controllers, so it never answers /utility or /logs.

Maintenance is temporary and says nothing about the configuration that
was asked for. The HTTP status cannot classify it on its own, because a
maintenance answer can arrive with a client-error status that otherwise
means the same request will be refused again, so callers must ask this
question before they act on a status.
A hard maintenance window took DNS down on endpoints that already had a
working configuration: the bootstrap preflight read the maintenance answer
as a failed fetch and exited, turning a temporary API outage into an
endpoint outage.

Startup now falls back to the configuration on disk when a content- and
identity-bound marker records that the API produced it. Without one, ctrld
stops with API_MAINTENANCE (exit 37). Maintenance never self-uninstalls or
counts as a permanent rejection, a fallback run retries every 5-7 minutes
until a recovered configuration is applied, and deactivation is refused
with 503 while the PIN cannot be verified.

Master adaptation: pending-recovery and PIN-known handling live in
applyFetchedResolverConfig; logging uses *ctrld.Logger; the reload fetch
seam is fetchResolverConfigFn; the control-server and self-uninstall
probes use the existing fetchResolverConfig seam with loggerCtx.
…nce start

A maintenance fallback start holds no resolver config, and Allowed
Destinations (a master-only Firewall Mode feature) are not persisted with
the configuration on disk. Under Firewall Mode the exception set is
therefore empty until the API answers, so raw-IP destinations the
organization allowed stay blocked for the window, and a persisted pf
table from the previous run is replaced by the empty set.

Keep that fail-closed on purpose: the list is the organization's network
topology and is kept out of persisted state. Journal a Warn when a
fallback start applies the empty set under Firewall Mode so the blocked
destinations are explained, and pin that the first answer after recovery
re-applies the list.

Also capitalize the two managed-config marker log messages to match the
rest of master.
Internal Domains now support three modes. "os" is unchanged.
"resolvers" becomes explicit resolver with network fallback: the
configured resolvers are tried first, and a timeout, unreachable
resolver, SERVFAIL or NXDOMAIN hands the query to matching VPN DNS
servers, then domain-less VPN DNS servers, then the OS resolver's LAN
nameservers through the new LanOnlyQueryCtx. "resolvers_only" keeps
the previous strict behavior. Down fallback-mode resolvers are skipped
and re-checked in the background at most every 30s. Generated upstreams
are identified by the UpstreamConfig.InternalDomain marker instead of
the internal_ key prefix.

Master adaptations: the fallback state lives on proxyRequest and is
applied in tryUpstreams, shouldContinueWithNextUpstream and
handleAllUpstreamsFailure instead of v1.0's monolithic proxy loop.
Resolver construction uses master's context-aware NewResolver, and the
background re-check marks its context private so transport errors that
name the resolver stay at debug. Tests use master's zap JSON capture
and VPN manager logger.
- Stop closes the control server right after the deactivation-pin check,
  so a stopping process no longer answers /started for its successor.
  releaseResources still closes it for stop paths that skip Stop; a
  sync.Once makes the double close safe.
- upgrade and restart wait for a 200 from /started via
  newReadySocketControlClient; a 408 (start never finished) now rolls the
  upgrade back and makes restart warn instead of reporting success.
- The plain socket client still accepts any answer, for the
  deactivation-pin path.
`ctrld start --cd-org <code>` logged its arguments as-is in the debug line
`intercept upgrade check: args=[...]`. redactedArgs formats them through
redactSecrets with provisionSecrets, covering --cd-org and both parts of
--cd, in either flag form.
TestNetworkShutdownCancelsIndefiniteRecoveryWait relied on an empty
upstream map never recovering. Since 73db7adc an empty pool checks
upstream.os instead, so the test probed the host's real OS resolver.
On a host with working DNS that check succeeds, recovery completes, and
it reads the upstream monitor the test never built.

Recover against a loopback port nothing listens on, so the wait can
never end on its own, and give the prog the upstream monitor that run
always creates before network monitoring can start a recovery. The
panic is test-only: production never reaches recovery without one.
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.

4 participants