Run on Node 24 - #2858
Merged
Merged
Run on Node 24#2858
Conversation
Part of the Node.js 22 -> 24 upgrade: move the runtime images and the test workflow to the Node 24 bookworm-slim base, so what we build and what we test against match the version we now support. Issue: BB-888
Node 24 broke the native module chain: bucketclient 8.2.x pulled arsenal as a full dependency, dragging in diskusage and aws-sdk v2, which fail to build. bucketclient 8.2.10 makes arsenal a peer dependency, and arsenal 8.6.0-preview.1 moves to the AWS SDK v3 and drops diskusage entirely; a resolution keeps the transitive copies on 8.2.10 too. ioctl is loaded at runtime but only ever reached us through arsenal as an optional dependency, where a failed build is silently ignored -- declare it directly so it is a real, visible dependency. With the floor at >=24, --ignore-engines is no longer needed anywhere. Issue: BB-888
url.parse() is deprecated and its quirks are load-bearing for S3 request targets: WHATWG URL collapses dot segments, reads a leading // as an authority and percent-encodes non-ASCII, so it is not a drop-in for routes. Use arsenal's requestUrl.parseRequestTarget, which is built for exactly that, and plain URL for the ingestion endpoint, which really is an absolute URL. Issue: BB-888
backo is archived upstream and unmaintained; arsenal 8.6 ships an equivalent Backoff with the same min/max/factor/jitter options, so there is no reason to keep a separate dependency for it. Issue: BB-888
Contributor
Hello francoisferrand,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 2 files with indirect coverage changes
@@ Coverage Diff @@
## development/9.6 #2858 +/- ##
===================================================
+ Coverage 76.74% 76.75% +0.01%
===================================================
Files 207 207
Lines 14488 14487 -1
===================================================
+ Hits 11119 11120 +1
+ Misses 3359 3357 -2
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
francoisferrand
requested review from
a team,
SylvainSenechal and
benzekrimaha
September 24, 2026 06:59
Contributor
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
benzekrimaha
approved these changes
Sep 24, 2026
SylvainSenechal
approved these changes
Sep 24, 2026
The actions we pin were all still on the node20 runtime, which is end-of-life on the runner; their latest majors move to node24, in line with the rest of this branch. Nothing we pass them changed: the inputs removed along the way (buildx and build-push deprecations) were unused, and checkout v7's fork restriction only concerns pull_request_target and workflow_run, which we don't use. Issue: BB-888
Contributor
Author
|
/approve |
Contributor
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
This pull request did not target the following hotfix branch(es) so they
Please check the status of the associated issue BB-888. Goodbye francoisferrand. The following options are set: approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the OS-1155 Node.js 22 → 24 upgrade.
Runtime images, the syntheticbucketd test image and the 5 CI jobs move to Node 24, and
engines.nodegoes to>=24. With a real floor,--ignore-enginesis no longer needed anywhere — it was only there to mask the native module breakage below.That breakage was bucketclient 8.2.x pulling arsenal as a full dependency, which dragged in
diskusageand aws-sdk v2 and failed to build on 24. bucketclient 8.2.10 makes arsenal a peer dependency, and arsenal 8.6.0-preview.1 is on the AWS SDK v3 with nodiskusageat all. A resolution keeps the transitive bucketclient copies on 8.2.10 too. vaultclient and httpagent follow to their tagged releases (httpagent 1.1.1 also matches what arsenal pulls, so the duplicate unpack warning goes away).ioctlis loaded at runtime but only ever reached us through arsenal as an optional dependency, where a failed native build is silently ignored and yarn tells you it is safe. Declared directly so it is a real, visible dependency.Two smaller cleanups riding along, both enabled by the arsenal bump:
url.parse()is deprecated, but its quirks are load-bearing for S3 request targets — WHATWGURLcollapses dot segments, reads a leading//as an authority and percent-encodes non-ASCII.BackbeatRequestnow uses arsenal'srequestUrl.parseRequestTarget, which is built for exactly this;IngestionPopulatorgets plainURL, since that one really is an absolute URL.backois archived upstream; arsenal 8.6 ships an equivalentBackoff, so the separate dependency goes.Verified
On Node 24.21.0: install clean, lint clean, 703 unit tests passing (
api,lib,utils,credentials,clients,gc— the rest of the unit suite needs a live Kafka broker).yarn audit, deduplicated by advisory × path, prod dependencies only: 86 → 84, with two highs gone (@opentelemetry/propagator-jaegerandbase-x, both via arsenal). What's left is upstream:fcntl,@aws-sdk/client-iam→fast-xml-parser, arsenal's socket.io/ws chain,async@2→lodash,breakbeat→axios. The larger dev-side count is almost entirely@zenko/cloudserverand never reaches the production image.Issue: BB-888