Skip to content

Commit d84a76b

Browse files
Randy424claude
Randy424
and
claude
committed
ACM-38826 fix(backend): use HEAD /api for token validation to eliminate response body drain (#6537)
getAuthenticatedToken() called isAuthenticated() on every authenticated request, which issued GET /apis to the kube API. The response body (up to several MB on CRD-heavy clusters) was never consumed on the success path, preventing the socket from returning to the keepAlive pool. Under sustained load, native (external) memory accumulated proportionally to request volume. Replace GET /apis with HEAD /api: - HEAD responses have no message body by HTTP spec — nothing to drain - /api (core group) is ~200 bytes of headers; it does not grow with CRDs - Returns HTTP status so callers preserve 401/403/5xx distinctions - No client-side caching required: OpenShift oauth-apiserver caches valid tokens ~30 seconds server-side; failures are not cached isAuthenticated() now returns Promise<number> (HTTP status) instead of Promise<Response> so callers preserve upstream status codes. authenticated.ts updated accordingly. All route tests updated to mock HEAD /api instead of GET /apis. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-authored-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 638f6a8 commit d84a76b

14 files changed

Lines changed: 73 additions & 59 deletions

‎backend/src/lib/authenticated.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ export function authenticated(req: Http2ServerRequest, res: Http2ServerResponse)
77
const token = getToken(req)
88
if (!token) return unauthorized(req, res)
99
isAuthenticated(token)
10-
.then((response) => {
11-
res.writeHead(response.status).end()
12-
void response.blob()
10+
.then((status) => {
11+
res.writeHead(status).end()
1312
})
1413
.catch(catchInternalServerError(res))
1514
}

‎backend/src/lib/token.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,16 @@ export function getToken(req: Http2ServerRequest): string | undefined {
2828
return token
2929
}
3030

31-
export async function isAuthenticated(token: string) {
32-
return fetchRetry(process.env.CLUSTER_API_URL + '/apis', {
31+
// HEAD /api returns headers only — no response body — so no drain is needed and
32+
// the payload is ~200 bytes regardless of how many CRDs are registered.
33+
// Returns the HTTP status so callers can distinguish 401 (invalid token) from
34+
// 403 (valid token, insufficient permission) and 5xx (transient upstream error).
35+
export async function isAuthenticated(token: string): Promise<number> {
36+
const response = await fetchRetry(process.env.CLUSTER_API_URL + '/api', {
37+
method: 'HEAD',
3338
headers: { [HTTP2_HEADER_AUTHORIZATION]: `Bearer ${token}` },
3439
})
40+
return response.status
3541
}
3642

3743
export const isHttp2ServerResponse = (
@@ -51,22 +57,19 @@ export async function getAuthenticatedToken(
5157
const token = getToken(req)
5258

5359
if (token) {
54-
const authResponse = await isAuthenticated(token)
60+
const status = await isAuthenticated(token)
5561
/* istanbul ignore if */
56-
if (authResponse.status === constants.HTTP_STATUS_OK) {
62+
if (status === constants.HTTP_STATUS_OK) {
5763
if (process.env.NODE_ENV === 'development') {
5864
const localStorage = new LocalStorage(LOCAL_STORAGE)
5965
localStorage.setItem(ADMIN_TOKEN, token)
6066
}
6167
return token
68+
}
69+
if (isHttp2ServerResponse(resOrSocket)) {
70+
resOrSocket.writeHead(status).end()
6271
} else {
63-
if (isHttp2ServerResponse(resOrSocket)) {
64-
resOrSocket.writeHead(authResponse.status).end()
65-
} else {
66-
resOrSocket.destroy()
67-
}
68-
69-
void authResponse.blob()
72+
resOrSocket.destroy()
7073
}
7174
} else if (isHttp2ServerResponse(resOrSocket)) {
7275
unauthorized(req, resOrSocket)

‎backend/test/routes/aggregator.test.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ import type { IResource } from '../../src/resources/resource'
1616
/// to get exact nock request body, put bp at line 303 in /backend/node_modules/nock/lib/intercepted_request_router.js
1717
describe(`aggregator Route`, function () {
1818
it(`should page Unfiltered Applications`, async function () {
19-
resetApplicationCache()
20-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
19+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200)
2120

2221
// initialize events
2322
await Promise.all(resources.map((resource) => cacheResource(resource)))
@@ -62,8 +61,7 @@ describe(`aggregator Route`, function () {
6261
expect(await parseResponseJsonBody(res)).toEqual(responseNoFilter)
6362
})
6463
it(`should page Filtered Applications`, async function () {
65-
resetApplicationCache()
66-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
64+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200)
6765

6866
// initialize events
6967
await Promise.all(resources.map((resource) => cacheResource(resource)))
@@ -92,8 +90,7 @@ describe(`aggregator Route`, function () {
9290
expect(await parseResponseJsonBody(res)).toEqual(responseFiltered)
9391
})
9492
it(`should return application counts`, async function () {
95-
resetApplicationCache()
96-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
93+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200)
9794

9895
// initialize events
9996
await Promise.all(resources.map((resource) => cacheResource(resource)))
@@ -114,9 +111,8 @@ describe(`aggregator Route`, function () {
114111
expect(res.statusCode).toEqual(200)
115112
expect(await parseResponseJsonBody(res)).toEqual(responseCount)
116113
})
117-
it(`should return ui data`, async function () {
118-
resetApplicationCache()
119-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
114+
it(`should return appset data`, async function () {
115+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200)
120116

121117
// initialize events
122118
resources.forEach((resource) => cacheResource(resource))

‎backend/test/routes/ansibletower.test.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import nock from 'nock'
77
const TOWER_HOST = 'https://ansible-tower.com'
88

99
describe(`ansibletower Route`, function () {
10-
it(`should list Ansible TowerJobs`, async function () {
11-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
10+
it(`should list Ansible Automation controller Jobs`, async function () {
11+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200)
1212
nock(TOWER_HOST).get(ansiblePaths[0]).reply(200, response)
1313
const res = await request('POST', '/ansibletower', {
1414
towerHost: TOWER_HOST + ansiblePaths[0],
@@ -18,8 +18,8 @@ describe(`ansibletower Route`, function () {
1818
expect(JSON.stringify(await parsePipedJsonBody(res))).toEqual(JSON.stringify(response))
1919
})
2020

21-
it(`when bad things happen to Ansible TowerJobs 1`, async function () {
22-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
21+
it(`when bad things happen to Ansible Automation controller Jobs 1`, async function () {
22+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200)
2323
nock(TOWER_HOST).get(ansiblePaths[0]).reply(200, response)
2424
const res = await request('POST', '/ansibletower', {
2525
towerHost: TOWER_HOST + '/badPath',
@@ -29,18 +29,19 @@ describe(`ansibletower Route`, function () {
2929
expect(JSON.stringify(await parsePipedJsonBody(res))).toEqual(JSON.stringify({}))
3030
})
3131

32-
it(`when bad things happen to Ansible TowerJobs 2`, async function () {
33-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
32+
it(`when bad things happen to Ansible Automation controller Jobs 2`, async function () {
33+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200)
3434
nock(TOWER_HOST).get(ansiblePaths[0]).reply(200, response)
3535
const res = await request('POST', '/ansibletower', {
3636
token: '12345',
3737
})
3838
expect(JSON.stringify(await parsePipedJsonBody(res))).toEqual(JSON.stringify({}))
3939
})
4040

41-
it(`when bad things happen to Ansible TowerJobs 3`, async function () {
42-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(400)
41+
it(`when bad things happen to Ansible Automation controller Jobs 3`, async function () {
42+
nock(process.env.CLUSTER_API_URL).head('/api').reply(401)
4343
const res = await request('POST', '/ansibletower')
44+
expect(res.statusCode).toEqual(401)
4445
expect(JSON.stringify(await parsePipedJsonBody(res))).toEqual(JSON.stringify({}))
4546
})
4647
})

‎backend/test/routes/apiPath.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ describe(`apiPath Route`, function () {
1515

1616
nock(process.env.CLUSTER_API_URL).get(paths[0]).reply(200, response)
1717

18-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
18+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
1919
status: 200,
2020
paths: response,
2121
})

‎backend/test/routes/hub.test.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ import { request } from '../mock-request'
55

66
describe('global hub', function () {
77
it('should return the boolean', async function () {
8-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
9-
status: 200,
10-
})
8+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200)
119
nock(process.env.CLUSTER_API_URL)
1210
.get('/apis/apiextensions.k8s.io/v1/customresourcedefinitions') // .reply(200, { isGlobalHub: true })
1311
.reply(200, {

‎backend/test/routes/hypershift-status.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { parseResponseJsonBody } from '../../src/lib/body-parser'
44
import nock from 'nock'
55

66
describe('hypershift-status Route', function () {
7-
const mockAuth = () => nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, { status: 200 })
7+
const mockAuth = () => nock(process.env.CLUSTER_API_URL).head('/api').reply(200, { status: 200 })
88

99
const mockMCE = (hypershiftEnabled = true, localHostingEnabled = true) =>
1010
nock(process.env.CLUSTER_API_URL)

‎backend/test/routes/metricsProxy.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ import { request } from '../mock-request'
44

55
describe('metrics proxy route', function () {
66
it('Successfully calls prometheus endpoint', async function () {
7-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
7+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
88
status: 200,
99
})
1010
const res = await request('GET', '/prometheus/query')
1111
expect(res.statusCode).toEqual(200)
1212
})
1313
it(`Successfully calls observability endpoint`, async function () {
14-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
14+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
1515
status: 200,
1616
})
1717
const res = await request('GET', '/observability/query')

‎backend/test/routes/operatorCheck.test.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const subscriptionOperators = {
2323

2424
describe(`operatorCheck Route`, function () {
2525
it(`returns valid response with version for installed operator`, async function () {
26-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
26+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
2727
status: 200,
2828
})
2929
nock(process.env.CLUSTER_API_URL)
@@ -38,7 +38,7 @@ describe(`operatorCheck Route`, function () {
3838
})
3939
})
4040
it(`returns valid response for not-installed operator`, async function () {
41-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
41+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
4242
status: 200,
4343
})
4444
nock(process.env.CLUSTER_API_URL)
@@ -52,7 +52,7 @@ describe(`operatorCheck Route`, function () {
5252
})
5353
})
5454
it(`returns bad request for arbitrary operator`, async function () {
55-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
55+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
5656
status: 200,
5757
})
5858
nock(process.env.CLUSTER_API_URL)
@@ -61,4 +61,20 @@ describe(`operatorCheck Route`, function () {
6161
const res = await request('POST', '/operatorCheck', { operator: 'multicluster-engine' })
6262
expect(res.statusCode).toEqual(400)
6363
})
64+
65+
it('correctly parses request body received in multiple chunks', async function () {
66+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
67+
status: 200,
68+
})
69+
nock(process.env.CLUSTER_API_URL)
70+
.get('/apis/operators.coreos.com/v1alpha1/subscriptions')
71+
.reply(200, subscriptionOperators)
72+
const res = await requestMultiChunk('POST', '/operatorCheck', { operator: 'openshift-gitops-operator' })
73+
expect(res.statusCode).toEqual(200)
74+
expect(await parseResponseJsonBody(res)).toEqual({
75+
operator: 'openshift-gitops-operator',
76+
installed: true,
77+
version: 'openshift-gitops-operator.v1.8.2',
78+
})
79+
})
6480
})

‎backend/test/routes/search.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import nock from 'nock'
44

55
describe(`search Route`, function () {
66
it(`uses search-api in the namespace of the MultiClusterHub`, async function () {
7-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
7+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
88
status: 200,
99
})
1010
nock(process.env.CLUSTER_API_URL)
@@ -27,7 +27,7 @@ describe(`search Route`, function () {
2727
//expect(res.statusCode).toEqual(200)
2828
})
2929
it(`uses search-api in namespace of pod if no MultiClusterHub`, async function () {
30-
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200, {
30+
nock(process.env.CLUSTER_API_URL).head('/api').reply(200, {
3131
status: 200,
3232
})
3333
nock(process.env.CLUSTER_API_URL).get('/apis/operator.open-cluster-management.io/v1/multiclusterhubs').reply(200, {

0 commit comments

Comments
 (0)