Skip to content

server: evict saved namespace picks whose namespace was deleted#1115

Open
hisco wants to merge 5 commits into
mainfrom
fix-stale-namespace-pick
Open

server: evict saved namespace picks whose namespace was deleted#1115
hisco wants to merge 5 commits into
mainfrom
fix-stale-namespace-pick

Conversation

@hisco

@hisco hisco commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Problem

Radar persists the header namespace switcher's pick (no-auth local case) and applies it to every read. If the picked namespace is later deleted from the cluster, every endpoint silently returns empty — the UI looks like an empty cluster with no hint that a stale filter is the cause, and there is no recovery: the existing stale-pick recovery only triggers when RBAC filtering empties the pick, which never happens in no-auth mode (getUserNamespaces is a pass-through there). The pick re-seeds from settings on every request, so the state is permanent until the user manually finds and clears the namespace filter.

Fix

  • Saved picks are pruned against the live namespace list when applied: deleted namespaces drop out; when none survive, the pick is fully evicted and reads fall back to unfiltered.
  • Evict only on positive knowledge: when the namespace informer can't answer (namespace-scoped cache, restricted RBAC), picks pass through untouched — wrongly dropping a valid pick is worse than keeping a stale one.
  • Explicit ?namespaces= query filters are unaffected; only the saved-pick fallback path prunes.
  • The eviction persists to settings — in-memory-only eviction would be undone on the next request when the saved preference re-seeds from disk. Persistence is skipped under --namespace-scope, where the saved pick doubles as the cache-scope restore value.
  • Pick mutations are serialized (nsPickMu), and the prune revalidates its snapshot under the lock before writing: a concurrent pick change or a kubeconfig context switch mid-request skips the write, so a slow read can't revert a fresh pick or persist old-context survivors under the new context's key.
  • handleGetNamespaceScope reports through the same prune, so the scope endpoint and the read path can't disagree.

Testing

  • 6 new tests: full eviction, partial trim, valid-pick-unaffected, informer-unavailable pass-through, and two race-guard regressions (pick changed mid-flight, context switch mid-flight). Full internal/server suite green including -race.
  • Live verification on a kind cluster: a settings file pinning a nonexistent namespace previously made /api/namespaces return [] forever; with the fix the first request returns all 8 namespaces and rewrites settings with the stale pick removed (other settings preserved). A valid pick still filters correctly after restart.

https://claude.ai/code/session_019aHPAeh1q88KxMGgyDpESc


Note

Medium Risk
Touches shared namespace-filter state on every read and POST with new locking; incorrect pruning or persistence could change multi-user/no-auth visibility, though guards and conservative informer behavior limit blast radius.

Overview
Fixes a case where a persisted namespace switcher pick for a deleted namespace made every read look empty (especially in no-auth mode), with no self-healing because disk kept re-seeding the stale filter.

On the saved-pick fallback path, picks are now pruned against live cluster namespaces (pruneToExistingNamespaces / pruneDeletedNamespacePicks): partial trims drop gone names; full eviction clears the pick and reads fall back to unfiltered. Pruning is skipped when the informer cannot list namespaces (empty/nil existing), so valid picks are not dropped on uncertainty. Deleted-namespace eviction persists to settings.json for the no-auth single-user case; RBAC trims, empty-fallback clears, and disk seeding use commitPickMutation with persist=false.

commitPickMutation plus nsPickMu centralize read-path writes: compare-and-swap on the snapshotted pick and kube context, skip on concurrent POST or context switch, and avoid deadlocking handleSetActiveNamespace (release lock before the trailing GET). getActiveNamespaceForUserInContext pairs pick + context for one atomic snapshot. The same prune runs in handleGetNamespaceScope and parseNamespacesForUser so scope API and resource reads stay aligned.

Tests cover full/partial eviction, informer-unavailable pass-through, and race/context-switch guards.

Reviewed by Cursor Bugbot for commit 9a8dc6b. Bugbot is set up for automated code reviews on this repo. Configure here.

A persisted namespace pick (the header switcher's selection, saved in
settings for the no-auth local case) is applied to every read. When the
picked namespace is later deleted from the cluster, every endpoint
silently returns empty — the UI looks like an empty cluster with no
hint that a stale filter is the cause. The existing recovery only
triggers when RBAC filtering empties the pick, which never happens in
no-auth mode where getUserNamespaces is a pass-through.

Saved picks are now pruned against the live namespace list when they
are applied: deleted namespaces drop out, and when none survive the
pick is fully evicted and reads fall back to unfiltered. Eviction is
taken only on positive knowledge — an unavailable namespace informer
(namespace-scoped cache, restricted RBAC) leaves picks untouched, since
wrongly dropping a valid pick is worse than keeping a stale one.
Explicit ?namespaces= query filters are unaffected.

The eviction persists to settings; dropping only the in-memory entry
would be undone on the next request when the saved preference re-seeds
it from disk. Persistence is skipped under --namespace-scope, where the
saved pick doubles as the cache-scope restore value and the set-active
handler owns that lifecycle.

Pick mutations are serialized (nsPickMu) and the prune revalidates its
snapshot under the lock before writing: a concurrent pick change or a
kubeconfig context switch mid-request skips the write instead of
reverting a fresh pick or persisting old-context survivors under the
new context's key.

Verified against a live cluster: a stale pick that previously blanked
every read now returns the full namespace list on the first request,
with the pick durably evicted; a valid pick still filters.

Claude-Session: https://claude.ai/code/session_019aHPAeh1q88KxMGgyDpESc
@hisco hisco requested a review from nadaverell as a code owner July 7, 2026 06:56
Comment thread internal/server/namespace_scope.go Outdated
The set-active handler's closing handleGetNamespaceScope render runs the
stale-pick prune, which takes nsPickMu itself; holding the mutex across
the render self-deadlocks when the prune has something to evict (a
namespace deleted between the handler's validation and the render). The
lock is now released explicitly once the persist+set pair completes;
OnceFunc keeps every early error return covered by the defer.

Claude-Session: https://claude.ai/code/session_019aHPAeh1q88KxMGgyDpESc
@nadaverell

Copy link
Copy Markdown
Contributor

Review finding:

nsPickMu only protects the new deleted-namespace prune and the POST persist+set path. The existing read-path RBAC trim/clear still writes nsPreferences without the mutex or a “current pick still equals my snapshot” guard (handleGetNamespaceScope trims actives, and parseNamespacesForUser clears on empty fallback). A concurrent scope GET based on an old pick can still land after a user’s fresh POST and overwrite it with old survivors or clear it. This is the same lost-update class this PR is trying to close.

Fix: route all read-path pick mutations through the same locked compare-current-then-store helper, so stale read snapshots cannot mutate the live pick.

…ded writer

The prior change locked only the deleted-namespace prune and the POST
persist+set. Three other read paths still wrote nsPreferences with no
mutex and no compare-current guard: the RBAC trim in handleGetNamespaceScope,
the empty-fallback clear in parseNamespacesForUser, and the first-request
seed in loadSavedNamespacePreference. A concurrent scope GET or read based
on a stale snapshot could land after a user's fresh POST and revert or clear
it — the same lost-update class the prune guard was meant to close.

Extract commitPickMutation: a single locked compare-and-store that writes
only if the stored pick still equals the caller's snapshot under the still-
current context. It binds the whole read/compare/write to the passed snapshot
ctxName and touches nsPreferences directly, so a context switch mid-critical-
section can't compare against one context's pick and store under another's.
Callers read the pick and its context together via
getActiveNamespaceForUserInContext so the snapshot is atomic.

persist=true only for the prune (settings.json rewrite for no-auth single
user); the trim, clear, and seed stay in-memory view filters.

Adds direct guard tests: skip-on-mid-flight-pick-change, apply-when-live,
persist isolation, context-switch skip, and seed-doesn't-clobber. Full
internal/server suite green under -race.

Claude-Session: https://claude.ai/code/session_01EsXeXKxCUyA3dpYMKSKcce
Comment thread internal/server/namespace_scope.go
The no-op fast path in pruneDeletedNamespacePicks used len(survivors) ==
len(picks). That is correct only because the prune is a pure order- and
duplicate-preserving filter, so equal length implies identical contents.
Compare contents directly so the invariant is explicit and survives any
future change to the filter. No behavior change today.

Adds duplicate-preservation cases to TestPruneToExistingNamespaces.

Claude-Session: https://claude.ai/code/session_01EsXeXKxCUyA3dpYMKSKcce

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 338619c. Configure here.

Comment thread internal/server/namespace_scope.go
if len(existing) == 0 {
return picks
}
return intersectPicksWithAllowed(picks, existing)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty namespace list skips eviction

Low Severity

pruneToExistingNamespaces treats any len(existing)==0 list as “informer unavailable” and leaves picks unchanged, but allNamespaceNames returns nil when the informer cannot answer and a non-nil empty slice when listing succeeds with zero namespaces. A cluster with no namespaces therefore never evicts stale saved picks.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 338619c. Configure here.

pruneDeletedNamespacePicks re-read k8s.GetContextName() at prune time while
its callers had already snapshotted the pick and its context together via
getActiveNamespaceForUserInContext. A kubeconfig context switch between those
two reads could let the prune persist old-context survivors under the new
context's key when the new context's stored pick happened to match. Thread the
snapshot ctxName through prune so its commit binds to the same context the
picks were read under, matching the trim and clear paths.

Strengthens the context-switch test to preseed the new context with a matching
pick — proving the prune no longer trims it.

Claude-Session: https://claude.ai/code/session_01EsXeXKxCUyA3dpYMKSKcce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants