Conversation
Pin Yarn via the packageManager field only (no committed release binary); CI enables Corepack explicitly. - .yarnrc.yml: nodeLinker node-modules, and a Scality-scoped approvedGitRepositories allowlist rather than the "**" Yarn writes by default. npmMinimalAgeGate is left at Yarn's 1-day default. - Build scripts stay disabled globally (Yarn 4 default); the six packages that genuinely need compiling are allowlisted via dependenciesMeta. Yarn 1 by contrast lets all ~1100 packages run install scripts. - prepare: tsc instead of yarn build, so the package manager used by Arsenal never leaks into consumers that clone it as a git dependency - test/coverage: inline NODE_OPTIONS; Berry's portable shell has no export - CI: corepack enable must precede actions/setup-node, otherwise its `cache: yarn` probe runs Yarn 1, which refuses once packageManager is set. Also --immutable, and drop --network-concurrency (unsupported), --prefer-offline, and the removed global --silent flag. - The tests/workflows job keeps Yarn 1 and therefore drops `cache: yarn`, since setup-node probes Yarn from the repo root. - Pin the two nested legacy install roots (tests/workflows, lib/executables/pensieveCreds) to yarn@1.22.22; without this the root packageManager field makes Yarn 1 refuse to install in them. Touching them also forces a Prettier reformat, as CI only checks changed files and both were already non-conforming.
Yarn repacks non-registry packages after running their build. fcntl is a git dependency with a node-gyp step, so the archive differs between machines and even between runs, and CI failed with YN0018. Removing only that one checksum keeps integrity checking for the other 1103 packages, including the three pure-JS git dependencies, which is preferable to checksumBehavior: update or ignore. The real fix is to publish scality/node-fcntl to npm as fcntl@0.3.0: the repo is public and Apache-2.0, the bare name is unclaimed, and the package.json already declares name "fcntl", so consumers would only change the dependency spec, with no code change and no registry auth.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The previous commit still carried one: verifying with 'yarn install' re-adds a checksum to the lockfile, and it got committed. Yarn tolerates a missing checksum on install but always writes one back, so this entry has to be committed without an install in between.
Berry's 'yarn run' does still accept --silent (it is consumed, not passed to the script, and the step exits 0), so this was not broken. But the flag is absent from 'yarn run --help', so it is undocumented and could disappear; Berry is quiet by default anyway. Using the plain form matches the other script invocations.
| "text" | ||
| ] | ||
| }, | ||
| "packageManager": "yarn@4.18.0", |
There was a problem hiding this comment.
packageManager lacks a +sha512.… integrity hash. Without it, Corepack downloads whatever is published as yarn@4.18.0 without cryptographic verification — a supply-chain risk the PR otherwise works to reduce. Run corepack use yarn@4.18.0 to let Corepack write the field with the correct hash, or grab it from https://github.com/nodejs/corepack/blob/main/config.json.
Suggested change
| "packageManager": "yarn@4.18.0", | |
| "packageManager": "yarn@4.18.0+sha512.e899d6ee24ba2e5b2923add4e7e9c97b85c46a3ae3dc1561be28c575fceb8e0e1cb61e1f8e2c1e1de0c25c7fcf31ed1c35202dfc8e36d480cb752e4ab7fe205ba", |
The repo's yamllint config sets document-start to error level, so lint_yml failed on the new file. Yarn parses it fine with the marker.
Without the +sha512 suffix, Corepack fetches whatever is published as yarn@4.18.0 with no integrity check. The hash was generated locally with 'corepack use yarn@4.18.0' and verified: 'corepack install' accepts it and rejects a wrong one.
| # the packageManager field would make Yarn 1 refuse. This job deliberately | ||
| # stays on Yarn 1 for tests/workflows, which pins yarn@1.22.22 itself. | ||
| - name: Install dependencies | ||
| run: yarn --cwd tests/workflows install --frozen-lockfile |
There was a problem hiding this comment.
yarn --cwd sets Yarn's internal working directory but not the process CWD, so Corepack still resolves packageManager from the repo root (Yarn 4.18.0) rather than from tests/workflows/ (Yarn 1.22.22). The nested packageManager: yarn@1.22.22 added in tests/workflows/package.json only takes effect when the process CWD is inside that directory.
Today this is safe because Corepack is not enabled in this job. But if it ever is — or if Node enables it by default — this step would silently use Yarn 4, which rejects --frozen-lockfile and can't read the Yarn 1 lockfile.
Using working-directory (the same pattern used for pensieveCreds at line 54) would make the nested packageManager field effective:
- name: Install dependencies
run: yarn install --frozen-lockfile
working-directory: tests/workflows
- name: Run workflow tests
run: yarn test
working-directory: tests/workflowsCorepack resolves packageManager from the process CWD, and --cwd does not change that: with Corepack enabled, 'yarn --cwd tests/workflows' reports 4.18.0 (the repo root) while 'cd tests/workflows && yarn' reports 1.22.22. The nested pin was therefore inert. Harmless today because Corepack is not enabled in that job, but it would break silently the moment it is. working-directory matches how the pensieveCreds step already does it.
| "async": "~2.6.1", | ||
| "node-forge": "^0.7.1" | ||
| }, | ||
| "packageManager": "yarn@1.22.22" |
There was a problem hiding this comment.
packageManager: yarn@1.22.22 has no +sha512.… integrity hash. The test job in tests.yaml enables Corepack at the job level, so when this step runs yarn install, Corepack downloads Yarn 1.22.22 without cryptographic verification. The root package.json already pins its hash — doing the same here keeps the supply-chain posture consistent.
The same applies to tests/workflows/package.json (line 23), though Corepack is deliberately not enabled in that job today.
You can get the hash with:
corepack use yarn@1.22.22 --cwd lib/executables/pensieveCreds
Suggested change
| "packageManager": "yarn@1.22.22" | |
| "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e2" |
Dropping Yarn 1's --network-concurrency 1 turned out to be wrong. Berry rejects the CLI flag but still honours the setting, and it is load bearing: dependencies that are Git repos with Yarn 1 lockfiles get bootstrapped with Yarn Classic, and concurrent Classic installs race on its shared cache, failing to pack with ENOENT (YN0058). Arsenal's CI happens to pass without it today, but it has the same five Git dependencies and the same exposure; this was demonstrated in the Cloudserver Docker build. Can be removed once those repos move to Berry.
Setting packageManager flips Yarn's git-dependency bootstrap from the Yarn Classic branch to the Berry branch. Classic ran 'yarn install', which executes prepare; Berry runs 'yarn pack --install-if-needed', which executes prepack and not prepare. Verified on a pristine 'git archive HEAD' checkout: without prepack the bootstrap produces a 3-entry, 6969-byte tarball with no build/ at all, so every Berry consumer would install an empty Arsenal. Cloudserver is already such a consumer. With prepack: tsc the same command produces 701 entries including the full build/ tree. Also drop level and protobufjs from the build allowlist (their install scripts only print notices, they do not compile), and disable telemetry.
…ists Telemetry was disabled on review advice, but that was not asked for and Berry's default is fine. approvedGitRepositories is not optional hardening: Yarn's default is an empty allowlist that blocks every Git fetch, so a repo with Git dependencies cannot install without it. Verified in Cloudserver by removing it -- the install fails with YN0080. The only real choice is between Yarn's migration-written "**" and a scoped list.
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
1 participant
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.
Draft / PoC — not for merge. Exploring what it actually takes to move a Yarn 1 repo to Yarn 4 (Berry), to inform ZENKO-4051 (open since 2022). Arsenal was picked as the pilot; no ticket yet.
Choices made
packageManageronly — no committed release binary. CI enables Corepack explicitly.nodeLinker: node-modules— not PnP. Matchesbench-vault, the only other Berry repo in the org, and keeps compatibility with the 5 native modules and the git-protocol dependencies.tests/workflowsandlib/executables/pensieveCredsstay on Yarn 1, pinned explicitly (see below).Security posture improves
Berry disables build scripts by default. Arsenal needs six packages compiled, allowlisted via
dependenciesMeta:Under Yarn 1, all ~1100 packages may run install scripts. This is arguably the strongest argument for the migration.
Note that
yarn installsilently wrotenpmMinimalAgeGate: 0andapprovedGitRepositories: ["**"]into.yarnrc.yml— disabling the publish-age gate and the git-source allowlist. Both were reverted: the age gate is back to Yarn's 1-day default, and the allowlist is scoped togithub.com/scality/*. Anyone repeating this migration should re-read.yarnrc.ymlbefore committing.Gotchas worth knowing for the other repos
packageManagerfield breaks nested Yarn 1 installs.yarn --cwd sub installexits 1, andcd subdoesn't help — Yarn 1 walks up and finds the root field. Both nested install roots needed their ownpackageManager: yarn@1.22.22. Any repo with nested install roots hits this.corepack enablemust precedeactions/setup-node. Itscache: yarnprobe shells out to Yarn to find the cache folder; run it after, and Yarn 1 refuses. See Ifcache: yarnis specified, this action fails actions/setup-node#1027.export.testandcoverageboth beganexport NODE_OPTIONS=... &&and failed instantly. Inlined instead.preparechanged fromyarn buildtotscso that no package manager is invoked when a consumer clones Arsenal as a git dependency. Verified against a Yarn 1 consumer.--network-concurrencyand the global--silentflag are removed in Berry;--frozen-lockfileis deprecated in favour of--immutable.package.jsonfiles were already Prettier non-conforming — invisible until now because CI only checks changed files.Verification
Locally green: install,
--immutable,build, unit tests (164 suites / 3947 tests),lint,lint_md,build_doc,prettier:diff, and a Yarn 1 consumer installing this branch as a git dependency.Not verifiable locally, so CI is the real test:
ft_test,ft_pykmip_test(need services) andlint_yml(needs runner-providedyamllint).Caveats
yarn.lockdiff is ~21k lines and cannot meaningfully be read; it is verified by testing, not review.nodeLinker: node-modulesgives no disk dedup — that needsnmMode: hardlinks-global, which is a per-developer preference rather than a repo setting.