Repository navigation
fix(inventory): own writes, overlays, and lookup names consistently - #32
Merged
Merged
Conversation
Provider writes chose the last-merged source for inventory.providers.<p>, so any root-config block for a provider captured every later write and shadowed the included provider file. Prefer the included file that declares the provider type, and prune the root's now-redundant host, group, or provider copy after writing the merged value there. findInventoryHost returned whichever same-named entry the unstable sort put first, so local hosts listed by a provider too could be treated as provider-owned and get root auth overrides. Prefer the local definition. The host catalog filled aliases from Go map iteration with last-writer-wins, so a short name shared by two hosts resolved randomly between them. Rank names (full name > explicit alias > derived short name) and break ties by canonical name. inv set now skips a derived short alias already owned by another host and rejects a colliding explicit --alias.
The connect catalog and the inv commands resolved provider hosts with different rules. The catalog treated provider YAML hosts as overlays but let a cached provider object replace a local host of the same name; the inv listing showed each override as a second row beside an unmerged provider object, so inv set picked between them at random. Share one rule set in internal/connect: a provider YAML host overlays the provider object it names, and a local inventory host shadows a provider object with the same name. Local inventory is operator-owned and provider state is a refreshable cache, so precedence is decided at read time rather than by filtering during refresh, and it takes effect without a refresh. inv list/get/set now see one row per host with overrides applied. inv status reports shadowed objects and overrides that match no provider object; inv get shows what a local host shadows.
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.
Summary
Inventory edits made through nssh kept getting written into the root
config.yamlinstead of the included provider files, and aliases shared by two hosts resolved to a random host. A host defined locally and also cached from NetBox could also lose its local settings, andinv listshowed overridden NetBox hosts twice. This PR fixes all four.Implementation Notes
Choosing the write file
InventoryProviderSourcenow picks the included file that declares a provider'stypeover a root-config block for the same provider. Before, the root file always won because it is loaded last, so one strayinventory.providers.<p>block captured every laterinv set, compat-fix, and group save.PruneRootInventoryProviderShadowdeletes the root's redundant copy of a host, group, or whole provider after its merged value is written to the owning file. This keeps the effective config the same while stopping the root from overriding the file. It does nothing when the root itself owns the provider.Lookup names
inv setnow skips an automatic short alias that another host already has (with a warning) and rejects an explicit--aliascollision.Overrides and local shadowing
internal/connect/provider_hosts.go, used by the catalog, theinvlisting, andinv status/get:inventory.providers.<p>.hostsoverlays the provider object it names; it is not a second host.invlisting showed each override as its own row next to the cached NetBox object without the override applied, andfindInventoryHostchose between those rows at random.invcommands now see one row per host with overrides applied, and local definitions win.inv status <provider>lists shadowed objects and overrides that match no provider object; the dashboard shows the counts.inv getshows which NetBox object a local host shadows and suggests a provider override instead.Docs
skills/nssh/references/configuration-inventory-credentials.md: write ownership, lookup-name precedence, overlays, and shadowing.Verification
make container-test: all packages pass, golangci-lint reports 0 issues, and the release cross-builds succeed.inv listwent from 1,363 to 1,347 rows: 12 duplicate override rows and 4 shadowed objects are gone. Override rows now show the resolved username, andinv status netbox-prodnames the 4 shadowed hosts.TestManagedProxyAskpassPrefixRunsUnderFishfails on a macOS host with a fish shell onmaintoo; it passes in the container.