Skip to content

Commit a710e8c

Browse files
dchaudhari7177nadaverellclaude
authored
Scope the topology filter sidebar to the selected namespaces (#1686)
Closes #1647. `TopologyFilterSidebar` was passed `topology?.nodes` — the raw, cluster-wide, unbuffered node list — while the graph rendered `filteredTopology`. With a namespace selected, the sidebar's kind rows and counts therefore described the whole cluster rather than the canvas (the `Ingress (12)` case in the issue), and because it read `topology` rather than `displayedTopology`, its counts kept moving while the graph was paused. ## Change The sidebar now receives a **namespace-scoped** set derived from `displayedTopology`. Scoping is namespace-only, deliberately. `TopologyFilterSidebar` builds `availableKinds` from `kindCounts` over its `nodes` prop and computes the footer's visible/hidden totals by intersecting that with `visibleKinds` (`TopologyFilterSidebar.tsx:150-170`, `:400-415`). Handing it the already kind-filtered graph nodes would drop every hidden kind out of the list instead of showing it with a count, and `hidden` would always read zero — which is what the issue warns about. The namespace carve-out is now a single shared helper (`web/src/utils/topology-namespace.ts`) used by **both** the graph's `filteredTopology` and the sidebar, so the two cannot drift apart again — that is the acceptance criterion stated as code rather than as a convention. Nodes with no namespace are cluster-scoped (`Node`, `PersistentVolume`, `Namespace`) and stay visible in every scope, matching the existing behaviour. `hiddenKinds` is read from `displayedTopology` for the same pause semantics. ## Acceptance criteria | criterion | how it is met | | --- | --- | | Count and list kinds from the namespace-filtered node set | `filterSidebarNodes` = `scopeNodesToNamespaces(displayedTopology.nodes, namespaces)` | | Retain cluster-scoped nodes with no namespace | `isInNamespaceScope` returns `true` when `data.namespace` is absent or empty | | Use the pause-buffered `displayedTopology` | the memo reads `displayedTopology`, not `topology` | | Do **not** pass the kind-filtered graph nodes | the helper applies the namespace carve-out only; kind filtering stays in `filteredTopology` | ## Verification - `npm run tsc` (typescript-7, `--noEmit`) — clean - `npm run lint` — **0 errors** (351 pre-existing warnings, unchanged) - `npx vitest run src` — **914 passed**, including 9 new tests in `web/src/utils/topology-namespace.test.ts` Two tests in `src/api/client.authRedirect.test.ts` fail, but they are **pre-existing and flaky**, unrelated to this change: on a clean stash of this branch that file alone still reports `1 failed | 5 passed`, and across runs it reports 1 or 2 failures on the same untouched code. I could not exercise this against a live cluster, so the namespace behaviour is covered by unit tests on the extracted helper rather than end to end. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > UI-only topology filtering refactor with extracted helpers and tests; no auth, API, or data-path changes. > > **Overview** > **Aligns the topology filter sidebar with the namespace-scoped graph** so kind rows and counts match what you see on the canvas when namespaces are selected, instead of reflecting the full cluster. > > Adds shared helpers in `topology-namespace.ts` (`isInNamespaceScope`, `scopeNodesToNamespaces`) used by both the graph’s `filteredTopology` and a new `filterSidebarNodes` memo. Sidebar nodes are **namespace-scoped only** (from pause-buffered `displayedTopology`), not kind-filtered, so hidden kinds still appear in the list with correct visible/hidden footer counts. `hiddenKinds` now comes from `displayedTopology` so it stays stable while the graph is paused. > > Unit tests cover namespace scoping and cluster-scoped nodes (no/empty namespace). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4915b72. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Nadav Erell <nadaverell@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0e3dc48 commit a710e8c

3 files changed

Lines changed: 96 additions & 4 deletions

File tree

‎web/src/App.tsx‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import { DiagnosticsOverlay } from './components/ui/DiagnosticsOverlay'
5353
import { useEventSource } from './hooks/useEventSource'
5454
import { debugNamespaceLog, useNamespaces, useNamespaceScope, useSetActiveNamespace, useSwitchContext, useAuthMe, useAudit } from './api/client'
5555
import { buildAuditSeverityMap } from './utils/auditBadges'
56+
import { isInNamespaceScope, scopeNodesToNamespaces } from './utils/topology-namespace'
5657
import { routePath, apiUrl, getAuthHeaders, getCredentialsMode, stripBasename } from './api/config'
5758
import { KeyboardShortcutProvider, useRegisterShortcut, useRegisterShortcuts, useSuppressBaseShortcuts } from './hooks/useKeyboardShortcuts'
5859
import { useAnimatedUnmount } from './hooks/useAnimatedUnmount'
@@ -1604,8 +1605,7 @@ function AppInner({ manageDocumentTitle = false, documentTitleSuffix, onClusterL
16041605
// Filter by namespace (client-side) and by visible kinds
16051606
const nsSet = namespaces.length > 0 ? new Set(namespaces) : null
16061607
const filteredNodes = displayedTopology.nodes.filter(node =>
1607-
effectiveKinds.has(node.kind) &&
1608-
(!nsSet || nsSet.has(node.data.namespace as string) || !(node.data.namespace as string))
1608+
effectiveKinds.has(node.kind) && isInNamespaceScope(node, nsSet)
16091609
)
16101610
const filteredNodeIds = new Set(filteredNodes.map(n => n.id))
16111611

@@ -1630,6 +1630,16 @@ function AppInner({ manageDocumentTitle = false, documentTitleSuffix, onClusterL
16301630
}
16311631
}, [displayedTopology, visibleKinds, namespaces, topologyMode])
16321632

1633+
// Namespace-scoped but NOT kind-filtered: the sidebar derives its kind list
1634+
// and visible/hidden footer counts from this prop, so handing it the
1635+
// kind-filtered graph nodes would drop every hidden kind from the list and
1636+
// "hidden" would always read zero. Reads displayedTopology so the counts
1637+
// freeze with the graph while paused.
1638+
const filterSidebarNodes = useMemo(() => {
1639+
if (!displayedTopology) return []
1640+
return scopeNodesToNamespaces(displayedTopology.nodes, namespaces)
1641+
}, [displayedTopology, namespaces])
1642+
16331643
// Cluster Audit findings, joined onto topology nodes by the audit key the
16341644
// backend stamps on each node (data.auditKey). Only badge-worthy findings
16351645
// reach the graph; the raw auditDanger/auditWarning property names remain at
@@ -2182,14 +2192,14 @@ function AppInner({ manageDocumentTitle = false, documentTitleSuffix, onClusterL
21822192
<>
21832193
{/* Filter sidebar */}
21842194
<TopologyFilterSidebar
2185-
nodes={topology?.nodes || []}
2195+
nodes={filterSidebarNodes}
21862196
visibleKinds={visibleKinds}
21872197
onToggleKind={handleToggleKind}
21882198
onShowAll={handleShowAllKinds}
21892199
onHideAll={handleHideAllKinds}
21902200
collapsed={filterSidebarCollapsed}
21912201
onToggleCollapse={() => setFilterSidebarCollapsed(prev => !prev)}
2192-
hiddenKinds={topology?.hiddenKinds}
2202+
hiddenKinds={displayedTopology?.hiddenKinds}
21932203
onEnableHiddenKind={(kind) => {
21942204
setVisibleKinds(prev => new Set(prev).add(kind as NodeKind))
21952205
console.log(`[topology] User requested to show hidden kind: ${kind}`)
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { describe, expect, it } from 'vitest'
2+
import type { TopologyNode } from '@skyhook-io/k8s-ui/types/core'
3+
import { isInNamespaceScope, scopeNodesToNamespaces } from './topology-namespace'
4+
5+
function node(id: string, kind: string, namespace?: string): TopologyNode {
6+
return {
7+
id,
8+
kind,
9+
name: id,
10+
status: 'healthy',
11+
data: namespace === undefined ? {} : { namespace },
12+
} as TopologyNode
13+
}
14+
15+
const nodes = [
16+
node('web', 'Deployment', 'prod'),
17+
node('ing', 'Ingress', 'staging'),
18+
node('db', 'StatefulSet', 'prod'),
19+
node('node-1', 'Node'), // cluster-scoped
20+
node('pv-1', 'PersistentVolume'), // cluster-scoped
21+
]
22+
23+
describe('scopeNodesToNamespaces', () => {
24+
it('returns every node when no namespace is selected', () => {
25+
expect(scopeNodesToNamespaces(nodes, [])).toHaveLength(5)
26+
})
27+
28+
it('keeps only the selected namespace, plus cluster-scoped nodes', () => {
29+
const scoped = scopeNodesToNamespaces(nodes, ['prod'])
30+
expect(scoped.map(n => n.id)).toEqual(['web', 'db', 'node-1', 'pv-1'])
31+
})
32+
33+
it('drops a kind that exists only outside the scope', () => {
34+
const scoped = scopeNodesToNamespaces(nodes, ['prod'])
35+
expect(scoped.some(n => n.kind === 'Ingress')).toBe(false)
36+
})
37+
38+
it('accepts several namespaces at once', () => {
39+
const scoped = scopeNodesToNamespaces(nodes, ['prod', 'staging'])
40+
expect(scoped).toHaveLength(5)
41+
})
42+
43+
it('returns the same array identity when the scope is empty', () => {
44+
expect(scopeNodesToNamespaces(nodes, [])).toBe(nodes)
45+
})
46+
})
47+
48+
describe('isInNamespaceScope', () => {
49+
it('passes everything for a null scope', () => {
50+
expect(isInNamespaceScope(node('x', 'Pod', 'any'), null)).toBe(true)
51+
})
52+
53+
it('always passes a node with no namespace', () => {
54+
expect(isInNamespaceScope(node('node-1', 'Node'), new Set(['prod']))).toBe(true)
55+
})
56+
57+
it('treats an empty-string namespace as cluster-scoped', () => {
58+
expect(isInNamespaceScope(node('n', 'Node', ''), new Set(['prod']))).toBe(true)
59+
})
60+
61+
it('rejects a namespaced node outside the scope', () => {
62+
expect(isInNamespaceScope(node('ing', 'Ingress', 'staging'), new Set(['prod']))).toBe(false)
63+
})
64+
})
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import type { TopologyNode } from '@skyhook-io/k8s-ui/types/core'
2+
3+
// A node with no namespace is cluster-scoped (Node, PersistentVolume,
4+
// Namespace, ...) and stays visible in every scope.
5+
export function isInNamespaceScope(node: TopologyNode, nsSet: Set<string> | null): boolean {
6+
if (!nsSet) return true
7+
const ns = node.data?.namespace as string | undefined
8+
return !ns || nsSet.has(ns)
9+
}
10+
11+
// Shared by the topology graph and the filter sidebar so the two cannot
12+
// disagree about which nodes are in scope. Namespace carve-out only — kind
13+
// filtering stays on the graph.
14+
export function scopeNodesToNamespaces(nodes: TopologyNode[], namespaces: string[]): TopologyNode[] {
15+
if (namespaces.length === 0) return nodes
16+
const nsSet = new Set(namespaces)
17+
return nodes.filter(node => isInNamespaceScope(node, nsSet))
18+
}

0 commit comments

Comments
 (0)