Skip to content

Commit 80621f7

Browse files
Auto-merge main into odh-release
2 parents fa331a8 + 2db1c8a commit 80621f7

468 files changed

Lines changed: 49045 additions & 3247 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
name: Test / MaaS BFF Tests
2+
on:
3+
push:
4+
paths:
5+
- "packages/maas/**"
6+
- "!LICENSE*"
7+
- "!DOCKERFILE*"
8+
- "!**.gitignore"
9+
- "!**.md"
10+
11+
pull_request:
12+
paths:
13+
- "packages/maas/**"
14+
- ".github/workflows/**"
15+
- "!LICENSE*"
16+
- "!DOCKERFILE*"
17+
- "!**.gitignore"
18+
- "!**.md"
19+
20+
jobs:
21+
test:
22+
runs-on: ubuntu-latest
23+
steps:
24+
- uses: actions/checkout@v6.0.1
25+
26+
- name: Setup Go
27+
uses: actions/setup-go@v6
28+
with:
29+
go-version: "1.24.6"
30+
31+
- name: Lint
32+
working-directory: packages/maas/bff
33+
run: make lint
34+
35+
- name: Build (BFF)
36+
working-directory: packages/maas/bff
37+
run: make build
38+
39+
- name: Install envtest assets
40+
working-directory: packages/maas/bff
41+
run: ./bin/setup-envtest-release-0.17 use 1.29.0 --bin-dir ./bin
42+
43+
- name: Test (BFF API)
44+
working-directory: packages/maas/bff
45+
env:
46+
ENVTEST_ASSETS_DIR: ${{ github.workspace }}/packages/maas/bff/bin
47+
run: go test ./internal/api
48+
49+
- name: Check if there are uncommitted file changes
50+
working-directory: packages/maas
51+
run: |
52+
clean=$(git status --porcelain)
53+
if [[ -z "$clean" ]]; then
54+
echo "Empty git status --porcelain: $clean"
55+
else
56+
echo "Uncommitted file changes detected: $clean"
57+
git diff
58+
exit 1
59+
fi

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,5 +83,6 @@ Before performing certain tasks, read and follow the corresponding specialized r
8383
| **Contract Tests** | [docs/agent-rules/contract-tests.md](docs/agent-rules/contract-tests.md) | When working on contract tests or BFF API validation |
8484
| **Cypress E2E Tests** | [docs/agent-rules/cypress-e2e.md](docs/agent-rules/cypress-e2e.md) | When creating or modifying E2E tests, Robot Framework migrations |
8585
| **Cypress Mock Tests**| [docs/agent-rules/cypress-mock.md](docs/agent-rules/cypress-mock.md) | When creating or modifying mock/component tests |
86+
| **Unit Tests** | [docs/agent-rules/unit-tests.md](docs/agent-rules/unit-tests.md) | When creating or modifying Jest unit tests for utilities, hooks, or components |
8687

8788
**Important**: Always read the relevant rule file before starting the task to ensure you follow the project's conventions and patterns.

OWNERS

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ filters:
1616
- workbenches-approvers
1717
- gen-ai-approvers
1818
- nim-serving-approvers
19+
- autorag-approvers
1920

2021
# Catchall
2122
'.*':
@@ -147,3 +148,12 @@ filters:
147148
- notebooks-reviewers
148149
labels:
149150
- 'area/notebooks'
151+
152+
# AutoRAG
153+
'packages/autorag/.*':
154+
approvers:
155+
- autorag-approvers
156+
reviewers:
157+
- autorag-reviewers
158+
labels:
159+
- 'area/autorag'

OWNERS_ALIASES

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ aliases:
6464
- mturley
6565
- ppadti
6666
- YuliaKrimerman
67+
- Philip-Carneiro
6768

6869
# Model Serving & Model Metrics
6970
model-serving-metrics-approvers:
@@ -101,6 +102,7 @@ aliases:
101102
- antowaddle
102103
- FedeAlonso
103104
- manosnoam
105+
- sridarna
104106
quality-e2e-testing-reviewers:
105107
- ConorOM1
106108
- antowaddle
@@ -148,7 +150,7 @@ aliases:
148150
- rhdedgar
149151
- iamemilio
150152
- s-akhtar-baig
151-
153+
152154
# Notebooks
153155
notebooks-approvers:
154156
- andyatmiami
@@ -167,5 +169,29 @@ aliases:
167169
# NIM / Ecosystem Engineering
168170
nim-serving-approvers:
169171
- TomerFi
172+
- TheiaSurette
173+
- trujillm
174+
- mtalvi
175+
- xieshenzh
176+
- swati-kale
170177
nim-serving-reviewers:
171178
- TomerFi
179+
- TheiaSurette
180+
- trujillm
181+
- mtalvi
182+
- xieshenzh
183+
- swati-kale
184+
185+
# AutoRAG
186+
autorag-approvers:
187+
- nickmazzi
188+
- chrjones-rh
189+
autorag-reviewers:
190+
- nickmazzi
191+
- chrjones-rh
192+
- srtanish1992
193+
- daniduong
194+
- GAUNSD
195+
- jefho-rh
196+
- MatthewAThompson
197+
- jkyaw

backend/src/__tests__/websocket-proxy.spec.ts

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,36 @@ describe('WebSocket K8s Proxy', () => {
586586
expect.stringContaining('Unexpected response from K8s API'),
587587
);
588588
});
589+
590+
it('should convert reserved close code 1006 to 1011 when forwarding to client', async () => {
591+
// Code 1006 (Abnormal Closure) cannot be sent in a close frame per RFC 6455
592+
// The ws module enforces this. We convert it to 1011 (Internal Error).
593+
await routeHandler(mockConnection, mockRequest);
594+
595+
const targetCloseHandler = mockTargetSocket.on.mock.calls.find(
596+
(call: any) => call[0] === 'close',
597+
)?.[1];
598+
599+
// K8s API closes with code 1006 (abnormal closure - e.g., network drop)
600+
targetCloseHandler(1006, 'Abnormal closure');
601+
602+
// Client socket should receive 1011 instead of 1006
603+
expect(mockSourceSocket.close).toHaveBeenCalledWith(1011, 'Abnormal closure');
604+
});
605+
606+
it('should pass through normal close codes unchanged', async () => {
607+
await routeHandler(mockConnection, mockRequest);
608+
609+
const targetCloseHandler = mockTargetSocket.on.mock.calls.find(
610+
(call: any) => call[0] === 'close',
611+
)?.[1];
612+
613+
// K8s API closes with normal code 1000
614+
targetCloseHandler(1000, 'Normal closure');
615+
616+
// Client socket should receive 1000 unchanged
617+
expect(mockSourceSocket.close).toHaveBeenCalledWith(1000, 'Normal closure');
618+
});
589619
});
590620

591621
describe('Stale Connection Cleanup', () => {
@@ -602,7 +632,7 @@ describe('WebSocket K8s Proxy', () => {
602632
// Connection is still active, so no cleanup warning yet
603633
expect(mockLog.warn).not.toHaveBeenCalledWith(
604634
expect.anything(),
605-
expect.stringContaining('Removing stale connection'),
635+
expect.stringContaining('Closing stale websocket connection'),
606636
);
607637

608638
// Advance time beyond stale threshold
@@ -614,9 +644,78 @@ describe('WebSocket K8s Proxy', () => {
614644
expect.objectContaining({
615645
inactivityDuration: expect.any(Number),
616646
}),
617-
expect.stringContaining('Removing stale connection from tracking'),
647+
expect.stringContaining('Closing stale websocket connection'),
648+
);
649+
});
650+
651+
/**
652+
* This test verifies the fix for the zombie connection bug:
653+
*
654+
* When a connection becomes stale, both websockets are now properly closed
655+
* so that the client receives the close event and can reconnect.
656+
*/
657+
it('should close websockets when cleaning up stale connections', async () => {
658+
await wssK8sRoutes(mockFastify);
659+
routeHandler = (mockFastify.get as jest.Mock).mock.calls[0][2];
660+
661+
// Create a connection
662+
await routeHandler(mockConnection, mockRequest);
663+
664+
// Verify sockets haven't been closed yet
665+
expect(mockSourceSocket.close).not.toHaveBeenCalled();
666+
expect(mockTargetSocket.close).not.toHaveBeenCalled();
667+
668+
// Let connection become stale (advance past threshold + cleanup cycle)
669+
jest.advanceTimersByTime(STALE_CONNECTION_MS + 60000);
670+
671+
// Verify stale cleanup ran
672+
expect(mockLog.warn).toHaveBeenCalledWith(
673+
expect.objectContaining({
674+
inactivityDuration: expect.any(Number),
675+
}),
676+
expect.stringContaining('Closing stale websocket connection'),
677+
);
678+
679+
// Both websockets should be closed with code 1001 (Going Away)
680+
expect(mockSourceSocket.close).toHaveBeenCalledWith(
681+
1001,
682+
'Connection stale due to inactivity',
683+
);
684+
expect(mockTargetSocket.close).toHaveBeenCalledWith(
685+
1001,
686+
'Connection stale due to inactivity',
618687
);
619688
});
689+
690+
/**
691+
* This test verifies that heartbeat intervals are properly cleared
692+
* when cleaning up stale connections.
693+
*/
694+
it('should clear heartbeat interval when cleaning up stale connections', async () => {
695+
await wssK8sRoutes(mockFastify);
696+
routeHandler = (mockFastify.get as jest.Mock).mock.calls[0][2];
697+
698+
// Create a connection
699+
await routeHandler(mockConnection, mockRequest);
700+
701+
// Simulate connection opening to start heartbeat
702+
const openHandler = mockTargetSocket.on.mock.calls.find(
703+
(call: any) => call[0] === 'open',
704+
)?.[1];
705+
openHandler();
706+
707+
// Verify heartbeat is running
708+
jest.advanceTimersByTime(HEARTBEAT_INTERVAL_MS);
709+
expect(mockTargetSocket.ping).toHaveBeenCalledTimes(1);
710+
711+
// Advance time beyond stale threshold + cleanup cycle
712+
jest.advanceTimersByTime(STALE_CONNECTION_MS + 60000);
713+
714+
// After cleanup, heartbeat should be stopped - no more pings
715+
const pingCountAfterCleanup = mockTargetSocket.ping.mock.calls.length;
716+
jest.advanceTimersByTime(HEARTBEAT_INTERVAL_MS * 3);
717+
expect(mockTargetSocket.ping.mock.calls.length).toBe(pingCountAfterCleanup);
718+
});
620719
});
621720

622721
describe('Configuration Constants', () => {

0 commit comments

Comments
 (0)