Skip to content

Commit 0da1de7

Browse files
authored
fix(desktop): don't crash when Prometheus discovery runs without a kubeconfig (#1787)
Fixes #1782 ## What Radar Desktop crashed the moment you opened Settings (or the Cost page) on a machine with no usable kubeconfig — the exact state a fresh install lands in. ## Why Desktop deliberately keeps running when kubeconfig init fails so the UI can show the error. But `RegisterCallbacks` still passed `k8s.GetClient()` — a nil `*kubernetes.Clientset` — into `prometheuspkg.Initialize` / `Reinitialize` / `traffic.ReinitializeWithConfig`, all of which take `kubernetes.Interface`. A typed nil in an interface is non-nil, so the `k8sClient == nil` guard in `discover()` never fired. `/api/opencost/summary` → `EnsureConnected` → `prom.Discover` then dereferenced it inside a `wg.Go` worker, where chi's Recoverer can't catch the panic, and the process exited. ## Fix - `k8s.GetClientInterface()` returns an untyped nil when there is no client; used at the three bootstrap sites that store the client in an interface. - Regression tests pin the untyped-nil and the pass-through case. ## Verified - Reproduced on macOS with `make desktop-binary` + empty `$HOME`: click Settings → `panic: nil pointer dereference` in `pkg/prom.collectWellKnownLocations`, process gone. - With the fix, the same build answers `/api/opencost/summary` with `{"available":false,"reason":"query_error",…}` and `POST /api/prometheus/connect` with `no Kubernetes client available for discovery`; the process stays up. - `go test ./internal/k8s ./internal/prometheus ./internal/app` and `pkg/prom` green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, targeted change at client handoff sites with tests; no auth or data-path logic changes beyond preventing nil dereference when the cluster client is missing. > > **Overview** > Fixes a **Radar Desktop process exit** when kubeconfig init fails but the UI still runs (e.g. fresh install with no kubeconfig). Opening Settings or hitting cost/Prometheus APIs could trigger discovery with a **typed nil** `*kubernetes.Clientset` stored in `kubernetes.Interface`, so downstream `client == nil` checks never ran and discovery panicked. > > Adds **`k8s.GetClientInterface()`**, which returns an **untyped nil** when there is no clientset, and switches Prometheus/traffic bootstrap and reinit callbacks in `RegisterCallbacks` to use it instead of `GetClient()`. **Regression tests** lock in untyped-nil vs pass-through behavior. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 6edb345. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 4b9f2e2 commit 0da1de7

3 files changed

Lines changed: 42 additions & 3 deletions

File tree

‎internal/app/bootstrap.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ func RegisterCallbacks(cfg AppConfig, timelineStoreCfg timeline.StoreConfig) {
281281
})
282282

283283
// Initialize Prometheus metrics client (must come before SetManualURL)
284-
prometheuspkg.Initialize(k8s.GetClient(), k8s.GetConfig(), k8s.GetContextName())
284+
prometheuspkg.Initialize(k8s.GetClientInterface(), k8s.GetConfig(), k8s.GetContextName())
285285

286286
if cfg.PrometheusURL != "" {
287287
u, err := url.Parse(cfg.PrometheusURL)
@@ -319,14 +319,14 @@ func RegisterCallbacks(cfg AppConfig, timelineStoreCfg timeline.StoreConfig) {
319319
}
320320

321321
k8s.RegisterTrafficFuncs(traffic.Reset, func() error {
322-
return traffic.ReinitializeWithConfig(k8s.GetClient(), k8s.GetConfig(), k8s.GetContextName())
322+
return traffic.ReinitializeWithConfig(k8s.GetClientInterface(), k8s.GetConfig(), k8s.GetContextName())
323323
})
324324

325325
// Reinitialize carries the current manual URL + headers forward (including any
326326
// applied live via /integrations/prometheus). Re-applying the captured startup
327327
// cfg here would revert a live change on context switch, so we don't.
328328
k8s.RegisterPrometheusFuncs(prometheuspkg.Reset, func() error {
329-
prometheuspkg.Reinitialize(k8s.GetClient(), k8s.GetConfig(), k8s.GetContextName())
329+
prometheuspkg.Reinitialize(k8s.GetClientInterface(), k8s.GetConfig(), k8s.GetContextName())
330330
return nil
331331
})
332332
// Reinitialize only builds the new client; discovery for the new cluster has

‎internal/k8s/client.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,19 @@ func GetClient() *kubernetes.Clientset {
732732
return k8sClient
733733
}
734734

735+
// GetClientInterface returns the clientset as kubernetes.Interface, or an
736+
// untyped nil when no client exists. Callers that store the result in an
737+
// interface must use this rather than GetClient(): a nil *Clientset wrapped in
738+
// an interface is non-nil, so "client == nil" guards downstream never fire and
739+
// the first API call panics.
740+
func GetClientInterface() kubernetes.Interface {
741+
c := GetClient()
742+
if c == nil {
743+
return nil
744+
}
745+
return c
746+
}
747+
735748
func GetClientSafetySnapshot() (*kubernetes.Clientset, string) {
736749
clientMu.RLock()
737750
defer clientMu.RUnlock()
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package k8s
2+
3+
import (
4+
"testing"
5+
6+
"k8s.io/client-go/kubernetes"
7+
)
8+
9+
func TestGetClientInterfaceIsUntypedNilWithoutClient(t *testing.T) {
10+
prev := SetTestClient(nil)
11+
t.Cleanup(func() { SetTestClient(prev) })
12+
13+
if got := GetClientInterface(); got != nil {
14+
t.Fatalf("GetClientInterface() = %#v, want untyped nil", got)
15+
}
16+
}
17+
18+
func TestGetClientInterfaceReturnsClient(t *testing.T) {
19+
c := &kubernetes.Clientset{}
20+
prev := SetTestClient(c)
21+
t.Cleanup(func() { SetTestClient(prev) })
22+
23+
if got := GetClientInterface(); got != kubernetes.Interface(c) {
24+
t.Fatalf("GetClientInterface() = %#v, want the configured clientset", got)
25+
}
26+
}

0 commit comments

Comments
 (0)