Skip to content

ACM-40502 fix(backend): use GET /api with body drain instead of GET /apis [release-2.14] - #6671

Open
Randy424 wants to merge 1 commit into
stolostron:release-2.14from
Randy424:ACM-38826-token-validation-fix-release-2.14
Open

ACM-40502 fix(backend): use GET /api with body drain instead of GET /apis [release-2.14]#6671
Randy424 wants to merge 1 commit into
stolostron:release-2.14from
Randy424:ACM-38826-token-validation-fix-release-2.14

Conversation

@Randy424

Copy link
Copy Markdown
Contributor

Summary

Backport of the memory leak fix to release-2.14.

isAuthenticated() in backend/src/lib/token.ts was calling GET /apis on every authenticated request and never consuming the response body on the success path. This left sockets stuck outside the keepAlive pool, causing unbounded native (external) memory growth in console-chart-console-v2 pods.

Reproduced at 5,899 MB peak RSS (+5,512 MB above baseline) with 615 stuck sockets under 150 concurrent connections. V8 heap oscillated normally throughout — the leak is entirely in native/external memory outside the GC's reach.

Changes

  • isAuthenticated() now uses GET /api (~200 bytes) instead of GET /apis (35+ KB, scales with CRDs)
  • Response body is drained via response.body?.on('error', () => undefined).resume() so the socket returns to the keepAlive pool immediately
  • Return type changed from Promise<Response> to Promise<number> — callers no longer hold a Response reference
  • authenticated.ts updated to consume numeric status
  • All route test auth mocks updated: .get('/apis').get('/api')

Related

  • ACM-40502 (this backport)
  • ACM-38826 (parent — release-2.15, 2.16, 2.17 backports)
  • Customer case: 04433290

Test plan

  • All 84 affected backend route tests pass locally
  • ci/prow/check and ci/prow/unit-tests-sonarcloud pass in prow

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: stolostron/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5b06c6a9-0342-4ebd-8316-1a5839a5c08f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Randy424

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…apis [release-2.14]

Backport of the memory leak fix to release-2.14.

isAuthenticated() was calling GET /apis on every authenticated request
and never consuming the response body on the success path. This left
sockets stuck outside the keepAlive pool, causing unbounded native
(external) memory growth in console-chart-console-v2 pods.

Fix: switch to GET /api (~200 bytes vs 35+ KB for /apis which scales
with CRDs), drain the response body via .resume() so the socket returns
to the keepAlive pool immediately, and return the numeric status so
callers no longer hold a Response reference.

Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@Randy424
Randy424 force-pushed the ACM-38826-token-validation-fix-release-2.14 branch from 64dc9b1 to f373402 Compare August 17, 2026 04:27
@Randy424

Copy link
Copy Markdown
Contributor Author

Rebased onto the latest release-2.14 (8898db028) to resolve the merge conflict.

Conflict: backend/test/routes/ansibletower.test.tsrelease-2.14 had since picked up #6702 (glasswing RBAC-secret fix), which added nockCredentialSecret(TOWER_HOST) calls to several tests that were still mocking the auth check via .get('/apis').

Resolution: Reviewed how main resolved the equivalent code — it converts each .get('/apis') auth mock individually to .get('/api'), leaving everything else untouched. Applied the same pattern here: kept main's new nockCredentialSecret calls, changed the conflicting auth mocks to /api.

Additional fix: A few other .get('/apis') mocks in the same file weren't flagged as conflicts (new tests release-2.14 added that merged cleanly) but would have failed silently, since isAuthenticated() now requests /api and nock.disableNetConnect() is on — an unmatched request throws instead of returning the mocked response. Converted those too for consistency.

Verified: all 172 backend tests pass, lint clean.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant