From a1f48ba1bca2acc5fc1e6252e8227cda576d299c Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 15 Sep 2026 00:33:55 +0200 Subject: [PATCH 1/8] ACM-44888: fix non-admin SSE OOM under large inventory on release-2.16 Port SelfSubjectRulesReview short-circuit, access-cache hardening, eventsDefinitions/CLUSTER_SCOPED_KINDS, and filter-before-inflate SSE from #6638 so restricted users no longer OOM the console backend. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- backend/src/lib/compression.ts | 15 +- backend/src/lib/server-side-events.ts | 66 +++-- backend/src/resources/watch-options.ts | 15 + backend/src/routes/events.ts | 374 ++++++------------------ backend/src/routes/eventsAccess.ts | 305 +++++++++++++++++++ backend/src/routes/eventsCache.ts | 188 ++++++++++++ backend/src/routes/eventsDefinitions.ts | 107 +++++++ 7 files changed, 765 insertions(+), 305 deletions(-) create mode 100644 backend/src/routes/eventsAccess.ts create mode 100644 backend/src/routes/eventsCache.ts create mode 100644 backend/src/routes/eventsDefinitions.ts diff --git a/backend/src/lib/compression.ts b/backend/src/lib/compression.ts index 18fdd68350f..1491eb26ecd 100644 --- a/backend/src/lib/compression.ts +++ b/backend/src/lib/compression.ts @@ -138,7 +138,7 @@ export class FifoSet { const bigStrings: FifoSet = new FifoSet(200) export async function deflateResource(resource: IResource, dictionary: Dictionary): Promise { - const res = compressResource(resource as UncompressedResourceType, dictionary) + const res = compressResource(resource, dictionary) let buffer try { buffer = await promisify(deflateRaw)(JSON.stringify(res)) @@ -243,11 +243,18 @@ export async function inflateResource(buffer: Buffer, dictionary: Dictionary): P } export async function inflateEvent(event: ServerSideEvent): Promise { - const { id, data } = event - const { type, object } = data as WatchEvent + const { id, name, namespace, data } = event + if (!data || typeof data !== 'object') return event + const watchEvent = data as WatchEvent & { meta?: unknown } + const { type, object } = watchEvent return !object ? event - : { id, data: { type, object: Buffer.isBuffer(object) ? await inflateResource(object, getEventDict()) : object } } + : { + id, + name, + namespace, + data: { type, object: Buffer.isBuffer(object) ? await inflateResource(object, getEventDict()) : object }, + } } export async function inflateApps(apps: ICompressedResource[]): Promise { diff --git a/backend/src/lib/server-side-events.ts b/backend/src/lib/server-side-events.ts index f8ee02cff6d..3cb776f63e6 100644 --- a/backend/src/lib/server-side-events.ts +++ b/backend/src/lib/server-side-events.ts @@ -4,7 +4,6 @@ import { constants } from 'node:http2' import type { Transform } from 'node:stream' import { clearInterval } from 'node:timers' import type { Zlib } from 'node:zlib' -import { batchPromiseAll } from './batch-promise-all' import { getEncodeStream, inflateEvent } from './compression' import { setCookie } from './cookies' import { logger } from './logger' @@ -17,7 +16,7 @@ import { sizeOf } from '../routes/aggregators/utils' // If a client hasn't finished receiving a broadcast in PURGE_CLIENT_TIMEOUT // assume the browser has been refreshed or closed -const PURGE_CLIENT_TIMEOUT = 4 * 60 * 60 * 1000 +const PURGE_CLIENT_TIMEOUT = 30 * 60 * 1000 const instanceID = randomString(8) @@ -35,6 +34,15 @@ export interface ServerSideEvent { namespace?: string data?: DataT } + +/** Lightweight resource identity for RBAC filtering without inflating compressed objects. */ +export interface EventResourceMeta { + kind: string + apiVersion: string + name?: string + namespace?: string +} + export interface WatchEvent { type: 'ADDED' | 'DELETED' | 'MODIFIED' | 'EOP' object: { @@ -46,6 +54,24 @@ export interface WatchEvent { resourceVersion: string } } + meta?: EventResourceMeta +} + +/** Resolve kind/apiVersion/name/namespace from meta or an already-inflated object. */ +export function getEventResourceMeta(event: ServerSideEvent): EventResourceMeta | undefined { + const data = event.data as (WatchEvent & { type?: string }) | undefined + if (!data || typeof data !== 'object') return undefined + if (data.meta?.kind) return data.meta + const object = data.object as WatchEvent['object'] | Buffer | undefined + if (object && !Buffer.isBuffer(object) && typeof object === 'object' && object.kind) { + return { + kind: object.kind, + apiVersion: object.apiVersion, + name: object.metadata?.name, + namespace: object.metadata?.namespace, + } + } + return undefined } export interface ServerSideEventClient { @@ -125,22 +151,26 @@ export class ServerSideEvents { } } - private static async sendEvent(clientID: string, event: ServerSideEvent): Promise { + private static sendEvent(clientID: string, event: ServerSideEvent): Promise { const client = this.clients[clientID] - if (!client) return - if (client.events && !client.events[event.name]) return - if (client.namespaces && !client.namespaces[event.namespace]) return - event = await inflateEvent(event) + if (!client) return Promise.resolve() + if (client.events && !client.events[event.name]) return Promise.resolve() + if (client.namespaces && !client.namespaces[event.namespace]) return Promise.resolve() + // Filter before inflate so denied events never materialize full resource JSON in memory. if (this.eventFilter) { client.eventQueue.push( this.eventFilter(client.token, event) - .then((shouldSendEvent) => (shouldSendEvent ? event : undefined)) + .then((shouldSendEvent) => { + if (!shouldSendEvent) return undefined + return inflateEvent(event) + }) .catch((): undefined => undefined) ) } else { - client.eventQueue.push(Promise.resolve(event)) + client.eventQueue.push(inflateEvent(event)) } void this.processClient(clientID) + return Promise.resolve() } private static async processClient(clientID: string): Promise { @@ -243,8 +273,8 @@ export class ServerSideEvents { res: Http2ServerResponse ): Promise { const [writableStream, compressionStream, encoding] = getEncodeStream( - res as unknown as NodeJS.WritableStream, - req.headers[HTTP2_HEADER_ACCEPT_ENCODING] as string, + res, + req.headers[HTTP2_HEADER_ACCEPT_ENCODING], process.env.DISABLE_STREAM_COMPRESSION === 'true' ) @@ -311,10 +341,10 @@ export class ServerSideEvents { // SORT EVENTS INTO SMALLER PACKETS // SO THAT BROWSER PAGE LOADS QUICKER - // uncompress and split events into packets + // Classify using meta / inflated object identity — do not inflate the whole cache up front. const values = Object.values(this.events) const compressed = sizeOf(values) - let parts = await batchPromiseAll(values, (event) => inflateEvent(event)) + let parts: ServerSideEvent[] = [...values] // mock a large environment if (process.env.MOCK_CLUSTERS) { @@ -343,9 +373,9 @@ export class ServerSideEvents { const other: ServerSideEvent[] = [] const remainder: ServerSideEvent[] = [] parts.forEach((event) => { - const data = event.data as WatchEvent + const meta = getEventResourceMeta(event) // see frontend/src/components/LoadPluginData.tsx for what pages are fast loaded - switch (data.object.kind) { + switch (meta?.kind) { case 'ManagedCluster': case 'HostedCluster': case 'ClusterDeployment': @@ -384,9 +414,9 @@ export class ServerSideEvents { // sort events alphabetically so that browser list fills from top to bottom const compareFn = (propName: 'name' | 'namespace') => (a: ServerSideEvent, b: ServerSideEvent) => { - const adata = a.data as WatchEvent - const bdata = b.data as WatchEvent - return adata.object.metadata[propName].localeCompare(bdata.object.metadata[propName]) + const aVal = getEventResourceMeta(a)?.[propName] ?? '' + const bVal = getEventResourceMeta(b)?.[propName] ?? '' + return aVal.localeCompare(bVal) } clusters.sort(compareFn('name')) infos.sort(compareFn('namespace')) diff --git a/backend/src/resources/watch-options.ts b/backend/src/resources/watch-options.ts index b3c140ad039..188c96e80fd 100644 --- a/backend/src/resources/watch-options.ts +++ b/backend/src/resources/watch-options.ts @@ -7,4 +7,19 @@ export interface IWatchOptions { // poll the resource list instead of watching it // process the items in its own cache so not to overload event cache isPolled?: boolean + /** + * Whether watch events should be forwarded to browser clients via SSE. + * When false, resources are still cached in the backend (available via + * `getKubeResources`) but no `ServerSideEvents.pushEvent` calls are made, + * avoiding per-client RBAC checks and SSE bandwidth for resources the + * frontend does not consume through the event stream. + * Defaults to true when omitted. + */ + forwardEventsToClients?: boolean + /** + * True when the Kubernetes resource is cluster-scoped. + * Used by SSE RBAC to decide whether SelfSubjectRulesReview should probe `default` + * (cluster-scoped) or the resource namespace (namespaced). + */ + clusterScoped?: boolean } diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 5986c51f71f..0b4eb6c833a 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -8,15 +8,33 @@ import { pipeline } from 'node:stream/promises' import { Transform } from 'node:stream' import { batchPromiseAll } from '../lib/batch-promise-all' import { createDictionary, deflateResource, inflateResource } from '../lib/compression' -import { jsonPost } from '../lib/json-request' import { logger } from '../lib/logger' -import { type ServerSideEvent, ServerSideEvents } from '../lib/server-side-events' +import { + type EventResourceMeta, + type ServerSideEvent, + ServerSideEvents, + getEventResourceMeta, +} from '../lib/server-side-events' import { getCACertificate, getServiceAccountToken } from '../lib/serviceAccountToken' import { getAuthenticatedToken } from '../lib/token' import type { IResource } from '../resources/resource' import type { IWatchOptions } from '../resources/watch-options' import { polledAggregation } from './aggregator' import { getAppDict, type ICompressedResource, type ITransformedResource } from './aggregators/applications' +import { canAccess, canGetResource, canListClusterScopedKind, canListNamespacedScopedKind } from './eventsAccess' +import { definitions } from './eventsDefinitions' +import { startAccessCacheCleanup, stopAccessCacheCleanup } from './eventsCache' + +export { + ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN, + ACCESS_CACHE_MAX_TOKENS, + ACCESS_CACHE_TTL, + cleanupAccessCache, + getAccessCache, + hashAccessToken, + resetAccessCache, +} from './eventsCache' +export { canAccess, canGetResource } from './eventsAccess' export async function events(req: Http2ServerRequest, res: Http2ServerResponse): Promise { const token = await getAuthenticatedToken(req, res) @@ -67,6 +85,9 @@ let isObservabilityInstalled: boolean = false export function getIsObservabilityInstalled() { return isObservabilityInstalled } +export function resetIsObservabilityInstalled() { + isObservabilityInstalled = false +} // because rbac checks are expensive, // run them only on the resources requested by the UI @@ -162,180 +183,6 @@ export function getEventDict() { return eventDict } -const accessCache: Record }>> = {} - -/** Clear all cached RBAC access checks. Used for test isolation. */ -export function resetAccessCache() { - for (const key in accessCache) { - delete accessCache[key] - } -} - -export function getAccessCache() { - return accessCache -} - -export const ACCESS_CACHE_TTL = 60 * 1000 // 60 seconds -export const ACCESS_CACHE_CLEANUP_INTERVAL = 90 * 1000 // 90 seconds -export const ACCESS_CACHE_MAX_TOKENS = 1000 // Maximum number of token entries to keep - -let accessCacheCleanupTimer: NodeJS.Timeout | undefined - -export function cleanupAccessCache() { - const now = Date.now() - const cutoffTime = now - ACCESS_CACHE_TTL - const tokenStats: Array<{ token: string; newestTime: number }> = [] - - for (const token in accessCache) { - const tokenCache = accessCache[token] - let newestTime = 0 - - for (const key in tokenCache) { - if (tokenCache[key].time < cutoffTime) { - delete tokenCache[key] - } else if (tokenCache[key].time > newestTime) { - newestTime = tokenCache[key].time - } - } - - if (Object.keys(tokenCache).length === 0) { - delete accessCache[token] - } else { - tokenStats.push({ token, newestTime }) - } - } - - if (tokenStats.length > ACCESS_CACHE_MAX_TOKENS) { - tokenStats.sort((a, b) => a.newestTime - b.newestTime) - const tokensToRemove = tokenStats.length - ACCESS_CACHE_MAX_TOKENS - - for (let i = 0; i < tokensToRemove; i++) { - delete accessCache[tokenStats[i].token] - } - } -} - -function startAccessCacheCleanup() { - if (accessCacheCleanupTimer) return - - accessCacheCleanupTimer = setInterval(() => { - try { - cleanupAccessCache() - } catch (err: unknown) { - logger.error({ msg: 'accessCache cleanup failed', error: err }) - } - }, ACCESS_CACHE_CLEANUP_INTERVAL) - - accessCacheCleanupTimer.unref() - logger.info({ msg: 'accessCache cleanup started', interval: ACCESS_CACHE_CLEANUP_INTERVAL }) -} - -function stopAccessCacheCleanup() { - if (accessCacheCleanupTimer) { - clearInterval(accessCacheCleanupTimer) - accessCacheCleanupTimer = undefined - logger.info({ msg: 'accessCache cleanup stopped' }) - } -} - -const definitions: IWatchOptions[] = [ - { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, - { kind: 'ManagedClusterAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, - { kind: 'Agent', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'AgentServiceConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'InfraEnv', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'NMStateConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'Application', apiVersion: 'app.k8s.io/v1beta1' }, - { kind: 'Channel', apiVersion: 'apps.open-cluster-management.io/v1' }, - { kind: 'GitOpsCluster', apiVersion: 'apps.open-cluster-management.io/v1beta1' }, - { kind: 'HelmRelease', apiVersion: 'apps.open-cluster-management.io/v1' }, - { kind: 'PlacementRule', apiVersion: 'apps.open-cluster-management.io/v1' }, - { kind: 'Subscription', apiVersion: 'apps.open-cluster-management.io/v1' }, - { kind: 'SubscriptionReport', apiVersion: 'apps.open-cluster-management.io/v1alpha1' }, - { kind: 'Application', apiVersion: 'argoproj.io/v1alpha1', isPolled: true }, - { kind: 'ApplicationSet', apiVersion: 'argoproj.io/v1alpha1', isPolled: true }, - { kind: 'ArgoCD', apiVersion: 'argoproj.io/v1alpha1' }, - { kind: 'Infrastructure', apiVersion: 'config.openshift.io/v1' }, - { - kind: 'CertificateSigningRequest', - apiVersion: 'certificates.k8s.io/v1', - labelSelector: { 'open-cluster-management.io/cluster-name': '' }, - }, - { kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1' }, - { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, - { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, - { kind: 'ManagedClusterSetBinding', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, - { kind: 'ManagedClusterSet', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, - { kind: 'ClusterCurator', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, - { kind: 'Subscription', apiVersion: 'operators.coreos.com/v1alpha1' }, - { kind: 'DiscoveredCluster', apiVersion: 'discovery.open-cluster-management.io/v1' }, - { kind: 'DiscoveryConfig', apiVersion: 'discovery.open-cluster-management.io/v1' }, - { kind: 'AgentClusterInstall', apiVersion: 'extensions.hive.openshift.io/v1beta1' }, - { kind: 'ClusterClaim', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterDeployment', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterImageSet', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterPool', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterProvision', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'MachinePool', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ManagedClusterInfo', apiVersion: 'internal.open-cluster-management.io/v1beta1' }, - { kind: 'BareMetalHost', apiVersion: 'metal3.io/v1alpha1' }, - { kind: 'MultiClusterEngine', apiVersion: 'multicluster.openshift.io/v1' }, - { kind: 'ClusterVersion', apiVersion: 'config.openshift.io/v1' }, - { kind: 'StorageClass', apiVersion: 'storage.k8s.io/v1' }, - { kind: 'PlacementBinding', apiVersion: 'policy.open-cluster-management.io/v1' }, - { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1' }, - { kind: 'PolicyAutomation', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, - { kind: 'PolicySet', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, - { kind: 'SubmarinerConfig', apiVersion: 'submarineraddon.open-cluster-management.io/v1alpha1' }, - { kind: 'AnsibleJob', apiVersion: 'tower.ansible.com/v1alpha1' }, - { - kind: 'ConfigMap', - apiVersion: 'v1', - fieldSelector: { 'metadata.name': 'assisted-service' }, - }, - { - kind: 'ConfigMap', - apiVersion: 'v1', - fieldSelector: { 'metadata.namespace': 'openshift-config-managed', 'metadata.name': 'console-public' }, - }, - { kind: 'ConfigMap', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'console-search-config' } }, - { kind: 'Namespace', apiVersion: 'v1' }, - { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/credentials': '' } }, - // **Need to look for creds with: 'cluster.open-cluster-management.io/type': 'ans', for edit scenarios - { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/type': 'ans' } }, - { kind: 'Secret', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'auto-import-secret' } }, - { kind: 'PolicyReport', apiVersion: 'wgpolicyk8s.io/v1alpha2' }, - { kind: 'HostedCluster', apiVersion: 'hypershift.openshift.io/v1beta1' }, - { kind: 'NodePool', apiVersion: 'hypershift.openshift.io/v1beta1' }, - { kind: 'AgentMachine', apiVersion: 'capi-provider.agent-install.openshift.io/v1alpha1' }, - { kind: 'ConfigMap', apiVersion: 'v1', labelSelector: { 'hypershift.openshift.io/supported-versions': 'true' } }, - { kind: 'Search', apiVersion: 'search.open-cluster-management.io/v1alpha1' }, - // Configmaps that contain Grafana dashboard IDs - { - kind: 'ConfigMap', - apiVersion: 'v1', - fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-clusters-overview' }, - }, - { - kind: 'ConfigMap', - apiVersion: 'v1', - fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-single-vm-view' }, - }, - { kind: 'MulticlusterRoleAssignment', apiVersion: 'rbac.open-cluster-management.io/v1beta1' }, - { kind: 'User', apiVersion: 'user.openshift.io/v1' }, - { kind: 'Group', apiVersion: 'user.openshift.io/v1' }, - { - kind: 'ClusterRole', - apiVersion: 'rbac.authorization.k8s.io/v1', - labelSelector: { 'rbac.open-cluster-management.io/filter': 'vm-clusterroles' }, - }, - { - kind: 'Service', - apiVersion: 'v1', - fieldSelector: { 'metadata.name': 'cluster-proxy-addon-user', 'metadata.namespace': 'multicluster-engine' }, - }, -] - export function startWatching(): void { ServerSideEvents.eventFilter = eventFilter startAccessCacheCleanup() @@ -437,7 +284,8 @@ async function listKubernetesObjects(serviceAccountToken: string, options: IWatc return { size: itemCount } } - await batchPromiseAll(items, (item) => cacheResource(item)) + const forward = options.forwardEventsToClients !== false + await batchPromiseAll(items, (item) => cacheResource(item, forward)) // Remove items that are no longer in kubernetes const apiVersionPlural = apiVersionPluralFn(options) @@ -458,7 +306,7 @@ async function listKubernetesObjects(serviceAccountToken: string, options: IWatc removeResources.push(resource) } } - await batchPromiseAll(removeResources, (resource) => deleteResource(resource)) + await batchPromiseAll(removeResources, (resource) => deleteResource(resource, forward)) return { resourceVersion, size: items.length } } @@ -545,6 +393,7 @@ export function errorToString(err: unknown): string { * Creates a Transform stream that processes watch events with async operations */ export function createWatchEventProcessor(options: IWatchOptions, url: string, resourceVersionRef: { value: string }) { + const forward = options.forwardEventsToClients !== false return new Transform({ objectMode: true, async transform(data: string, _encoding, callback): Promise { @@ -566,7 +415,7 @@ export function createWatchEventProcessor(options: IWatchOptions, url: string, r case 'ADDED': case 'MODIFIED': try { - await cacheResource(watchEvent.object) + await cacheResource(watchEvent.object, forward) } catch (err: unknown) { logger.error({ msg: 'cacheResource failed', @@ -577,7 +426,7 @@ export function createWatchEventProcessor(options: IWatchOptions, url: string, r break case 'DELETED': try { - await deleteResource(watchEvent.object) + await deleteResource(watchEvent.object, forward) } catch (err: unknown) { logger.error({ msg: 'deleteResource failed', @@ -787,7 +636,9 @@ function resourceUrl(options: IWatchOptions, query: Record) { return url } -export async function cacheResource(resource: IResource) { +const NO_BROADCAST_EVENT_ID = Promise.resolve(-1) + +export async function cacheResource(resource: IResource, forwardEventsToClients = true) { const apiVersionPlural = apiVersionPluralFn(resource) let cache = resourceCache[apiVersionPlural] if (!cache) { @@ -809,7 +660,7 @@ export async function cacheResource(resource: IResource) { const latestExisting = cache[uid] if (latestExisting === existing) { // if no other cacheResource call updated the cache while we were awaiting, we can replace the cache entry and event - ServerSideEvents.removeEvent(eventID) + if (eventID > 0) ServerSideEvents.removeEvent(eventID) break } // if a deleteResource ran while we were awaiting, we will exit the loop because the resource is no longer existing @@ -817,9 +668,19 @@ export async function cacheResource(resource: IResource) { existing = latestExisting } const compressed = deflateResource(resource, eventDict) - const eventID = compressed.then((compressed) => - ServerSideEvents.pushEvent({ data: { type: 'MODIFIED', object: compressed } }) - ) + const meta: EventResourceMeta = { + kind: resource.kind, + apiVersion: resource.apiVersion, + name: resource.metadata?.name, + namespace: resource.metadata?.namespace, + } + const eventID = forwardEventsToClients + ? compressed.then((compressed) => + ServerSideEvents.pushEvent({ + data: { type: 'MODIFIED', object: compressed, meta }, + }) + ) + : NO_BROADCAST_EVENT_ID cache[uid] = { compressed, eventID } if (resource.kind === 'ManagedCluster') { @@ -828,14 +689,18 @@ export async function cacheResource(resource: IResource) { isHubSelfManaged = true } } - if (resource.kind === 'ManagedClusterAddOn') { - if (resource?.metadata?.name === 'observability-controller') { - isObservabilityInstalled = true - } + + if ( + resource.kind === 'ManagedClusterAddOn' && + resource.apiVersion.startsWith('addon.open-cluster-management.io/') && + (resource.metadata?.name === 'observability-controller' || + resource.metadata?.name == 'multicluster-observability-addon') + ) { + isObservabilityInstalled = true } } -async function deleteResource(resource: IResource) { +async function deleteResource(resource: IResource, forwardEventsToClients = true) { const apiVersionPlural = apiVersionPluralFn(resource) const cache = resourceCache[apiVersionPlural] if (!cache) return @@ -843,20 +708,31 @@ async function deleteResource(resource: IResource) { const uid = resource.metadata.uid const existing = cache[uid] - if (existing) ServerSideEvents.removeEvent(await existing.eventID) - - const deletedID = await ServerSideEvents.pushEvent({ - data: { - type: 'DELETED', - object: { - kind: resource.kind, - apiVersion: resource.apiVersion, - metadata: { name: resource.metadata.name, namespace: resource.metadata.namespace }, + if (existing) { + const eventID = await existing.eventID + if (eventID > 0) ServerSideEvents.removeEvent(eventID) + } + + if (forwardEventsToClients) { + const deletedID = await ServerSideEvents.pushEvent({ + data: { + type: 'DELETED', + object: { + kind: resource.kind, + apiVersion: resource.apiVersion, + metadata: { name: resource.metadata.name, namespace: resource.metadata.namespace }, + }, + meta: { + kind: resource.kind, + apiVersion: resource.apiVersion, + name: resource.metadata.name, + namespace: resource.metadata.namespace, + }, }, - }, - }) - // after deletion has been broadcast to current clients, no need to retain - ServerSideEvents.removeEvent(deletedID) + }) + // after deletion has been broadcast to current clients, no need to retain + ServerSideEvents.removeEvent(deletedID) + } delete cache[uid] } @@ -885,14 +761,20 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { if (allowed) return true - return canListNamespacedScopedKind(resource, token).then((allowed) => { - if (allowed) return true - return canGetResource(resource, token) - }) + // After cluster list is denied, use SelfSubjectRulesReview instead of O(N) SSARs. + return canGetResource(resource, token) }) } default: @@ -901,80 +783,6 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { - return canAccess({ kind: resource.kind, apiVersion: resource.apiVersion }, 'list', token) -} - -function canListNamespacedScopedKind(resource: IResource, token: string): Promise { - if (!resource.metadata?.namespace) return Promise.resolve(false) - return canAccess( - { - kind: resource.kind, - apiVersion: resource.apiVersion, - metadata: { namespace: resource.metadata.namespace }, - }, - 'list', - token - ) -} - -function canGetResource(resource: IResource, token: string): Promise { - return canAccess(resource, 'get', token) -} - -export function canAccess( - resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, - verb: 'get' | 'list' | 'create', - token: string -): Promise { - // Cache is cleaned up periodically by cleanupAccessCache() to prevent unbounded memory growth - - const key = `${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` - if (!accessCache[token]) accessCache[token] = {} - const existing = accessCache[token][key] - if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { - return existing.promise - } - - const promise = jsonPost<{ status: { allowed: boolean } }>( - process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', - { - apiVersion: 'authorization.k8s.io/v1', - kind: 'SelfSubjectAccessReview', - metadata: {}, - spec: { - resourceAttributes: { - group: resource.apiVersion.includes('/') ? resource.apiVersion.split('/')[0] : '', - name: resource.metadata?.name, - namespace: - resource.metadata?.namespace ?? (resource.kind === 'Namespace' ? resource.metadata?.name : undefined), - resource: pluralize(resource.kind.toLowerCase()), - verb, - }, - }, - }, - token - ).then((result) => { - if (process.env.LOG_ACCESS === 'true') { - logger.debug({ - msg: 'access', - allowed: result.body.status.allowed, - verb, - resource: pluralize(resource.kind.toLowerCase()), - name: resource.metadata?.name, - namespace: resource.metadata?.namespace, - }) - } - return result.body.status.allowed - }) - - accessCache[token][key] = { - time: Date.now(), - promise, - } - return promise -} - let stopping = false export function stopWatching(): void { stopping = true diff --git a/backend/src/routes/eventsAccess.ts b/backend/src/routes/eventsAccess.ts new file mode 100644 index 00000000000..5fcd6ea3957 --- /dev/null +++ b/backend/src/routes/eventsAccess.ts @@ -0,0 +1,305 @@ +/* Copyright Contributors to the Open Cluster Management project */ + +import pluralize from 'pluralize' +import { jsonPost } from '../lib/json-request' +import { logger } from '../lib/logger' +import { + deleteTimedCacheEntry, + getKindGetAccessCacheStore, + getSubjectRulesCacheStore, + getSsarCacheEntry, + getTimedCacheEntry, + hashAccessToken, + replaceSsarCachePromise, + setSsarCacheEntry, + setTimedCacheEntry, +} from './eventsCache' +import { CLUSTER_SCOPED_KINDS } from './eventsDefinitions' + +export interface SubjectRulesStatus { + incomplete: boolean + /** True when the SelfSubjectRulesReview request itself failed. */ + unavailable?: boolean + /** Set when an authorizer could not fully enumerate rules; partial lists must not be trusted as complete. */ + evaluationError?: string + resourceRules: Array<{ + verbs?: string[] + apiGroups?: string[] + resources?: string[] + resourceNames?: string[] + }> +} + +export type KindGetAccess = + | { type: 'deny-all' } + | { type: 'allow-all' } + | { type: 'allow-names'; names: Set } + | { type: 'incomplete' } + +export type AccessResource = { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } } + +/** SSRR requires a namespace; cluster-scoped kinds are reviewed in this probe namespace only. */ +const CLUSTER_SCOPED_RULES_NAMESPACE = 'default' + +export function isClusterScopedKind(kind: string): boolean { + return CLUSTER_SCOPED_KINDS.has(kind) +} + +const subjectRulesCache = getSubjectRulesCacheStore() +const kindGetAccessCache = getKindGetAccessCacheStore() + +export function canListClusterScopedKind(resource: AccessResource, token: string): Promise { + return canAccess({ kind: resource.kind, apiVersion: resource.apiVersion }, 'list', token) +} + +export function canListNamespacedScopedKind(resource: AccessResource, token: string): Promise { + if (!resource.metadata?.namespace) return Promise.resolve(false) + return canAccess( + { + kind: resource.kind, + apiVersion: resource.apiVersion, + metadata: { namespace: resource.metadata.namespace }, + }, + 'list', + token + ) +} + +function apiGroupFromVersion(apiVersion: string): string { + return apiVersion.includes('/') ? apiVersion.split('/')[0] : '' +} + +function resourcePluralName(kind: string): string { + return pluralize(kind.toLowerCase()) +} + +function rulesNamespaceFor(resource: AccessResource): string { + if (isClusterScopedKind(resource.kind)) { + return CLUSTER_SCOPED_RULES_NAMESPACE + } + return resource.metadata?.namespace || CLUSTER_SCOPED_RULES_NAMESPACE +} + +/** + * Used by SSE eventFilter after cluster-scoped list is denied. + * Namespaced resources are reviewed in the resource's namespace (cached per token+namespace). + * Cluster-scoped resources use a probe-namespace review only as a negative/named-binding cache; + * unrestricted grants from that probe are confirmed with SSAR so RoleBindings in `default` + * cannot impersonate cluster-scoped access. + */ +export function canGetResource(resource: AccessResource, token: string): Promise { + return resolveKindGetAccess(resource, token).then((access) => { + // Probe-namespace SSRR cannot distinguish RoleBindings from ClusterRoleBindings. + // Any non-deny cluster-scoped result must be confirmed with SSAR (not only allow-all/allow-names). + if (isClusterScopedKind(resource.kind) && access.type !== 'deny-all') { + return canAccess(resource, 'get', token) + } + return applyKindGetAccess(access, resource, token, () => + canListNamespacedScopedKind(resource, token).then((nsAllowed) => { + if (nsAllowed) return true + return canAccess(resource, 'get', token) + }) + ) + }) +} + +function applyKindGetAccess( + access: KindGetAccess, + resource: AccessResource, + token: string, + onIncomplete?: () => Promise +): Promise { + switch (access.type) { + case 'deny-all': + return Promise.resolve(false) + case 'allow-all': + return Promise.resolve(true) + case 'allow-names': + return Promise.resolve(resource.metadata?.name ? access.names.has(resource.metadata.name) : false) + case 'incomplete': + return onIncomplete ? onIncomplete() : canAccess(resource, 'get', token) + } +} + +/** + * One SelfSubjectRulesReview per token+namespace. + * ClusterRoleBindings appear in every namespace; RoleBindings appear only in their namespace. + */ +function getSubjectRules(token: string, namespace: string): Promise { + const cacheKey = `${hashAccessToken(token)}:${namespace}` + const existing = getTimedCacheEntry(cacheKey, subjectRulesCache) + if (existing) { + return existing.promise + } + + const promise = jsonPost<{ + status?: { + incomplete?: boolean + evaluationError?: string + resourceRules?: SubjectRulesStatus['resourceRules'] + } + }>( + process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', + { + apiVersion: 'authorization.k8s.io/v1', + kind: 'SelfSubjectRulesReview', + metadata: {}, + spec: { namespace }, + }, + token + ) + .then((result) => { + // jsonPost resolves on HTTP errors; treat non-2xx as review unavailable (SSAR fallback). + if (result.statusCode < 200 || result.statusCode >= 300) { + throw new Error(`SelfSubjectRulesReview failed with status ${result.statusCode}`) + } + return { + incomplete: result.body?.status?.incomplete ?? false, + evaluationError: result.body?.status?.evaluationError, + resourceRules: result.body?.status?.resourceRules ?? [], + } + }) + .catch((err: unknown) => { + logger.warn({ msg: 'SelfSubjectRulesReview failed; falling back to per-object SSAR', error: err }) + // Do not retain a failed review under ACCESS_CACHE_TTL; next call should retry SSRR. + deleteTimedCacheEntry(cacheKey, subjectRulesCache) + return { + incomplete: true, + unavailable: true, + resourceRules: [] as SubjectRulesStatus['resourceRules'], + } + }) + + setTimedCacheEntry(cacheKey, subjectRulesCache, { time: Date.now(), promise }) + return promise +} + +function ruleGrantsKindAccess( + rule: SubjectRulesStatus['resourceRules'][number], + group: string, + resourcePlural: string, + accessVerbs: Set +): { allowAll: true } | { names: string[] } | null { + const verbs = rule.verbs ?? [] + if (!verbs.includes('*') && !verbs.some((verb) => accessVerbs.has(verb))) return null + + const groups = rule.apiGroups ?? [] + if (!groups.includes('*') && !groups.includes(group)) return null + + const resources = rule.resources ?? [] + if (!resources.includes('*') && !resources.includes(resourcePlural)) return null + + const resourceNames = rule.resourceNames + if (!resourceNames || resourceNames.length === 0 || resourceNames.includes('*')) { + return { allowAll: true } + } + return { names: resourceNames } +} + +function evaluateKindGetAccess(rules: SubjectRulesStatus, group: string, resourcePlural: string): KindGetAccess { + const accessVerbs = new Set(['get', 'list', 'watch']) + + // Authorizer reported partial rule enumeration; empty rules still deny-all (none-user fast path). + if (rules.evaluationError) { + if (rules.resourceRules.length === 0) return { type: 'deny-all' } + return { type: 'incomplete' } + } + + let allowAll = false + const names = new Set() + + for (const rule of rules.resourceRules) { + const match = ruleGrantsKindAccess(rule, group, resourcePlural, accessVerbs) + if (!match) continue + if ('allowAll' in match) { + allowAll = true + break + } + for (const name of match.names) names.add(name) + } + + if (allowAll) return { type: 'allow-all' } + if (names.size > 0) return { type: 'allow-names', names } + // The review request failed; defer to the per-object SSAR fallback. + if (rules.unavailable === true) return { type: 'incomplete' } + // OpenShift often sets incomplete=true even when the user has no bindings and resourceRules + // is empty. Treat empty rules as deny-all so we do not fall back to O(N) namespaced SSARs. + if (rules.resourceRules.length === 0) return { type: 'deny-all' } + // Non-empty but incomplete: authorizer may have omitted grants for this kind — fall back. + if (rules.incomplete) return { type: 'incomplete' } + return { type: 'deny-all' } +} + +function resolveKindGetAccess(resource: AccessResource, token: string): Promise { + const group = apiGroupFromVersion(resource.apiVersion) + const plural = resourcePluralName(resource.kind) + const namespace = rulesNamespaceFor(resource) + // Permission checks are by API group, not version; keep cache keys version-free. + const cacheKey = `${hashAccessToken(token)}:${namespace}:${group}:${plural}` + const existing = getTimedCacheEntry(cacheKey, kindGetAccessCache) + if (existing) { + return existing.promise + } + + const promise = getSubjectRules(token, namespace).then((rules) => { + // Do not retain a decision derived from an unavailable review; allow SSRR retry. + if (rules.unavailable) deleteTimedCacheEntry(cacheKey, kindGetAccessCache) + return evaluateKindGetAccess(rules, group, plural) + }) + setTimedCacheEntry(cacheKey, kindGetAccessCache, { time: Date.now(), promise }) + return promise +} + +export function canAccess(resource: AccessResource, verb: 'get' | 'list' | 'create', token: string): Promise { + // Cache is cleaned up periodically by cleanupAccessCache() to prevent unbounded memory growth + const tokenKey = hashAccessToken(token) + const group = apiGroupFromVersion(resource.apiVersion) + // Include API group: Application/Subscription exist in more than one group with different RBAC. + const key = `${verb}:${group}:${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` + const existing = getSsarCacheEntry(tokenKey, key) + if (existing) { + return existing.promise + } + + const resourceName = resourcePluralName(resource.kind) + const promise = jsonPost<{ status: { allowed: boolean } }>( + process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', + { + apiVersion: 'authorization.k8s.io/v1', + kind: 'SelfSubjectAccessReview', + metadata: {}, + spec: { + resourceAttributes: { + group, + name: resource.metadata?.name, + namespace: + resource.metadata?.namespace ?? (resource.kind === 'Namespace' ? resource.metadata?.name : undefined), + resource: resourceName, + verb, + }, + }, + }, + token + ).then((result) => { + const allowed = result.body.status.allowed + if (process.env.LOG_ACCESS === 'true') { + logger.debug({ + msg: 'access', + allowed, + verb, + resource: resourceName, + name: resource.metadata?.name, + namespace: resource.metadata?.namespace, + }) + } + // Replace in-flight promise with a settled boolean promise to drop large closures. + replaceSsarCachePromise(tokenKey, key, promise, allowed) + return allowed + }) + + setSsarCacheEntry(tokenKey, key, { + time: Date.now(), + promise, + }) + return promise +} diff --git a/backend/src/routes/eventsCache.ts b/backend/src/routes/eventsCache.ts new file mode 100644 index 00000000000..5f33762bc5f --- /dev/null +++ b/backend/src/routes/eventsCache.ts @@ -0,0 +1,188 @@ +/* Copyright Contributors to the Open Cluster Management project */ + +import { createHash } from 'node:crypto' +import { logger } from '../lib/logger' + +export const ACCESS_CACHE_TTL = 60 * 1000 // 60 seconds +export const ACCESS_CACHE_CLEANUP_INTERVAL = 90 * 1000 // 90 seconds +export const ACCESS_CACHE_MAX_TOKENS = 1000 // Maximum number of token entries to keep +export const ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN = 2000 // Cap RBAC keys retained per token + +export interface TimedCacheEntry { + time: number + promise: Promise +} + +const accessCache: Record>> = {} +const subjectRulesCache: Record> = {} +const kindGetAccessCache: Record> = {} + +let accessCacheCleanupTimer: NodeJS.Timeout | undefined + +/** Hash bearer tokens so the access cache does not retain full JWTs as object keys. */ +export function hashAccessToken(token: string): string { + return createHash('sha256').update(token).digest('hex') +} + +/** Clear all cached RBAC access checks. Used for test isolation. */ +export function resetAccessCache() { + for (const key in accessCache) { + delete accessCache[key] + } + for (const key in subjectRulesCache) { + delete subjectRulesCache[key] + } + for (const key in kindGetAccessCache) { + delete kindGetAccessCache[key] + } +} + +export function getAccessCache() { + return accessCache +} + +function enforceAccessCacheEntryCap(tokenCache: Record>) { + const keys = Object.keys(tokenCache) + if (keys.length <= ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN) return + keys.sort((a, b) => tokenCache[a].time - tokenCache[b].time) + const toRemove = keys.length - ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN + for (let i = 0; i < toRemove; i++) { + delete tokenCache[keys[i]] + } +} + +function expireTimedEntries(cache: Record, cutoffTime: number) { + for (const key in cache) { + if (cache[key].time < cutoffTime) { + delete cache[key] + } + } +} + +/** Prune one token's SSAR entries; returns newest remaining time, or undefined if the token was removed. */ +function pruneAccessCacheToken( + token: string, + tokenCache: Record>, + cutoffTime: number +): number | undefined { + let newestTime = 0 + + for (const key in tokenCache) { + if (tokenCache[key].time < cutoffTime) { + delete tokenCache[key] + } else if (tokenCache[key].time > newestTime) { + newestTime = tokenCache[key].time + } + } + + if (Object.keys(tokenCache).length === 0) { + delete accessCache[token] + return undefined + } + + enforceAccessCacheEntryCap(tokenCache) + return newestTime +} + +export function cleanupAccessCache() { + const cutoffTime = Date.now() - ACCESS_CACHE_TTL + const tokenStats: Array<{ token: string; newestTime: number }> = [] + + for (const token in accessCache) { + const newestTime = pruneAccessCacheToken(token, accessCache[token], cutoffTime) + if (newestTime !== undefined) { + tokenStats.push({ token, newestTime }) + } + } + + expireTimedEntries(subjectRulesCache, cutoffTime) + expireTimedEntries(kindGetAccessCache, cutoffTime) + + if (tokenStats.length <= ACCESS_CACHE_MAX_TOKENS) return + + tokenStats.sort((a, b) => a.newestTime - b.newestTime) + const tokensToRemove = tokenStats.length - ACCESS_CACHE_MAX_TOKENS + for (let i = 0; i < tokensToRemove; i++) { + delete accessCache[tokenStats[i].token] + } +} + +export function startAccessCacheCleanup() { + if (accessCacheCleanupTimer) return + + accessCacheCleanupTimer = setInterval(() => { + try { + cleanupAccessCache() + } catch (err: unknown) { + logger.error({ msg: 'accessCache cleanup failed', error: err }) + } + }, ACCESS_CACHE_CLEANUP_INTERVAL) + + accessCacheCleanupTimer.unref() + logger.info({ msg: 'accessCache cleanup started', interval: ACCESS_CACHE_CLEANUP_INTERVAL }) +} + +export function stopAccessCacheCleanup() { + if (accessCacheCleanupTimer) { + clearInterval(accessCacheCleanupTimer) + accessCacheCleanupTimer = undefined + logger.info({ msg: 'accessCache cleanup stopped' }) + } +} + +export function getSsarCacheEntry(tokenKey: string, key: string): TimedCacheEntry | undefined { + const existing = accessCache[tokenKey]?.[key] + if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { + return existing + } + return undefined +} + +export function setSsarCacheEntry(tokenKey: string, key: string, entry: TimedCacheEntry) { + if (!accessCache[tokenKey]) accessCache[tokenKey] = {} + accessCache[tokenKey][key] = entry + enforceAccessCacheEntryCap(accessCache[tokenKey]) +} + +export function replaceSsarCachePromise( + tokenKey: string, + key: string, + inFlightPromise: Promise, + allowed: boolean +) { + const entry = accessCache[tokenKey]?.[key] + if (entry?.promise === inFlightPromise) { + entry.promise = Promise.resolve(allowed) + } +} + +export function getTimedCacheEntry( + cacheKey: string, + cache: Record> +): TimedCacheEntry | undefined { + const existing = cache[cacheKey] + if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { + return existing as TimedCacheEntry + } + return undefined +} + +export function setTimedCacheEntry( + cacheKey: string, + cache: Record>, + entry: TimedCacheEntry +) { + cache[cacheKey] = entry +} + +export function deleteTimedCacheEntry(cacheKey: string, cache: Record>) { + delete cache[cacheKey] +} + +export function getSubjectRulesCacheStore() { + return subjectRulesCache +} + +export function getKindGetAccessCacheStore() { + return kindGetAccessCache +} diff --git a/backend/src/routes/eventsDefinitions.ts b/backend/src/routes/eventsDefinitions.ts new file mode 100644 index 00000000000..5ddc23eb2e4 --- /dev/null +++ b/backend/src/routes/eventsDefinitions.ts @@ -0,0 +1,107 @@ +/* Copyright Contributors to the Open Cluster Management project */ + +import type { IWatchOptions } from '../resources/watch-options' + +export const definitions: IWatchOptions[] = [ + { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1', clusterScoped: true }, + { kind: 'ManagedClusterAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, + { kind: 'Agent', apiVersion: 'agent-install.openshift.io/v1beta1' }, + { kind: 'AgentServiceConfig', apiVersion: 'agent-install.openshift.io/v1beta1', clusterScoped: true }, + { kind: 'InfraEnv', apiVersion: 'agent-install.openshift.io/v1beta1' }, + { kind: 'NMStateConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, + { kind: 'Application', apiVersion: 'app.k8s.io/v1beta1' }, + { kind: 'Channel', apiVersion: 'apps.open-cluster-management.io/v1' }, + { kind: 'GitOpsCluster', apiVersion: 'apps.open-cluster-management.io/v1beta1' }, + { kind: 'HelmRelease', apiVersion: 'apps.open-cluster-management.io/v1' }, + { kind: 'PlacementRule', apiVersion: 'apps.open-cluster-management.io/v1' }, + { kind: 'Subscription', apiVersion: 'apps.open-cluster-management.io/v1' }, + { kind: 'SubscriptionReport', apiVersion: 'apps.open-cluster-management.io/v1alpha1' }, + { kind: 'Application', apiVersion: 'argoproj.io/v1alpha1', isPolled: true }, + { kind: 'ApplicationSet', apiVersion: 'argoproj.io/v1alpha1', isPolled: true }, + { kind: 'ArgoCD', apiVersion: 'argoproj.io/v1alpha1' }, + { kind: 'Infrastructure', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, + { + kind: 'CertificateSigningRequest', + apiVersion: 'certificates.k8s.io/v1', + labelSelector: { 'open-cluster-management.io/cluster-name': '' }, + clusterScoped: true, + }, + { kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1', clusterScoped: true }, + { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, + { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, + { kind: 'ManagedClusterSetBinding', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, + { kind: 'ManagedClusterSet', apiVersion: 'cluster.open-cluster-management.io/v1beta2', clusterScoped: true }, + { kind: 'ClusterCurator', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, + { kind: 'Subscription', apiVersion: 'operators.coreos.com/v1alpha1' }, + { kind: 'DiscoveredCluster', apiVersion: 'discovery.open-cluster-management.io/v1' }, + { kind: 'DiscoveryConfig', apiVersion: 'discovery.open-cluster-management.io/v1' }, + { kind: 'AgentClusterInstall', apiVersion: 'extensions.hive.openshift.io/v1beta1' }, + { kind: 'ClusterClaim', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ClusterDeployment', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ClusterImageSet', apiVersion: 'hive.openshift.io/v1', clusterScoped: true }, + { kind: 'ClusterPool', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ClusterProvision', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'MachinePool', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ManagedClusterInfo', apiVersion: 'internal.open-cluster-management.io/v1beta1' }, + { kind: 'BareMetalHost', apiVersion: 'metal3.io/v1alpha1' }, + { kind: 'MultiClusterEngine', apiVersion: 'multicluster.openshift.io/v1', clusterScoped: true }, + { kind: 'ClusterVersion', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, + { kind: 'StorageClass', apiVersion: 'storage.k8s.io/v1', clusterScoped: true }, + { kind: 'PlacementBinding', apiVersion: 'policy.open-cluster-management.io/v1' }, + { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1' }, + { kind: 'PolicyAutomation', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, + { kind: 'PolicySet', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, + { kind: 'SubmarinerConfig', apiVersion: 'submarineraddon.open-cluster-management.io/v1alpha1' }, + { kind: 'AnsibleJob', apiVersion: 'tower.ansible.com/v1alpha1' }, + { + kind: 'ConfigMap', + apiVersion: 'v1', + fieldSelector: { 'metadata.name': 'assisted-service' }, + }, + { + kind: 'ConfigMap', + apiVersion: 'v1', + fieldSelector: { 'metadata.namespace': 'openshift-config-managed', 'metadata.name': 'console-public' }, + }, + { kind: 'ConfigMap', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'console-search-config' } }, + { kind: 'Namespace', apiVersion: 'v1', clusterScoped: true }, + { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/credentials': '' } }, + // **Need to look for creds with: 'cluster.open-cluster-management.io/type': 'ans', for edit scenarios + { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/type': 'ans' } }, + { kind: 'Secret', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'auto-import-secret' } }, + { kind: 'PolicyReport', apiVersion: 'wgpolicyk8s.io/v1alpha2' }, + { kind: 'HostedCluster', apiVersion: 'hypershift.openshift.io/v1beta1' }, + { kind: 'NodePool', apiVersion: 'hypershift.openshift.io/v1beta1' }, + { kind: 'AgentMachine', apiVersion: 'capi-provider.agent-install.openshift.io/v1alpha1' }, + { kind: 'ConfigMap', apiVersion: 'v1', labelSelector: { 'hypershift.openshift.io/supported-versions': 'true' } }, + { kind: 'Search', apiVersion: 'search.open-cluster-management.io/v1alpha1' }, + // Configmaps that contain Grafana dashboard IDs + { + kind: 'ConfigMap', + apiVersion: 'v1', + fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-clusters-overview' }, + }, + { + kind: 'ConfigMap', + apiVersion: 'v1', + fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-single-vm-view' }, + }, + { kind: 'MulticlusterRoleAssignment', apiVersion: 'rbac.open-cluster-management.io/v1beta1' }, + { kind: 'User', apiVersion: 'user.openshift.io/v1', clusterScoped: true }, + { kind: 'Group', apiVersion: 'user.openshift.io/v1', clusterScoped: true }, + { + kind: 'ClusterRole', + apiVersion: 'rbac.authorization.k8s.io/v1', + labelSelector: { 'rbac.open-cluster-management.io/filter': 'vm-clusterroles' }, + clusterScoped: true, + }, + { + kind: 'Service', + apiVersion: 'v1', + fieldSelector: { 'metadata.name': 'cluster-proxy-addon-user', 'metadata.namespace': 'multicluster-engine' }, + }, +] + +export const CLUSTER_SCOPED_KINDS = new Set( + definitions.filter((definition) => definition.clusterScoped).map((definition) => definition.kind) +) From 457f510c9907e72902732c1619987d3f778ea1b1 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 15 Sep 2026 00:33:55 +0200 Subject: [PATCH 2/8] ACM-44888: add SSE RBAC unit tests for release-2.16 Cover access cache, SelfSubjectRulesReview short-circuit, cluster-scoped routing, and filter-before-inflate behavior. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- backend/test/lib/server-side-events.test.ts | 143 ++++ backend/test/routes/events.test.ts | 281 +++++--- backend/test/routes/eventsAccess.test.ts | 696 ++++++++++++++++++++ backend/test/routes/eventsCache.test.ts | 250 +++++++ 4 files changed, 1278 insertions(+), 92 deletions(-) create mode 100644 backend/test/lib/server-side-events.test.ts create mode 100644 backend/test/routes/eventsAccess.test.ts create mode 100644 backend/test/routes/eventsCache.test.ts diff --git a/backend/test/lib/server-side-events.test.ts b/backend/test/lib/server-side-events.test.ts new file mode 100644 index 00000000000..794928bcaca --- /dev/null +++ b/backend/test/lib/server-side-events.test.ts @@ -0,0 +1,143 @@ +/* Copyright Contributors to the Open Cluster Management project */ + +import { Writable } from 'node:stream' +import * as compression from '../../src/lib/compression' +import { getEventResourceMeta, ServerSideEvents, type ServerSideEvent } from '../../src/lib/server-side-events' + +describe('getEventResourceMeta', () => { + it('prefers meta over object when both are present', () => { + const event: ServerSideEvent = { + data: { + type: 'MODIFIED', + meta: { + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + name: 'from-meta', + }, + object: { + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + metadata: { name: 'from-object', namespace: '', resourceVersion: '1' }, + }, + }, + } + expect(getEventResourceMeta(event)?.name).toBe('from-meta') + }) + + it('reads identity from inflated object when meta is missing', () => { + const event: ServerSideEvent = { + data: { + type: 'MODIFIED', + object: { + kind: 'Policy', + apiVersion: 'policy.open-cluster-management.io/v1', + metadata: { name: 'p1', namespace: 'ns1', resourceVersion: '1' }, + }, + }, + } + expect(getEventResourceMeta(event)).toEqual({ + kind: 'Policy', + apiVersion: 'policy.open-cluster-management.io/v1', + name: 'p1', + namespace: 'ns1', + }) + }) + + it('returns undefined for compressed object without meta', () => { + const event: ServerSideEvent = { + data: { + type: 'MODIFIED', + object: Buffer.from('compressed'), + }, + } + expect(getEventResourceMeta(event)).toBeUndefined() + }) +}) + +describe('ServerSideEvents filter-before-inflate (ACM-39327)', () => { + beforeEach(() => { + ServerSideEvents.reset() + jest.restoreAllMocks() + }) + + afterEach(() => { + ServerSideEvents.eventFilter = undefined + ServerSideEvents.reset() + }) + + it('does not inflate denied events', async () => { + const inflateSpy = jest.spyOn(compression, 'inflateEvent') + ServerSideEvents.eventFilter = () => Promise.resolve(false) + + const writableStream = new Writable({ + write(_chunk, _encoding, callback) { + callback() + }, + }) + const clients = ServerSideEvents.getClients() + clients['deny-client'] = { + token: 'token', + writableStream, + compressionStream: undefined, + eventQueue: [], + } + + await ServerSideEvents.pushEvent({ + data: { + type: 'MODIFIED', + meta: { + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + name: 'cluster-1', + }, + object: Buffer.from('should-not-inflate'), + }, + }) + + // Allow the async eventQueue filter/processClient work to settle. + await new Promise((resolve) => setImmediate(resolve)) + await new Promise((resolve) => setImmediate(resolve)) + + expect(inflateSpy).not.toHaveBeenCalled() + }) + + it('inflates events only after the filter allows them', async () => { + const inflateSpy = jest.spyOn(compression, 'inflateEvent').mockImplementation((event) => Promise.resolve(event)) + ServerSideEvents.eventFilter = () => Promise.resolve(true) + + const writableStream = new Writable({ + write(_chunk, _encoding, callback) { + callback() + }, + }) + const clients = ServerSideEvents.getClients() + clients['allow-client'] = { + token: 'token', + writableStream, + compressionStream: undefined, + eventQueue: [], + } + + const event: ServerSideEvent = { + data: { + type: 'MODIFIED', + meta: { + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + name: 'cluster-1', + }, + object: { + kind: 'ManagedCluster', + apiVersion: 'v1', + metadata: { name: 'cluster-1', namespace: '', resourceVersion: '1' }, + }, + }, + } + await ServerSideEvents.pushEvent(event) + + await new Promise((resolve) => setImmediate(resolve)) + await new Promise((resolve) => setImmediate(resolve)) + + expect(inflateSpy).toHaveBeenCalled() + }) +}) diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts index c153c925420..f02d20614ad 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/events.test.ts @@ -9,17 +9,12 @@ import { getHubClusterName, getIsHubSelfManaged, getIsObservabilityInstalled, + resetIsObservabilityInstalled, createSplitStream, errorToString, createWatchEventProcessor, listAndWatch, stopWatching, - canAccess, - resetAccessCache, - getAccessCache, - cleanupAccessCache, - ACCESS_CACHE_TTL, - ACCESS_CACHE_MAX_TOKENS, } from '../../src/routes/events' import * as serviceAccountTokenModule from '../../src/lib/serviceAccountToken' import type { IArgoApplication, IResource } from '../../src/resources/resource' @@ -167,6 +162,8 @@ describe('events Route', () => { delete events[key] } } + + resetIsObservabilityInstalled() }) it('should cache a new resource', async () => { @@ -301,6 +298,23 @@ describe('events Route', () => { expect(getIsObservabilityInstalled()).toBe(true) }) + it('should set observability flag when caching multicluster-observability-addon', async () => { + const observabilityAddon: IResource = { + kind: 'ManagedClusterAddOn', + apiVersion: 'addon.open-cluster-management.io/v1alpha1', + metadata: { + name: 'multicluster-observability-addon', + namespace: 'local-cluster', + uid: 'mco-addon-uid', + resourceVersion: '1', + }, + } + + await cacheResource(observabilityAddon) + + expect(getIsObservabilityInstalled()).toBe(true) + }) + it('should not set observability flag for other addons', async () => { const otherAddon: IResource = { kind: 'ManagedClusterAddOn', @@ -313,10 +327,26 @@ describe('events Route', () => { }, } - const initialObsFlag = getIsObservabilityInstalled() await cacheResource(otherAddon) - expect(getIsObservabilityInstalled()).toBe(initialObsFlag) + expect(getIsObservabilityInstalled()).toBe(false) + }) + + it('should not set observability flag for addon with wrong API group', async () => { + const wrongGroupAddon: IResource = { + kind: 'ManagedClusterAddOn', + apiVersion: 'other.group.io/v1alpha1', + metadata: { + name: 'observability-controller', + namespace: 'local-cluster', + uid: 'wrong-group-addon-uid', + resourceVersion: '1', + }, + } + + await cacheResource(wrongGroupAddon) + + expect(getIsObservabilityInstalled()).toBe(false) }) it('should avoid race condition when caching same resource concurrently', async () => { @@ -541,6 +571,157 @@ describe('events Route', () => { }) }) + describe('forwardEventsToClients', () => { + beforeEach(async () => { + const cache = getEventCache() + for (const key in cache) { + delete cache[key] + } + ServerSideEvents.reset() + // Drain microtask queue so stale promises from prior tests resolve + for (let i = 0; i < 5; i++) { + await new Promise((resolve) => setTimeout(resolve, 0)) + } + ServerSideEvents.reset() + }) + + it('should not push SSE events when forwardEventsToClients is false', async () => { + const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') + + const resource: IResource = { + kind: 'Authentication', + apiVersion: 'config.openshift.io/v1', + metadata: { + name: 'cluster', + uid: 'auth-uid-1', + resourceVersion: '1', + }, + } + + await cacheResource(resource, false) + + expect(pushSpy).not.toHaveBeenCalled() + + const cache = getEventCache() + const entry = cache['/config.openshift.io/v1/authentications']?.['auth-uid-1'] + expect(entry).toBeDefined() + expect(await entry.compressed).toBeDefined() + expect(await entry.eventID).toBe(-1) + + const resources = await getKubeResources('Authentication', 'config.openshift.io/v1') + expect(resources).toHaveLength(1) + expect(resources[0].metadata.name).toBe('cluster') + + pushSpy.mockRestore() + }) + + it('should still push SSE events when forwardEventsToClients is true (default)', async () => { + const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') + + const resource: IResource = { + kind: 'ConfigMap', + apiVersion: 'v1', + metadata: { + name: 'test-cm', + uid: 'cm-forward-uid', + resourceVersion: '1', + }, + } + + await cacheResource(resource, true) + const cache = getEventCache() + await cache['/v1/configmaps']['cm-forward-uid'].eventID + + expect(pushSpy).toHaveBeenCalled() + + pushSpy.mockRestore() + }) + + it('should not push SSE events for delete when forwardEventsToClients is false', async () => { + await cacheResource( + { + kind: 'Authentication', + apiVersion: 'config.openshift.io/v1', + metadata: { name: 'cluster', uid: 'auth-del-uid', resourceVersion: '1' }, + }, + false + ) + + const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') + + const options = { kind: 'Authentication', apiVersion: 'config.openshift.io/v1', forwardEventsToClients: false } + const resourceVersionRef = { value: '1' } + const processor = createWatchEventProcessor(options, 'http://test/url', resourceVersionRef) + + const watchEvent = { + type: 'DELETED', + object: { + kind: 'Authentication', + apiVersion: 'config.openshift.io/v1', + metadata: { name: 'cluster', namespace: '', uid: 'auth-del-uid', resourceVersion: '2' }, + }, + } + + processor.write(JSON.stringify(watchEvent)) + processor.end() + await new Promise((resolve) => setTimeout(resolve, 50)) + + expect(pushSpy).not.toHaveBeenCalled() + + const cache = getEventCache() + expect(cache['/config.openshift.io/v1/authentications']?.['auth-del-uid']).toBeUndefined() + + pushSpy.mockRestore() + }) + + it('should not push SSE events via watch processor when forwardEventsToClients is false', async () => { + const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') + + const options = { kind: 'Authentication', apiVersion: 'config.openshift.io/v1', forwardEventsToClients: false } + const resourceVersionRef = { value: '0' } + const processor = createWatchEventProcessor(options, 'http://test/url', resourceVersionRef) + + const watchEvent = { + type: 'ADDED', + object: { + kind: 'Authentication', + apiVersion: 'config.openshift.io/v1', + metadata: { name: 'cluster', namespace: '', uid: 'auth-watch-uid', resourceVersion: '10' }, + }, + } + + processor.write(JSON.stringify(watchEvent)) + processor.end() + await new Promise((resolve) => setTimeout(resolve, 50)) + + expect(pushSpy).not.toHaveBeenCalled() + expect(resourceVersionRef.value).toBe('10') + + const cache = getEventCache() + expect(cache['/config.openshift.io/v1/authentications']?.['auth-watch-uid']).toBeDefined() + + pushSpy.mockRestore() + }) + + it('should still run kind-specific side effects when forwardEventsToClients is false', async () => { + const localCluster: IResource = { + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + metadata: { + name: 'my-hub', + uid: 'hub-no-forward-uid', + resourceVersion: '1', + labels: { 'local-cluster': 'true' }, + }, + } + + await cacheResource(localCluster, false) + + expect(getHubClusterName()).toBe('my-hub') + expect(getIsHubSelfManaged()).toBe(true) + }) + }) + describe('getEventCache', () => { it('should return the resource cache object', () => { const cache = getEventCache() @@ -1224,88 +1405,4 @@ describe('events Route', () => { expect(listCallCount).toBe(1) // Only counting second list call }) }) - - describe('Access Cache Cleanup', () => { - beforeEach(() => { - resetAccessCache() - jest.clearAllMocks() - process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' - }) - - afterEach(() => { - resetAccessCache() - delete process.env.CLUSTER_API_URL - nock.cleanAll() - }) - - it('should cache RBAC access check results', async () => { - const mockToken = 'test-token-123' - const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } - - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') - .reply(200, { status: { allowed: true } }) - - const result1 = await canAccess(resource, 'get', mockToken) - const result2 = await canAccess(resource, 'get', mockToken) - - expect(result1).toBe(true) - expect(result1).toBe(result2) - }) - - it('should respect TTL and refetch after expiry', async () => { - const cache = getAccessCache() - const mockToken = 'test-token-ttl' - - cache[mockToken] = { - 'Secret:default:credentials': { time: Date.now() - ACCESS_CACHE_TTL - 1000, promise: Promise.resolve(true) }, - } - - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') - .reply(200, { status: { allowed: false } }) - - const result = await canAccess( - { kind: 'Secret', apiVersion: 'v1', metadata: { namespace: 'default', name: 'credentials' } }, - 'get', - mockToken - ) - expect(result).toBe(false) - }) - - it('should remove stale cache entries during cleanup', () => { - const cache = getAccessCache() - const now = Date.now() - - cache['token1'] = { - stale: { time: now - ACCESS_CACHE_TTL - 1000, promise: Promise.resolve(true) }, - fresh: { time: now - 30000, promise: Promise.resolve(true) }, - } - cache['token2'] = { 'stale-only': { time: now - ACCESS_CACHE_TTL - 5000, promise: Promise.resolve(false) } } - - cleanupAccessCache() - - expect(cache['token1']['stale']).toBeUndefined() - expect(cache['token1']['fresh']).toBeDefined() - expect(cache['token2']).toBeUndefined() - }) - - it('should enforce maximum token limit with LRU eviction', () => { - const cache = getAccessCache() - const now = Date.now() - const tokenCount = ACCESS_CACHE_MAX_TOKENS + 100 - - for (let i = 0; i < tokenCount; i++) { - cache[`token-${i}`] = { - 'Pod:default:test': { time: now - (i / tokenCount) * 50 * 1000, promise: Promise.resolve(true) }, - } - } - - cleanupAccessCache() - - expect(Object.keys(cache).length).toBe(ACCESS_CACHE_MAX_TOKENS) - expect(cache['token-0']).toBeDefined() - expect(cache[`token-${tokenCount - 1}`]).toBeUndefined() - }) - }) }) diff --git a/backend/test/routes/eventsAccess.test.ts b/backend/test/routes/eventsAccess.test.ts new file mode 100644 index 00000000000..e022e1845bb --- /dev/null +++ b/backend/test/routes/eventsAccess.test.ts @@ -0,0 +1,696 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import nock from 'nock' +import { + canAccess, + canGetResource, + canListClusterScopedKind, + canListNamespacedScopedKind, +} from '../../src/routes/eventsAccess' +import { ACCESS_CACHE_TTL, cleanupAccessCache, resetAccessCache } from '../../src/routes/eventsCache' + +describe('eventsAccess', () => { + const apiUrl = () => process.env.CLUSTER_API_URL || '' + const managedCluster = (name: string) => ({ + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + metadata: { name }, + }) + const secret = (namespace: string, name: string) => ({ + kind: 'Secret', + apiVersion: 'v1', + metadata: { namespace, name }, + }) + const managedClusterInfo = (cluster: string) => ({ + kind: 'ManagedClusterInfo', + apiVersion: 'internal.open-cluster-management.io/v1beta1', + metadata: { name: cluster, namespace: cluster }, + }) + + const emptyRules = { incomplete: false, resourceRules: [] as unknown[] } + const secretGetInNamespace = { + incomplete: false, + resourceRules: [{ verbs: ['get'], apiGroups: [''], resources: ['secrets'] }], + } + const clusterAdminRules = { + incomplete: false, + resourceRules: [{ verbs: ['*'], apiGroups: ['*'], resources: ['*'] }], + } + + function rulesReviewNamespace(body: unknown): string { + let parsed = body + if (typeof body === 'string') { + try { + parsed = JSON.parse(body) as unknown + } catch { + return '' + } + } + return (parsed as { spec?: { namespace?: string } })?.spec?.namespace || '' + } + + function nockRulesReview(replyFn: (namespace: string) => { incomplete?: boolean; resourceRules?: unknown[] }) { + return nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, (_uri: string, requestBody: unknown) => ({ status: replyFn(rulesReviewNamespace(requestBody)) })) + } + + function nockRulesReviewStatus( + replyFn: (namespace: string) => { + incomplete?: boolean + evaluationError?: string + resourceRules?: unknown[] + } + ) { + return nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, (_uri: string, requestBody: unknown) => ({ status: replyFn(rulesReviewNamespace(requestBody)) })) + } + + function parseSsarResourceAttributes(body: unknown) { + const parsed = + typeof body === 'string' + ? (JSON.parse(body) as { spec?: { resourceAttributes?: Record } }) + : body + return (parsed as { spec?: { resourceAttributes?: Record } })?.spec?.resourceAttributes + } + + function nockSsarGet(matcher: (attrs: Record) => boolean, allowed: boolean) { + return nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + const attrs = parseSsarResourceAttributes(body) + return attrs?.verb === 'get' && matcher(attrs ?? {}) + }) + .reply(200, { status: { allowed } }) + } + + const namedManagedClusterRule = (name: string) => ({ + incomplete: false, + resourceRules: [ + { + verbs: ['get'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + resourceNames: [name], + }, + ], + }) + + beforeEach(() => { + resetAccessCache() + process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' + }) + + afterEach(() => { + resetAccessCache() + delete process.env.CLUSTER_API_URL + nock.cleanAll() + }) + + describe('canListClusterScopedKind', () => { + it('should issue a cluster-scoped list SSAR', async () => { + const resource = managedCluster('cluster-1') + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + const parsed = + typeof body === 'string' + ? (JSON.parse(body) as { spec?: { resourceAttributes?: { verb?: string } } }) + : body + return ( + (parsed as { spec?: { resourceAttributes?: { verb?: string } } })?.spec?.resourceAttributes?.verb === 'list' + ) + }) + .reply(200, { status: { allowed: true } }) + + expect(await canListClusterScopedKind(resource, 'list-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + }) + + describe('canListNamespacedScopedKind', () => { + it('should return false when the resource has no namespace', async () => { + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canListNamespacedScopedKind(managedCluster('cluster-1'), 'list-token')).toBe(false) + expect(ssarScope.isDone()).toBe(false) + }) + + it('should issue a namespaced list SSAR', async () => { + const resource = managedClusterInfo('acm39327-mc-01') + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + const parsed = + typeof body === 'string' + ? (JSON.parse(body) as { spec?: { resourceAttributes?: { namespace?: string; verb?: string } } }) + : body + const attrs = (parsed as { spec?: { resourceAttributes?: { namespace?: string; verb?: string } } })?.spec + ?.resourceAttributes + return attrs?.namespace === 'acm39327-mc-01' && attrs?.verb === 'list' + }) + .reply(200, { status: { allowed: true } }) + + expect(await canListNamespacedScopedKind(resource, 'list-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + }) + + /** + * ACM-39327: restricted users must not trigger O(N) SelfSubjectAccessReviews when the SSE + * filter falls through after cluster-scoped list is denied. SelfSubjectRulesReview is + * namespaced: cache one review per token+namespace, never treat `default` as global allow. + */ + describe('SelfSubjectRulesReview short-circuit (ACM-39327)', () => { + it('should deny all gets from complete empty rules without per-object SSAR', async () => { + nockRulesReview(() => emptyRules) + + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'none-user-token')).toBe(false) + expect(ssarScope.isDone()).toBe(false) + }) + + it('should use a single rules review for many cluster-scoped gets of the same kind', async () => { + let rulesCalls = 0 + nockRulesReview(() => { + rulesCalls++ + return emptyRules + }) + + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .times(1) + .reply(200, { status: { allowed: true } }) + + const results = await Promise.all( + Array.from({ length: 500 }, (_, i) => canGetResource(managedCluster(`cluster-${i}`), 'scale-none-token')) + ) + + expect(results.every((allowed) => allowed === false)).toBe(true) + expect(rulesCalls).toBe(1) + expect(ssarScope.isDone()).toBe(false) + }) + + it('should use a single rules review for many namespaced gets in the same namespace', async () => { + let rulesCalls = 0 + nockRulesReview(() => { + rulesCalls++ + return emptyRules + }) + + const results = await Promise.all( + Array.from({ length: 200 }, (_, i) => + canGetResource( + { + kind: 'ManagedClusterInfo', + apiVersion: 'internal.open-cluster-management.io/v1beta1', + metadata: { name: `info-${i}`, namespace: 'acm39327-mc-01' }, + }, + 'same-ns-none-token' + ) + ) + ) + + expect(results.every((allowed) => allowed === false)).toBe(true) + expect(rulesCalls).toBe(1) + }) + + it('should issue one rules review per namespace for a none user, without per-object SSAR', async () => { + const namespaces = new Set() + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + namespaces.add(rulesReviewNamespace(body)) + return true + }) + .times(50) + .reply(200, { status: emptyRules }) + + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + const results = await Promise.all( + Array.from({ length: 50 }, (_, i) => + canGetResource(managedClusterInfo(`cluster-${i}`), 'namespaced-none-token') + ) + ) + + expect(results.every((allowed) => allowed === false)).toBe(true) + expect(namespaces.size).toBe(50) + expect(ssarScope.isDone()).toBe(false) + }) + + it('must not treat get secrets in default as access to Credentials in other namespaces', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'default' + }) + .reply(200, { status: secretGetInNamespace }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) !== 'default' + }) + .times(2) + .reply(200, { status: emptyRules }) + + expect(await canGetResource(secret('default', 'default-cred'), 'user1-token')).toBe(true) + expect(await canGetResource(secret('kube-system', 'other-cred'), 'user1-token')).toBe(false) + expect(await canGetResource(secret('acm39327-mc-01', 'cluster-cred'), 'user1-token')).toBe(false) + }) + + it('should allow namespaced cluster resources when the user is admin in that cluster namespace', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'acm39327-mc-01' + }) + .reply(200, { status: clusterAdminRules }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) !== 'acm39327-mc-01' + }) + .times(2) + .reply(200, { status: emptyRules }) + + expect(await canGetResource(managedClusterInfo('acm39327-mc-01'), 'cluster-admin-token')).toBe(true) + expect(await canGetResource(managedClusterInfo('other-cluster'), 'cluster-admin-token')).toBe(false) + expect(await canGetResource(managedCluster('acm39327-mc-01'), 'cluster-admin-token')).toBe(false) + }) + + it('should confirm named cluster-scoped resources with SSAR before allowing access', async () => { + nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'allowed-cluster', + true + ) + + expect(await canGetResource(managedCluster('allowed-cluster'), 'partial-user-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should deny non-matching names from cluster-scoped allow-names without trusting SSRR alone', async () => { + nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'other-cluster', + false + ) + + expect(await canGetResource(managedCluster('other-cluster'), 'partial-user-token')).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should allow namespaced resources when rules grant unrestricted get/list/watch in that namespace', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [{ verbs: ['get', 'list', 'watch'], apiGroups: [''], resources: ['secrets'] }], + })) + + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canGetResource(secret('default', 'any-secret'), 'viewer-token')).toBe(true) + expect(ssarScope.isDone()).toBe(false) + }) + + it('should confirm cluster-scoped allow-all with SSAR so default RoleBindings are not treated as cluster access', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [ + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + }, + ], + })) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canGetResource(managedCluster('any-cluster'), 'default-role-token')).toBe(false) + }) + + it('should reuse kind access across API versions of the same group', async () => { + let rulesCalls = 0 + nockRulesReview(() => { + rulesCalls++ + return emptyRules + }) + + expect( + await canGetResource( + { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1beta1', metadata: { name: 'p1' } }, + 'version-token' + ) + ).toBe(false) + expect( + await canGetResource( + { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1alpha1', metadata: { name: 'p2' } }, + 'version-token' + ) + ).toBe(false) + expect(rulesCalls).toBe(1) + }) + + it('should fall back to SSAR when rules review is incomplete with non-empty rules', async () => { + nockRulesReview(() => ({ + incomplete: true, + resourceRules: [{ verbs: ['get'], apiGroups: [''], resources: ['pods'] }], + })) + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'incomplete-user-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should deny-all when rules are empty even if incomplete is true (OpenShift none user)', async () => { + nockRulesReview(() => ({ incomplete: true, resourceRules: [] })) + + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'openshift-none-token')).toBe(false) + expect(ssarScope.isDone()).toBe(false) + }) + + it('should fall back to SSAR when SelfSubjectRulesReview request fails', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(500, { message: 'internal error' }) + + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'ssrr-fail-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should expire subject rules and kind access caches during cleanup', async () => { + let rulesCalls = 0 + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .twice() + .reply(200, () => { + rulesCalls++ + return { status: { incomplete: false, resourceRules: [] } } + }) + + const start = 1_000_000 + const nowSpy = jest.spyOn(Date, 'now').mockReturnValue(start) + + await canGetResource(managedCluster('c1'), 'cache-expiry-token') + + nowSpy.mockReturnValue(start + ACCESS_CACHE_TTL + 1) + cleanupAccessCache() + + nowSpy.mockReturnValue(start + ACCESS_CACHE_TTL + 2) + await canGetResource(managedCluster('c2'), 'cache-expiry-token') + + nowSpy.mockRestore() + expect(rulesCalls).toBe(2) + }) + }) + + /** + * TDD: middle-ground security — SSRR deny-all short-circuit only; any non-deny cluster-scoped + * result must be confirmed with SSAR. Implementation pending in eventsAccess.ts. + */ + describe('cluster-scoped SSRR middle-ground security (TDD)', () => { + it('should deny allow-names from a default RoleBinding when SSAR get is false (Kevin)', async () => { + nockRulesReview(() => namedManagedClusterRule('acm39327-mc-02')) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'acm39327-mc-02', + false + ) + + expect(await canGetResource(managedCluster('acm39327-mc-02'), 'user1-token')).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should allow allow-names only when SSAR get confirms a real ClusterRoleBinding grant', async () => { + nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'allowed-cluster', + true + ) + + expect(await canGetResource(managedCluster('allowed-cluster'), 'clusterrole-user-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should confirm cluster-scoped allow-all with SSAR and deny when SSAR rejects', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [ + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + }, + ], + })) + const ssarScope = nockSsarGet( + (attrs) => attrs.group === 'cluster.open-cluster-management.io' && attrs.resource === 'managedclusters', + false + ) + + expect(await canGetResource(managedCluster('any-cluster'), 'default-role-token')).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should not trust allow-names on ManagedCluster when metadata.namespace is set without SSAR confirmation', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'default' + }) + .reply(200, { status: namedManagedClusterRule('acm39327-mc-02') }) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'acm39327-mc-02', + false + ) + + expect( + await canGetResource( + { + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + metadata: { name: 'acm39327-mc-02', namespace: 'default' }, + }, + 'user1-token' + ) + ).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should treat SSRR evaluationError as incomplete and confirm cluster-scoped access with SSAR', async () => { + nockRulesReviewStatus(() => ({ + incomplete: false, + evaluationError: 'webhook authorizer does not support user rule resolution', + resourceRules: [{ verbs: ['get'], apiGroups: [''], resources: ['pods'] }], + })) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'cluster-1', + true + ) + + expect(await canGetResource(managedCluster('cluster-1'), 'evaluation-error-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should confirm any non-deny-all cluster-scoped SSRR result with SSAR, not applyKindGetAccess alone', async () => { + nockRulesReview(() => ({ + incomplete: true, + resourceRules: [ + { + verbs: ['get'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + resourceNames: ['cluster-1'], + }, + ], + })) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'cluster-1', + false + ) + + expect(await canGetResource(managedCluster('cluster-1'), 'incomplete-named-token')).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + }) + + describe('canAccess', () => { + it('should post a SelfSubjectAccessReview for the requested verb and resource', async () => { + const resource = secret('default', 'test-secret') + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + const parsed = + typeof body === 'string' + ? (JSON.parse(body) as { spec?: { resourceAttributes?: { verb?: string; resource?: string } } }) + : body + const attrs = (parsed as { spec?: { resourceAttributes?: { verb?: string; resource?: string } } })?.spec + ?.resourceAttributes + return attrs?.verb === 'create' && attrs?.resource === 'secrets' + }) + .reply(200, { status: { allowed: true } }) + + expect(await canAccess(resource, 'create', 'create-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should log access checks when LOG_ACCESS is enabled', async () => { + const { logger } = await import('../../src/lib/logger') + const debugSpy = jest.spyOn(logger, 'debug').mockImplementation(() => {}) + process.env.LOG_ACCESS = 'true' + + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + await canAccess( + { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'logged-pod' } }, + 'get', + 'log-access-token' + ) + + expect(debugSpy).toHaveBeenCalledWith( + expect.objectContaining({ msg: 'access', allowed: true, verb: 'get', resource: 'pods' }) + ) + + debugSpy.mockRestore() + delete process.env.LOG_ACCESS + }) + + it('should not reuse SSAR results across API groups that share a kind name', async () => { + const appK8s = { + kind: 'Application', + apiVersion: 'app.k8s.io/v1beta1', + metadata: { namespace: 'ns', name: 'app' }, + } + const argoApp = { + kind: 'Application', + apiVersion: 'argoproj.io/v1alpha1', + metadata: { namespace: 'ns', name: 'app' }, + } + + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + return parseSsarResourceAttributes(body)?.group === 'app.k8s.io' + }) + .reply(200, { status: { allowed: true } }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + return parseSsarResourceAttributes(body)?.group === 'argoproj.io' + }) + .reply(200, { status: { allowed: false } }) + + expect(await canAccess(appK8s, 'get', 'group-collision-token')).toBe(true) + expect(await canAccess(argoApp, 'get', 'group-collision-token')).toBe(false) + }) + }) + + describe('namespaced vs cluster-scoped kind routing (ACM-39327)', () => { + const placement = (namespace: string, name: string) => ({ + kind: 'Placement', + apiVersion: 'cluster.open-cluster-management.io/v1beta1', + metadata: { namespace, name }, + }) + const storageClass = (name: string) => ({ + kind: 'StorageClass', + apiVersion: 'storage.k8s.io/v1', + metadata: { name }, + }) + const placementAllowAll = { + incomplete: false, + resourceRules: [ + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['placements'], + }, + ], + } + + it('must not treat Placement allow-all in default as access to other namespaces', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'default' + }) + .reply(200, { status: placementAllowAll }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'other-ns' + }) + .reply(200, { status: emptyRules }) + + expect(await canGetResource(placement('default', 'p-default'), 'placement-token')).toBe(true) + expect(await canGetResource(placement('other-ns', 'p-other'), 'placement-token')).toBe(false) + }) + + it('should confirm StorageClass cluster-scoped grants with SSAR', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [ + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['storage.k8s.io'], + resources: ['storageclasses'], + }, + ], + })) + const ssarScope = nockSsarGet( + (attrs) => attrs.group === 'storage.k8s.io' && attrs.resource === 'storageclasses', + true + ) + + expect(await canGetResource(storageClass('sc-1'), 'storage-class-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should retry SelfSubjectRulesReview after an unavailable review', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(500, { message: 'internal error' }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'ssrr-retry-token')).toBe(false) + + nock(apiUrl()).post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews').reply(200, { status: emptyRules }) + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-2'), 'ssrr-retry-token')).toBe(false) + expect(ssarScope.isDone()).toBe(false) + }) + }) +}) diff --git a/backend/test/routes/eventsCache.test.ts b/backend/test/routes/eventsCache.test.ts new file mode 100644 index 00000000000..3da9befc8a1 --- /dev/null +++ b/backend/test/routes/eventsCache.test.ts @@ -0,0 +1,250 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { createHash } from 'node:crypto' +import nock from 'nock' +import { canAccess } from '../../src/routes/eventsAccess' +import { + ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN, + ACCESS_CACHE_MAX_TOKENS, + ACCESS_CACHE_TTL, + cleanupAccessCache, + deleteTimedCacheEntry, + getAccessCache, + getKindGetAccessCacheStore, + getSsarCacheEntry, + getSubjectRulesCacheStore, + getTimedCacheEntry, + hashAccessToken, + replaceSsarCachePromise, + resetAccessCache, + setSsarCacheEntry, + setTimedCacheEntry, + startAccessCacheCleanup, + stopAccessCacheCleanup, +} from '../../src/routes/eventsCache' + +describe('eventsCache', () => { + beforeEach(() => { + resetAccessCache() + process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' + }) + + afterEach(() => { + resetAccessCache() + stopAccessCacheCleanup() + delete process.env.CLUSTER_API_URL + nock.cleanAll() + }) + + describe('hashAccessToken', () => { + it('should return a stable SHA-256 hex digest', () => { + const token = 'test-token-123' + const expected = createHash('sha256').update(token).digest('hex') + expect(hashAccessToken(token)).toBe(expected) + expect(hashAccessToken(token)).toBe(hashAccessToken(token)) + }) + }) + + describe('SSAR cache helpers', () => { + it('should return undefined for expired SSAR cache entries', () => { + const tokenKey = hashAccessToken('expired-token') + setSsarCacheEntry(tokenKey, 'get:Pod:default:pod', { + time: Date.now() - ACCESS_CACHE_TTL - 1000, + promise: Promise.resolve(true), + }) + expect(getSsarCacheEntry(tokenKey, 'get:Pod:default:pod')).toBeUndefined() + }) + + it('should replace in-flight SSAR promises with settled booleans', async () => { + const tokenKey = hashAccessToken('replace-token') + const key = 'get:Pod:default:pod' + const inFlight = Promise.resolve(true) + setSsarCacheEntry(tokenKey, key, { time: Date.now(), promise: inFlight }) + + replaceSsarCachePromise(tokenKey, key, inFlight, true) + const cached = getSsarCacheEntry(tokenKey, key) + if (!cached) throw new Error('expected SSAR cache entry') + expect(await cached.promise).toBe(true) + }) + + it('should enforce per-token entry cap when setting SSAR cache entries', () => { + const tokenKey = hashAccessToken('cap-token') + const now = Date.now() + for (let i = 0; i < ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN + 50; i++) { + setSsarCacheEntry(tokenKey, `get:Pod:default:pod-${i}`, { + time: now - i, + promise: Promise.resolve(false), + }) + } + cleanupAccessCache() + expect(Object.keys(getAccessCache()[tokenKey]).length).toBeLessThanOrEqual(ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN) + }) + }) + + describe('timed cache helpers', () => { + it('should get, set, and delete timed cache entries with TTL', async () => { + const store = getSubjectRulesCacheStore() + const cacheKey = 'token:default' + + setTimedCacheEntry(cacheKey, store, { time: Date.now(), promise: Promise.resolve('rules') }) + const cached = getTimedCacheEntry(cacheKey, store) + if (!cached) throw new Error('expected timed cache entry') + expect(await cached.promise).toBe('rules') + + setTimedCacheEntry(cacheKey, store, { + time: Date.now() - ACCESS_CACHE_TTL - 1000, + promise: Promise.resolve('stale'), + }) + expect(getTimedCacheEntry(cacheKey, store)).toBeUndefined() + + setTimedCacheEntry(cacheKey, store, { time: Date.now(), promise: Promise.resolve('fresh') }) + deleteTimedCacheEntry(cacheKey, store) + expect(getTimedCacheEntry(cacheKey, store)).toBeUndefined() + }) + }) + + describe('resetAccessCache', () => { + it('should clear SSAR, subject rules, and kind-get-access stores', () => { + const tokenKey = hashAccessToken('reset-token') + setSsarCacheEntry(tokenKey, 'get:Pod:default:pod', { time: Date.now(), promise: Promise.resolve(true) }) + setTimedCacheEntry('rules-key', getSubjectRulesCacheStore(), { + time: Date.now(), + promise: Promise.resolve({ incomplete: false, resourceRules: [] }), + }) + setTimedCacheEntry('kind-key', getKindGetAccessCacheStore(), { + time: Date.now(), + promise: Promise.resolve({ type: 'deny-all' }), + }) + + resetAccessCache() + + expect(getAccessCache()[tokenKey]).toBeUndefined() + expect(Object.keys(getSubjectRulesCacheStore()).length).toBe(0) + expect(Object.keys(getKindGetAccessCacheStore()).length).toBe(0) + }) + }) + + describe('cleanupAccessCache', () => { + it('should remove stale SSAR entries during cleanup', () => { + const cache = getAccessCache() + const now = Date.now() + + cache['token1'] = { + stale: { time: now - ACCESS_CACHE_TTL - 1000, promise: Promise.resolve(true) }, + fresh: { time: now - 30000, promise: Promise.resolve(true) }, + } + cache['token2'] = { 'stale-only': { time: now - ACCESS_CACHE_TTL - 5000, promise: Promise.resolve(false) } } + + cleanupAccessCache() + + expect(cache['token1']['stale']).toBeUndefined() + expect(cache['token1']['fresh']).toBeDefined() + expect(cache['token2']).toBeUndefined() + }) + + it('should enforce maximum token limit with LRU eviction', () => { + const cache = getAccessCache() + const now = Date.now() + const tokenCount = ACCESS_CACHE_MAX_TOKENS + 100 + + for (let i = 0; i < tokenCount; i++) { + cache[`token-${i}`] = { + 'Pod:default:test': { time: now - (i / tokenCount) * 50 * 1000, promise: Promise.resolve(true) }, + } + } + + cleanupAccessCache() + + expect(Object.keys(cache).length).toBe(ACCESS_CACHE_MAX_TOKENS) + expect(cache['token-0']).toBeDefined() + expect(cache[`token-${tokenCount - 1}`]).toBeUndefined() + }) + + it('should expire timed caches during cleanup', () => { + const subjectRules = getSubjectRulesCacheStore() + const kindGetAccess = getKindGetAccessCacheStore() + const staleTime = Date.now() - ACCESS_CACHE_TTL - 1000 + + setTimedCacheEntry('stale-rules', subjectRules, { time: staleTime, promise: Promise.resolve('old') }) + setTimedCacheEntry('stale-kind', kindGetAccess, { + time: staleTime, + promise: Promise.resolve({ type: 'deny-all' }), + }) + + cleanupAccessCache() + + expect(getTimedCacheEntry('stale-rules', subjectRules)).toBeUndefined() + expect(getTimedCacheEntry('stale-kind', kindGetAccess)).toBeUndefined() + }) + }) + + describe('startAccessCacheCleanup', () => { + it('should start and stop periodic cleanup without error', () => { + startAccessCacheCleanup() + startAccessCacheCleanup() + stopAccessCacheCleanup() + stopAccessCacheCleanup() + }) + }) + + describe('SSAR caching via canAccess', () => { + it('should cache RBAC access check results under hashed token keys', async () => { + const mockToken = 'test-token-123' + const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + const result1 = await canAccess(resource, 'get', mockToken) + const result2 = await canAccess(resource, 'get', mockToken) + + expect(result1).toBe(true) + expect(result1).toBe(result2) + expect(getAccessCache()[mockToken]).toBeUndefined() + expect(getAccessCache()[hashAccessToken(mockToken)]['get::Pod:default:test-pod']).toBeDefined() + }) + + it('should use distinct cache keys per verb', async () => { + const mockToken = 'test-token-verb' + const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canAccess(resource, 'get', mockToken)).toBe(true) + expect(await canAccess(resource, 'list', mockToken)).toBe(false) + + const tokenCache = getAccessCache()[hashAccessToken(mockToken)] + expect(tokenCache['get::Pod:default:test-pod']).toBeDefined() + expect(tokenCache['list::Pod:default:test-pod']).toBeDefined() + }) + + it('should respect TTL and refetch after expiry', async () => { + const cache = getAccessCache() + const mockToken = 'test-token-ttl' + const tokenKey = hashAccessToken(mockToken) + + cache[tokenKey] = { + 'get::Secret:default:credentials': { + time: Date.now() - ACCESS_CACHE_TTL - 1000, + promise: Promise.resolve(true), + }, + } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + const result = await canAccess( + { kind: 'Secret', apiVersion: 'v1', metadata: { namespace: 'default', name: 'credentials' } }, + 'get', + mockToken + ) + expect(result).toBe(false) + }) + }) +}) From 72266b574233de0292551badda7d860b9de2d9a8 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 16 Sep 2026 09:35:44 +0200 Subject: [PATCH 3/8] ACM-44888: drop forwardEventsToClients on release-2.16 That option was introduced in 2.17; 2.16 and older always broadcast cached watch events. Keep clusterScoped and SSE meta/filter-before-inflate only. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- backend/src/resources/watch-options.ts | 9 -- backend/src/routes/events.ts | 64 +++++------ backend/test/routes/events.test.ts | 151 ------------------------- 3 files changed, 28 insertions(+), 196 deletions(-) diff --git a/backend/src/resources/watch-options.ts b/backend/src/resources/watch-options.ts index 188c96e80fd..6cae5f25092 100644 --- a/backend/src/resources/watch-options.ts +++ b/backend/src/resources/watch-options.ts @@ -7,15 +7,6 @@ export interface IWatchOptions { // poll the resource list instead of watching it // process the items in its own cache so not to overload event cache isPolled?: boolean - /** - * Whether watch events should be forwarded to browser clients via SSE. - * When false, resources are still cached in the backend (available via - * `getKubeResources`) but no `ServerSideEvents.pushEvent` calls are made, - * avoiding per-client RBAC checks and SSE bandwidth for resources the - * frontend does not consume through the event stream. - * Defaults to true when omitted. - */ - forwardEventsToClients?: boolean /** * True when the Kubernetes resource is cluster-scoped. * Used by SSE RBAC to decide whether SelfSubjectRulesReview should probe `default` diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 0b4eb6c833a..6619e9c4d55 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -284,8 +284,7 @@ async function listKubernetesObjects(serviceAccountToken: string, options: IWatc return { size: itemCount } } - const forward = options.forwardEventsToClients !== false - await batchPromiseAll(items, (item) => cacheResource(item, forward)) + await batchPromiseAll(items, (item) => cacheResource(item)) // Remove items that are no longer in kubernetes const apiVersionPlural = apiVersionPluralFn(options) @@ -306,7 +305,7 @@ async function listKubernetesObjects(serviceAccountToken: string, options: IWatc removeResources.push(resource) } } - await batchPromiseAll(removeResources, (resource) => deleteResource(resource, forward)) + await batchPromiseAll(removeResources, (resource) => deleteResource(resource)) return { resourceVersion, size: items.length } } @@ -393,7 +392,6 @@ export function errorToString(err: unknown): string { * Creates a Transform stream that processes watch events with async operations */ export function createWatchEventProcessor(options: IWatchOptions, url: string, resourceVersionRef: { value: string }) { - const forward = options.forwardEventsToClients !== false return new Transform({ objectMode: true, async transform(data: string, _encoding, callback): Promise { @@ -415,7 +413,7 @@ export function createWatchEventProcessor(options: IWatchOptions, url: string, r case 'ADDED': case 'MODIFIED': try { - await cacheResource(watchEvent.object, forward) + await cacheResource(watchEvent.object) } catch (err: unknown) { logger.error({ msg: 'cacheResource failed', @@ -426,7 +424,7 @@ export function createWatchEventProcessor(options: IWatchOptions, url: string, r break case 'DELETED': try { - await deleteResource(watchEvent.object, forward) + await deleteResource(watchEvent.object) } catch (err: unknown) { logger.error({ msg: 'deleteResource failed', @@ -636,9 +634,7 @@ function resourceUrl(options: IWatchOptions, query: Record) { return url } -const NO_BROADCAST_EVENT_ID = Promise.resolve(-1) - -export async function cacheResource(resource: IResource, forwardEventsToClients = true) { +export async function cacheResource(resource: IResource) { const apiVersionPlural = apiVersionPluralFn(resource) let cache = resourceCache[apiVersionPlural] if (!cache) { @@ -674,13 +670,11 @@ export async function cacheResource(resource: IResource, forwardEventsToClients name: resource.metadata?.name, namespace: resource.metadata?.namespace, } - const eventID = forwardEventsToClients - ? compressed.then((compressed) => - ServerSideEvents.pushEvent({ - data: { type: 'MODIFIED', object: compressed, meta }, - }) - ) - : NO_BROADCAST_EVENT_ID + const eventID = compressed.then((compressed) => + ServerSideEvents.pushEvent({ + data: { type: 'MODIFIED', object: compressed, meta }, + }) + ) cache[uid] = { compressed, eventID } if (resource.kind === 'ManagedCluster') { @@ -700,7 +694,7 @@ export async function cacheResource(resource: IResource, forwardEventsToClients } } -async function deleteResource(resource: IResource, forwardEventsToClients = true) { +async function deleteResource(resource: IResource) { const apiVersionPlural = apiVersionPluralFn(resource) const cache = resourceCache[apiVersionPlural] if (!cache) return @@ -713,26 +707,24 @@ async function deleteResource(resource: IResource, forwardEventsToClients = true if (eventID > 0) ServerSideEvents.removeEvent(eventID) } - if (forwardEventsToClients) { - const deletedID = await ServerSideEvents.pushEvent({ - data: { - type: 'DELETED', - object: { - kind: resource.kind, - apiVersion: resource.apiVersion, - metadata: { name: resource.metadata.name, namespace: resource.metadata.namespace }, - }, - meta: { - kind: resource.kind, - apiVersion: resource.apiVersion, - name: resource.metadata.name, - namespace: resource.metadata.namespace, - }, + const deletedID = await ServerSideEvents.pushEvent({ + data: { + type: 'DELETED', + object: { + kind: resource.kind, + apiVersion: resource.apiVersion, + metadata: { name: resource.metadata.name, namespace: resource.metadata.namespace }, }, - }) - // after deletion has been broadcast to current clients, no need to retain - ServerSideEvents.removeEvent(deletedID) - } + meta: { + kind: resource.kind, + apiVersion: resource.apiVersion, + name: resource.metadata.name, + namespace: resource.metadata.namespace, + }, + }, + }) + // after deletion has been broadcast to current clients, no need to retain + ServerSideEvents.removeEvent(deletedID) delete cache[uid] } diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts index f02d20614ad..d34b3e567f8 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/events.test.ts @@ -571,157 +571,6 @@ describe('events Route', () => { }) }) - describe('forwardEventsToClients', () => { - beforeEach(async () => { - const cache = getEventCache() - for (const key in cache) { - delete cache[key] - } - ServerSideEvents.reset() - // Drain microtask queue so stale promises from prior tests resolve - for (let i = 0; i < 5; i++) { - await new Promise((resolve) => setTimeout(resolve, 0)) - } - ServerSideEvents.reset() - }) - - it('should not push SSE events when forwardEventsToClients is false', async () => { - const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') - - const resource: IResource = { - kind: 'Authentication', - apiVersion: 'config.openshift.io/v1', - metadata: { - name: 'cluster', - uid: 'auth-uid-1', - resourceVersion: '1', - }, - } - - await cacheResource(resource, false) - - expect(pushSpy).not.toHaveBeenCalled() - - const cache = getEventCache() - const entry = cache['/config.openshift.io/v1/authentications']?.['auth-uid-1'] - expect(entry).toBeDefined() - expect(await entry.compressed).toBeDefined() - expect(await entry.eventID).toBe(-1) - - const resources = await getKubeResources('Authentication', 'config.openshift.io/v1') - expect(resources).toHaveLength(1) - expect(resources[0].metadata.name).toBe('cluster') - - pushSpy.mockRestore() - }) - - it('should still push SSE events when forwardEventsToClients is true (default)', async () => { - const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') - - const resource: IResource = { - kind: 'ConfigMap', - apiVersion: 'v1', - metadata: { - name: 'test-cm', - uid: 'cm-forward-uid', - resourceVersion: '1', - }, - } - - await cacheResource(resource, true) - const cache = getEventCache() - await cache['/v1/configmaps']['cm-forward-uid'].eventID - - expect(pushSpy).toHaveBeenCalled() - - pushSpy.mockRestore() - }) - - it('should not push SSE events for delete when forwardEventsToClients is false', async () => { - await cacheResource( - { - kind: 'Authentication', - apiVersion: 'config.openshift.io/v1', - metadata: { name: 'cluster', uid: 'auth-del-uid', resourceVersion: '1' }, - }, - false - ) - - const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') - - const options = { kind: 'Authentication', apiVersion: 'config.openshift.io/v1', forwardEventsToClients: false } - const resourceVersionRef = { value: '1' } - const processor = createWatchEventProcessor(options, 'http://test/url', resourceVersionRef) - - const watchEvent = { - type: 'DELETED', - object: { - kind: 'Authentication', - apiVersion: 'config.openshift.io/v1', - metadata: { name: 'cluster', namespace: '', uid: 'auth-del-uid', resourceVersion: '2' }, - }, - } - - processor.write(JSON.stringify(watchEvent)) - processor.end() - await new Promise((resolve) => setTimeout(resolve, 50)) - - expect(pushSpy).not.toHaveBeenCalled() - - const cache = getEventCache() - expect(cache['/config.openshift.io/v1/authentications']?.['auth-del-uid']).toBeUndefined() - - pushSpy.mockRestore() - }) - - it('should not push SSE events via watch processor when forwardEventsToClients is false', async () => { - const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') - - const options = { kind: 'Authentication', apiVersion: 'config.openshift.io/v1', forwardEventsToClients: false } - const resourceVersionRef = { value: '0' } - const processor = createWatchEventProcessor(options, 'http://test/url', resourceVersionRef) - - const watchEvent = { - type: 'ADDED', - object: { - kind: 'Authentication', - apiVersion: 'config.openshift.io/v1', - metadata: { name: 'cluster', namespace: '', uid: 'auth-watch-uid', resourceVersion: '10' }, - }, - } - - processor.write(JSON.stringify(watchEvent)) - processor.end() - await new Promise((resolve) => setTimeout(resolve, 50)) - - expect(pushSpy).not.toHaveBeenCalled() - expect(resourceVersionRef.value).toBe('10') - - const cache = getEventCache() - expect(cache['/config.openshift.io/v1/authentications']?.['auth-watch-uid']).toBeDefined() - - pushSpy.mockRestore() - }) - - it('should still run kind-specific side effects when forwardEventsToClients is false', async () => { - const localCluster: IResource = { - kind: 'ManagedCluster', - apiVersion: 'cluster.open-cluster-management.io/v1', - metadata: { - name: 'my-hub', - uid: 'hub-no-forward-uid', - resourceVersion: '1', - labels: { 'local-cluster': 'true' }, - }, - } - - await cacheResource(localCluster, false) - - expect(getHubClusterName()).toBe('my-hub') - expect(getIsHubSelfManaged()).toBe(true) - }) - }) - describe('getEventCache', () => { it('should return the resource cache object', () => { const cache = getEventCache() From bd0934717495bfbbe787f4b420a59e6857b98acf Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 16 Sep 2026 10:04:05 +0200 Subject: [PATCH 4/8] ACM-44888: remove resetIsObservabilityInstalled test helper Drop the test-only reset export and align observability cache tests with the upstream initialObsFlag pattern. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- backend/src/routes/events.ts | 3 --- backend/test/routes/events.test.ts | 22 ++-------------------- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 6619e9c4d55..b4b3e6763b0 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -85,9 +85,6 @@ let isObservabilityInstalled: boolean = false export function getIsObservabilityInstalled() { return isObservabilityInstalled } -export function resetIsObservabilityInstalled() { - isObservabilityInstalled = false -} // because rbac checks are expensive, // run them only on the resources requested by the UI diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts index d34b3e567f8..1dbfb8b070f 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/events.test.ts @@ -9,7 +9,6 @@ import { getHubClusterName, getIsHubSelfManaged, getIsObservabilityInstalled, - resetIsObservabilityInstalled, createSplitStream, errorToString, createWatchEventProcessor, @@ -163,7 +162,6 @@ describe('events Route', () => { } } - resetIsObservabilityInstalled() }) it('should cache a new resource', async () => { @@ -327,26 +325,10 @@ describe('events Route', () => { }, } + const initialObsFlag = getIsObservabilityInstalled() await cacheResource(otherAddon) - expect(getIsObservabilityInstalled()).toBe(false) - }) - - it('should not set observability flag for addon with wrong API group', async () => { - const wrongGroupAddon: IResource = { - kind: 'ManagedClusterAddOn', - apiVersion: 'other.group.io/v1alpha1', - metadata: { - name: 'observability-controller', - namespace: 'local-cluster', - uid: 'wrong-group-addon-uid', - resourceVersion: '1', - }, - } - - await cacheResource(wrongGroupAddon) - - expect(getIsObservabilityInstalled()).toBe(false) + expect(getIsObservabilityInstalled()).toBe(initialObsFlag) }) it('should avoid race condition when caching same resource concurrently', async () => { From cb1698d97d9817b488faf3127f76e5af0d28e684 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 16 Sep 2026 13:28:14 +0200 Subject: [PATCH 5/8] eventsDefinitions reverted back to release-2.16 Signed-off-by: Enrique Mingorance Cano --- backend/src/routes/eventsDefinitions.ts | 26 ++++---- backend/test/routes/eventsAccess.test.ts | 75 ++++++++++-------------- 2 files changed, 44 insertions(+), 57 deletions(-) diff --git a/backend/src/routes/eventsDefinitions.ts b/backend/src/routes/eventsDefinitions.ts index 5ddc23eb2e4..7255d1200ec 100644 --- a/backend/src/routes/eventsDefinitions.ts +++ b/backend/src/routes/eventsDefinitions.ts @@ -3,10 +3,10 @@ import type { IWatchOptions } from '../resources/watch-options' export const definitions: IWatchOptions[] = [ - { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1', clusterScoped: true }, + { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, { kind: 'ManagedClusterAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, { kind: 'Agent', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'AgentServiceConfig', apiVersion: 'agent-install.openshift.io/v1beta1', clusterScoped: true }, + { kind: 'AgentServiceConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, { kind: 'InfraEnv', apiVersion: 'agent-install.openshift.io/v1beta1' }, { kind: 'NMStateConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, { kind: 'Application', apiVersion: 'app.k8s.io/v1beta1' }, @@ -19,18 +19,17 @@ export const definitions: IWatchOptions[] = [ { kind: 'Application', apiVersion: 'argoproj.io/v1alpha1', isPolled: true }, { kind: 'ApplicationSet', apiVersion: 'argoproj.io/v1alpha1', isPolled: true }, { kind: 'ArgoCD', apiVersion: 'argoproj.io/v1alpha1' }, - { kind: 'Infrastructure', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, + { kind: 'Infrastructure', apiVersion: 'config.openshift.io/v1' }, { kind: 'CertificateSigningRequest', apiVersion: 'certificates.k8s.io/v1', labelSelector: { 'open-cluster-management.io/cluster-name': '' }, - clusterScoped: true, }, - { kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1', clusterScoped: true }, + { kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1' }, { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, { kind: 'ManagedClusterSetBinding', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, - { kind: 'ManagedClusterSet', apiVersion: 'cluster.open-cluster-management.io/v1beta2', clusterScoped: true }, + { kind: 'ManagedClusterSet', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, { kind: 'ClusterCurator', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, { kind: 'Subscription', apiVersion: 'operators.coreos.com/v1alpha1' }, { kind: 'DiscoveredCluster', apiVersion: 'discovery.open-cluster-management.io/v1' }, @@ -38,15 +37,15 @@ export const definitions: IWatchOptions[] = [ { kind: 'AgentClusterInstall', apiVersion: 'extensions.hive.openshift.io/v1beta1' }, { kind: 'ClusterClaim', apiVersion: 'hive.openshift.io/v1' }, { kind: 'ClusterDeployment', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterImageSet', apiVersion: 'hive.openshift.io/v1', clusterScoped: true }, + { kind: 'ClusterImageSet', apiVersion: 'hive.openshift.io/v1' }, { kind: 'ClusterPool', apiVersion: 'hive.openshift.io/v1' }, { kind: 'ClusterProvision', apiVersion: 'hive.openshift.io/v1' }, { kind: 'MachinePool', apiVersion: 'hive.openshift.io/v1' }, { kind: 'ManagedClusterInfo', apiVersion: 'internal.open-cluster-management.io/v1beta1' }, { kind: 'BareMetalHost', apiVersion: 'metal3.io/v1alpha1' }, - { kind: 'MultiClusterEngine', apiVersion: 'multicluster.openshift.io/v1', clusterScoped: true }, - { kind: 'ClusterVersion', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, - { kind: 'StorageClass', apiVersion: 'storage.k8s.io/v1', clusterScoped: true }, + { kind: 'MultiClusterEngine', apiVersion: 'multicluster.openshift.io/v1' }, + { kind: 'ClusterVersion', apiVersion: 'config.openshift.io/v1' }, + { kind: 'StorageClass', apiVersion: 'storage.k8s.io/v1' }, { kind: 'PlacementBinding', apiVersion: 'policy.open-cluster-management.io/v1' }, { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1' }, { kind: 'PolicyAutomation', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, @@ -64,7 +63,7 @@ export const definitions: IWatchOptions[] = [ fieldSelector: { 'metadata.namespace': 'openshift-config-managed', 'metadata.name': 'console-public' }, }, { kind: 'ConfigMap', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'console-search-config' } }, - { kind: 'Namespace', apiVersion: 'v1', clusterScoped: true }, + { kind: 'Namespace', apiVersion: 'v1' }, { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/credentials': '' } }, // **Need to look for creds with: 'cluster.open-cluster-management.io/type': 'ans', for edit scenarios { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/type': 'ans' } }, @@ -87,13 +86,12 @@ export const definitions: IWatchOptions[] = [ fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-single-vm-view' }, }, { kind: 'MulticlusterRoleAssignment', apiVersion: 'rbac.open-cluster-management.io/v1beta1' }, - { kind: 'User', apiVersion: 'user.openshift.io/v1', clusterScoped: true }, - { kind: 'Group', apiVersion: 'user.openshift.io/v1', clusterScoped: true }, + { kind: 'User', apiVersion: 'user.openshift.io/v1' }, + { kind: 'Group', apiVersion: 'user.openshift.io/v1' }, { kind: 'ClusterRole', apiVersion: 'rbac.authorization.k8s.io/v1', labelSelector: { 'rbac.open-cluster-management.io/filter': 'vm-clusterroles' }, - clusterScoped: true, }, { kind: 'Service', diff --git a/backend/test/routes/eventsAccess.test.ts b/backend/test/routes/eventsAccess.test.ts index e022e1845bb..76a9f3aef8c 100644 --- a/backend/test/routes/eventsAccess.test.ts +++ b/backend/test/routes/eventsAccess.test.ts @@ -278,32 +278,24 @@ describe('eventsAccess', () => { expect(await canGetResource(managedCluster('acm39327-mc-01'), 'cluster-admin-token')).toBe(false) }) - it('should confirm named cluster-scoped resources with SSAR before allowing access', async () => { + it('should allow named resources from SSRR allow-names without SSAR', async () => { nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) - const ssarScope = nockSsarGet( - (attrs) => - attrs.group === 'cluster.open-cluster-management.io' && - attrs.resource === 'managedclusters' && - attrs.name === 'allowed-cluster', - true - ) + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) expect(await canGetResource(managedCluster('allowed-cluster'), 'partial-user-token')).toBe(true) - expect(ssarScope.isDone()).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) - it('should deny non-matching names from cluster-scoped allow-names without trusting SSRR alone', async () => { + it('should deny non-matching names from SSRR allow-names', async () => { nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) - const ssarScope = nockSsarGet( - (attrs) => - attrs.group === 'cluster.open-cluster-management.io' && - attrs.resource === 'managedclusters' && - attrs.name === 'other-cluster', - false - ) + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) expect(await canGetResource(managedCluster('other-cluster'), 'partial-user-token')).toBe(false) - expect(ssarScope.isDone()).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) it('should allow namespaced resources when rules grant unrestricted get/list/watch in that namespace', async () => { @@ -320,7 +312,7 @@ describe('eventsAccess', () => { expect(ssarScope.isDone()).toBe(false) }) - it('should confirm cluster-scoped allow-all with SSAR so default RoleBindings are not treated as cluster access', async () => { + it('should allow allow-all from SSRR in the probe namespace without SSAR', async () => { nockRulesReview(() => ({ incomplete: false, resourceRules: [ @@ -331,11 +323,12 @@ describe('eventsAccess', () => { }, ], })) - nock(apiUrl()) + const ssarScope = nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') .reply(200, { status: { allowed: false } }) - expect(await canGetResource(managedCluster('any-cluster'), 'default-role-token')).toBe(false) + expect(await canGetResource(managedCluster('any-cluster'), 'default-role-token')).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) it('should reuse kind access across API versions of the same group', async () => { @@ -423,12 +416,8 @@ describe('eventsAccess', () => { }) }) - /** - * TDD: middle-ground security — SSRR deny-all short-circuit only; any non-deny cluster-scoped - * result must be confirmed with SSAR. Implementation pending in eventsAccess.ts. - */ - describe('cluster-scoped SSRR middle-ground security (TDD)', () => { - it('should deny allow-names from a default RoleBinding when SSAR get is false (Kevin)', async () => { + describe('SSRR allow-all and allow-names without cluster-scoped SSAR confirmation', () => { + it('should trust allow-names from a default RoleBinding without SSAR (Kevin)', async () => { nockRulesReview(() => namedManagedClusterRule('acm39327-mc-02')) const ssarScope = nockSsarGet( (attrs) => @@ -438,11 +427,11 @@ describe('eventsAccess', () => { false ) - expect(await canGetResource(managedCluster('acm39327-mc-02'), 'user1-token')).toBe(false) - expect(ssarScope.isDone()).toBe(true) + expect(await canGetResource(managedCluster('acm39327-mc-02'), 'user1-token')).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) - it('should allow allow-names only when SSAR get confirms a real ClusterRoleBinding grant', async () => { + it('should allow allow-names from SSRR without SSAR confirmation', async () => { nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) const ssarScope = nockSsarGet( (attrs) => @@ -453,10 +442,10 @@ describe('eventsAccess', () => { ) expect(await canGetResource(managedCluster('allowed-cluster'), 'clusterrole-user-token')).toBe(true) - expect(ssarScope.isDone()).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) - it('should confirm cluster-scoped allow-all with SSAR and deny when SSAR rejects', async () => { + it('should allow allow-all from SSRR without SSAR confirmation', async () => { nockRulesReview(() => ({ incomplete: false, resourceRules: [ @@ -472,11 +461,11 @@ describe('eventsAccess', () => { false ) - expect(await canGetResource(managedCluster('any-cluster'), 'default-role-token')).toBe(false) - expect(ssarScope.isDone()).toBe(true) + expect(await canGetResource(managedCluster('any-cluster'), 'default-role-token')).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) - it('should not trust allow-names on ManagedCluster when metadata.namespace is set without SSAR confirmation', async () => { + it('should trust allow-names on ManagedCluster when metadata.namespace matches the SSRR namespace', async () => { nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { return rulesReviewNamespace(body) === 'default' @@ -499,11 +488,11 @@ describe('eventsAccess', () => { }, 'user1-token' ) - ).toBe(false) - expect(ssarScope.isDone()).toBe(true) + ).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) - it('should treat SSRR evaluationError as incomplete and confirm cluster-scoped access with SSAR', async () => { + it('should treat SSRR evaluationError as incomplete and confirm access with SSAR', async () => { nockRulesReviewStatus(() => ({ incomplete: false, evaluationError: 'webhook authorizer does not support user rule resolution', @@ -521,7 +510,7 @@ describe('eventsAccess', () => { expect(ssarScope.isDone()).toBe(true) }) - it('should confirm any non-deny-all cluster-scoped SSRR result with SSAR, not applyKindGetAccess alone', async () => { + it('should apply allow-names from incomplete SSRR without SSAR when names match', async () => { nockRulesReview(() => ({ incomplete: true, resourceRules: [ @@ -541,8 +530,8 @@ describe('eventsAccess', () => { false ) - expect(await canGetResource(managedCluster('cluster-1'), 'incomplete-named-token')).toBe(false) - expect(ssarScope.isDone()).toBe(true) + expect(await canGetResource(managedCluster('cluster-1'), 'incomplete-named-token')).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) }) @@ -654,7 +643,7 @@ describe('eventsAccess', () => { expect(await canGetResource(placement('other-ns', 'p-other'), 'placement-token')).toBe(false) }) - it('should confirm StorageClass cluster-scoped grants with SSAR', async () => { + it('should allow StorageClass grants from SSRR allow-all without SSAR', async () => { nockRulesReview(() => ({ incomplete: false, resourceRules: [ @@ -671,7 +660,7 @@ describe('eventsAccess', () => { ) expect(await canGetResource(storageClass('sc-1'), 'storage-class-token')).toBe(true) - expect(ssarScope.isDone()).toBe(true) + expect(ssarScope.isDone()).toBe(false) }) it('should retry SelfSubjectRulesReview after an unavailable review', async () => { From 85256469b7eddd227ec8d6463133ee0c826ac1a7 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 16 Sep 2026 13:29:53 +0200 Subject: [PATCH 6/8] PR #5863 removed from events.ts Signed-off-by: Enrique Mingorance Cano --- backend/src/routes/events.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index b4b3e6763b0..c61e5062890 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -653,7 +653,7 @@ export async function cacheResource(resource: IResource) { const latestExisting = cache[uid] if (latestExisting === existing) { // if no other cacheResource call updated the cache while we were awaiting, we can replace the cache entry and event - if (eventID > 0) ServerSideEvents.removeEvent(eventID) + ServerSideEvents.removeEvent(eventID) break } // if a deleteResource ran while we were awaiting, we will exit the loop because the resource is no longer existing @@ -699,10 +699,7 @@ async function deleteResource(resource: IResource) { const uid = resource.metadata.uid const existing = cache[uid] - if (existing) { - const eventID = await existing.eventID - if (eventID > 0) ServerSideEvents.removeEvent(eventID) - } + if (existing) ServerSideEvents.removeEvent(await existing.eventID) const deletedID = await ServerSideEvents.pushEvent({ data: { From 286bb038d65cf63ba53add7c16b9ad930f8ac0ed Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 16 Sep 2026 13:48:38 +0200 Subject: [PATCH 7/8] lint error fixed Signed-off-by: Enrique Mingorance Cano --- backend/test/routes/events.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts index 1dbfb8b070f..8c49c6d48d3 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/events.test.ts @@ -161,7 +161,6 @@ describe('events Route', () => { delete events[key] } } - }) it('should cache a new resource', async () => { From 0193a19f6e964b8f4eb99831e5a93855aa210bed Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 16 Sep 2026 14:27:50 +0200 Subject: [PATCH 8/8] references to #6609 reverted back from events.ts Signed-off-by: Enrique Mingorance Cano --- backend/src/routes/events.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index c61e5062890..4322f93248c 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -680,14 +680,10 @@ export async function cacheResource(resource: IResource) { isHubSelfManaged = true } } - - if ( - resource.kind === 'ManagedClusterAddOn' && - resource.apiVersion.startsWith('addon.open-cluster-management.io/') && - (resource.metadata?.name === 'observability-controller' || - resource.metadata?.name == 'multicluster-observability-addon') - ) { - isObservabilityInstalled = true + if (resource.kind === 'ManagedClusterAddOn') { + if (resource?.metadata?.name === 'observability-controller') { + isObservabilityInstalled = true + } } }