Skip to content

[GO MIGRATION BACKPORT] ACM-44885: Fix non-admin SSE OOM under large inventory for Go as a part of the ACM-42568 effort - #73

Open
Ginxo wants to merge 2 commits into
go/migrationfrom
go/migration_6638
Open

Ginxo wants to merge 2 commits into
go/migrationfrom
go/migration_6638

Conversation

@Ginxo

@Ginxo Ginxo commented Sep 23, 2026

Copy link
Copy Markdown
Owner

📝 Summary

Backports ACM-44885 / #6638 to the Go GET /events path as part of ACM-42568 / ACM-45223.

Depends on: #6842 (go/migration). This PR should merge into go/migration, not main directly.

Type of Change: 🐞 Bug Fix


Problem

Restricted (non-admin) users could drive the console backend into high memory use and CPU saturation on large inventories. After cluster-scoped list SSAR was denied, the Go hub fell through to O(N) per-object SelfSubjectAccessReviews (list namespaced → get) for every cached resource in the SSE snapshot/live stream.

Node fixed the same class of bug in stolostron#6638 with two ideas:

  1. RBAC short-circuit — one SelfSubjectRulesReview per token+namespace, then local deny-all / allow-all / allow-names decisions (with SSAR confirmation for cluster-scoped kinds).
  2. Filter before inflate — do not decompress cached resource blobs until RBAC allows the event.

Only (1) is ported here. That is the change that bounds non-admin inventory filtering and prevents the OOM.


What changed (Go)

  • SSARAccess.canSee: cluster list SSAR → SelfSubjectRulesReview → local kind decision; SSAR get only for incomplete fallback or cluster-scoped confirmation (RoleBinding in default cannot impersonate ManagedCluster access).
  • Caches: hashed token keys, SSRR per namespace, kind-access per namespace+group+resource, per-token SSAR cap (2000), 60s TTL.
  • WatchSpec.ClusterScoped: same cluster-scoped kinds as Node CLUSTER_SCOPED_KINDS (ManagedCluster, Namespace, StorageClass, …). Scope is not inferred from metadata.namespace.
  • Tests: Go parity with Node eventsAccess.test.ts (e.g. 500 ManagedCluster → 1 SSRR, 0 SSAR get).
  • Cleanup: remove orphaned Node TS from the ACM-44885: Fix non-admin SSE OOM under large inventory stolostron/console#6638 merge (backend/src/routes/events*.ts and tests) — not executed by the Go backend.
  • Docs: backend/AGENTS.md, docs/ARCHITECTURE.md.

Why no zlib resource cache or per-client inflate

The Node backend kept SSE resources in memory as dictionary-compressed zlib blobs (deflateResource / inflateEvent). Each connected client could force decompression to full JSON before RBAC filtering (the pre-stolostron#6638 OOM path). stolostron#6638 added lightweight meta and filter-before-inflate so denied events never expanded.

The Go backend has a different architecture; those Node mechanisms are not needed for parity:

Node (stolostron#6638) Go (this PR)
In-process resourceCache of zlib Buffers client-go informer cache — one unstructured copy per resource, shared across clients
RBAC needs meta or inflate to read identity Identity from Event.Object / GVR without a decompress step
inflateEvent per client before send writeFiltered → Allow → json.Marshal only if allowed — denied events are never serialized to the wire
zlib on stored resources gzip/deflate on the SSE HTTP stream (internal/events/hub/encode.go) — compression in transit, not in the event store

So:

  • There is no inflate step because nothing is stored as a compressed blob between cache and client.
  • There is no per-resource zlib cache because informers already hold the canonical object; duplicating Node’s dictionary+zlib layer would add CPU and complexity without addressing the real Go gap (RBAC O(N)).
  • The security and performance invariant we preserve is: do not do expensive per-object authorization work for users who cannot list cluster-scoped resources — achieved here with SSRR, not with compression.

Test plan

  • go test ./... (hub SSRR tests including 500-cluster scale case)
  • golangci-lint on backend
  • Manual on hub: kubeadmin vs none user on Inventory — SSE completes, backend RSS/CPU stay bounded (no MOCK_CLUSTERS in Go; regression gate is unit tests above)

✅ Checklist

General

  • PR title follows the convention (e.g. ACM-12340 Fix bug with...)
  • Code builds and runs locally without errors
  • No console logs, commented-out code, or unnecessary files
  • All commits are meaningful and well-labeled
  • All new display strings are externalized for localization (English only)
  • (Nice to have) JSDoc comments added for new functions and interfaces

If Bugfix

  • Root cause and fix summary are documented in the ticket (for future reference / errata)
  • Fix tested thoroughly and resolves the issue (pending hub validation)
  • Test(s) added to prevent regression

🗒️ Notes for Reviewers

…rt of the ACM-42568 effort

Signed-off-by: Enrique Mingorance Cano <ginxaco@gmail.com>
@Ginxo Ginxo changed the title ACM-44885: Fix non-admin SSE OOM under large inventory for Go as a part of the ACM-42568 effort [GO MIGRATION BACKPORT] ACM-44885: Fix non-admin SSE OOM under large inventory for Go as a part of the ACM-42568 effort Sep 23, 2026
…6638

Signed-off-by: Enrique Mingorance Cano <ginxaco@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant