fix: back-fill InventoryItem.SiteName at the cache layer#56
Merged
Conversation
For Meraki devices, InventoryItem.SiteName is always empty — convertDeviceToInventoryItem only copies NetworkID off the source payload (no network name is on that object). reset_ap's fallback chain (device.SiteName || device.SiteID) then resolves to the raw L_… network ID, which surfaces in both the confirmation prompt and the success message: Rebooting AP ap15-2.mex1 at L_3732358191183298569 via meraki (meraki)… Resolve the site once via the cache accessor (already used a few lines below for the optional `site <name>` guardrail) and prefer the resolved name. Failure to resolve is non-fatal — we still fall back through device.SiteName → device.SiteID so unassigned-device edge cases keep working. Side effect: the `site <name>` guard now compares against the same resolved label, so its error message reads "site X, not Y" using the human name even when InventoryItem.SiteName was empty. Backfilling SiteName at the inventory-build stage (Meraki converter or cache index) would fix this everywhere at once but needs cross- referencing the sites cache during conversion; out of scope here.
Vendor converters (notably Meraki) only copy the site/network ID off
the device payload — the human-readable name lives on the sites
endpoint, not the device one. Every consumer that needed to display a
site name was working around this individually (or, more commonly,
not at all — surfacing raw IDs like L_3732358191183298569).
Fix the root cause at the cache layer:
- APICache.BackfillInventorySiteNames walks Inventory.{AP,Switch,Gateway}
and fills SiteName from SiteIndex.ByID.
- Called from saveAPICacheLocked right after RebuildSiteIndex, so
every freshly saved cache has correct names.
- Also called from GetAPICache after unmarshal, so caches written
by older binaries get back-filled on read — no forced refresh
needed for the fix to take effect.
cmd/reset_ap simplifies: device.SiteName is now reliable. The
GetSiteByID fallback stays as defense against pre-fix caches and
inventory records that pre-date their site entries.
Test:
TestCacheManager_SaveAndLoadAPICache now seeds the AP with
SiteID="site-001" and asserts the loaded record has
SiteName="US-LAB-01", proving the cache layer back-fills correctly.
ravinald
changed the title
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
1 participant
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
Surface human-readable site names everywhere by fixing the root cause at the cache layer, instead of having each command work around it.
Before (Meraki AP via
reset ap):```
Rebooting AP ap15-2.mex1 at L_3732358191183298569 via meraki (meraki)…
Reboot request accepted for ap15-2.mex1 at L_3732358191183298569 (meraki:meraki). AP will restart in a few seconds.
```
After:
```
Rebooting AP ap15-2.mex1 at MEX-OFFICE-01 via meraki (meraki)…
Reboot request accepted for ap15-2.mex1 at MEX-OFFICE-01 (meraki:meraki). AP will restart in a few seconds.
```
Root cause
Vendor inventory converters only copy the site/network ID off the device payload — the name lives on a separate sites endpoint. Result: `InventoryItem.SiteName` is empty for every Meraki device (and most Ubiquiti ones too). Every caller that wants to display a site name has been either falling back to the raw ID or doing its own `GetSiteByID` lookup.
Fix
Add `APICache.BackfillInventorySiteNames()` (`internal/vendors/cache_types.go`) that walks `Inventory.{AP,Switch,Gateway}` and sets `SiteName` from `SiteIndex.ByID`. Wire it into two chokepoints:
`cmd/reset_ap.go` simplifies: `device.SiteName` is now the primary path, with the `GetSiteByID` lookup demoted to a fallback (covers the narrow window between an old cache and the first save).
Files
Downstream impact
Every consumer that reads `InventoryItem.SiteName` from the cache now gets a populated value. Confirmed cache consumers via grep: `cmd/show`, `cmd/search`, `cmd/refresh`, the new `cmd/reset` — all should display the configured site name where they previously may have shown empty or the raw ID.
Test plan