Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Registries with non-conventional tarball paths #238

Closed
paul-sachs opened this issue Jun 18, 2019 · 21 comments · Fixed by #240 or #457
Closed

[Bug] Registries with non-conventional tarball paths #238

paul-sachs opened this issue Jun 18, 2019 · 21 comments · Fixed by #240 or #457
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone

Comments

@paul-sachs
Copy link

paul-sachs commented Jun 18, 2019

Describe the bug

I am trying to setup yarn v2 but I have some custom registries. When i run yarn config -c, and error is output saying that the key is not correct. From what i've seen here, the config looks correct.

I've tried using the yaml format for v2, but since my primary yarn command is v1, it seems necessary that the .yarnrc file is marked as v1.

To Reproduce

.yarnrc

# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1

npmScopes.org.npmRegistryServer "<url>"
yarn-path ".yarn/releases/yarn-berry.js"
enableGlobalCache true

Running yarn config -v outputs the following:

YN0034: Invalid configuration key "npmScopes.org.npmRegistryServer" in ~/.yarnrc

Environment if relevant (please complete the following information):

  • OS: MacOS 10.14.5
  • Node version 8.15.0
  • Yarn version 1.13.0 (with latest berry version installed from the readme instructions)
@paul-sachs paul-sachs added the bug Something isn't working label Jun 18, 2019
@paul-sachs
Copy link
Author

paul-sachs commented Jun 19, 2019

I have been able to work around this issue by putting the two yarnrc files (one in the project directory, one in the user directory):

./.yarnrc

# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1

yarn-path ".yarn/releases/yarn-berry.js"

~/.yarnrc

npmScopes:
    fss:
        npmRegistryServer: "<url>"

So I'm guessing it's probably because yarnrc v1 doesn't support the nested objects that are used in v2?

@paul-sachs
Copy link
Author

paul-sachs commented Jun 19, 2019

Next hurdle: The url to my custom registry is built on artifactory, and as such, looks something like this: https://my-host.com/artifactory/api/npm/npm-virtual/ so my yarnrc looks like:

npmScopes:
    org:
        npmRegistryServer: "https://my-host.com/artifactory/api/npm/npm-virtual/"
npmRegistries:
    //my-host.com/artifactory/api/npm/npm-virtual/:
        npmAlwaysAuth: true
        npmAuthIdent: "<email>:<password>"

And yarn is returning a 404. On quick investigation, it seems like yarn is trying to fetch the package at https://my-host.com/@org%2fpackage where it should be https://my-host.com/artifactory/api/npm/npm-virtual/@org%2fpackage... At least that's what I THINK yarn v1 fetches information from. The resolved in yarn.lock looks like https://my-host.com/artifactory/api/npm/npm-virtual/@org/package/-/@org/package-1.32.2.tgz#<sha>.

I'll try to dig into yarn berry to see if i can tweak this myself but some direction could be very useful.

@arcanis
Copy link
Member

arcanis commented Jun 19, 2019

So I'm guessing it's probably because yarnrc v1 doesn't support the nested objects that are used in v2?

Yes, the v1. To be honest I'm starting to wonder whether we shouldn't use a different name for the rc files, this whole thing is more confusing than I'd like 🙁

I'll try to dig into yarn berry to see if i can tweak this myself but some direction could be very useful.

Interesting - you can look for the following string and add some debug before to figure out what's the actual URL and headers sent to the registry:

const res = await got_1.default(target, { agent, body, headers, json, method, encoding: null });

I'd suggest to use a condition like if (target.includes('my-host')) to quickly spot the relevant information.

@paul-sachs
Copy link
Author

paul-sachs commented Jun 19, 2019

yep, just found it:

async function get(path, _a) {
  ...
  return await core_1.httpUtils.get(url_1.resolve(registry, path), Object.assign({ configuration, headers }, rest));
}

Which maps to this

This line resolves the registry (https://my-host.com/artifactory/api/npm/npm-virtual/) with a path (/@org%2fpackage), which, according to url docs, will resolve to https://my-host.com/@org%2fpackage

@paul-sachs
Copy link
Author

Now i just gotta figure out what our expected behavior is... should we just always join instead of resolve? Is there some weird support for always going to the host and then resolve from there? Does path (/@org%2fpackage) need a different resolution (ie somehow get to /artifactory/api/npm/npm-virtual/@org%2fpackage)

@arcanis
Copy link
Member

arcanis commented Jun 19, 2019

Oh 😮

No, this is definitely a bug I introduced in #233. Someone had reported a bug on Discord where Yarn was incorrectly joining paths (so it ended up as http://foo.com//hello, with two slashes), and I used url.resolve without thinking about the nested use case.

Ideally we should have:

  • http//foo.com + /bar = http//foo.com/bar
  • http//foo.com/ + /bar = http//foo.com/bar
  • http//foo.com/hello + /bar = http//foo.com/hello/bar
  • http//foo.com/hello/ + /bar = http//foo.com/hello/bar

@paul-sachs
Copy link
Author

Yeah that makes sense. I'll see if i can put a PR together with some tests... Though looking through the example PR, i don't know if i understand the test infrastructure. Looks entirely snapshot based. Might take me some time.

@paul-sachs
Copy link
Author

New fun issue: Authorization. I'm now getting 401 errors once i'm hitting my url. Best guess right now is that the authorization header doesn't work with artifactory. This was working through these old settings in npmrc:

//my-host.com/artifactory/api/npm/npm-virtual/:_password=<password>
//my-host.com/artifactory/api/npm/npm-virtual/:username=<username>
//my-host.com/artifactory/api/npm/npm-virtual/:email=<email>
//my-host.com/artifactory/api/npm/npm-virtual/:always-auth=true

I've tried configuring the new v2 settings with what i figured were the analogues:

npmRegistries:
    //my-host.com/artifactory/api/npm/npm-virtual/:
        npmAlwaysAuth: true
        npmAuthIdent: <username>:<password>

But it seems that is not sufficient. Whatever v1 was doing to authorize, v2 doesn't seem to following the same scheme.

@paul-sachs paul-sachs changed the title [Bug] Setting up custom registries [Bug] Setting up custom registries (artifactory) Jun 19, 2019
@paul-sachs
Copy link
Author

paul-sachs commented Jun 19, 2019

Alright, one more step up the ladder and one more blocker. I've fixed the login issue, had to use the artifactory provided _auth field with npmAuthIdent. That got me past the auth problem. Now it says it can't find the package again, though now it says it's trying to follow the following path:
https://my-host.com/artifactory/api/npm/npm-virtual/@org/package/-/package-1.3.5.tgz

Looking at verbose output from npm proper, i believe the path it needs to try is actually: https://my-host.com/artifactory/api/npm/npm-virtual/@org/package/-/@org/package-1.3.5.tgz

Notice the extra @org in the npm version.

Not sure where the resolution logic is for this one but npm install logic is still very much a black box to me.

@arcanis
Copy link
Member

arcanis commented Jun 20, 2019

Re: the invalid path concatenation, I've submitted a fix via #240.

Re: the invalid auth, I believe it's because you need to encode the string to base64 (npmAuthIdent is the equivalent of _auth, which was base64 encoded, ie echo -n 'username:password' | openssl base64).


Re: the @org thing, this seems to be a compatibility problem with Artifactory. In more details: the npm registry returns a field called tarball when fetching the package metadata. This field contains the url of the tarball on the server.

The v2 currently ignores this field and loads the package based on the convention that the tarball is located at <scope>/<name>/-/<name>-<version>.tgz. This is needed to finally get rid of the registry hostnames in the lockfile (which was one of the most requested features: yarnpkg/yarn#5892).

The problem is that your Artifactory instance doesn't seem to follow this convention, and instead uses the tarball field to know where to get the package from 🙁

@arcanis arcanis reopened this Jun 20, 2019
@paul-sachs
Copy link
Author

@arcanis that's a bummer about artifactory going against convention. I suppose filing a bug there might be my only way to get around that... Is there anyway this can be done with a plugin/hook? I've seen code around allowing the plugin architecture but am not sure about if it's possible to intercept the resolution and make that change. If I was able to look, see the scope resolution and add in the additional logic specific to artifactory?

@arcanis
Copy link
Member

arcanis commented Jun 20, 2019

I could see one way to make this work: when resolving the metadata, we could detect whether the tarball field is convention-compliant. If it is, then we resolve it to npm:x.y.z, as we currently do. If it isn't, we instead resolve it to its fully-qualified url (so https://...), and print a small warning to inform of the potential issue.

You won't be able to change the registry url for such packages without manually editing the lockfile (just like it was in the v1), but at least it wouldn't affect most projects and wouldn't break compatibility.

Would you like to give it a try? I think it should just be about adding a line here to check what's contained in registryData.versions[version].dist.tarball.

@paul-sachs
Copy link
Author

Yeah i'll give it a shot. Just got pulled onto something else for today, so i'll take a look at this sometime tomorrow. Thanks for all your help!

@paul-sachs
Copy link
Author

So i'm getting back to this sooner than i thought. Interesting behavior I just noticed:

I configured a new project and installed yarn-berry there, and then i found that my ~/.yarnrc file (which had npmScopes configured) was wiped out at some point. Not sure exactly what happened but i had to reset the npmScopes in the config. Not sure what triggered this. Might file a separate issue to track that thing.

@arcanis arcanis changed the title [Bug] Setting up custom registries (artifactory) [Bug] Registries with non-conventional tarball paths Jun 24, 2019
@arcanis arcanis added the help wanted Extra attention is needed label Aug 7, 2019
@arcanis arcanis added this to the 2.0.0 milestone Aug 20, 2019
@arcanis
Copy link
Member

arcanis commented Sep 13, 2019

@psachs21 If you get the chance, can you confirm that master works with your Artifactory setup? You should be able to upgrade to master by running yarn set version berry && yarn set version from sources.

@paul-sachs
Copy link
Author

@arcanis I'll give it a shot. Sorry I wasn't able to make the modifications myself, i played around a little and then got pulled onto something entirely unrelated. I'll give it a shot.

@paul-sachs
Copy link
Author

@arcanis hmm, still failing to resolve the packages on artifactory. It's just reporting 404s. I'll need to spend some more time digging into the details, see what url it's trying and why it's failing.

@paul-sachs
Copy link
Author

So the new version is attempting to pull from registry.yarnpkg.com instead of the registry specified in .npmrc file. This looks like a totally different error.

@paul-sachs
Copy link
Author

paul-sachs commented Sep 14, 2019

ah... new version wiped out previous yarnrc options. Getting unauth now, but i think i'll need to wrangle some more with the config.

Edit So the auth is not being sent, despite a proper registry in the config.

Inspecting the response and associated request, i don't see anything auth being sent over the wire.

.yarnrc.yml

npmScopes:
  org:
    npmRegistryServer: "https://my-host.com/artifactory/api/npm/npm-virtual/"
npmRegistries:
  //my-host.com/artifactory/api/npm/npm-virtual/:
    npmAlwaysAuth: true
    npmAuthIdent: "...."

The npmAuthIdent is copied from the _auth in the .npmrc file which was working before. Not sure if some auth changes are needed.

@paul-sachs
Copy link
Author

more digging, getting a little farther. Turns out, unlike current .npmrc, npmRegistryServer must match npmRegistries entry pretty close (protocol and final / must match). This then seems to pick up npmAuthIdent. Still returning 401 but at least it's trying to send authentication now.

npmScopes:
  org:
    npmRegistryServer: "https://my-host.com/artifactory/api/npm/npm-virtual"
npmRegistries:
  https://my-host.com/artifactory/api/npm/npm-virtual:
    npmAlwaysAuth: true
    npmAuthIdent: "...."

@Eli-Black-Work
Copy link

@paul-sachs, I want to say thank you. Your comments were invaluable in figuring out how to authenticate with Artifactory using Yarn Berry 🙂

For anyone else who is trying to get Yarn Berry working with Artifactory, your .yarnrc.yml file should look like this (unless your using scopes):

npmRegistryServer: "..."
npmAuthIdent: "your_artifactory_auth_token_not_your_username_and_password"
npmAlwaysAuth: true

npmAlwaysAuth must be set to true in order for your user to be truly authenticated. If this is not set, your user may be able to authenticate with the Artifactory server but then may have issues when trying to download a new version of a package that's not already in the Artifactory cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
3 participants