Skip to content

Conversation

@tomheller
Copy link
Contributor

Adds preliminary support for alternate npm registry

Fixes #148

@changeset-bot
Copy link

changeset-bot bot commented Apr 28, 2023

🦋 Changeset detected

Latest commit: 044cf6f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@codeshift/cli Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@danieldelcore danieldelcore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @tomheller 👋 Thanks so much for contributing, it's very much appreciated!

I think your changes will work for public registries, but unsure of the best way to approach private registries and if that should change our overall strategy here 🤔 I'd love to get your advice if it's something you're familiar with.

AFAIK, (and as stated here) there are multiple ways to auth against a private registry.

  1. Via an NPM token
  2. Via username, password and email

Perhaps instead of a --registry flag, we could create an --npmrc flag which would accept a path to your custom .npmrc. Which would then include one or more credentials for various registries.

For example, here's an excerpt from one of mine:

@foobar:registry=https://packages.foo.com/custom-npm-registry/npm/
@baz:registry=https://packages.baz.com/custom-npm-registry/npm/

//packages.foo.com/custom-npm-registry/npm/:_password=fff==
//packages.foo.com/custom-npm-registry/npm/:username=dd
//packages.foo.com/custom-npm-registry/npm/:email=dd@foo.com
//packages.foo.com/custom-npm-registry/npm/:always-auth=true

//packages.baz.com/custom-npm-registry/npm/:_password=fff==
//packages.baz.com/custom-npm-registry/npm/:username=dd
//packages.baz.com/custom-npm-registry/npm/:email=dd@foo.com
//packages.baz.com/custom-npm-registry/npm/:always-auth=true

That file could then be parsed and sent to the PluginManager via the npmRegistryConfig property, but unsure if that can handle multiple authentication methods at once.

Perhaps a combination of the two approaches could work here:

  • User provides the registry URL via --registry
  • User provides the path to --npmrc
  • The CLI will parse the npmrc and extract the credentials that match the registry (not sure if people would be comfortable with this)
  • The credentials will be passed onto PluginManager
  • Fin.

It all seems a bit messy 🤔 ...

Alternatively, we could just expose --registry and --registryToken and that would be all that we care about. 🤔 If you're running the CLI in a CI context then the token would just be an ENV variable but could be awkward for local runners...

@tomheller
Copy link
Contributor Author

tomheller commented May 1, 2023

Hey @danieldelcore! Always happy to help.

I feel like the whole npmrc parsing, as you as well have pointed out, could get quite messy. It also seems to still be an option for the future, if the simple --registry and --registryToken approach turns out to be too limited.

I think your changes will work for public registries, but unsure of the best way to approach private registries and if that should change our overall strategy here thinking I'd love to get your advice if it's something you're familiar with.

For our current setup, we have a internal registry deployed which has no authentication read protection other than physical network access. But an approach without any authentication will obviously not work for private access registries on npm or anywhere else.

We have never had the idea of actually using the CLI in an CI environment. In our cases, we have always had a developer running an update on packages.


I'd love to add the --registryToken flag authentication to this PR and some tests as well.

@danieldelcore
Copy link
Contributor

I'd love to add the --registryToken flag authentication to this PR and some tests as well.
That would be wonderful – thanks!

I agree, this approach seems like the best way forward and if we really need to go down the npmrc route and take on all of that complexity we can cross that bridge when we get there!

Thanks again! LMK when you're ready for a review!

@tomheller
Copy link
Contributor Author

@danieldelcore I have pushed a fixup with the following changes:

  • Added the registryToken flag as well to the CLI
  • Added registry and registryToken to the docs/website as well
  • Added some tests. For those I'm not sure how thorough I should go, as the majority of the logic is side loaded to the PluginManager anyways.

Let me know what you think.

Copy link
Contributor

@danieldelcore danieldelcore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, this looks great! Thanks again!

@danieldelcore danieldelcore marked this pull request as ready for review May 2, 2023 23:53
@danieldelcore danieldelcore merged commit c4a39a3 into hypermod-io:main May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(@codeshift/cli): fetching packages from private registry

2 participants