Repository navigation
Patching corepack to work with private registries聽#296
Description
TL;DR suggestion: read engine.config from environment vars.
Hi there 馃憢
Where I work we want to use corepack to manage package manager versions, however we find that we need to patch it in a few ways. I'm hoping we can find a solution that doesn't rely on patching as it leads to more maintenance over time and I believe that many other companies would benefit from solving this.
Default headers
var DEFAULT_HEADERS = {
- [`Accept`]: `application/vnd.npm.install-v1+json`
+ [`Accept`]: `application/vnd.npm.install-v1+json;q=1.0, application/json;1=0.8`
};Our private registry expects specific headers.
A simple solution here would be to add an environment variable to be spread onto these headers. WDYT?
Yarn config
">=2.0.0": {
name: "yarn",
- url: "https://repo.yarnpkg.com/{}/packages/yarnpkg-cli/bin/yarn.js",
- bin: [
- "yarn",
- "yarnpkg"
- ],
+ url: "https://<our-private-repo>/@yarnpkg/cli-dist/-/cli-dist-{}.tgz",
+ bin: {
+ yarn: "./bin/yarn.js",
+ yarnpkg: "./bin/yarn.js"
+ },Yarn is unique in that it is the only package manager that is not fetched from npm. If it was then we could easily use the COREPACK_NPM_REGISTRY environment variable to change this to our private repo. Today we don't have the ability to modify this via env vars.
Due to the way our private repo mirrors external repositories we also need to change the location of the bins for yarn 2+. There is no way to address this other than changing our private repo or patching corepack.
Suggestion 1: Allow custom configs
Since the Engine already supports a config being provided, what if we implemented a new environment variable that can point to the location of a custom config: COREPACK_CONFIG. This could even be spread onto the default config, thereby allowing users to only change what is needed.
Pros:
- Super simple to do
- Already supported in Engine
- Only adds one more env var
- Addresses bin file patching
Cons:
- Restricts the schema of the config in the future
- Although it could be versioned, or simply fail
Suggestion 2: Add more env vars
For the registry we could add a yarn specific setting, or allow a more dynamic resolution such as (replacing this part of corepackUtils.installVersion):
const packageManagerRegistry = `COREPACK_${locator.name.toUpperCase()}_REGISTRY`;
const registryUrl = process.env[packageManagerRegistry] || (process.env.COREPACK_NPM_REGISTRY ?
spec.url.replace(
npmRegistryUtils.DEFAULT_NPM_REGISTRY_URL,
() => process.env.COREPACK_NPM_REGISTRY!,
) :
spec.url);
const url = registryUrl.replace(`{}`, version);This would allow us to specify COREPACK_YARN_REGISTRY and open the door to other ones too, such as COREPACK_PNPM_REGISTRY. Granted, it is not super clean.
Pros:
- Targeted changes to config values
- Config schema is unaffected
Cons:
- A bit messy, adds several more env variables
- Doesn't address bin file patching
Conclusion
I'm happy to implement any / all of these, but I'd like to first get some feedback on the design.
Activity
mikeduminy commentedon Aug 31, 2023
An alternative would be to standardise the registries so that even yarn is loaded from NPM via the
@yarnpkg/cli-distpackage. This would make theCOREPACK_NPM_REGISTRYwork for yarn too.arcanis commentedon Aug 31, 2023
Couple of notes:
Of course the preferred solution here should be to allowlist the
repo.yarnpkg.comdomain. The rest of the discussion is around "how to workaround that the ideal solution isn't achievable due to non-Corepack-related reasons" - so that weights a bit in the level of complexity it's reasonable to add into Corepack, imo.I'd prefer to keep the configuration file an implementation detail, as I could imagine it changing from time to time (perhaps its content, or perhaps we'd add a new range, etc). Allowing unconstrained modification sounds risky.
I considered using
@yarnpkg/cli-dist, but it'd make it complicated to compute the same checksum between people using old Corepack (which relies on the url) and possible new Corepack (which would rely on the package).Some options I could be more favorable to:
COREPACK_YARN_PATTERNvariable to override the default one (it could be awkward if we add another range later on with a different url pattern, but I don't see that happening)corepack.config.jsfile where you could export a list of regexp to apply to any requested urls (but it requires a configuration file, which we currently avoid to allow easier configuration in cloud environments).It's probably fine to add
application/json;q=0.8to the headers by default, it seems a small change and makes sense from a purely semantical point of view.Regarding your bin patching, it's not clear to me why this would be needed, can you detail more?
mikeduminy commentedon Aug 31, 2023
Thanks for taking the time to consider this and reply. I'll get back to you on the bin patching question, but the rest of your suggestions seem fine to me.
I originally suggested the config approach because it would avoid these hyper specific env vars which could make it tougher to maintain in the future. These things are always a trade-off and I value your perspective here.
One quick question:
What would the effect of this checksum mismatch be? Would it basically cause corepack to re-download a previously downloaded yarn, or would it hard fail for users?
adityasrini commentedon Sep 27, 2023
Co-sign and wanted to say that I'm running into this issue at my workplace.
@arcanis Happy to make a relatively small PR contribution if it would incrementally solve this!
rotu commentedon Oct 8, 2023
I think a better way to specify this is to override yarn with a package specifier like
yarn@https://our-private-repo/@yarnpkg/cli-dist/-/cli-dist-{}.tgz. It's much more explicit than an environment variable. Unfortunately this seems currently broken (#313).simonhaenisch commentedon Oct 24, 2023
I believe the first problem with the headers was solved by #314?
I actually just ran into that cause I ran
corepack use yarn@4in a project to update the version which was saved with one checksum (fromrepo.yarnpkg.com), then our CI used Corepack with the above patch to use our internal registry where@yarnpkg/cli-dist@4.0.0apparently has a different checksum, so it failed to runyarn install -- immutablewithWe definitely need a way to configure those registries without having to patch corepack which is not feasible to do on every dev's machine.
christianblos commentedon Feb 15, 2024
I would love to see this feature as well. The lack of specifying a custom repository for yarn (including username and password for connecting to the repository) prevents me from using corepack at the moment.
lsrocha commentedon Apr 24, 2024
@mikeduminy The issues reported here were fixed in versions 0.22 and 0.26.
Setting a private registry using
COREPACK_NPM_REGISTRYnow leads to Yarn Berry being fetched correctly.lsrocha commentedon Apr 24, 2024
@simonhaenisch The checksum error was fixed in version 0.28