Skip to content

Remove always-auth config option - #211

Open
tkrah wants to merge 1 commit into
jenkinsci:masterfrom
tkrah:feature/remove-always-auth
Open

Remove always-auth config option#211
tkrah wants to merge 1 commit into
jenkinsci:masterfrom
tkrah:feature/remove-always-auth

Conversation

@tkrah

@tkrah tkrah commented Aug 11, 2026

Copy link
Copy Markdown

Testing done

mvn clean install did run successfully.

There is no test, I removed the option and the test for that option.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests that demonstrate the feature works or the issue is fixed

- option gone since 2021 / npm v7, see
  npm/cli@72a7eeb
- https://docs.npmjs.com/cli/v6/using-npm/config is the last legacy
  version supporting this
- npm 12.x does warn about this with:

  npm warn Unknown user config "always-auth"  ...
  This will stop working in the next major version of npm. ...
@tkrah
tkrah requested a review from a team as a code owner August 11, 2026 15:30
@nfalco79

nfalco79 commented Aug 11, 2026

Copy link
Copy Markdown
Member

What about old npm versions? I mean I have an old NodeJS 5.x project. Is backward compatbile?
Should the option be removed based on the selected installed NodeJS version?

@tkrah

tkrah commented Aug 11, 2026

Copy link
Copy Markdown
Author

What about old npm versions? I mean I have an old NodeJS 5.x project. Is backward compatbile? Should the option be removed based on the selected installed NodeJS version?

The option never did in the first place what it should, see upstream change:

...
Prior to those changes, it was used, but not in the manner documented.
...

So if you really still use such an old npm version and it was used in an undocumented manner, you need to stay with an old version of this plugin - it is not backward compatible (if the plugin does use semantic versioning, this would mean a new major version).
I removed it because at least I thought, having glue code just to support an option which was removed 5 years ago already is not worth the effort to support that.

But if you want to support this, I could revert the change and add a whole new checkbox like the "force npm 9 format" one.
The user could than enable / disable the generation of this option.

Question would be what would be the default? Default to generate that legacy option to be backward compatible or default to omit it, so that users of that old version need to check it and all users of current versions just have nothing to do?

PS:

Using the version of Node.js to make this decision is not sufficient, because the used Node.js installation could use a different version of npm (different from what upstream did default package), so letting the user, who knows which npm version is used in the pipeline, is imho the better approach.

@nfalco79

Copy link
Copy Markdown
Member

the option was introduced to authenticate against private repository that worked in a different way than npm registry. Those variables indeed are included in the .npmrc file.
I mean NodeJS because each version bundle a specific version of npm so it's true you can change the npm version but into one of specific supported range, and typically you do not do.
Since the this plugin supports NodeJS 0.10 (and older). Unless it's a blocking issue (we use NodeJS 21.x in production and artifactory as private reigstry, we do not have any issues) I'm not so prone to accept changes.

@tkrah

tkrah commented Aug 11, 2026

Copy link
Copy Markdown
Author

the option was introduced to authenticate against private repository that worked in a different way than npm registry. Those variables indeed are included in the .npmrc file. I mean NodeJS because each version bundle a specific version of npm so it's true you can change the npm version but into one of specific supported range, and typically you do not do. Since the this plugin supports NodeJS 0.10 (and older). Unless it's a blocking issue (we use NodeJS 21.x in production and artifactory as private reigstry, we do not have any issues) I'm not so prone to accept changes.

I do use Node.js 24 with npm 12.0.2 (Artifactory) and I don't use that always-auth option since ages (because since npm version 7 is is not used anyway anymore) and it works as expected - which warns me that with npm version 13.x this will not work anymore. I would rather add an option (backward compatible to be able to omit that option) to support the upcoming npm version 13.x and not wait for it to break when that happens, don't you think?

It is not a blocking issue now, but with the release of npm 13 it will be (and the whole option does not exist anymore since 5 years / npm 7.x - which would be Node.js 14 with npm 6.x if we take the default packaging) - so it should never have been generated in the first place anymore for npm 7+ imho:

npm warn Unknown user config "always-auth" ... This will stop working in the next major version of npm.

@nfalco79

Copy link
Copy Markdown
Member

In version 11 was the same warning, it's just a message to say "hey i have said that, i'm justified to run exception".

https://jfrog.com/help/r/artifactory-changes-to-the-login-behavior-in-npm-v9/artifactory-changes-to-the-login-behavior-in-npm-v9
There why i implemented the flag npm9format in the config file.

@tkrah

tkrah commented Aug 11, 2026

Copy link
Copy Markdown
Author

In version 11 was the same warning, it's just a message to say "hey i have said that, i'm justified to run exception".

https://jfrog.com/help/r/artifactory-changes-to-the-login-behavior-in-npm-v9/artifactory-changes-to-the-login-behavior-in-npm-v9 There why i implemented the flag npm9format in the config file.

Yeah, they changed the format of the npmrc file - which demanded a switch here.

But why would you generate a config option which is not valid anymore for npm 7+?
We could at least omit the whole thing, if we detect a Node.js version greater than Node.js 14 right? The option is gone in that version of the bundled npm anyway.

I just don't think we should wait until it breaks, we could just remove / omit it already for non ancient versions of Node.js (npm) - but your decision ;)

PS: Btw, npm 12.0.x has those engines defined:

  "engines": {
    "node": "^22.22.2 || ^24.15.0 || >=26.0.0"
  }

So running e.g. Node.js 24.x with npm 12.x is not unexpected imho.

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.

2 participants