From 1fff2c07a98690c8ef4a41b2c33b37b3cf0a46e7 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 28 Jun 2026 12:05:44 +0200 Subject: [PATCH] feat(health): show device name and IP on per-device findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-device health findings previously labelled the device by account and device IDs only (e.g. "account 3230304 · device 08DF1F0BA325"), which is hard to place at a glance. Add display-only Name and IP fields to health.Target and fill them centrally via EnrichTargets after the checks run, so individual checks don't each have to look up the device record. Both the live health endpoint and the diagnostic export go through the new Server.runHealthChecks helper, and the Health tab renders the friendly name first, then account/device IDs, then IP. Fixes match on Account+Device only, so the new fields don't affect quick-fix dispatch. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/service/handlers/handlers_export.go | 2 +- .../handlers/handlers_health_checks.go | 34 ++++++++++++++- pkg/service/handlers/web/js/script.js | 4 +- pkg/service/health/health.go | 39 ++++++++++++++++- pkg/service/health/health_test.go | 43 +++++++++++++++++++ 5 files changed, 118 insertions(+), 4 deletions(-) diff --git a/pkg/service/handlers/handlers_export.go b/pkg/service/handlers/handlers_export.go index 5281d08..c50f4d9 100644 --- a/pkg/service/handlers/handlers_export.go +++ b/pkg/service/handlers/handlers_export.go @@ -781,7 +781,7 @@ func (s *Server) buildDiagnosticReport(redirectCfgs map[string]*redirectConfig) } if s.healthRegistry != nil { - report.HealthChecks = s.healthRegistry.RunAll() + report.HealthChecks = s.runHealthChecks() } devices, err := s.ds.ListAllDevices() diff --git a/pkg/service/handlers/handlers_health_checks.go b/pkg/service/handlers/handlers_health_checks.go index 71017a5..9234c8c 100644 --- a/pkg/service/handlers/handlers_health_checks.go +++ b/pkg/service/handlers/handlers_health_checks.go @@ -44,7 +44,7 @@ func (s *Server) HandleHealthChecks(w http.ResponseWriter, _ *http.Request) { resp := healthChecksResponse{ GeneratedAt: time.Now().UTC().Format(time.RFC3339), - Checks: s.healthRegistry.RunAll(), + Checks: s.runHealthChecks(), } w.Header().Set("Content-Type", "application/json") @@ -55,6 +55,38 @@ func (s *Server) HandleHealthChecks(w http.ResponseWriter, _ *http.Request) { } } +// runHealthChecks runs every registered check and enriches each +// per-device finding's Target with the device name and IP (display +// only) so the UI and the diagnostic export can show a friendly label +// instead of bare IDs. Shared by the live health endpoint and the +// diagnostic export. +func (s *Server) runHealthChecks() []health.CheckResult { + results := s.healthRegistry.RunAll() + + if s.ds == nil { + return results + } + + devices, err := s.ds.ListAllDevices() + if err != nil { + return results + } + + name := make(map[string]string, len(devices)) + ip := make(map[string]string, len(devices)) + + for i := range devices { + name[devices[i].DeviceID] = devices[i].Name + ip[devices[i].DeviceID] = devices[i].IPAddress + } + + health.EnrichTargets(results, func(deviceID string) (string, string) { + return name[deviceID], ip[deviceID] + }) + + return results +} + // HandleHealthFix dispatches a quick-fix identified by // (checkId, fixId) against the supplied target. Returns 404 when // the fix isn't registered (typically a stale UI), 400 on a diff --git a/pkg/service/handlers/web/js/script.js b/pkg/service/handlers/web/js/script.js index 942e376..d6e666f 100644 --- a/pkg/service/handlers/web/js/script.js +++ b/pkg/service/handlers/web/js/script.js @@ -4305,14 +4305,16 @@ function renderFinding(checkId, finding) { row.appendChild(title); const target = finding.target || {}; - if (target.account || target.device) { + if (target.account || target.device || target.name || target.ip) { const t = document.createElement("div"); t.style.fontSize = "0.8em"; t.style.color = "#666"; t.style.marginTop = "4px"; const parts = []; + if (target.name) parts.push(target.name); if (target.account) parts.push(`account ${target.account}`); if (target.device) parts.push(`device ${target.device}`); + if (target.ip) parts.push(target.ip); t.textContent = parts.join(" · "); row.appendChild(t); } diff --git a/pkg/service/health/health.go b/pkg/service/health/health.go index bbf5017..6e2f2c2 100644 --- a/pkg/service/health/health.go +++ b/pkg/service/health/health.go @@ -32,14 +32,21 @@ const ( SeverityError Severity = "error" ) -// Target identifies what a Finding is about. Both fields are +// Target identifies what a Finding is about. Account and Device are // optional: a service-wide finding leaves both empty, a // per-account finding fills Account only, and the common // per-device case fills both. The UI displays the populated fields // as a small label next to the finding. +// +// Name and IP are display-only conveniences for per-device findings, +// filled in centrally by EnrichTargets after the checks run (the +// checks themselves don't all have them in scope). Fixes match on +// Account+Device only, so these never affect RunFix dispatch. type Target struct { Account string `json:"account,omitempty"` Device string `json:"device,omitempty"` + Name string `json:"name,omitempty"` + IP string `json:"ip,omitempty"` } // QuickFix is a remediation a user can trigger from the UI with a @@ -207,6 +214,36 @@ func (r *Registry) RunAll() []CheckResult { return out } +// EnrichTargets fills the display-only Name and IP on every finding +// Target that carries a Device but doesn't already have them, using +// resolve. resolve should return ("", "") for an unknown device, which +// leaves the Target unchanged. A nil resolve is a no-op. This is a +// post-processing pass so individual checks don't each have to look up +// the device record just to label their findings. +func EnrichTargets(results []CheckResult, resolve func(deviceID string) (name, ip string)) { + if resolve == nil { + return + } + + for i := range results { + for j := range results[i].Findings { + t := &results[i].Findings[j].Target + if t.Device == "" || (t.Name != "" && t.IP != "") { + continue + } + + name, ip := resolve(t.Device) + if t.Name == "" { + t.Name = name + } + + if t.IP == "" { + t.IP = ip + } + } + } +} + // RunFix dispatches to the FixFunc registered for (checkID, fixID). // Returns the user-facing success message, whether the UI should // re-fetch health afterwards, and any execution error. diff --git a/pkg/service/health/health_test.go b/pkg/service/health/health_test.go index 63bde3a..56e1193 100644 --- a/pkg/service/health/health_test.go +++ b/pkg/service/health/health_test.go @@ -99,3 +99,46 @@ func TestRegistry_Register_ReplacesByID(t *testing.T) { t.Errorf("expected title to be replaced, got %q", results[0].Title) } } + +func TestEnrichTargets_FillsNameAndIPForDeviceFindings(t *testing.T) { + results := []CheckResult{{ + ID: "c1", + Findings: []Finding{ + {Target: Target{Account: "3230304", Device: "08DF1F0BA325"}}, // per-device: enriched + {Target: Target{Account: "3230304"}}, // account-only: untouched + {Target: Target{}}, // service-wide: untouched + {Target: Target{Device: "UNKNOWNDEV"}}, // not in resolver: left as-is + }, + }} + + resolve := func(deviceID string) (string, string) { + if deviceID == "08DF1F0BA325" { + return "Cantina", "192.0.2.9" + } + return "", "" + } + + EnrichTargets(results, resolve) + + got := results[0].Findings + if got[0].Target.Name != "Cantina" || got[0].Target.IP != "192.0.2.9" { + t.Errorf("per-device finding not enriched: %+v", got[0].Target) + } + if got[1].Target.Name != "" || got[1].Target.IP != "" { + t.Errorf("account-only finding should be untouched: %+v", got[1].Target) + } + if got[2].Target.Name != "" || got[2].Target.IP != "" { + t.Errorf("service-wide finding should be untouched: %+v", got[2].Target) + } + if got[3].Target.Name != "" || got[3].Target.IP != "" { + t.Errorf("unknown device should be left as-is: %+v", got[3].Target) + } +} + +func TestEnrichTargets_NilResolveIsNoOp(t *testing.T) { + results := []CheckResult{{Findings: []Finding{{Target: Target{Device: "D1"}}}}} + EnrichTargets(results, nil) // must not panic + if results[0].Findings[0].Target.Name != "" { + t.Error("nil resolve should be a no-op") + } +}