server: evict saved namespace picks whose namespace was deleted

internal/server/namespace_scope.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"log"
 	"net/http"
+	"slices"
 	"strings"
 
 	"github.com/skyhook-io/radar/internal/auth"
@@ -149,6 +150,56 @@ func (s *Server) loadSavedNamespacePreference(r *http.Request) {
 	}
 }
 
+// pruneToExistingNamespaces returns picks minus namespaces absent from
+// existing. An empty existing list means the namespace informer can't answer
+// (namespace-scoped cache, restricted RBAC) — picks pass through unchanged,
+// since wrongly evicting a valid pick is worse than keeping a stale one.
+func pruneToExistingNamespaces(picks, existing []string) []string {
+	if len(existing) == 0 {
+		return picks
+	}
+	return intersectPicksWithAllowed(picks, existing)
+}
+
+// pruneDeletedNamespacePicks drops saved picks whose namespaces were deleted
+// from the cluster. Without this, a stale pick silently empties every read —
+// in no-auth mode nothing downstream re-validates it (getUserNamespaces is a
+// pass-through), so the UI looks like an empty cluster forever.
+//
+// Survivors are written back to the in-memory pick and, for the no-auth
+// single-user case, to settings.json — otherwise loadSavedNamespacePreference
+// re-seeds the stale pick from disk on the next request and the eviction is
+// undone. Skipped under --namespace-scope, where the saved pick doubles as
+// the cache-scope restore value and handleSetActiveNamespace owns its
+// lifecycle.
+func (s *Server) pruneDeletedNamespacePicks(r *http.Request, picks []string) []string {
+	ctxName := k8s.GetContextName()
+	survivors := pruneToExistingNamespaces(picks, s.allNamespaceNames())
+	if len(survivors) == len(picks) || ctxName == "" {
+		return survivors
+	}
+
+	// The prune validated a snapshot; mutate only if that snapshot is still
+	// the live state. A concurrent POST replacing the pick, or a context
+	// switch mid-request, makes the survivors stale — skip, and let the new
+	// pick be pruned on its own next read. Without the guard a slow read
+	// could land after a user's fresh pick and silently revert it, or
+	// persist old-context survivors under the new context's key.
+	s.nsPickMu.Lock()
+	defer s.nsPickMu.Unlock()
+	if k8s.GetContextName() != ctxName || !slices.Equal(s.getActiveNamespaceForUser(r), picks) {
+		return survivors
+	}
+
+	s.setActiveNamespaceForUser(r, survivors)
+	if auth.UserFromContext(r.Context()) == nil && !k8s.ForceNamespaceScope {
+		if err := persistNamespacePick(ctxName, survivors); err != nil {
+			log.Printf("[namespace] failed to persist pruned namespace pick for context %q: %v", ctxName, err)
+		}
+	}
+	return survivors
+}
+
 // intersectPicksWithAllowed returns the picks that survive RBAC filtering.
 // allowed=nil means cluster-admin / auth-disabled — all picks pass through.
 // Returns nil when the input picks are empty (no narrowing in effect).
@@ -178,7 +229,7 @@ func (s *Server) handleGetNamespaceScope(w http.ResponseWriter, r *http.Request)
 	}
 
 	s.loadSavedNamespacePreference(r)
-	actives := s.getActiveNamespaceForUser(r)
+	actives := s.pruneDeletedNamespacePicks(r, s.getActiveNamespaceForUser(r))
 	kubeNs := k8s.GetContextNamespace()
 	cacheScopeNs := k8s.GetNamespaceScopeTarget()
 
@@ -365,6 +416,12 @@ func (s *Server) handleSetActiveNamespace(w http.ResponseWriter, r *http.Request
 		s.scopeMutationMu.Lock()
 		defer s.scopeMutationMu.Unlock()
 	}
+	// Pairs this handler's persist+set with the read-path stale-pick prune:
+	// the prune re-checks the live pick under the same lock before mutating,
+	// so it can't revert a pick set here from a stale snapshot. Lock order is
+	// scopeMutationMu → nsPickMu; the prune takes only nsPickMu.
+	s.nsPickMu.Lock()
+	defer s.nsPickMu.Unlock()
 
 	// Persist the no-auth (single-user) pick across restarts before acting on it.
 	// Auth-enabled deploys skip persistence — it'd require user-keyed storage we

internal/server/namespace_scope_test.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/skyhook-io/radar/internal/k8s"
+	"github.com/skyhook-io/radar/internal/settings"
 	pkgauth "github.com/skyhook-io/radar/pkg/auth"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -305,6 +306,113 @@ func restoreHelmNamespaceFallbackState(t *testing.T) {
 	t.Cleanup(func() { k8s.SetFallbackNamespace("") })
 }
 
+// The shared TestMain cache holds exactly two namespaces: "default" and
+// "broken". Anything else counts as deleted from the cluster.
+
+func TestParseNamespacesForUser_EvictsDeletedSavedPick(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	s := newTestServer(t)
+
+	// A pick saved in a previous session names a namespace that has since
+	// been deleted from the cluster.
+	if _, err := settings.Update(func(st *settings.Settings) {
+		st.ActiveNamespaces = map[string][]string{"test-ctx": {"ghost"}}
+	}); err != nil {
+		t.Fatalf("settings.Update: %v", err)
+	}
+
+	req := httptest.NewRequest("GET", "/api/resources/pods", nil)
+	if got := s.parseNamespacesForUser(req); got != nil {
+		t.Fatalf("parseNamespacesForUser = %v, want nil (unfiltered) after stale-pick eviction", got)
+	}
+	if picks := s.getActiveNamespaceForUser(reqAs("")); len(picks) != 0 {
+		t.Errorf("stale pick survived in memory: %v", picks)
+	}
+	// The eviction must reach settings.json — otherwise loadSavedNamespace-
+	// Preference re-seeds the stale pick on the next request.
+	if saved := settings.Load().ActiveNamespaces["test-ctx"]; len(saved) != 0 {
+		t.Errorf("stale pick survived in settings: %v", saved)
+	}
+}
+
+func TestParseNamespacesForUser_TrimsPartiallyDeletedPick(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	s := newTestServer(t)
+
+	if _, err := settings.Update(func(st *settings.Settings) {
+		st.ActiveNamespaces = map[string][]string{"test-ctx": {"default", "ghost"}}
+	}); err != nil {
+		t.Fatalf("settings.Update: %v", err)
+	}
+
+	req := httptest.NewRequest("GET", "/api/resources/pods", nil)
+	if got := s.parseNamespacesForUser(req); !slices.Equal(got, []string{"default"}) {
+		t.Fatalf("parseNamespacesForUser = %v, want [default]", got)
+	}
+	if picks := s.getActiveNamespaceForUser(reqAs("")); !slices.Equal(picks, []string{"default"}) {
+		t.Errorf("in-memory pick = %v, want [default]", picks)
+	}
+	if saved := settings.Load().ActiveNamespaces["test-ctx"]; !slices.Equal(saved, []string{"default"}) {
+		t.Errorf("saved pick = %v, want [default]", saved)
+	}
+}
+
+func TestParseNamespacesForUser_ValidSavedPickStillFilters(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	s := newTestServer(t)
+	s.setActiveNamespaceForUser(reqAs(""), []string{"default"})
+
+	req := httptest.NewRequest("GET", "/api/resources/pods", nil)
+	if got := s.parseNamespacesForUser(req); !slices.Equal(got, []string{"default"}) {
+		t.Fatalf("parseNamespacesForUser = %v, want [default]", got)
+	}
+	if picks := s.getActiveNamespaceForUser(reqAs("")); !slices.Equal(picks, []string{"default"}) {
+		t.Errorf("valid pick was disturbed: %v", picks)
+	}
+}
+
+func TestPruneToExistingNamespaces(t *testing.T) {
+	tests := []struct {
+		name     string
+		picks    []string
+		existing []string
+		want     []string
+	}{
+		{
+			name:     "nil existing (informer unavailable) leaves picks alone",
+			picks:    []string{"alpha", "beta"},
+			existing: nil,
+			want:     []string{"alpha", "beta"},
+		},
+		{
+			name:     "empty existing leaves picks alone",
+			picks:    []string{"alpha"},
+			existing: []string{},
+			want:     []string{"alpha"},
+		},
+		{
+			name:     "deleted namespace dropped, survivors kept",
+			picks:    []string{"alpha", "ghost"},
+			existing: []string{"alpha", "beta"},
+			want:     []string{"alpha"},
+		},
+		{
+			name:     "all deleted returns empty",
+			picks:    []string{"ghost", "phantom"},
+			existing: []string{"alpha"},
+			want:     []string{},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := pruneToExistingNamespaces(tt.picks, tt.existing)
+			if !slices.Equal(got, tt.want) {
+				t.Errorf("pruneToExistingNamespaces(%v, %v) = %v, want %v", tt.picks, tt.existing, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestParseNamespacesForUser_ForcedCacheScope(t *testing.T) {
 	s := newTestServer(t)
 	k8s.ForceNamespaceScope = true
@@ -333,3 +441,59 @@ func TestParseNamespacesForUser_ForcedCacheScope(t *testing.T) {
 		})
 	}
 }
+
+// The prune validates a snapshot of the pick; if a concurrent POST replaces
+// the pick before the prune's write, the write must be skipped — otherwise a
+// slow read reverts the user's fresh pick to pruned survivors of the old one.
+func TestPruneDeletedNamespacePicks_SkipsWhenPickChangedMidFlight(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	s := newTestServer(t)
+	req := reqAs("")
+
+	// The stale snapshot a slow read is working from.
+	snapshot := []string{"default", "ghost"}
+	s.setActiveNamespaceForUser(req, snapshot)
+
+	// A user POST lands while the read is pruning its snapshot.
+	s.setActiveNamespaceForUser(req, []string{"broken"})
+
+	survivors := s.pruneDeletedNamespacePicks(req, snapshot)
+	if !slices.Equal(survivors, []string{"default"}) {
+		t.Fatalf("survivors = %v, want [default] (this request still filters by its own snapshot)", survivors)
+	}
+	// The fresh pick must be untouched — the prune's write was skipped.
+	if picks := s.getActiveNamespaceForUser(req); !slices.Equal(picks, []string{"broken"}) {
+		t.Errorf("fresh pick reverted by stale prune: %v, want [broken]", picks)
+	}
+	if saved := settings.Load().ActiveNamespaces["test-ctx"]; len(saved) != 0 {
+		t.Errorf("stale prune persisted survivors over the fresh pick: %v", saved)
+	}
+}
+
+// A context switch between snapshot and write must also skip the mutation —
+// old-context survivors must not persist under the new context's key.
+func TestPruneDeletedNamespacePicks_SkipsAcrossContextSwitch(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	s := newTestServer(t)
+	req := reqAs("")
+
+	snapshot := []string{"default", "ghost"}
+	s.setActiveNamespaceForUser(req, snapshot)
+
+	// Simulate the interleaving: the prune's entry snapshot saw "test-ctx";
+	// wrap the call so the context flips before it runs (the guard re-reads
+	// the context under the lock, so a flip before the call exercises the
+	// same skip path as one mid-call).
+	prev := k8s.SetTestContextName("other-ctx")
+	survivorsUnderOther := s.pruneDeletedNamespacePicks(req, snapshot)
+	k8s.SetTestContextName(prev)
+	_ = survivorsUnderOther
+
+	// The original context's pick and settings are untouched.
+	if picks := s.getActiveNamespaceForUser(req); !slices.Equal(picks, snapshot) {
+		t.Errorf("old-context pick mutated across switch: %v, want %v", picks, snapshot)
+	}
+	if saved := settings.Load().ActiveNamespaces["other-ctx"]; len(saved) != 0 {
+		t.Errorf("survivors persisted under the new context's key: %v", saved)
+	}
+}

internal/server/server.go

@@ -90,6 +90,12 @@ type Server struct {
 	// rebuild, not this handler's persist step).
 	scopeMutationMu sync.Mutex
 
+	// nsPickMu serializes namespace-pick mutations: the POST handler's
+	// persist+set pair and the read-path stale-pick prune. Without it, a
+	// prune computed from a stale snapshot can land after a user's fresh
+	// pick and silently revert it.
+	nsPickMu sync.Mutex
+
 	// Short-TTL cache for topology builds. The Topology graph is a
 	// deterministic projection of the informer cache; rebuilding it walks
 	// every resource of every kind. A 5s TTL absorbs the typical bursts
@@ -866,11 +872,17 @@ func (s *Server) parseNamespacesForUser(r *http.Request) []string {
 		}
 	}
 	if namespaces == nil {
-		// No explicit filter — use the user's saved picks if any.
+		// No explicit filter — use the user's saved picks if any, pruned of
+		// namespaces that were deleted from the cluster since the pick was made.
+		// When every pick is stale, fall through with no filter so the user sees
+		// the full cluster instead of a silently-empty UI.
 		s.loadSavedNamespacePreference(r)
 		if picks := s.getActiveNamespaceForUser(r); len(picks) > 0 {
-			namespaces = picks
-			pickFallback = true
+			picks = s.pruneDeletedNamespacePicks(r, picks)
+			if len(picks) > 0 {
+				namespaces = picks
+				pickFallback = true
+			}
 		}
 	}
 	filtered := s.getUserNamespaces(r, namespaces)