Closed cdaringe closed 5 months ago
What versions are you using of semantic-release and the npm plugin?
Ok, i've learned some more.
npm config list
reads data from <workspace>/.npmrc
and my <prefix>/globalconfig
.<workspace>/.npmrc
and ../../<workspace>/.npmrc
, which is a grantparent. More specifically:13:04:37 "configs": [
13:04:37 "/mnt/jenkinspan/.npmrc", # this config file is not processed by npm list config when run in my project checkout
13:04:37 "/mnt/jenkinspan/workspace/myorg/foo/ws/.npmrc"
13:04:37 ]
// set-npmrc-auth.js
import rc from "rc";
rc();
tldr, sem-rels use of rc() writes an incompatible npm config with my system, where if it used just my checkout's npm config, it would work successfully. rc
does not appear to match the way npm sources configuration files? still digging.
My intuition says that this config generation method creates opporitunity for desync.
Instead of reading files and checking for valid auth, we should seriously consider querying npm to produce it's full configuration. We're we to have captured npm config list
as the source of config before calling verify-auth, it would have said "ya, this system is auth ready." However, instead of consulting npm, we tried to re-create what npm does, but it was not aligned, and can lead to two different failures:
tldr, sem-rels use of rc() writes an incompatible npm config with my system
the npm plugin used to write a config that use the _auth
approach that your error describes. this is because it used to be a valid approach. however, we dropped support for that approach in v10.0.0 of the npm plugin once npm dropped support. this is why i was curious about what versions you were using.
since you are using a version that is significantly newer than that change, it does not directly explain your problem. does the version you mentioned above match the one in the release workflow output? this feels like you have another (older) version of semantic-release involved somewhere.
also, are NPM_USERNAME
and NPM_PASSWORD
provided to your release workflow as environment variables?
Instead of reading files and checking for valid auth, we should seriously consider querying npm to produce it's full configuration. We're we to have captured
npm config list
as the source of config before calling verify-auth, it would have said "ya, this system is auth ready." However, instead of consulting npm, we tried to re-create what npm does, but it was not aligned, and can lead to two different failures:
- first, reading in a previously unread file (which corrupted my config), and second,
- not consulting the prefix/global config file (which dropped my critical, correct auth config!)
i wont disagree that there is opportunity for improvement, but there are reasons that the auth process is how it is today. our goal is to leverage npm config as much as possible, but it is important to understand that auth is expected to be provided in the NPM_TOKEN
environment variable. if this is done as expected, semantic-release will produce a valid config that factors in the remaining config from your project .npmrc
with auth injected correctly.
npm plugin once npm dropped support ... does the version you mentioned above match the one
The error message is directly from sem-rel running of the latest version of npm
(latest at the time of writing). And indeed, I can assert latest versions of all sem-rel packages were being used. The failure mode is/was isolated to the latest version of the npm sem-rel plugin (this package).
This package can generate an incorrect .npmrc, used in its temdir. My CI system had an old, incorrect evil floating .npmrc file on disk somewhere in a parent folder, which I did not place, nor do I service as a user of this CI system. I talked with the CI admin to remove it. NPM_TOKEN or no NPM_TOKEN, this package has code which concats all parent npmrcs which is no longer correct behavior, ref https://docs.npmjs.com/cli/v7/configuring-npm/npmrc#files
I think we're aligned on that, but re-expressing just for clarity sake.
but it is important to understand ... if this is done as expected
This is incorrect. The following is all stated with courtesy. This lib currently supports checking for _auth
, and can fail incorrectly due to bad concat'ing. "done as expected" is surprising language given the state of the code. The lib supports this now despite the unsupported claim: https://github.com/semantic-release/npm/blob/86011db51801fedf17eec1e9611b1cbe8e46cafa/lib/set-npmrc-auth.js#L21-L26
_Even if I had set NPMTOKEN, that failing code block would have triggered and my publish failed due to the rc
concatenating in a dead rc file. FWIW, I do rely on my auth being provided by my CI providing team, so NPM_TOKEN
isn't available to me, which is... a thing 😵 .
It's a subtle and nuanced failure mode, no doubt.
My CI system had an old, incorrect evil floating .npmrc file on disk somewhere in a parent folder, which I did not place, nor do I service as a user of this CI system. I talked with the CI admin to remove it.
does this mean that the trouble you faced is resolved in your case? if so, that gives us a bit of space to gather the details more clearly for improving the future handling of the configs
NPM_TOKEN or no NPM_TOKEN, this package has code which concats all parent npmrcs which is no longer correct behavior, ref docs.npmjs.com/cli/v7/configuring-npm/npmrc#files
as i mentioned, our goal is to align with npm as closely as possible, but we know we are currently not fully in alignment. appreciate you highlighting the details that you found that resulted in misalignment and surprise. i'll open an issue to track this separately
This is incorrect. The following is all stated with courtesy.
honestly appreciate the persistence to get the right details in the open. i'll be honest, i've never provided auth through an .npmrc when using semantic-release, so i forgot that we supported that approach officially. for a long time, the env var was the only supported method of providing a token. i understand the cases where it fits how some teams prefer to configure CI servers, but i honestly thought we hadnt added that support yet. i can say with certainty that we no longer inject auth as _auth
since that was removed as part of https://github.com/semantic-release/npm/pull/437
This lib currently supports checking for
_auth
, and can fail incorrectly due to bad concat'ing.
i'm still a little bit confused here, so i'm hoping to just clarify some specifics. since it has been a bit since i've explored this particular part of our implementation for a while, i looked a little more closely. i see that registry-auth-token does find and return a token from an .npmrc file that uses the legacy unscoped approach. what i'm not clear about from your statement is what you mean about "bad concat'ing". are you saying that the approach of combining the files is introducing a syntax or formatting error, that the _auth
property should be stripped, or is this referring to your point that some found files should not be concat'ed because of the mismatch with default npm behvaior?
i'll open an issue to track this separately
does this mean that the trouble you faced is resolved in your case?
Yep!
honestly appreciate the persistence ... since #437
ya no problem.
what i'm not clear about from your statement is what you mean about "bad concat'ing".
Indeed, I should have used more precise language. I intended to mean "[referring my point] that some found files should not be concat'ed because of the mismatch with default npm behavior".
Edit: I realized you did grok my point above 😵 , so my expansion below wasn't needed :)
got it. that makes sense.
I realized you did grok my point above 😵 , so my expansion below wasn't needed :)
i do appreciate the confirmation so that i know for sure we are on the same page now
i agree that we should address this, and i think i have this covered in the additional issues created as a result of this thread. since you have a path forward with the current state of things, would it be fair to close this issue in favor of the others to address aligning better with current npm behavior?
Awesome, ya let's close!
Problem
Unable to release due to a common auth error regarding the
_auth
field which is not present in my config.The associated stack:
Research
Observation 1: The error claims
_auth
format is wrong, but my npm config has no_auth
set.npm config list
in my checkout in CI shows no _auth (evidence attached below).//npme.foo.com/:_auth=(redacted)
in it. It does not have_auth
!npm config list, immediately before calling semantic release
Observation 2: The command fails on
npm version
, per the stack trace, but the samenpm version
invocation works when run locally in my CI checkout, vs the/tmp/...
semrel uses.npm version 0.0.3 --userconfig ./.npmrc --no-git-tag-version --allow-same-version
Discussion
npm version
work in CI immediately before semantic-release, but not when sem-rel does stuff off in a temp dir?