Install arsenal and scubaclient from the npm registry instead of git - #6308
Open
francoisferrand wants to merge 11 commits into
Open
francoisferrand wants to merge 11 commits into
francoisferrand wants to merge 11 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files
@@ Coverage Diff @@
## improvement/CLDSRV-996-node-24 #6308 +/- ##
===============================================================
Coverage 86.52% 86.52%
===============================================================
Files 213 213
Lines 14619 14619
===============================================================
Hits 12649 12649
Misses 1970 1970
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
francoisferrand
force-pushed
the
improvement/CLDSRV-997
branch
from
September 24, 2026 19:29
1eb6a71 to
39b7e85
Compare
francoisferrand
force-pushed
the
improvement/CLDSRV-996-node-24
branch
2 times, most recently
from
September 25, 2026 09:56
e65085d to
4d4bdeb
Compare
francoisferrand
force-pushed
the
improvement/CLDSRV-997
branch
from
September 25, 2026 09:58
39b7e85 to
4b05680
Compare
delthas
approved these changes
Sep 25, 2026
francoisferrand
force-pushed
the
improvement/CLDSRV-996-node-24
branch
from
September 25, 2026 16:48
01f506f to
4a12afc
Compare
francoisferrand
force-pushed
the
improvement/CLDSRV-997
branch
from
September 25, 2026 16:53
8976712 to
53a628f
Compare
francoisferrand
requested review from
a team,
SylvainSenechal and
benzekrimaha
September 25, 2026 16:55
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
francoisferrand
force-pushed
the
improvement/CLDSRV-997
branch
from
September 25, 2026 17:00
bd62865 to
27069e0
Compare
Both were git dependencies that compiled their TypeScript sources
during yarn install via prepare/postinstall hooks. Arsenal is aliased
to the registry package (arsenal -> npm:@scality/arsenal@8.6.0-preview.1)
to avoid touching the ~285 require('arsenal') call sites; scubaclient
is renamed outright to @scality/scubaclient since only 3 files require
it. Both now ship precompiled JS, so no install-time tsc is needed for
either package anymore.
Issue: CLDSRV-997
They were only needed to compile arsenal and scubaclient's TypeScript sources at install time when both were git dependencies. Now that both are installed as precompiled registry packages, no git dependency in package.json still needs an install-time build, so the toolchain is no longer needed in the image. Issue: CLDSRV-997
The npm registry alias ("arsenal": "npm:@scality/arsenal@...") let us
install from the registry while keeping the historical bare require,
but it means every dependency (bucketclient, utapi, ...) that itself
declares a peer/dependency on @scality/arsenal ends up with two
separate copies installed side by side instead of sharing one.
Rename the dependency to its real @scality/arsenal name and update all
require('arsenal') call sites accordingly, so the package resolves and
dedupes the same way as any other consumer of it.
Issue: CLDSRV-997
Tag 8.3.0 renames the package from bucketclient to @scality/bucketclient and moves its arsenal peer/dev dependency to the @scality/arsenal package name, matching the rename just made here. Update the sole require site accordingly. Issue: CLDSRV-997
A plain yarn upgrade floats @aws-sdk/client-s3 and @smithy/core to
their latest mutually-compatible versions (3.1140.0 / 3.35.0). The
newer @smithy/core no longer declares @smithy/util-stream as a
dependency even though its bundled code still requires it at runtime
(upstream packaging gap), so it's added as an explicit direct
dependency to keep arsenal's AWS SDK usage (requestUrl,
network/kmip client) working.
Verified require('ioctl'), require('@scality/arsenal'),
require('@scality/bucketclient'), require('@scality/scubaclient') and
requestUrl.parseRequestTarget all still work after a clean install.
Issue: CLDSRV-997
Same pattern as arsenal and bucketclient: the git tag's own package.json already declares itself as @scality/utapi, so alias it under that name instead of the bare utapi key. Issue: CLDSRV-997
francoisferrand
force-pushed
the
improvement/CLDSRV-997
branch
from
September 26, 2026 09:35
7cd934b to
2ea0ab1
Compare
francoisferrand
force-pushed
the
improvement/CLDSRV-996-node-24
branch
from
September 26, 2026 09:35
4a12afc to
d454d68
Compare
francoisferrand
force-pushed
the
improvement/CLDSRV-997
branch
3 times, most recently
from
September 27, 2026 08:28
96e59c2 to
061b781
Compare
The @aws-sdk/client-s3 bump pulled in by the yarn.lock refresh moved the CRC64NVME checksum container from @aws-sdk/middleware-flexible-checksums to @aws-sdk/checksums. Update the two functional test files that imported it directly so they no longer fail with "Cannot find module '@aws-sdk/middleware-flexible-checksums'". Also drop jsonwebtoken and fast-xml-parser from package.json dependencies. Both are only needed as transitive dependencies of @azure/storage-blob (via @azure/identity/@azure/msal-node and @azure/core-xml respectively) and are already version-pinned through the existing resolutions block; promoting them to direct dependencies during the yarn.lock refresh was unintentional. Removing them does not change yarn.lock or the installed versions. @smithy/util-stream was unintentionally promoted to a direct dependency during the yarn.lock refresh, same as the jsonwebtoken/fast-xml-parser case fixed earlier. Unlike those, it is not required transitively by anything else in the dependency tree either (it was the only consumer of that package in yarn.lock), so removing it drops the package entirely instead of just de-duplicating it. It is not require()'d anywhere in cloudserver source. Flagged by Claude Code Review. Issue: CLDSRV-997
instrumentation-http ~0.218 -> ~0.222, instrumentation-ioredis ~0.64 -> ~0.70, instrumentation-mongodb ~0.69 -> ~0.75, all sharing the same @opentelemetry/instrumentation@0.222.0 core. Also refreshed yarn.lock, bumping other transitive deps within their existing semver ranges. Issue: CLDSRV-997
The @aws-sdk/client-s3 bump pulled in by the yarn.lock refresh made the S3 REST XML serializer reject an empty Bucket path label client-side, before any request is sent. The test now gets a plain Error with no $metadata instead of a 405 response from the server, so assert on the new client-side error message instead. Issue: CLDSRV-997 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The test only checked the ETag of the final GetObject and left the 30000-byte body unread, so the `after` hook's DeleteObject raced the server still streaming parts: once the data is gone the server destroys the response mid-stream and the client socket closes with an incomplete body. With @aws-sdk/client-s3 3.1141 the response checksum stream (ChecksumStream, now in @smithy/core) forwards the underlying IncomingMessage's `aborted` error onto `Body`. Nothing listens to it in the test, so mocha dies with an uncaught `Error: aborted` in the after hook. Older SDKs piped the source without forwarding its errors, which is why this used to pass. Read the body to completion (and check its content) so the object is only deleted once the stream is done. Issue: CLDSRV-997
francoisferrand
force-pushed
the
improvement/CLDSRV-997
branch
from
September 27, 2026 11:30
061b781 to
54f72c1
Compare
This branch has not been deployed
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
2 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.
Arsenal and scubaclient were installed from git, so both compiled during
yarn installvia theirprepare/postinstallhooks. That's exactly the kind of install-time build that's fragile on a new runtime, and the Dockerfile carried a globaltypescript@4.9.5(older than either package's own TS 5.x requirement) just to make it work "by luck".Both are now published on the npm registry as precompiled artifacts, so this removes the install-time build, the global typescript/node-gyp, and the frozen git refs.
arsenalis aliased (npm:@scality/arsenal@8.6.0-preview.1) rather than renamed, since there are ~285require('arsenal')call sites — the alias keeps the diff topackage.json/yarn.lockonly.scubaclientis renamed to@scality/scubaclient(only 3 require sites:lib/utilization/scuba/wrapper.js,tests/unit/quotas/scuba/wrapper.js,tests/unit/api/apiUtils/quotas/quotaUtils.js), API-compatible (getLatestMetrics/healthCheck/constructor unchanged).typescript@4.9.5/node-gypinstall is dropped since no remaining git dependency needs an install-time TS compile.Verified with a fresh
yarn install --frozen-lockfileon Node 24:require('ioctl')still works (CLDSRV-996's nan/ioctl fix undisturbed),arsenal.requestUrl.parseRequestTargetis present,ScubaClient.getLatestMetrics/healthCheckare present, lint is clean, and quota/scuba/objectCopy unit tests pass (231 passing).Stacked on #6307 ("Run on Node 24") — targets
improvement/CLDSRV-996-node-24, notdevelopment/9.5.Issue: CLDSRV-997