-
Notifications
You must be signed in to change notification settings - Fork 116
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
fix: allow _auth
to be set via environment
#428
base: master
Are you sure you want to change the base?
Conversation
I'm unsure why non-standard environment variables are used in this project (e.g. |
Could somebody please check this pull request? |
When using legacy auth, the env var `NPM_CONFIG__AUTH` can be used. Fixes semantic-release#324
74804f3
to
ddb390a
Compare
based on https://semantic-release.gitbook.io/semantic-release/support/faq#can-i-use-semantic-release-to-publish-a-package-on-artifactory, it looks like these were used to support legacy versions of artifcatory. this is a great example of why i'm hesitant to expand support for environment variables at all. i would rather remove than add additional complexity.
i prefer the idea of aligning with the standard if we do proceed down this path, but before putting additional effort into an implementation, let's first decide whether it is a path that is worth pursuing for the project.
you mentioned this in the related issue, but I'm still trying to get my head around the need for this. i'm familiar with artifactory as a registry, but i don't understand what you mean by "Artifactory uses NPM_CONFIG__AUTH or :_auth in .npmrc". in my current use of artifactory, packages are published to artifactory from a CI pipeline. the auth details are needed from that CI pipeline, so i set can you explain what you mean by "Artifactory uses NPM_CONFIG__AUTH or :_auth in .npmrc" and why only those options are available from where you are publishing from? |
From my reading of the original PR it doesn't seem like the pre-existing NPM environment variables were considered. If @pvdlg is still around maybe he could chime in? "Legacy auth" for npm is simply using username, password and email and doesn't define what the environment variables should be - they are defined separately and all prefixed with
Apologies for the lack of clarity. Artifactory can use In other words, when legacy auth was added to semantic-release, it overlooked the fact that the |
from both the original PR that you linked to and from the docs link that i included, the reason for supporting legacy auth was because of tools that did not yet support the use of tokens. that is no longer true for either tool that was mentioned. tokens should be preferred over un/pw at this point. the official registry no longer supports legacy auth at all rather than expanding support for legacy auth, i would rather consider removing support from semantic-release altogether. this would encourage more secure registry interactions and simplify the implementation of this plugin. what prevents you from convincing your ops team from making a token available to your builds. i recently worked through a similar situation for the artifactory instance that i work with and the improved security of using tokens helped with the decision. |
Does that mean that those organizations that do not use the official registry, or who use a registry without token support are left out in the cold?
For the general development ecosystem, I agree with you. However, for encouraging the use of semantic-release I disagree since it would prevent people adopting semantic-release that otherwise could.
This is in progress, but larger organizations tend to have some inertia to overcome, I would prefer to use semantic-release today. I guess it comes down to the goal of this library. Either it lightly wraps |
the intention is not to leave anyone out in the cold because of limitations of supported tooling. however, artifactory does support using tokens, but your organization has chosen to only enable the less-secure legacy auth approach at this point. i am not aware of any alternative npm registries that still do not support token auth at this point. this problem is solvable by your organization (although i do understand how difficult it can be to influence such changes and have battled similar situations myself).
"npm" can refer to multiple things. the "npm" registry no longer supports legacy auth. how much of the "npm" cli we lightly wrap is a decision that we do have to consider while maintaining the project, but it is not as simple assuming that only wrapping is always the best answer. we do prefer to use an existing standard implemented by the npm cli when it accomplishes the goals of the project. however, the core reason that there will be a point when legacy auth is dropped, even by the npm cli. i'm simply attempting to balance trade-offs to think through what the best steps are for this project.
i understand the situation you are in, but when considering the ongoing maintenance and encouraging good practices, i'm not convinced that this change deserves our attention over more pressing maintenance needs when you have a workaround and a path toward moving to the more appropriate use of tokens. i would be more interested in a similar change related to environment variables not related to legacy auth, but even alternatively named env vars could be handled like if there are documentation improvements that you think could better encourage the approach of using tokens over legacy auth, even to help explain the situation to ops teams of others in similar situations, we would appreciate suggestions as a PR. |
When using legacy auth, the env var
NPM_CONFIG__AUTH
can be used.Fixes #324