Properly infer .npmrc for PNPM

[deivid-rodriguez]

There are two issues:

• PNPM lockfile updater is not using NpmrcBuilder to properly configure private registries. This was an oversight from the initial PNPM implementation since all other lockfile updaters (NPM and Yarn) do it.

This issue is fixed by cherry-picking #8091 from @mburumaxwell.

• Even if an npmrc file is generated, the logic for generating it is incomplete. The NpmrcBuilder only looks in the lockfile for finding the proper registry vs scope configuration, but can't work if the lockfile does not include registries or if parsing a specific lockfile is not implemented. However, we have a smarter class, RegistryFinder, that can also infer the proper registry by directly querying it and checking which dependencies it serves. This class is already used by the UpdateChecker and there's a TODO note on top of NpmrcBuilder to refactor it to use RegistryFinder:

  dependabot-core/npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npmrc_builder.rb

  Lines 9 to 11 in 0d64ef2

  	# Build a .npmrc file from the lockfile content, credentials, and any
  	# committed .npmrc
  	# We should refactor this to use UpdateChecker::RegistryFinder

  .

The second commit refactors NpmrcBuilder to use RegistryFinder, but just for PNPM. Once I validate that this works properly, I can migrate NPM & Yarn too and completely remove lockfile parsing code from this class.

This PR should fix #7731.

[deivid-rodriguez]

This is hard to test, but I did add a test to check that a properly scoped registry is generated in the .npmrc file when only one of the dependencies is listed in a configured private registry.

I added a commit to simplify some npmrc generation tests at 0c0cc10. I could extract it to a separate PR but I'll set this as ready in any case.

I plan to follow up with other PRs implementing this for NPM and Yarn, and eventually completely removing direct parsing of lockfiles from this class. But I think this is a good low risk first step.

[jakecoffman]

Let's try it!

[deivid-rodriguez]

Ok let's try this!

[mburumaxwell]

I just got a chance to test this. I can confirm that the credentials issue is now fixed. However, updating one dependency took about half an hour on my machine. Looking at the logs, a request is made to the private registry for each dependency available in the lock file for each dependency that needs to be updated. I stopped after the first. My thoughts are that came from using the new RegistryFinder ...

How can we speed up the operation for finding the registry? Or run it once? I haven't studied much of how it works to offer valid ideas yet.

[deivid-rodriguez]

Thanks for testing @mburumaxwell. Yes, this is not performant. I see several improvements that can be made, like the one you suggest of running it just once, not once per dependency. I will look into it.

As a workaround, you can commit an npmrc file with proper registry and scope configuration so that Dependabot does not need to infer it.

[mburumaxwell]

I am not entirely sure that works. I tested this out with a repo that only pulls a few packages from one private feed and the rest from the default registry. Maybe my understanding of the npm ecosystem is low but here's the npmrc file:

auto-install-peers = true

registry=https://registry.npmjs.org/
@tingle:registry=https://pkgs.dev.azure.com/tingle/_packaging/tingle/npm/registry/
always-auth=true

It still goes slow.

I tried executing step by step and through the RegistryFinder, I noticed the value for registry_source_url will always be null because the public registry is in use.

dependabot-core/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/registry_finder.rb

Lines 268 to 274 in 3887e23

	def registry_source_url
	sources = dependency.requirements
	.map { |r| r.fetch(:source) }.uniq.compact
	.sort_by { |source| self.class.central_registry?(source[:url]) ? 1 : 0 }
	sources.find { |s| s[:type] == "registry" }&.fetch(:url)
	end

Consequently, that will fall on first_registry_with_dependency_details

dependabot-core/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/registry_finder.rb

Lines 33 to 35 in 3887e23

	def registry
	locked_registry || first_registry_with_dependency_details
	end

first_registry_with_dependency_details will always make at least one HTTP requests.

I created a sample to try figure out what is happening or how to solve it.

• https://dev.azure.com/tingle/dependabot/_build/results?buildId=66063&view=logs
• https://dev.azure.com/tingle/dependabot/_git/pnpm-auth-test

Could there be something I am missing?

[deivid-rodriguez]

Thanks for your investigation. I think that may be a problem with my patch. If scopes are already configured, I was not expecting to go through this logic. I'll look into it!

[mburumaxwell]

@deivid-rodriguez As you look into this, I made an assumption that if the registry is not present in the lockfile, the global one should be used. It increased the speed significantly (the numerous HTTP requests are no longer made).
See https://github.com/mburumaxwell/dependabot-core/commit/c1148dd2df82b48d3adad914fdbe607474e86e33

I do not know what else it affects but at least it works as expected and could be a hint on which direction to check.
If by chance it is the right direction, let me know then I can create a PR.

[deivid-rodriguez]

@yeikel Working on a fix at #8330, feel free to give it a try!