From b49f073db68c2dad76485cda385f341db4ea7dc3 Mon Sep 17 00:00:00 2001 From: Arlen Vasconcelos Date: Tue, 25 Aug 2026 15:13:20 -0400 Subject: [PATCH] feat(context): reopen Desktop on the last used cluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Radar Desktop starts on the cluster you were last working in instead of the kubeconfig's current-context. A window you reopen should come back where you left it, while a command typed right after `kubectl config use-context` must run where the shell says it will — so only cmd/desktop opts in. `kubectl radar`, the `radar` CLI and `radar diagnose --standalone` start on current-context and record nothing, so a terminal session can neither be steered by nor move the Desktop pick. The memory lives in its own ~/.radar/desktop-state.json, not settings.json. Sharing the settings file would make that separation a flag rather than a fact: both binaries read it, and /api/settings serializes the whole struct verbatim, so the remembered cluster would be served to every viewer of a shared instance and could be round-tripped away by a PUT from a client that never saw the field. The record carries the kubeconfig file the context came from, not just its name, and resolution requires both to match. With several kubeconfigs loaded, two files can define the same context name and which one keeps the unqualified form depends on directory read order — a name fallback would let a newly added file silently redirect the restore to a different cluster, under the name the user last used. That trade only ever goes one way: falling back to current-context is what Radar does with no memory at all, so an unresolvable memory costs one click, while a wrong cluster costs more with apply, scale, drain and rollback a button away. Nothing is recorded or restored when auth is enabled or when serving through a cloud tunnel — there the kubeconfig context is shared state and one user's switch must not steer everyone else's next start. CAPI workload clusters are never remembered: their kubeconfig is a temp file that no longer exists on the next run. The behaviour explains itself rather than just happening. A miss is recorded to the diagnostics surface as well as the log, and the cannot-connect screen says when Radar opened on the remembered cluster — "the cluster you were last using is unreachable" is a different problem from "your current-context is unreachable", same error, different fix. Settings → Connection gains a "Reopen on the last used cluster" toggle, so turning the behaviour off does not mean hand-editing ~/.radar/config.json — which Desktop users are the least likely to do. Turning it off clears the stored pick instead of freezing it; freezing would mean that re-enabling months later reopens whichever cluster happened to be recorded back then, a worse surprise than the behaviour the user turned off. config.json "restoreLastContext" sets the same switch, and AppConfig.RestoreLastContext is opt-in so an entrypoint that never mentions the field gets current-context. settings.Update now refuses to write over a settings file it could not read. Context switches call it automatically, so a single unreadable file would otherwise erase every other preference on the next switch. --- cmd/desktop/main.go | 5 + docs/configuration.md | 38 ++++ internal/app/bootstrap.go | 22 +- internal/app/context_scope_test.go | 75 +++++++ internal/app/last_context.go | 88 ++++++++ internal/app/last_context_test.go | 142 +++++++++++++ internal/config/config.go | 42 ++-- internal/k8s/client.go | 16 +- internal/k8s/context_preference_test.go | 121 +++++++++++ internal/k8s/context_registry.go | 31 ++- internal/k8s/context_registry_test.go | 6 +- internal/k8s/context_source.go | 145 +++++++++++++ internal/k8s/last_context_test.go | 201 ++++++++++++++++++ internal/k8s/testing.go | 21 ++ internal/server/last_context_privacy_test.go | 65 ++++++ internal/server/server.go | 1 + internal/server/sse.go | 26 +-- internal/settings/desktop_state.go | 86 ++++++++ internal/settings/desktop_state_test.go | 93 ++++++++ internal/settings/settings.go | 41 +++- internal/settings/settings_test.go | 19 ++ web/src/components/ConnectionErrorView.tsx | 6 + .../components/settings/SettingsDialog.tsx | 16 +- web/src/context/ConnectionContext.tsx | 6 +- 24 files changed, 1264 insertions(+), 48 deletions(-) create mode 100644 internal/app/context_scope_test.go create mode 100644 internal/app/last_context.go create mode 100644 internal/app/last_context_test.go create mode 100644 internal/k8s/context_preference_test.go create mode 100644 internal/k8s/context_source.go create mode 100644 internal/k8s/last_context_test.go create mode 100644 internal/server/last_context_privacy_test.go create mode 100644 internal/settings/desktop_state.go create mode 100644 internal/settings/desktop_state_test.go diff --git a/cmd/desktop/main.go b/cmd/desktop/main.go index 6fbc408df..01b981978 100644 --- a/cmd/desktop/main.go +++ b/cmd/desktop/main.go @@ -147,6 +147,7 @@ func main() { cfg := app.AppConfig{ Kubeconfig: *kubeconfig, KubeconfigDirs: app.ParseKubeconfigDirs(*kubeconfigDir), + RestoreLastContext: fileCfg.RestoreLastContextOr(true), Namespace: resolvedNamespace, Namespaces: resolvedNamespaces, Port: fileCfg.PortOr(0), // Configured port, or random to avoid conflicts with CLI @@ -173,6 +174,10 @@ func main() { AIHistoryDBPath: fileCfg.AIHistoryDBPath, } + if !cfg.RestoreLastContext { + app.ForgetLastContext() + } + app.SetGlobals(cfg) versionpkg.SetDesktop(true) desktopenv.SetGPUPolicy(gpuPolicyName(linuxGPUPolicy)) diff --git a/docs/configuration.md b/docs/configuration.md index f3f9fb969..1a23ce57a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -58,6 +58,7 @@ All fields are optional — omitted fields use built-in defaults. |-------|-------------| | `kubeconfig` | Path to kubeconfig file (same as `--kubeconfig`) | | `kubeconfigDirs` | Directories containing kubeconfig files (same as `--kubeconfig-dir`) | +| `restoreLastContext` | Desktop app only: reopen on the cluster last used (default: enabled). `false` always opens on the kubeconfig's `current-context` — see [Startup Context](#startup-context) | | `namespace` | Initial namespace filter | | `namespaces` | Initial namespace filters as a list (same as `--namespaces ns1,ns2,ns3`) | | `port` | Server port (default 9280) | @@ -94,6 +95,14 @@ User preferences for the UI. Managed via the Settings dialog or `PUT /api/settin | `theme` | `light`, `dark`, `system` | UI theme preference | | `pinnedKinds` | Array of `{name, kind, group}` | Resource kinds pinned to the sidebar | +### Desktop State File (`~/.radar/desktop-state.json`) + +Written by the Desktop app for itself — not served over the API, and never read by `kubectl radar` or the `radar` CLI. + +| Field | Values | Description | +|-------|--------|-------------| +| `lastContext` | `{name, sourceFile, inFileName}` | The cluster the Desktop window was last switched to, reopened on the next launch — see [Startup Context](#startup-context) | + ## Cluster Connection Precedence Radar connects to Kubernetes clusters using the same configuration sources as `kubectl`: @@ -145,6 +154,35 @@ Radar supports switching between Kubernetes contexts at runtime through the UI. When running in-cluster (using the pod's service account), context switching is disabled. +Switching contexts in the UI never rewrites your kubeconfig — `kubectl` keeps pointing wherever it pointed before. + +## Startup Context + +Which cluster Radar comes up on depends on how you launched it. + +**The Desktop app reopens where you left off.** Every context switch is recorded in `~/.radar/desktop-state.json`, and the next launch reconnects to it — the natural behaviour for a window you closed and reopened. + +**`kubectl radar`, `radar`, and `radar diagnose --standalone` start on the kubeconfig's `current-context`**, as `kubectl` would. A command typed right after `kubectl config use-context staging` runs against staging, and a cluster picked in the Desktop app days ago never redirects it. Terminal runs don't record switches either, so nothing you do in one moves where the Desktop app reopens. + +The separation is structural, not a preference: the remembered cluster lives in a Desktop-owned file that the CLI never reads or writes, and there is no setting that opts the CLI in. + +To stop the Desktop app reopening on the last cluster, turn off **Reopen on the last used cluster** in Settings → Connection, or set in `~/.radar/config.json`: + +```json +{ + "restoreLastContext": false +} +``` + +Details worth knowing: + +- The remembered context records the kubeconfig file it came from, not just its name — the name alone is not a stable handle. With several kubeconfigs loaded, two files can define the same context name, and which one keeps the unqualified name depends on the order the files are read, so adding a file can hand that name to a different cluster. +- Radar reopens only on an exact match: the same context, in the same file. Anything else — the context renamed or deleted, the file moved or no longer loaded — opens the kubeconfig's `current-context` instead, and says so in Diagnostics. A same-named context in another file is not treated as evidence that it is the same cluster; losing the convenience costs a click, landing on the wrong cluster costs more. +- If the remembered cluster is unreachable (VPN down, for instance), Radar reports the connection failure rather than silently connecting to a different cluster. Pick another cluster from the header. +- Clusters connected through CAPI are never remembered: their kubeconfig is a temporary file that no longer exists on the next run. +- Turning the memory off clears it as well as stopping new recording, so turning it back on later starts fresh rather than reopening a cluster you stopped using months ago. +- When Radar cannot reach a cluster it restored, the connection screen says so — "reopened on the cluster you were last using" is a different problem from your `current-context` being unreachable. + ## Namespace Picker The header has a namespace picker on the right. Pick a single namespace to focus the view, or **All namespaces** to see everything you have access to. Cluster-scoped resources (Nodes, Namespaces, PVs, StorageClasses) appear regardless of the pick if your RBAC permits them — they have no namespace to filter on. Namespace-restricted users without their own cluster-scoped RBAC won't see cluster-scoped sections at all. diff --git a/internal/app/bootstrap.go b/internal/app/bootstrap.go index 7c511b6ff..b8f017a00 100644 --- a/internal/app/bootstrap.go +++ b/internal/app/bootstrap.go @@ -32,8 +32,17 @@ var clusterConnectionProbe = k8s.TestClusterConnection // AppConfig holds all parsed configuration for the Radar application. type AppConfig struct { - Kubeconfig string - KubeconfigDirs []string + Kubeconfig string + KubeconfigDirs []string + // RestoreLastContext reopens on the cluster the last session was switched + // to instead of the kubeconfig's current-context, recording every switch so + // the next start finds it. Only cmd/desktop sets it: a window you reopen + // should come back where you left it, while a command typed right after + // `kubectl config use-context` must run where the shell says it will. The + // zero value is the deterministic one on purpose — an entrypoint that never + // mentions this field gets current-context rather than another session's + // state. + RestoreLastContext bool Namespace string Namespaces []string Port int @@ -107,8 +116,9 @@ func validateNamespaceFanout(namespaces []string, ctxNs string, maxCandidates in // InitializeK8s creates and configures the Kubernetes client. func InitializeK8s(cfg AppConfig) error { err := k8s.Initialize(k8s.InitOptions{ - KubeconfigPath: cfg.Kubeconfig, - KubeconfigDirs: cfg.KubeconfigDirs, + KubeconfigPath: cfg.Kubeconfig, + KubeconfigDirs: cfg.KubeconfigDirs, + PreferredContext: startupContextPreference(cfg), }) if err != nil { return fmt.Errorf("failed to initialize K8s client: %w", err) @@ -219,6 +229,8 @@ func BuildTimelineStoreConfig(cfg AppConfig) timeline.StoreConfig { func RegisterCallbacks(cfg AppConfig, timelineStoreCfg timeline.StoreConfig) { k8s.RegisterHelmFuncs(helm.ResetClient, helm.ReinitClient) + RegisterLastContextMemory(cfg) + k8s.RegisterTimelineFuncs(func() { // Reset the store AND the per-cluster event-pipeline metrics: RecentDrops // name resources from the previous cluster and must not survive the switch. @@ -262,6 +274,7 @@ func RegisterCallbacks(cfg AppConfig, timelineStoreCfg timeline.StoreConfig) { // CreateServer creates the HTTP server with the given configuration. func CreateServer(cfg AppConfig) *server.Server { + restoreLastContext := remembersLastContext(cfg) effectiveCfg := &config.Config{ Kubeconfig: cfg.Kubeconfig, KubeconfigDirs: cfg.KubeconfigDirs, @@ -280,6 +293,7 @@ func CreateServer(cfg AppConfig) *server.Server { DebugImage: cfg.DebugImage, ReachabilityImage: cfg.ReachabilityImage, MCP: &cfg.MCPEnabled, + RestoreLastContext: &restoreLastContext, } serverCfg := server.Config{ diff --git a/internal/app/context_scope_test.go b/internal/app/context_scope_test.go new file mode 100644 index 000000000..087b08e86 --- /dev/null +++ b/internal/app/context_scope_test.go @@ -0,0 +1,75 @@ +package app + +import ( + "os" + "path/filepath" + "testing" +) + +// The zero value is what a terminal entrypoint passes: `kubectl radar` and +// `radar diagnose --standalone` start where the shell says they do, so a +// switch made in Desktop can't steer a command typed after +// `kubectl config use-context`. +func TestTerminalEntrypointDoesNotRememberTheContext(t *testing.T) { + useTempHome(t) + + persistLastContext(AppConfig{}, "prod-eu") + + if saved := rememberedName(); saved != "" { + t.Errorf("remembered context = %q, want empty for an entrypoint that doesn't opt in", saved) + } +} + +func TestTerminalEntrypointStartsOnCurrentContext(t *testing.T) { + useTempHome(t) + remember(t, "prod-eu") + + got := startupContextPreference(AppConfig{}) + if got.Name != "" { + t.Errorf("startupContextPreference() = %q, want empty so current-context stands", got.Name) + } +} + +func TestPersistLastContextSkippedWhenRestoreDisabled(t *testing.T) { + useTempHome(t) + + cfg := remembering() + cfg.RestoreLastContext = false + persistLastContext(cfg, "prod-eu") + + if saved := rememberedName(); saved != "" { + t.Errorf("remembered context = %q, want empty when restore is turned off", saved) + } +} + +func TestStartupContextPreferenceSkippedWhenRestoreDisabled(t *testing.T) { + useTempHome(t) + remember(t, "prod-eu") + + cfg := remembering() + cfg.RestoreLastContext = false + if got := startupContextPreference(cfg); got.Name != "" { + t.Errorf("startupContextPreference() = %q, want empty when restore is turned off", got.Name) + } +} + +func TestForgetLastContextClearsTheMemory(t *testing.T) { + useTempHome(t) + remember(t, "prod-eu") + + ForgetLastContext() + + if saved := rememberedName(); saved != "" { + t.Errorf("remembered context = %q, want it cleared", saved) + } +} + +func TestForgetLastContextLeavesOtherDesktopStateAlone(t *testing.T) { + dir := useTempHome(t) + + ForgetLastContext() + + if _, err := os.Stat(filepath.Join(dir, ".radar", "desktop-state.json")); !os.IsNotExist(err) { + t.Errorf("desktop-state.json exists after clearing nothing (err=%v)", err) + } +} diff --git a/internal/app/last_context.go b/internal/app/last_context.go new file mode 100644 index 000000000..905ba7a6f --- /dev/null +++ b/internal/app/last_context.go @@ -0,0 +1,88 @@ +package app + +import ( + "log" + + "github.com/skyhook-io/radar/internal/k8s" + "github.com/skyhook-io/radar/internal/settings" +) + +// remembersLastContext reports whether this process may record the active +// cluster on disk and start on it next time. Only the Desktop app opts in — +// see AppConfig.RestoreLastContext. The auth and cloud-tunnel checks are +// belt-and-braces for the day Desktop grows either: there the kubeconfig +// context is shared state, and one user's switch must not steer everyone +// else's next start. +func remembersLastContext(cfg AppConfig) bool { + return cfg.RestoreLastContext && !cfg.AuthConfig.Enabled() && !cfg.CloudTunnelConfigured +} + +// startupContextPreference resolves which context this run starts on: the one +// the last session ended on, or an empty ref to keep the kubeconfig's +// current-context. +func startupContextPreference(cfg AppConfig) k8s.ContextRef { + if !remembersLastContext(cfg) { + return k8s.ContextRef{} + } + // The error matters: a store we failed to read must not look like "the + // user never picked a cluster". + saved, err := settings.LoadDesktopState() + if err != nil || saved.LastContext == nil { + return k8s.ContextRef{} + } + return k8s.ContextRef{ + Name: saved.LastContext.Name, + SourceFile: saved.LastContext.SourceFile, + InFileName: saved.LastContext.InFileName, + } +} + +// RegisterLastContextMemory records every successful context switch so the next +// start comes back on the cluster the user was working in. Recording the switch +// rather than the exit is deliberate — a force-quit or crash would otherwise +// lose the pick. +func RegisterLastContextMemory(cfg AppConfig) { + if !remembersLastContext(cfg) { + return + } + k8s.OnContextSwitch(func(name string) { + persistLastContext(cfg, name) + }) +} + +// ForgetLastContext drops the remembered cluster, so turning the memory off and +// back on later doesn't reopen a cluster the user stopped using long ago. +func ForgetLastContext() { + saved, err := settings.LoadDesktopState() + if err != nil || saved.LastContext == nil { + return + } + if _, err := settings.UpdateDesktopState(func(st *settings.DesktopState) { + st.LastContext = nil + }); err != nil { + log.Printf("[context] failed to clear the remembered context: %v", err) + } +} + +func persistLastContext(cfg AppConfig, name string) { + if name == "" || !remembersLastContext(cfg) { + return + } + // A CAPI workload cluster lives in a temp kubeconfig that's gone next run, + // so its context could never be restored. + if k8s.IsEphemeralContext(name) { + return + } + // Record the file too: across multiple kubeconfigs the display name alone + // can be reassigned to another file's context between runs. + ref := k8s.ContextSourceFor(name) + if _, err := settings.UpdateDesktopState(func(st *settings.DesktopState) { + st.LastContext = &settings.LastContext{ + Name: ref.Name, + SourceFile: ref.SourceFile, + InFileName: ref.InFileName, + } + }); err != nil { + log.Printf("[context] failed to remember last used context %q: %v", name, err) + } +} diff --git a/internal/app/last_context_test.go b/internal/app/last_context_test.go new file mode 100644 index 000000000..a3d9ed518 --- /dev/null +++ b/internal/app/last_context_test.go @@ -0,0 +1,142 @@ +package app + +import ( + "path/filepath" + "testing" + + "github.com/skyhook-io/radar/internal/auth" + "github.com/skyhook-io/radar/internal/k8s" + "github.com/skyhook-io/radar/internal/settings" +) + +func useTempHome(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) + return dir +} + +func TestPersistLastContextRemembersTheSwitch(t *testing.T) { + useTempHome(t) + + persistLastContext(remembering(), "prod-eu") + + if saved := rememberedName(); saved != "prod-eu" { + t.Errorf("remembered context = %q, want %q", saved, "prod-eu") + } +} + +func TestPersistLastContextIgnoresEmptyName(t *testing.T) { + useTempHome(t) + + persistLastContext(remembering(), "") + + if saved := rememberedName(); saved != "" { + t.Errorf("remembered context = %q, want empty for an empty context name", saved) + } +} + +// A shared server must not remember one user's cluster pick on disk — the +// switch belongs to whoever made it, not to the machine. +func TestPersistLastContextSkippedWhenAuthEnabled(t *testing.T) { + useTempHome(t) + + persistLastContext(withAuth(auth.Config{Mode: "oidc"}), "prod-eu") + + if saved := rememberedName(); saved != "" { + t.Errorf("remembered context = %q, want empty when auth is enabled", saved) + } +} + +func TestPersistLastContextSkippedForCloudTunnel(t *testing.T) { + useTempHome(t) + + persistLastContext(cloudTunnelled(), "prod-eu") + + if saved := rememberedName(); saved != "" { + t.Errorf("remembered context = %q, want empty in cloud-tunnel mode", saved) + } +} + +func TestStartupContextPreferenceReturnsLastUsedContext(t *testing.T) { + useTempHome(t) + remember(t, "prod-eu") + + if got := startupContextPreference(remembering()); got.Name != "prod-eu" { + t.Errorf("startupContextPreference() = %q, want %q", got, "prod-eu") + } +} + +func TestStartupContextPreferenceEmptyWithoutSavedContext(t *testing.T) { + useTempHome(t) + + if got := startupContextPreference(remembering()); got.Name != "" { + t.Errorf("startupContextPreference() = %q, want empty", got.Name) + } +} + +func TestStartupContextPreferenceSkippedWhenAuthEnabled(t *testing.T) { + useTempHome(t) + remember(t, "prod-eu") + + if got := startupContextPreference(withAuth(auth.Config{Mode: "proxy"})); got.Name != "" { + t.Errorf("startupContextPreference() = %q, want empty when auth is enabled", got.Name) + } +} + +// remembering returns the config of an entrypoint that opts into the memory — +// Desktop's shape. The zero AppConfig deliberately does not. +func remembering() AppConfig { + return AppConfig{RestoreLastContext: true} +} + +func withAuth(c auth.Config) AppConfig { + cfg := remembering() + cfg.AuthConfig = c + return cfg +} + +func cloudTunnelled() AppConfig { + cfg := remembering() + cfg.CloudTunnelConfigured = true + return cfg +} + +// rememberedName reads back the recorded context name, treating "nothing +// recorded" as the empty string. +func rememberedName() string { + saved, _ := settings.LoadDesktopState() + if saved.LastContext != nil { + return saved.LastContext.Name + } + return "" +} + +func remember(t *testing.T, name string) { + t.Helper() + if _, err := settings.UpdateDesktopState(func(st *settings.DesktopState) { + st.LastContext = &settings.LastContext{Name: name} + }); err != nil { + t.Fatalf("UpdateDesktopState: %v", err) + } +} + +// The switch is recorded with the file it came from, not just the name the +// header showed — see settings.LastContext for why the name alone is not a +// stable handle across kubeconfigs. +func TestPersistLastContextRecordsWhereTheContextCameFrom(t *testing.T) { + useTempHome(t) + path := filepath.Join(t.TempDir(), "team.yaml") + t.Cleanup(k8s.SetTestRegistryEntry("dev (team)", path, "dev")) + + persistLastContext(remembering(), "dev (team)") + + saved, _ := settings.LoadDesktopState() + if saved.LastContext == nil { + t.Fatal("nothing recorded") + } + if got := *saved.LastContext; got.Name != "dev (team)" || got.SourceFile != path || got.InFileName != "dev" { + t.Errorf("recorded %+v, want name/file/in-file-name all pinned", saved.LastContext) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index c529902ac..1bbc4f423 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -14,19 +14,26 @@ import ( // Config holds startup configuration persisted across restarts. // Values are used as flag defaults; explicit CLI flags always take precedence. type Config struct { - Kubeconfig string `json:"kubeconfig,omitempty"` - KubeconfigDirs []string `json:"kubeconfigDirs,omitempty"` - Namespace string `json:"namespace,omitempty"` - Namespaces []string `json:"namespaces,omitempty"` - Port int `json:"port,omitempty"` - NoBrowser bool `json:"noBrowser,omitempty"` - Browser string `json:"browser,omitempty"` - TimelineStorage string `json:"timelineStorage,omitempty"` - TimelineDBPath string `json:"timelineDbPath,omitempty"` - TimelineRetention string `json:"timelineRetention,omitempty"` // Go duration (e.g. "168h" for 7d); "0" disables age cleanup - TimelineMaxSize string `json:"timelineMaxSize,omitempty"` // Byte size (e.g. "800Mi", "8Gi"); "0" disables - HistoryLimit int `json:"historyLimit,omitempty"` - PrometheusURL string `json:"prometheusUrl,omitempty"` + Kubeconfig string `json:"kubeconfig,omitempty"` + KubeconfigDirs []string `json:"kubeconfigDirs,omitempty"` + // RestoreLastContext turns off the Desktop app reopening on the cluster it + // was last switched to (nil = on). Desktop-only by design: the CLI always + // starts on the kubeconfig's current-context, so a command typed after + // `kubectl config use-context` runs where the shell says it will. There is + // deliberately no knob to opt the CLI in — that would put a Desktop switch + // back in the path of a later terminal command. + RestoreLastContext *bool `json:"restoreLastContext,omitempty"` + Namespace string `json:"namespace,omitempty"` + Namespaces []string `json:"namespaces,omitempty"` + Port int `json:"port,omitempty"` + NoBrowser bool `json:"noBrowser,omitempty"` + Browser string `json:"browser,omitempty"` + TimelineStorage string `json:"timelineStorage,omitempty"` + TimelineDBPath string `json:"timelineDbPath,omitempty"` + TimelineRetention string `json:"timelineRetention,omitempty"` // Go duration (e.g. "168h" for 7d); "0" disables age cleanup + TimelineMaxSize string `json:"timelineMaxSize,omitempty"` // Byte size (e.g. "800Mi", "8Gi"); "0" disables + HistoryLimit int `json:"historyLimit,omitempty"` + PrometheusURL string `json:"prometheusUrl,omitempty"` // PrometheusHeaders are sent with every request to the Prometheus API. // Required for auth-protected backends (Bearer tokens, X-Scope-OrgID, etc.). // Stored in plain text in ~/.radar/config.json — protect the file accordingly. @@ -205,6 +212,15 @@ func (c Config) MCPEnabledOr(def bool) bool { return def } +// RestoreLastContextOr returns *c.RestoreLastContext if non-nil, otherwise the +// provided default. +func (c Config) RestoreLastContextOr(def bool) bool { + if c.RestoreLastContext != nil { + return *c.RestoreLastContext + } + return def +} + // AIHistoryOr returns *c.AIHistory if non-nil, otherwise the provided default. func (c Config) AIHistoryOr(def bool) bool { if c.AIHistory != nil { diff --git a/internal/k8s/client.go b/internal/k8s/client.go index 2f341a02f..d420e3d49 100644 --- a/internal/k8s/client.go +++ b/internal/k8s/client.go @@ -105,6 +105,10 @@ func SetEnrichedKubeconfigFromShell(v bool) { type InitOptions struct { KubeconfigPath string KubeconfigDirs []string // Directories containing kubeconfig files + // PreferredContext is the context to start on instead of the kubeconfig's + // current-context. Ignored when it doesn't resolve, so a stale preference + // can never keep Radar from starting. + PreferredContext ContextRef } // Initialize initializes the K8s client with the given options @@ -143,6 +147,9 @@ func doInit(opts InitOptions) error { contextName = "in-cluster" clusterName = "in-cluster" kubeconfigMode = "in-cluster" + if !opts.PreferredContext.Empty() { + log.Printf("[k8s-init] ignoring preferred context %q: running in-cluster", opts.PreferredContext.Name) + } } } @@ -166,8 +173,9 @@ func doInit(opts InitOptions) error { kubeconfigMode = "multi-dir" if len(configs) == 1 { loadingRules = &clientcmd.ClientConfigLoadingRules{ExplicitPath: configs[0]} + applyContextPreference(configs[0], opts.PreferredContext, configOverrides) } else { - lr, ovr, err := setupIsolatedLoad(configs) + lr, ovr, err := setupIsolatedLoad(configs, opts.PreferredContext) if err != nil { return err } @@ -192,7 +200,7 @@ func doInit(opts InitOptions) error { if paths := filepath.SplitList(kubeconfig); len(paths) > 1 { kubeconfigPaths = paths kubeconfigMode = "multi-env" - lr, ovr, err := setupIsolatedLoad(paths) + lr, ovr, err := setupIsolatedLoad(paths, opts.PreferredContext) if err != nil { return err } @@ -202,6 +210,7 @@ func doInit(opts InitOptions) error { kubeconfigPath = kubeconfig kubeconfigMode = "single" loadingRules = &clientcmd.ClientConfigLoadingRules{ExplicitPath: kubeconfig} + applyContextPreference(kubeconfig, opts.PreferredContext, configOverrides) } } @@ -249,6 +258,9 @@ func doInit(opts InitOptions) error { } } else { contextName = rawConfig.CurrentContext + if configOverrides.CurrentContext != "" { + contextName = configOverrides.CurrentContext + } totalContextCount = len(rawConfig.Contexts) cmds, emptyAIs := collectExecPluginCommands(&rawConfig) execPluginCommands = cmds diff --git a/internal/k8s/context_preference_test.go b/internal/k8s/context_preference_test.go new file mode 100644 index 000000000..50460dcaa --- /dev/null +++ b/internal/k8s/context_preference_test.go @@ -0,0 +1,121 @@ +package k8s + +import ( + "path/filepath" + "testing" +) + +func TestPickInitialContextResolvesTheRecordedContext(t *testing.T) { + dir := t.TempDir() + f1 := writeKubeconfig(t, dir, "first.yaml", "from-first", []kubeEntry{ + {ctxName: "from-first", userName: "u1", clusterName: "c1"}, + }) + f2 := writeKubeconfig(t, dir, "second.yaml", "from-second", []kubeEntry{ + {ctxName: "from-second", userName: "u2", clusterName: "c2"}, + }) + + paths := []string{f1, f2} + registry, fileConfigs := buildContextRegistry(paths) + saved := ContextRef{Name: "from-second", SourceFile: f2, InFileName: "from-second"} + qName, entry, ok := pickInitialContext(paths, registry, fileConfigs, saved) + if !ok { + t.Fatal("pickInitialContext() found no context") + } + if qName != "from-second" { + t.Errorf("qName = %q, want the recorded context %q", qName, "from-second") + } + if entry.SourceFile != f2 { + t.Errorf("entry.SourceFile = %q, want %q", entry.SourceFile, f2) + } +} + +func TestPickInitialContextIgnoresAContextThatIsGone(t *testing.T) { + dir := t.TempDir() + f1 := writeKubeconfig(t, dir, "first.yaml", "from-first", []kubeEntry{ + {ctxName: "from-first", userName: "u1", clusterName: "c1"}, + }) + + paths := []string{f1} + registry, fileConfigs := buildContextRegistry(paths) + saved := ContextRef{Name: "ghost", SourceFile: f1, InFileName: "ghost"} + qName, _, ok := pickInitialContext(paths, registry, fileConfigs, saved) + if !ok { + t.Fatal("pickInitialContext() found no context") + } + if qName != "from-first" { + t.Errorf("qName = %q, want the current-context fallback %q", qName, "from-first") + } +} + +// The scenario the source file exists for: two kubeconfigs define "dev", and a +// file added later takes over the unqualified name. Resolving by name alone +// would connect to the newcomer's cluster under the name the user last used. +func TestPickInitialContextPrefersTheRecordedSourceFileOverTheName(t *testing.T) { + dir := t.TempDir() + newcomer := writeKubeconfig(t, dir, "aaa.yaml", "dev", []kubeEntry{ + {ctxName: "dev", userName: "u1", clusterName: "someone-elses-cluster"}, + }) + worked := writeKubeconfig(t, dir, "bbb.yaml", "dev", []kubeEntry{ + {ctxName: "dev", userName: "u2", clusterName: "the-cluster-i-was-on"}, + }) + + paths := []string{newcomer, worked} + registry, fileConfigs := buildContextRegistry(paths) + if entry := registry["dev"]; entry.SourceFile != newcomer { + t.Fatalf("precondition: expected %q to own the unqualified name, got %q", newcomer, entry.SourceFile) + } + + saved := ContextRef{Name: "dev", SourceFile: worked, InFileName: "dev"} + _, entry, ok := pickInitialContext(paths, registry, fileConfigs, saved) + if !ok { + t.Fatal("pickInitialContext() found no context") + } + if entry.SourceFile != worked { + t.Errorf("entry.SourceFile = %q, want the recorded file %q — the name was reassigned", entry.SourceFile, worked) + } +} + +// Once the recorded file is gone, a same-named context in another file is not +// evidence that it is the same cluster. Radar opens current-context instead of +// guessing — losing the convenience beats landing somewhere the user didn't pick. +func TestPickInitialContextDoesNotAdoptASameNamedContextFromAnotherFile(t *testing.T) { + dir := t.TempDir() + current := writeKubeconfig(t, dir, "first.yaml", "from-first", []kubeEntry{ + {ctxName: "from-first", userName: "u1", clusterName: "c1"}, + }) + impostor := writeKubeconfig(t, dir, "second.yaml", "", []kubeEntry{ + {ctxName: "prod", userName: "u2", clusterName: "someone-elses-prod"}, + }) + + paths := []string{current, impostor} + registry, fileConfigs := buildContextRegistry(paths) + saved := ContextRef{Name: "prod", SourceFile: filepath.Join(dir, "deleted.yaml"), InFileName: "prod"} + qName, entry, ok := pickInitialContext(paths, registry, fileConfigs, saved) + if !ok { + t.Fatal("pickInitialContext() found no context") + } + if qName != "from-first" || entry.SourceFile != current { + t.Errorf("resolved %q from %q, want the current-context fallback %q from %q", + qName, entry.SourceFile, "from-first", current) + } +} + +// A ref carrying only a name — nothing records one today — is not resolvable: +// the name is exactly the part another file can take over. +func TestPickInitialContextIgnoresANameOnlyReference(t *testing.T) { + dir := t.TempDir() + f1 := writeKubeconfig(t, dir, "first.yaml", "from-first", []kubeEntry{ + {ctxName: "from-first", userName: "u1", clusterName: "c1"}, + {ctxName: "other", userName: "u2", clusterName: "c2"}, + }) + + paths := []string{f1} + registry, fileConfigs := buildContextRegistry(paths) + qName, _, ok := pickInitialContext(paths, registry, fileConfigs, ContextRef{Name: "other"}) + if !ok { + t.Fatal("pickInitialContext() found no context") + } + if qName != "from-first" { + t.Errorf("qName = %q, want the current-context fallback %q", qName, "from-first") + } +} diff --git a/internal/k8s/context_registry.go b/internal/k8s/context_registry.go index dff95ded3..5fd2ef6bf 100644 --- a/internal/k8s/context_registry.go +++ b/internal/k8s/context_registry.go @@ -25,7 +25,7 @@ import ( // than one kubeconfig file: each file stays an island. A SwitchContext later // looks up the target entry in the registry and loads that one file, so // shared user/cluster names across files never collide — see issue #519. -func setupIsolatedLoad(paths []string) ( +func setupIsolatedLoad(paths []string, preferred ContextRef) ( *clientcmd.ClientConfigLoadingRules, *clientcmd.ConfigOverrides, error, @@ -34,7 +34,7 @@ func setupIsolatedLoad(paths []string) ( if len(registry) == 0 { return nil, nil, fmt.Errorf("no contexts found across %d kubeconfig files", len(paths)) } - qName, entry, ok := pickInitialContext(paths, registry, fileConfigs) + qName, entry, ok := pickInitialContext(paths, registry, fileConfigs, preferred) if !ok { return nil, nil, fmt.Errorf("no usable context found across %d kubeconfig files", len(paths)) } @@ -164,7 +164,16 @@ func pickInitialContext( paths []string, registry map[string]contextEntry, fileConfigs map[string]*clientcmdapi.Config, + preferred ContextRef, ) (string, contextEntry, bool) { + // Preference pass: the context the last session ended on. It resolves only + // on an exact (file, in-file name) match — see matchPreferred. + if qName, entry, ok := matchPreferred(registry, preferred); ok { + restoredContextName.Store(qName) + return qName, entry, true + } + reportContextPreferenceMiss(preferred) + // First pass: honor CurrentContext in file order. for _, path := range paths { cfg, ok := fileConfigs[path] @@ -195,6 +204,24 @@ func pickInitialContext( return "", contextEntry{}, false } +// matchPreferred resolves a saved reference against the registry on the exact +// (file, in-file name) pair, and nothing else. There is deliberately no +// fallback to the display name: another file may have taken that name over +// since it was recorded, so following it would connect to a different cluster +// under the name the user last used. Losing the convenience costs one click; +// landing on the wrong cluster costs more than that. +func matchPreferred(registry map[string]contextEntry, preferred ContextRef) (string, contextEntry, bool) { + if preferred.Empty() { + return "", contextEntry{}, false + } + for qName, entry := range registry { + if entry.SourceFile == preferred.SourceFile && entry.InFileName == preferred.InFileName { + return qName, entry, true + } + } + return "", contextEntry{}, false +} + // refreshContextRegistry reconciles the in-memory contextRegistry + // perFileConfigs against what's actually on disk RIGHT NOW. Returns // new map values (registry, fileConfigs, fileMtimes) plus a `changed` diff --git a/internal/k8s/context_registry_test.go b/internal/k8s/context_registry_test.go index f0bc4d62d..f3870c808 100644 --- a/internal/k8s/context_registry_test.go +++ b/internal/k8s/context_registry_test.go @@ -364,7 +364,7 @@ func TestPickInitialContext_PrefersFirstFileCurrentContext(t *testing.T) { paths := []string{f1, f2} registry, fileConfigs := buildContextRegistry(paths) - qName, entry, ok := pickInitialContext(paths, registry, fileConfigs) + qName, entry, ok := pickInitialContext(paths, registry, fileConfigs, ContextRef{}) if !ok { t.Fatal("expected initial context") } @@ -388,7 +388,7 @@ func TestPickInitialContext_FallsBackWhenCurrentContextEmpty(t *testing.T) { paths := []string{f1, f2} registry, fileConfigs := buildContextRegistry(paths) - qName, _, ok := pickInitialContext(paths, registry, fileConfigs) + qName, _, ok := pickInitialContext(paths, registry, fileConfigs, ContextRef{}) if !ok { t.Fatal("expected initial context") } @@ -405,7 +405,7 @@ func TestPickInitialContext_NoCurrentContextAnywhere(t *testing.T) { paths := []string{f1} registry, fileConfigs := buildContextRegistry(paths) - qName, _, ok := pickInitialContext(paths, registry, fileConfigs) + qName, _, ok := pickInitialContext(paths, registry, fileConfigs, ContextRef{}) if !ok { t.Fatal("expected initial context from any-ctx fallback") } diff --git a/internal/k8s/context_source.go b/internal/k8s/context_source.go new file mode 100644 index 000000000..eb1d00664 --- /dev/null +++ b/internal/k8s/context_source.go @@ -0,0 +1,145 @@ +package k8s + +import ( + "log" + "sync/atomic" + + "k8s.io/client-go/tools/clientcmd" + + "github.com/skyhook-io/radar/internal/errorlog" +) + +// restoredContextName is the context Radar came up on because the last session +// was left there. A name rather than a flag: a switch away answers for itself, +// while a retry against the same cluster — which runs the same switch path — +// keeps it. +var restoredContextName atomic.Value // string + +// ContextRestoredFromMemory reports whether the context Radar is on right now +// is one it restored at startup rather than one the kubeconfig chose. +func ContextRestoredFromMemory() bool { + name, _ := restoredContextName.Load().(string) + return name != "" && name == GetContextName() +} + +// ContextRef identifies a kubeconfig context precisely enough to survive a +// restart: the name Radar displays, plus the file it came from and the name it +// carries inside that file. +// +// The name alone is ambiguous once more than one kubeconfig is loaded. Two +// files can define the same context name, and which one keeps the unqualified +// form depends on the order discoverKubeconfigs walks the directory — so +// adding a file can silently reassign the name to a different cluster. A +// caller persisting a context across restarts must carry the file too. +// +// SourceFile + InFileName are the identity; Name is the label Radar shows. A +// ref carrying only a Name resolves to nothing, because a name is exactly the +// thing another file can take over. +type ContextRef struct { + Name string + SourceFile string + InFileName string +} + +// Empty reports whether the ref names nothing to resolve. +func (r ContextRef) Empty() bool { + return r.SourceFile == "" || r.InFileName == "" +} + +// ContextSourceFor returns the full reference for a context Radar currently +// knows, so callers persisting it can record where it came from. Outside the +// registry there is only one kubeconfig loaded, and it is the only place the +// active context can have come from. +func ContextSourceFor(name string) ContextRef { + clientMu.RLock() + defer clientMu.RUnlock() + + if entry, ok := contextRegistry[name]; ok { + return ContextRef{Name: name, SourceFile: entry.SourceFile, InFileName: entry.InFileName} + } + if name != "" && name == contextName { + if path := singleLoadedKubeconfig(); path != "" { + return ContextRef{Name: name, SourceFile: path, InFileName: name} + } + } + return ContextRef{Name: name} +} + +// singleLoadedKubeconfig returns the one kubeconfig backing this process, or "" +// when several are loaded (the registry answers there) or none is (in-cluster). +// --kubeconfig-dir records its find in kubeconfigPaths even when it finds +// exactly one file, so both globals have to be consulted. Callers must hold +// clientMu. +func singleLoadedKubeconfig() string { + if kubeconfigPath != "" { + return kubeconfigPath + } + if len(kubeconfigPaths) == 1 { + return kubeconfigPaths[0] + } + return "" +} + +// IsEphemeralContext reports whether a context lives in a temp kubeconfig Radar +// wrote itself for a CAPI workload cluster. That file is gone on the next run, +// so callers that persist a context across restarts must skip those. +func IsEphemeralContext(name string) bool { + clientMu.RLock() + defer clientMu.RUnlock() + + entry, ok := contextRegistry[name] + if !ok { + return false + } + for _, tmpPath := range capiKubeconfigs { + if tmpPath == entry.SourceFile { + return true + } + } + return false +} + +// applyContextPreference points overrides at the preferred context when the +// kubeconfig at path is the file it was recorded from and still defines it. +// Validating first matters twice: a context that has since been renamed or +// deleted would otherwise fail the whole startup, and the override has to be in +// place before the deferred loader builds its inner config — it captures +// CurrentContext on the first RawConfig()/ClientConfig() call and caches it. +func applyContextPreference(path string, preferred ContextRef, overrides *clientcmd.ConfigOverrides) { + if preferred.Empty() { + reportContextPreferenceMiss(preferred) + return + } + cfg, err := clientcmd.LoadFromFile(path) + if err != nil { + reportContextPreferenceMiss(preferred) + return + } + if preferred.SourceFile == path { + if _, ok := cfg.Contexts[preferred.InFileName]; ok { + overrides.CurrentContext = preferred.InFileName + restoredContextName.Store(preferred.InFileName) + return + } + } + reportContextPreferenceMiss(preferred) +} + +// reportContextPreferenceMiss explains why Radar did not come up where the last +// session left it. Falling back to current-context is the safe answer — the +// name alone is what another kubeconfig can take over — but a silent redirect +// leaves the user staring at a cluster they didn't pick, so it goes to the +// diagnostics surface and not only to the log. +func reportContextPreferenceMiss(preferred ContextRef) { + name := preferred.Name + if name == "" { + name = preferred.InFileName + } + if name == "" { + return + } + log.Printf("[k8s-init] last used context %q not found where it was recorded; using current-context", name) + errorlog.Record("k8s-init", "warning", + "could not reopen on %q: that context is no longer in the kubeconfig it was recorded from. Starting on the kubeconfig's current-context instead.", + name) +} diff --git a/internal/k8s/last_context_test.go b/internal/k8s/last_context_test.go new file mode 100644 index 000000000..e646530af --- /dev/null +++ b/internal/k8s/last_context_test.go @@ -0,0 +1,201 @@ +package k8s + +import ( + "strings" + "testing" +) + +// restoreClientGlobals snapshots the package state doInit writes and puts it +// back afterwards, so these tests can run a real init without leaking a fake +// cluster into sibling tests. +func restoreClientGlobals(t *testing.T) { + t.Helper() + clientMu.Lock() + var ( + savedPath = kubeconfigPath + savedPaths = kubeconfigPaths + savedMode = kubeconfigMode + savedRegistry = contextRegistry + savedConfigs = perFileConfigs + savedMtimes = perFileMtimes + savedContext = contextName + savedCluster = clusterName + savedNamespace = contextNamespace + savedUsesExec = contextUsesExec + savedTotal = totalContextCount + savedExecCmds = execPluginCommands + savedClient = k8sClient + savedConfig = k8sConfig + savedDiscovery = discoveryClient + savedDynamic = dynamicClient + ) + clientMu.Unlock() + + t.Cleanup(func() { + clientMu.Lock() + defer clientMu.Unlock() + kubeconfigPath = savedPath + kubeconfigPaths = savedPaths + kubeconfigMode = savedMode + contextRegistry = savedRegistry + perFileConfigs = savedConfigs + perFileMtimes = savedMtimes + contextName = savedContext + clusterName = savedCluster + contextNamespace = savedNamespace + contextUsesExec = savedUsesExec + totalContextCount = savedTotal + execPluginCommands = savedExecCmds + k8sClient = savedClient + k8sConfig = savedConfig + discoveryClient = savedDiscovery + dynamicClient = savedDynamic + }) +} + +func TestDoInitPrefersRequestedContext(t *testing.T) { + restoreClientGlobals(t) + dir := t.TempDir() + path := writeKubeconfig(t, dir, "config", "alpha", []kubeEntry{ + {ctxName: "alpha", userName: "ua", clusterName: "cluster-alpha", namespace: "ns-alpha"}, + {ctxName: "beta", userName: "ub", clusterName: "cluster-beta", namespace: "ns-beta"}, + }) + + saved := ContextRef{Name: "beta", SourceFile: path, InFileName: "beta"} + if err := doInit(InitOptions{KubeconfigPath: path, PreferredContext: saved}); err != nil { + t.Fatalf("doInit() error = %v", err) + } + + if got := GetContextName(); got != "beta" { + t.Errorf("GetContextName() = %q, want %q", got, "beta") + } + if got := GetContextNamespace(); got != "ns-beta" { + t.Errorf("GetContextNamespace() = %q, want %q", got, "ns-beta") + } + // The bookkeeping and the client must agree: a context name that says + // "beta" while the REST config still dials alpha is the failure mode + // this preference has to avoid. + if host := GetConfig().Host; !strings.Contains(host, "cluster-beta") { + t.Errorf("rest config Host = %q, want it to point at cluster-beta", host) + } +} + +func TestDoInitFallsBackWhenPreferredContextMissing(t *testing.T) { + restoreClientGlobals(t) + dir := t.TempDir() + path := writeKubeconfig(t, dir, "config", "alpha", []kubeEntry{ + {ctxName: "alpha", userName: "ua", clusterName: "cluster-alpha", namespace: "ns-alpha"}, + }) + + saved := ContextRef{Name: "ghost", SourceFile: path, InFileName: "ghost"} + if err := doInit(InitOptions{KubeconfigPath: path, PreferredContext: saved}); err != nil { + t.Fatalf("doInit() error = %v", err) + } + + if got := GetContextName(); got != "alpha" { + t.Errorf("GetContextName() = %q, want the kubeconfig current-context %q", got, "alpha") + } + if host := GetConfig().Host; !strings.Contains(host, "cluster-alpha") { + t.Errorf("rest config Host = %q, want it to point at cluster-alpha", host) + } +} + +func TestIsEphemeralContextSingleKubeconfig(t *testing.T) { + restoreClientGlobals(t) + clientMu.Lock() + kubeconfigPath = "/home/user/.kube/config" + kubeconfigPaths = nil + contextRegistry = nil + clientMu.Unlock() + + if IsEphemeralContext("prod") { + t.Error("IsEphemeralContext(prod) = true, want false for a context from the user's kubeconfig") + } +} + +func TestIsEphemeralContextReportsCAPIContext(t *testing.T) { + restoreClientGlobals(t) + dir := t.TempDir() + durable := writeKubeconfig(t, dir, "durable.yaml", "prod", []kubeEntry{ + {ctxName: "prod", userName: "u", clusterName: "c"}, + }) + temp := writeKubeconfig(t, dir, "radar-capi-kubeconfig-1234.yaml", "workload", []kubeEntry{ + {ctxName: "workload", userName: "u", clusterName: "c"}, + }) + + clientMu.Lock() + kubeconfigPath = "" + kubeconfigPaths = []string{durable, temp} + contextRegistry = map[string]contextEntry{ + "prod": {SourceFile: durable, InFileName: "prod"}, + "workload": {SourceFile: temp, InFileName: "workload"}, + } + savedCAPI := capiKubeconfigs + capiKubeconfigs = map[string]string{"workload": temp} + clientMu.Unlock() + t.Cleanup(func() { + clientMu.Lock() + capiKubeconfigs = savedCAPI + clientMu.Unlock() + }) + + if IsEphemeralContext("prod") { + t.Error("IsEphemeralContext(prod) = true for a durable kubeconfig") + } + if !IsEphemeralContext("workload") { + t.Error("IsEphemeralContext(workload) = false for a CAPI temp kubeconfig") + } +} + +// --kubeconfig-dir records its find in kubeconfigPaths even when it discovers +// exactly one file, and builds no registry because there is nothing to +// disambiguate. Without consulting both globals the source file would never be +// recorded, and a restore that requires an exact match could never resolve. +func TestContextSourceForRecordsTheFileFoundInAKubeconfigDir(t *testing.T) { + restoreClientGlobals(t) + clientMu.Lock() + kubeconfigPath = "" + kubeconfigPaths = []string{"/home/user/.kube/configs/prod.yaml"} + contextRegistry = nil + contextName = "prod" + clientMu.Unlock() + + got := ContextSourceFor("prod") + if got.SourceFile != "/home/user/.kube/configs/prod.yaml" || got.InFileName != "prod" { + t.Errorf("ContextSourceFor(prod) = %+v, want the discovered file recorded", got) + } + if got.Empty() { + t.Error("ref is not resolvable, so the memory could never be restored") + } +} + +func TestContextSourceForRecordsTheSingleKubeconfig(t *testing.T) { + restoreClientGlobals(t) + clientMu.Lock() + kubeconfigPath = "/home/user/.kube/config" + kubeconfigPaths = nil + contextRegistry = nil + contextName = "prod" + clientMu.Unlock() + + got := ContextSourceFor("prod") + if got.SourceFile != "/home/user/.kube/config" || got.InFileName != "prod" { + t.Errorf("ContextSourceFor(prod) = %+v, want the loaded kubeconfig recorded", got) + } +} + +// Several files loaded means the registry is the only thing that knows which +// one a context came from — no single-file guess applies. +func TestContextSourceForLeavesNoFileWhenSeveralAreLoaded(t *testing.T) { + restoreClientGlobals(t) + clientMu.Lock() + kubeconfigPath = "" + kubeconfigPaths = []string{"/a.yaml", "/b.yaml"} + contextRegistry = map[string]contextEntry{} + contextName = "prod" + clientMu.Unlock() + + if got := ContextSourceFor("prod"); !got.Empty() { + t.Errorf("ContextSourceFor(prod) = %+v, want an unresolvable ref rather than a guess", got) + } +} diff --git a/internal/k8s/testing.go b/internal/k8s/testing.go index 3d1c6074c..58d84ac79 100644 --- a/internal/k8s/testing.go +++ b/internal/k8s/testing.go @@ -177,6 +177,26 @@ func SetTestContextName(name string) string { return prev } +// SetTestRegistryEntry is a test-only helper that registers one context in the +// isolated-load registry, so callers can exercise resolution against a +// multi-kubeconfig layout. Returns a restore func. +func SetTestRegistryEntry(qualifiedName, sourceFile, inFileName string) func() { + clientMu.Lock() + prev := contextRegistry + next := make(map[string]contextEntry, len(prev)+1) + for k, v := range prev { + next[k] = v + } + next[qualifiedName] = contextEntry{SourceFile: sourceFile, InFileName: inFileName} + contextRegistry = next + clientMu.Unlock() + return func() { + clientMu.Lock() + contextRegistry = prev + clientMu.Unlock() + } +} + // SetTestContextNamespace is a test-only helper that overrides the package-level // kubeconfig context namespace. Returns the previous value so callers can // restore it on cleanup. @@ -232,6 +252,7 @@ func SetTestPolicyReportIndex(idx *policyreports.Index) *policyreports.Index { // This is intended for integration tests only. func ResetTestState() { policyReportIndex.Store(nil) + restoredContextName.Store("") // Reset resource cache ResetResourceCache() diff --git a/internal/server/last_context_privacy_test.go b/internal/server/last_context_privacy_test.go new file mode 100644 index 000000000..a3a4f1f01 --- /dev/null +++ b/internal/server/last_context_privacy_test.go @@ -0,0 +1,65 @@ +package server + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/skyhook-io/radar/internal/settings" + "github.com/skyhook-io/radar/pkg/auth" +) + +// The remembered cluster is Desktop's own state and lives in its own file, so +// /api/settings can't serve it — not on GET, not echoed back from a PUT. This +// pins that: a field re-added to the Settings struct would hand every viewer +// of a shared instance the cluster name from whenever this $HOME last ran the +// Desktop app. +func TestSettingsEndpointNeverCarriesTheRememberedCluster(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) + + if err := settings.Save(settings.Settings{Theme: "dark"}); err != nil { + t.Fatalf("seed settings: %v", err) + } + if _, err := settings.UpdateDesktopState(func(st *settings.DesktopState) { + st.LastContext = &settings.LastContext{Name: "prod-eu"} + }); err != nil { + t.Fatalf("seed desktop state: %v", err) + } + + for _, tc := range []struct { + name string + server *Server + }{ + {"local", &Server{}}, + {"auth-enabled", &Server{authConfig: auth.Config{Mode: "oidc"}}}, + } { + t.Run(tc.name, func(t *testing.T) { + get := httptest.NewRecorder() + tc.server.handleGetSettings(get, httptest.NewRequest(http.MethodGet, "/api/settings", nil)) + assertNoRememberedCluster(t, "GET", get.Body.String()) + + put := httptest.NewRecorder() + tc.server.handlePutSettings(put, httptest.NewRequest( + http.MethodPut, "/api/settings", strings.NewReader(`{"theme":"light"}`))) + assertNoRememberedCluster(t, "PUT", put.Body.String()) + }) + } +} + +func assertNoRememberedCluster(t *testing.T, verb, body string) { + t.Helper() + var payload map[string]any + if err := json.Unmarshal([]byte(body), &payload); err != nil { + t.Fatalf("%s decode: %v", verb, err) + } + if v, has := payload["lastContext"]; has { + t.Errorf("%s /api/settings carried the remembered cluster: %v", verb, v) + } + if strings.Contains(body, "prod-eu") { + t.Errorf("%s /api/settings body mentions the remembered cluster: %s", verb, body) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index c3075552c..4a2063870 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -4373,6 +4373,7 @@ func (s *Server) handleConnectionStatus(w http.ResponseWriter, r *http.Request) // Lets the browser stand down its auto-retry for the whole auth-loss // episode, even when the live errorType flips to non-auth values. "authRecoveryOwed": k8s.RuntimeAuthRecoveryOwed(), + "restoredLastUsed": k8s.ContextRestoredFromMemory(), } // Context enumeration re-reads kubeconfig files (under the client write // lock in multi-file mode) — too expensive for the UI's perpetual diff --git a/internal/server/sse.go b/internal/server/sse.go index ecb5a3bec..1f4e86d57 100644 --- a/internal/server/sse.go +++ b/internal/server/sse.go @@ -351,12 +351,13 @@ func (b *SSEBroadcaster) registerConnectionStateCallback() { b.Broadcast(SSEEvent{ Event: "connection_state", Data: map[string]any{ - "state": status.State, - "context": status.Context, - "clusterName": status.ClusterName, - "error": status.Error, - "errorType": status.ErrorType, - "progressMessage": status.ProgressMsg, + "state": status.State, + "context": status.Context, + "clusterName": status.ClusterName, + "error": status.Error, + "errorType": status.ErrorType, + "progressMessage": status.ProgressMsg, + "restoredLastUsed": k8s.ContextRestoredFromMemory(), }, }) @@ -1295,12 +1296,13 @@ func (b *SSEBroadcaster) HandleSSE(w http.ResponseWriter, r *http.Request, denie // Send current connection state immediately so client knows current status status := k8s.GetConnectionStatus() connData, err := json.Marshal(map[string]any{ - "state": status.State, - "context": status.Context, - "clusterName": status.ClusterName, - "error": status.Error, - "errorType": status.ErrorType, - "progressMessage": status.ProgressMsg, + "state": status.State, + "context": status.Context, + "clusterName": status.ClusterName, + "error": status.Error, + "errorType": status.ErrorType, + "progressMessage": status.ProgressMsg, + "restoredLastUsed": k8s.ContextRestoredFromMemory(), }) if err == nil { fmt.Fprintf(w, "event: connection_state\ndata: %s\n\n", connData) diff --git a/internal/settings/desktop_state.go b/internal/settings/desktop_state.go new file mode 100644 index 000000000..e72411864 --- /dev/null +++ b/internal/settings/desktop_state.go @@ -0,0 +1,86 @@ +package settings + +import ( + "errors" + "log" + "os" + "path/filepath" + "sync" +) + +// DesktopState is state the Desktop app owns alone: what the window was doing +// when it was last closed, so reopening it comes back to the same place. +// +// It lives in its own file, deliberately NOT in the Settings struct, for two +// reasons. /api/settings serializes Settings verbatim (including through a +// Cloud tunnel), so anything in there is served to every viewer and can be +// round-tripped away by a PUT from a client that never saw the field. And this +// is Desktop's state, not the machine's: `kubectl radar` shares $HOME with the +// Desktop app, so a value both could reach is one a Desktop switch could use +// to steer a later terminal command. Keeping the store separate makes that +// separation structural rather than a flag someone can flip. +type DesktopState struct { + // LastContext is the kubeconfig context the Desktop window was last + // switched to, restored on the next launch so it reopens on the cluster + // the user was working in rather than the kubeconfig's current-context. + LastContext *LastContext `json:"lastContext,omitempty"` +} + +// LastContext identifies a kubeconfig context precisely enough to survive a +// restart. The displayed name alone is not enough across multiple kubeconfig +// files: which file owns the unqualified form depends on directory read order, +// so dropping a new file into a watched directory can steal the name and point +// the restore at a different cluster. SourceFile + InFileName pin the exact +// context; the name is the fallback for when that file has moved. +type LastContext struct { + Name string `json:"name"` + SourceFile string `json:"sourceFile,omitempty"` + InFileName string `json:"inFileName,omitempty"` +} + +// desktopMu serializes load-mutate-save cycles on the desktop-state file. +var desktopMu sync.Mutex + +// DesktopStatePath returns the desktop-state file path +// (~/.radar/desktop-state.json). +func DesktopStatePath() string { + homeDir, err := os.UserHomeDir() + if err != nil { + log.Printf("[settings] Cannot determine home directory: %v (desktop state will not be persisted)", err) + return "" + } + return filepath.Join(homeDir, ".radar", "desktop-state.json") +} + +// LoadDesktopState reads the desktop state, distinguishing "no file yet" +// (zero value, nil error) from a failed read or parse (zero value, error). +// Callers restoring a remembered cluster must use the error: treating an +// unreadable file as "the user never picked one" would silently start +// somewhere else and then overwrite the pick they actually had. +func LoadDesktopState() (DesktopState, error) { + path := DesktopStatePath() + if path == "" { + return DesktopState{}, errors.New("desktop state path unavailable") + } + var s DesktopState + if err := readJSONFile(path, &s); err != nil { + return DesktopState{}, err + } + return s, nil +} + +// UpdateDesktopState atomically loads, applies a mutation, and saves. +// +// It refuses to write over a file it could not read, for the same reason +// Update does: a context switch calls this on its own, so one unreadable file +// would otherwise erase whatever else the store had picked up. +func UpdateDesktopState(mutate func(*DesktopState)) (DesktopState, error) { + desktopMu.Lock() + defer desktopMu.Unlock() + s, err := LoadDesktopState() + if err != nil { + return s, err + } + mutate(&s) + return s, writeJSONFile(DesktopStatePath(), s) +} diff --git a/internal/settings/desktop_state_test.go b/internal/settings/desktop_state_test.go new file mode 100644 index 000000000..1f1a7c5c2 --- /dev/null +++ b/internal/settings/desktop_state_test.go @@ -0,0 +1,93 @@ +package settings + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestDesktopStateRoundTrips(t *testing.T) { + useTempHome(t) + + if _, err := UpdateDesktopState(func(st *DesktopState) { + st.LastContext = &LastContext{Name: "prod-eu", SourceFile: "/kube/team.yaml", InFileName: "prod"} + }); err != nil { + t.Fatalf("UpdateDesktopState: %v", err) + } + + got, err := LoadDesktopState() + if err != nil { + t.Fatalf("LoadDesktopState: %v", err) + } + if got.LastContext == nil || *got.LastContext != (LastContext{ + Name: "prod-eu", SourceFile: "/kube/team.yaml", InFileName: "prod", + }) { + t.Errorf("LastContext = %+v, want name/file/in-file-name all preserved", got.LastContext) + } +} + +func TestLoadDesktopStateTreatsAMissingFileAsNothingRecorded(t *testing.T) { + useTempHome(t) + + got, err := LoadDesktopState() + if err != nil { + t.Fatalf("LoadDesktopState on a fresh home: %v", err) + } + if got.LastContext != nil { + t.Errorf("LastContext = %+v, want nil before anything is recorded", got.LastContext) + } +} + +func TestUpdateDesktopStateRefusesToOverwriteAnUnreadableFile(t *testing.T) { + dir := useTempHome(t) + + path := filepath.Join(dir, ".radar", "desktop-state.json") + os.MkdirAll(filepath.Dir(path), 0o755) + os.WriteFile(path, []byte("{bad"), 0o644) + + if _, err := UpdateDesktopState(func(st *DesktopState) { + st.LastContext = &LastContext{Name: "prod-eu"} + }); err == nil { + t.Fatal("UpdateDesktopState should fail when the existing file cannot be read") + } + + raw, _ := os.ReadFile(path) + if string(raw) != "{bad" { + t.Errorf("UpdateDesktopState rewrote an unreadable file: %s", raw) + } +} + +// The two stores are separate files on purpose — /api/settings serializes +// Settings verbatim, and this is Desktop's state rather than the machine's. +func TestDesktopStateStaysOutOfTheSettingsFile(t *testing.T) { + dir := useTempHome(t) + + if err := Save(Settings{Theme: "dark"}); err != nil { + t.Fatalf("Save: %v", err) + } + if _, err := UpdateDesktopState(func(st *DesktopState) { + st.LastContext = &LastContext{Name: "prod-eu"} + }); err != nil { + t.Fatalf("UpdateDesktopState: %v", err) + } + + raw, err := os.ReadFile(filepath.Join(dir, ".radar", "settings.json")) + if err != nil { + t.Fatalf("read settings.json: %v", err) + } + if strings.Contains(string(raw), "prod-eu") || strings.Contains(string(raw), "lastContext") { + t.Errorf("settings.json carries desktop state: %s", raw) + } + if loaded := Load(); loaded.Theme != "dark" { + t.Errorf("theme = %q, want the settings file untouched", loaded.Theme) + } +} + +func useTempHome(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) + return dir +} diff --git a/internal/settings/settings.go b/internal/settings/settings.go index 81b5c015a..48a536296 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -84,32 +84,48 @@ func LoadChecked() (Settings, error) { if path == "" { return Settings{}, errors.New("settings path unavailable") } + var s Settings + if err := readJSONFile(path, &s); err != nil { + return Settings{}, err + } + return s, nil +} + +// readJSONFile decodes path into v, treating a missing file as "nothing +// recorded yet" (v left untouched, nil error) and every other failure as an +// error the caller must distinguish from absence. +func readJSONFile(path string, v any) error { data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { - return Settings{}, nil + return nil } log.Printf("[settings] Failed to read %s: %v", path, err) - return Settings{}, err + return err } - var s Settings - if err := json.Unmarshal(data, &s); err != nil { + if err := json.Unmarshal(data, v); err != nil { log.Printf("[settings] Failed to parse %s: %v", path, err) - return Settings{}, err + return err } - return s, nil + return nil } // Save writes settings to disk using atomic rename. func Save(s Settings) error { - path := Path() + return writeJSONFile(Path(), s) +} + +// writeJSONFile writes v to path via a temp file and an atomic rename, so a +// crash mid-write leaves the previous contents intact rather than a truncated +// file the next start refuses to parse. +func writeJSONFile(path string, v any) error { if path == "" { return os.ErrNotExist } if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return err } - data, err := json.MarshalIndent(s, "", " ") + data, err := json.MarshalIndent(v, "", " ") if err != nil { return err } @@ -126,10 +142,17 @@ func Save(s Settings) error { // Update atomically loads, applies a mutation, and saves settings. // This prevents concurrent PUTs from overwriting each other's changes. +// +// LoadChecked, not Load: a settings file we failed to read must not be +// overwritten from a zero value, or saving one preference would silently erase +// every other one the file still held. func Update(mutate func(*Settings)) (Settings, error) { mu.Lock() defer mu.Unlock() - s := Load() + s, err := LoadChecked() + if err != nil { + return s, err + } mutate(&s) return s, Save(s) } diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go index 4671001f2..3071b9db0 100644 --- a/internal/settings/settings_test.go +++ b/internal/settings/settings_test.go @@ -194,3 +194,22 @@ func TestRolloutKeyConcurrentMintResolvesToOneWinner(t *testing.T) { t.Fatal("no identity persisted after the race") } } + +func TestUpdateRefusesToOverwriteAnUnreadableFile(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) + + path := filepath.Join(dir, ".radar", "settings.json") + os.MkdirAll(filepath.Dir(path), 0o755) + os.WriteFile(path, []byte("{bad"), 0o644) + + if _, err := Update(func(s *Settings) { s.Theme = "dark" }); err == nil { + t.Fatal("Update should fail when the existing settings cannot be read") + } + + raw, _ := os.ReadFile(path) + if string(raw) != "{bad" { + t.Errorf("Update rewrote an unreadable settings file, losing every other preference: %s", raw) + } +} diff --git a/web/src/components/ConnectionErrorView.tsx b/web/src/components/ConnectionErrorView.tsx index 51e4a741f..f9df20dc1 100644 --- a/web/src/components/ConnectionErrorView.tsx +++ b/web/src/components/ConnectionErrorView.tsx @@ -401,6 +401,12 @@ export function ConnectionErrorView({ connection, onRetry, isRetrying }: Connect Cluster: {connection.clusterName}

)} + + {connection.restoredLastUsed && connection.errorType !== 'config' && ( +

+ Radar reopened on the cluster you were last using — not your kubeconfig's current context. +

+ )}
diff --git a/web/src/components/settings/SettingsDialog.tsx b/web/src/components/settings/SettingsDialog.tsx index 865b17548..e84793df3 100644 --- a/web/src/components/settings/SettingsDialog.tsx +++ b/web/src/components/settings/SettingsDialog.tsx @@ -42,6 +42,7 @@ interface Config { argoCdUrl?: string argoCdInsecureTls?: boolean mcp?: boolean | null + restoreLastContext?: boolean | null } interface ConfigResponse { @@ -95,6 +96,7 @@ function normalizeStartup(c: Config) { timelineDbPath: c.timelineDbPath ?? '', historyLimit: c.historyLimit ?? null, mcp: c.mcp ?? true, + restoreLastContext: c.restoreLastContext ?? true, } } @@ -148,7 +150,8 @@ export function SettingsDialog({ const clusterDirty = edN.kubeconfig !== svN.kubeconfig || edN.kubeconfigDirs !== svN.kubeconfigDirs || - edN.namespace !== svN.namespace + edN.namespace !== svN.namespace || + edN.restoreLastContext !== svN.restoreLastContext const serverDirty = edN.port !== svN.port || edN.noBrowser !== svN.noBrowser || edN.browser !== svN.browser const mcpDirty = edN.mcp !== svN.mcp @@ -462,6 +465,7 @@ export function SettingsDialog({
@@ -986,10 +990,12 @@ function AIUnavailableNotice() { function ClusterSection({ config, effectiveConfig, + isDesktop, onChange, }: { config: Config effectiveConfig?: Config + isDesktop: boolean onChange: (field: K, value: Config[K]) => void }) { return ( @@ -1018,6 +1024,14 @@ function ClusterSection({ placeholder="All namespaces" onChange={(v) => onChange('namespace', v || undefined)} /> + {isDesktop && ( + onChange('restoreLastContext', v ? undefined : false)} + /> + )} ) } diff --git a/web/src/context/ConnectionContext.tsx b/web/src/context/ConnectionContext.tsx index 8628c6715..07c671217 100644 --- a/web/src/context/ConnectionContext.tsx +++ b/web/src/context/ConnectionContext.tsx @@ -12,6 +12,7 @@ export interface ConnectionState { error?: string errorType?: string // config, auth, auth-rejected, auth-plugin-stuck, rbac, network, timeout, tls, unknown progressMessage?: string + restoredLastUsed?: boolean } interface ConnectionStatusResponse extends ConnectionState { @@ -211,6 +212,7 @@ export function ConnectionProvider({ children }: { children: ReactNode }) { error: data.error, errorType: data.errorType, progressMessage: data.progressMessage, + restoredLastUsed: data.restoredLastUsed, }) if (becameConnected) { refreshCachesOnConnect() @@ -234,7 +236,7 @@ export function ConnectionProvider({ children }: { children: ReactNode }) { })) }, onSuccess: (result) => { - setConnection(result) + setConnection(prev => ({ ...result, restoredLastUsed: prev.restoredLastUsed })) if (result.state === 'connected') { // Keep the first-connect bookkeeping and the double-refresh window // honest — the SSE frame that follows must not re-invalidate. @@ -302,7 +304,7 @@ export function ConnectionProvider({ children }: { children: ReactNode }) { recovered = true autoRetryDelayRef.current = AUTO_RETRY_INITIAL_DELAY_MS sseActiveRef.current = false - setConnection(result) + setConnection(prev => ({ ...result, restoredLastUsed: prev.restoredLastUsed })) if (result.state === 'connected') { hasConnectedRef.current = true lastCacheRefreshAtRef.current = Date.now()