Fix linux detection when cluster contains windows hosts

[james-nesbitt]

What this accomplishes

Linux hosts are identified correctly again. They were reported as ID: "linux" with no version, but only in fleets that also contained a Windows host. Found migrating Mirantis/launchpad to v2, where it surfaced as unsupported OS: linux. The consumer got a plausible-looking Release instead of an error, which made it expensive to diagnose from outside.

Detection no longer depends on which host resolved first. Provider.Get used to move the matching factory to the front of the list, so one host's lookup changed the next one's. Beyond the bug above, this silently broke sudo: resolving an ordinary sudo host promoted Sudo past UID0Noop, and every root host after that had its commands wrapped in sudo -n -- sh -c ... instead of run directly. Lookups now consult factories in registration order and never reorder them.

Detection for separate hosts now runs concurrently. Reordering mutated the shared slice, so Get held an exclusive lock across the factory probes — which are remote commands — serialising all OS / init-system / sudo / package-manager detection process-wide. Get takes a read lock now.

Callers can override a built-in factory. Previously they couldn't: registries only append, and ResolveLinuxCompat matches every Linux host os-release cannot name, so a resolver added for such a host was unreachable. rig's own factories avoid this by standing down for hosts a more specific factory handles, but that is only available to whoever owns the broad factory.

• Provider.RegisterFirst(f) puts a factory at the front, ahead of everything registered before or after it.
• RegisterDefaults(reg) in os, sudo, initsystem, packagemanager and remotefs registers what DefaultRegistry holds, so building your own registry doesn't mean listing rig's factories by hand and missing ones added later.

reg := packagemanager.NewRegistry()
mypkg.RegisterFoo(reg)               // tried first
packagemanager.RegisterDefaults(reg) // then everything rig ships, now and later

Worth a reviewer's attention

• ResolveLinux now requires an ID from os-release. kv.Decoder returns no error for empty input, so it previously claimed hosts with ID: "". Those hosts now reach the compat resolver, which can still name them from their package manager.
• A Factory may now be called concurrently, and says so. Audited all five registries: none closes over mutable state.
• RegisterFirst called twice means the most recent call wins. This is deliberate — "first" means first.

Testing

Every guard was confirmed to fail with the change it covers reverted, including the RegisterFirst ones against an appending implementation. golangci-lint 0 issues · go vet clean · full suite green · -race clean. Each commit is green on its own, so the history bisects.

The RegisterFirst would let an operator override the built-in zypper factory in #419, but newUniversalPackageManager is unexported, so changing one install flag still means reimplementing PackageManager.

---

Credit to @james-nesbitt for the diagnosis, the reproduction and the test fixtures, which this keeps. Dropping the reordering was the alternative offered in the original description — thanks for laying the options out.

[Copilot]

Pull request overview

This PR prevents “superset” resolver factories (intended as last-resort fallbacks) from being promoted ahead of more specific factories during Provider.Get lookups, which previously could cause incorrect OS detection in the process-global os.DefaultRegistry.

Changes:

• Add Provider.RegisterFallback and a separate fallback factory list that is consulted only after all normal factories, and never promoted/reordered.
• Register ResolveLinuxCompat as a fallback in the OS default registry (and via RegisterLinuxCompat) to avoid overtaking ResolveLinux.
• Add targeted tests in plumbing and os to reproduce and guard against the ordering/promotion regression.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
plumbing/provider.go	Adds fallback factory support and updates Get/GetAll to consult fallbacks last without promotion.
plumbing/provider_test.go	Adds tests ensuring fallbacks aren’t promoted and are included by GetAll.
os/linux_compat.go	Switches Linux compat resolver registration to use RegisterFallback.
os/defaultprovider.go	Ensures DefaultRegistry registers ResolveLinuxCompat as a fallback to avoid ordering corruption.
os/defaultprovider_test.go	Adds regression tests reproducing the Windows-then-Linux misclassification and verifying fallback behavior remains intact.

Suppressed comments (1)

plumbing/provider.go:70

• The GetAll doc comment also refers to factories "not error"ing, but factories indicate matches via the boolean return value. Consider rewording to avoid implying factories can error.

// GetAll retrieves all values of type T from the Factories in the Provider,
// followed by any registered with RegisterFallback.
// If none that does not error can be found, the error supplied at creation time is returned.
func (p *Provider[R, T]) GetAll(r R) ([]T, error) {

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[kke]

Nice.

This seems to be similar problem as there was with #411

Copilot's problems seem to be nitpicking about comments only.

I can finish this.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

os/defaultprovider.go:20

• This registry tier does not implement the complementary resolver predicates promised by the PR. ResolveLinux still returns success for an empty-but-successfully-read os-release (linux.go:23-36), so this fallback is not reached and callers still receive an empty ID; ResolveLinuxCompat also still accepts hosts with a valid os-release (linux_compat.go:37-78) when invoked directly or registered normally. Please share the os-release read, require a non-empty ID in ResolveLinux, and make the compat resolver decline when that ID is present.

		// ResolveLinuxCompat accepts any Linux host, so it must never be able to
		// answer ahead of ResolveLinux. See RegisterFallback.
		provider.RegisterFallback(ResolveLinuxCompat)

plumbing/provider.go:42

• Keeping move-to-front for ordinary factories leaves precedence unstable. In the existing defaults, resolving a non-root sudo host can promote Sudo ahead of UID0Noop, causing a later root host with sudo installed to be wrapped unnecessarily; likewise, promoting WinSCM can put SysVinit ahead of Systemd on a systemd host with /etc/init.d. The exclusive lock also remains held across remote probes, so host detection is still serialized. Preserve registration order and use the read lock as described by the PR.

// The first factory that does not error is moved to the front of the list to optimize
// future lookups. Factories added with RegisterFallback are tried last, in
// registration order, and are never reordered.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (2)

plumbing/provider.go:27

• This reverses the precedence implemented below: Register appends, and Get checks from the beginning, so a factory added later cannot shadow an earlier factory; it can only be shadowed by it. Rephrase the guidance in terms of callers assembling registries in different orders so the public documentation does not misstate registration behavior.

// stands down for any host os-release can identify. A registry stays open for
// registration, so a factory that leans on being registered first can be shadowed
// by one a caller adds later.

plumbing/provider.go:9

• Get and GetAll can be invoked concurrently with the same input, and the read lock allows those calls to enter the same factory simultaneously. Restricting this warning to “different inputs” understates the new concurrency contract and could lead external factory implementations to keep unsynchronized per-input state. Document that factories must be safe for concurrent calls regardless of whether the input is the same.

This issue also appears on line 25 of the same file.

// A Factory may be called concurrently with itself for different inputs, so it
// must not depend on being called one at a time.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (3)

os/linux_compat.go:50

• This order-independence claim is broader than the implementation: the compat resolver excludes only inputs handled by ResolveLinux, so a caller's custom resolver for an ID-less Linux host can still be shadowed when registered after compat. Narrow the comment to the two built-in Linux resolvers to avoid promising unsupported extensibility.

	// host keeps the two complementary however the registry happens to be
	// ordered, including once a caller has added resolvers of their own. yum

os/linux_compat.go:98

• “The other resolvers” implies order independence with arbitrary registered resolvers, but this factory only self-excludes for ResolveLinux. A later custom resolver for Linux without a usable os-release remains unreachable, so document the actual pairwise guarantee.

// It excludes itself on any host ResolveLinux can identify, so it does not matter
// when it is registered relative to the other resolvers.

os/linux_compat.go:46

• The public function documentation still says this fallback is only for absent os-release files, but the new readOSRelease check intentionally also routes unreadable, malformed, and ID-less files here. Update the contract so callers understand the expanded behavior.

This issue also appears in the following locations of the same file:

• line 49
• line 97

	// ResolveLinux identifies any host whose os-release names the distribution,

[kke]

Yeah I think the optimization has to go.

The resolvers should be run in registration order and there should be a RegisterFirst or similar so consumers can prepend stuff without the built-ins taking precedence.

The more correct way for consumers to use it would still be cloning the default registry and then prepending stuff there, as mutating the global one is a bit ugly.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (1)

os/linux_compat.go:62

• This second os-release probe can eliminate the fallback after a transient command failure. In the default order, ResolveLinux may decline because its first read failed; if this retry succeeds, compat also declines, and there is no later ResolveLinux attempt, so Get returns ErrNotRecognized. Avoid independently probing the same condition from both factories—either rely on the now-stable Linux-before-compat order and let compat handle the prior failure, or share a single probe result across the two resolvers.

	if _, ok := readOSRelease(conn); ok {
		log.Trace(context.Background(), "linux compat resolver: os-release identifies the host, deferring to the standard resolver",
			log.HostAttr(conn),
		)

		return nil, false