chore(deps): upgrade backend to Fastify 5 on stable 2.x - #9247
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@PR3MM: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
PR3MM
removed
needs-rebase
|
/retest |
1 similar comment
|
/retest |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![odh-dashboard-agent[bot]](https://avatars.githubusercontent.com/in/3714079?s=60&v=4){kind=small}
There was a problem hiding this comment.
Preflight Agent Report
Verdict: ❌ NOT READY
Commit: 039f6e04
Checks
| Check | Status | Details |
|---|---|---|
| Conflicts | ❌ | CONFLICTING — PR targets main but branch contains stable-2.x commits. PR description says it should target v2.25.0-fixes; retargeting the base branch should resolve all Prow failures. |
| CI (Prow) | ❌ | 5 image mirror jobs failed (ci/prow/images, 4 *-pr-image-mirror). Root cause: merge conflict prevents image build — not a code regression. Resolve conflicts first. |
| CI (GitHub Actions) | Lint, Type-Check, Unit Tests pending (router check still running at time of evaluation). |
|
| Jira | ✅ | RHOAIENG-81841 — Migrate Fastify 4 → 5 (Closed). |
| Test Coverage | ✅ | No test files added; PR body explains: dep upgrade, no websocket proxy spec exists on this branch. |
| PR Body | ✅ | Follows template — Description, How Tested, Test Impact, checklist all complete. |
| Style | ➖ | N/A — no .scss/.css/.tsx files changed. |
| RBAC | ✅ | No concerns. Auth path (getDirectCallOptions + getAccessToken) unchanged. |
| Claude review | 🟠 1 major · 🟡 2 minor | Pre-existing issues in touched function — see details below. |
| Jira Eval | 4/6 criteria satisfied. WebSocket functional testing flagged "confirm 101 before merge" in PR body; 3.3/3.4 branches still outstanding. |
|
| Human review | REVIEW_REQUIRED — no approvals yet. | |
| CodeRabbit (PR) | ➖ | Review skipped: 213 files exceed the limit of 100. |
🟠 1 major · 🟡 2 minor — Claude review findings in backend/src/routes/wss/k8s/index.ts
These are pre-existing issues in the touched WebSocket handler — not introduced by this PR but exposed by the diff scope.
🟠 Major — fastify.server.address() null dereference (line 69)
server.address() can return null when the server is not yet fully bound. The code takes the typeof null === 'object' branch and accesses serverAddress.address, throwing TypeError. Unlikely in production but eliminable:
- const serverAddress = fastify.server.address();
+ const serverAddress = fastify.server.address() ?? { address: '127.0.0.1' };🟡 Minor — close callback declares reason: string but 'close' event emits Buffer (line 74)
The ws library emits (code: number, reason: Buffer) on close. TypeScript strict mode will flag this. closeWebSocket already handles Buffer via .toString(), so no runtime crash — but the parameter type should be widened:
- const close = (code: number, reason: string) => {
+ const close = (code: number, reason: string | Buffer) => {🟡 Minor — source.send(data, { binary }) with data: unknown (line 98)
WebSocket.send expects RawData (string | Buffer | ArrayBuffer | Buffer[]), not unknown. TypeScript strict mode rejects this, and a non-buffer unexpected value would be serialized as '[object Object]' and corrupt binary kubectl exec/log streams.
Automated by ODH Dashboard Agent
|
/hold this breaks the dashboard - can't merge |
|
closing PR as it needs rework based on changes in main branch |
2 participants
https://issues.redhat.com/browse/RHOAIENG-81841
Description
Ports the Fastify 5 backend migration from
main(#9202) onto thestable-2.xrelease branch. Bumpsfastifyand version-coupled@fastify/*plugins in the same change set.Dependency changes (
backend/package.json):fastifyfastify-plugin@fastify/autoload@fastify/http-proxy@fastify/reply-from@fastify/static@fastify/websocket@fastify/sensible@fastify/accepts@pinojs/redactundiciBackend code changes for Fastify 5 API compatibility:
server.ts:logger→loggerInstance;maxParamLengthmoved underrouterOptionsroutes/wss/k8s/index.ts: WebSocket handler usesconnectiondirectly (Fastify 5 /@fastify/websocketv11)package-lock.json: regenerated for Fastify 5 dependency treeHow Has This Been Tested?
Automated
npm run test(backend) — lint, type-check, jest 17/17 passnpm run build(backend) — passResolved lock versions:
fastify@5.11.3@fastify/websocket@11.3.0@fastify/http-proxy@11.6.0@fastify/autoload@6.5.0@fastify/reply-from@12.6.4@pinojs/redact@0.4.0undici@7.29.0Manual
Logged in as
htpasswd-cluster-admin-userviamake login+npm run dev./api/health,/api/status,/api/config/api/dsc/status,/api/dsci/status/api/k8s/...proxy/wss/k8s/...watch in browserTest Impact
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
stable-2.x