fix(plumbing): stop fallback factories overtaking specific ones - #420
Draft
james-nesbitt wants to merge 1 commit into
Draft
fix(plumbing): stop fallback factories overtaking specific ones#420james-nesbitt wants to merge 1 commit into
james-nesbitt wants to merge 1 commit into
Conversation
Provider.Get promotes a winning factory to the front of the list to speed up later lookups. That is only sound while no factory matches a superset of another's inputs, and DefaultRegistry violates it. os.DefaultRegistry registers [ResolveLinux, ResolveLinuxCompat, ResolveWindows, ResolveDarwin]. ResolveLinuxCompat is a last-resort resolver for hosts with no os-release file, and it accepts any Linux host, reporting ID "linux" with no version. Resolving a Windows host swaps index 0 with index 2, leaving [Windows, LinuxCompat, Linux, Darwin]. The compat resolver now sits ahead of ResolveLinux, so every Linux host resolved afterwards is reported as "linux" instead of its real distribution. Because the registry is a process-global built with sync.OnceValue, that persists for the life of the process. Any consumer managing a mixed Linux/Windows fleet hits this: the Linux hosts are identified correctly until the first Windows host is resolved, and incorrectly from then on. It is order-dependent, so it presents as Linux-only runs working while mixed runs fail. Add Provider.RegisterFallback for factories that match a superset of another's inputs. Fallbacks are consulted only after every Register'ed factory has declined, and are never reordered or promoted, so a superset factory can no longer answer in place of the specific one it backs up. Register ResolveLinuxCompat through it. Both tests fail without the change. The plumbing test registers the superset factory between the two specific ones, which is the position that breaks, and the os test reproduces the real sequence: resolve a Windows host, then check a Linux host is still identified correctly. Written by AI: claude-sonnet-5 Signed-off-by: James Nesbitt <jnesbitt@mirantis.com>
james-nesbitt
force-pushed
the
fix/provider-fallback-ordering
branch
from
August 6, 2026 15:54
3ba3070 to
9f4c72a
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
Adds
Provider.RegisterFallbackfor factories that match a superset of another factory's inputs, and registersResolveLinuxCompatthrough it.Why
Provider.Getpromotes a winning factory to the front of the list to speed up later lookups:That is only sound while no factory matches a superset of another's inputs — and
os.DefaultRegistryviolates it.The registry is
[ResolveLinux, ResolveLinuxCompat, ResolveWindows, ResolveDarwin].ResolveLinuxCompatis a last-resort resolver for hosts with no os-release file, and it accepts any Linux host, reportingID: "linux"with no version.Resolving a Windows host swaps index 0 with index 2:
[Linux, LinuxCompat, Windows, Darwin][Windows, LinuxCompat, Linux, Darwin]ResolveLinuxCompatnow sits ahead ofResolveLinux, so every Linux host resolved afterwards is reported aslinuxrather than its real distribution.DefaultRegistryis a process-global built withsync.OnceValue, so this persists for the life of the process.Any consumer managing a mixed Linux/Windows fleet is affected. It is order-dependent, so it presents as Linux-only runs working while mixed runs fail.
How this showed up
Found while migrating Mirantis/launchpad from rig v0 to v2. Linux hosts failed configurer lookup with
unsupported OS: linux, but only in clusters that also contained a Windows host — the Windows host resolved first and reordered the registry.Worth noting the failure is hard to diagnose from the outside: the consumer sees a plausible-looking
ReleasewithID: "linux", not an error. In our case that cost a full CI run to track down. I also verified against a live host thatResolveLinuxitself was working correctly (20/20 runs) before looking at ordering.Approach
RegisterFallbackkeeps fallbacks in a separate slice, consulted only after everyRegistered factory has declined. They are never reordered or promoted, so registration order between them is preserved.I chose this over removing the promotion optimisation, since the optimisation is worthwhile and the real problem is that a superset factory was registered as if it were a peer. It is additive — no change for existing
Registercallers.GetAllalso consults fallbacks, after the ordinary factories.Testing
Both tests were confirmed to fail without the change:
plumbing:TestRegisterFallbackIsNeverPromotedregisters the superset factory between the two specific ones, which is the position that actually breaks. With the fallback treated as an ordinary factory it returnsfallbackwherespecificis expected.os:TestDefaultRegistryOrderingIsStablereproduces the real sequence — resolve a Windows host, then assert a Linux host is stillubuntu/22.04. Without the fix:got ID "linux" version "".TestDefaultRegistryStillFallsBackToCompatconfirms the fallback is still reached for a host with no os-release file, which is the case it exists for.gofmt -lreports 8 files on this repo, all pre-existing onmainand untouched here.)Happy to adjust naming or reshape this if you would prefer a different mechanism — e.g. a priority argument on
Register, or simply dropping the promotion.Written by AI: claude-sonnet-5