Installation script is started even if node is installed

[testuser7]

Version report

Jenkins and plugins versions report:

Plugin version 0.3.0

• What Operating System are you using (both controller, and any agents involved in the problem)?

Official node docker container: node:16

Reproduction steps

Use NPMWrapper inside docker image:

    docker.image('node:16').inside('-u 1000') {
        withNPMWrapper(credentialsId: 'ID', yarnEnabled: false) {
            ...

Results

Expected result:

If there is a node available installation should be skipped.

Actual result:

Installation is started inside official node docker container:

[Pipeline] {
[Pipeline] withNPMWrapper
[PROJECT] $ docker exec --env ******** *hash* bash -c ./nvm-installer
=> Downloading nvm from git to '/home/node/.nvm'

=> Cloning into '/home/node/.nvm'...

=> Compressing and cleaning up git repository

=> Appending nvm source string to /home/node/.bashrc
=> Appending bash_completion source string to /home/node/.bashrc

=> You currently have modules installed globally with `npm`. These will no
=> longer be linked to the active version of Node when you install a new node
=> with `nvm`; and they may (depending on how you construct your `$PATH`)
=> override the binaries of modules installed with `nvm`:

/usr/local/lib
+-- corepack@0.10.0
=> If you wish to uninstall them at a later point (or re-install them under your
=> `nvm` Nodes), you can remove them from the system Node as follows:

     $ nvm use system
     $ npm uninstall -g a_module

=> Installing Node.js version 16.12.0

Downloading and installing node v16.12.0...
Downloading https://nodejs.org/dist/v16.12.0/node-v16.12.0-linux-x64.tar.xz...

#=#=#                                                                         

#                                                                          2.1%
########                                                                  11.4%
###########################                                               37.7%
####################################                                      50.5%
##################################################                        69.9%
#############################################################             85.0%
######################################################################## 100.0%
Computing checksum with sha256sum
Checksums matched!

Now using node v16.12.0 (npm v8.1.0)
Creating default alias: default -> 16.12.0 (-> v16.12.0 *)

=> Node.js version 16.12.0 has been successfully installed
=> Close and reopen your terminal to start using nvm or run the following to use it now:

export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"  # This loads nvm
[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"  # This loads nvm bash_completion
bash: /var/lib/jenkins/.nvm/nvm.sh: No such file or directory
[Pipeline] // withNPMWrapper
[Pipeline] }

[jameslafferty]

@testuser7 I'm not quite sure what the best behavior would be in this scenario. I do think it's important to always respect the Node version in the .nvmrc maybe followed by the version in the engines section of package.json, maybe then falling back to whatever system version is available.

However, it sounds like you might be better off just using sh in your pipeline rather than this plugin since Node is already available. In any case, I don't think it's really a bug. Rather, it seems like a potentially valid feature request. I'm going to relabel it and I'll think on what might be the right behavior. Hopefully the holidays will give me a bit of time to play around with this!

[testuser7]

@jameslafferty Perhaps the best solution would be to add an option to skip installation (disabled by default)?

[jameslafferty]

@testuser7 As it's structured right now, the plugin includes nvm install (basically a no-op if the Node version is already installed using nvm, but not if you're running in a Node Docker container) before each command. The value-add for this plugin is mostly in helping DevOps think less about Node when devs are using an nvmrc. I'd like to carry that philosophy a bit further, but there's definitely work involved.

[gafrs]

This is also a problem with our Jenkins jobs and this plugin. Same plugin version, 0.3.0, build and controller hosts running CentOS 7. We use a downloaded tarball from NodeJS stable repository, because we are locked into a specific version - don't ask. This is not a container/image. The NodeJS is not installed with "nvm", our systems engineer and project management will not allow the use of nvm for the install - also a don't ask.

Selecting "Set NPM Environment" in Jenkins and selecting our credentials for our local repository causes the job to download the latest NodeJS using nvm, which is completely wrong on two levels: the "npm" command is already in the build user's path and should be used over any version the plugin is trying to retrieve; and second, the version that is retrieved is a "latest" development version of NodeJS (version 17.x at this time) and that should not be used for production deliveries.

The feature of the plugin we want to use is the credentials passing to log in to our repository. We of course could put these credentials in a manual shell script, however keeping the credentials in a vault is a better security and configuration management option, especially if you have 50+ jobs to modify if the credentials change in a shell script.

You mention the focus of the plugin is helping DevOps, however the final destination of a pipeline is always the production stack. With this plugin presuming the version of NodeJs it wants to use and downloading that on its own, it violates the existing tool chain and perhaps introduces errors down the line. I would suggest testing for a NodeJS install on the build host system using a couple different techniques (e.g. "nvm ls", check for npm in the $PATH, etc.) and if it doesn't exist with those methods, raise an error and kill execution of the job. The Jenkins admin should rightly have installed NodeJS already, considering that they are using a NodeJS build job.

[jameslafferty]

@gafrs I don't know that I agree that the Jenkins admin should need to consider NodeJS or its installation. Production could mean publishing to an npm repository, which would mean the Node version is the responsibility of the eventual consumer. It could also mean publishing webpack bundles to a CDN, in which case Node version also isn't an issue, except as part of the build. In cases where deployment to production involves server-side Node, the DevOps organization probably does need a pretty solid understanding of which version's being used and to control for that.

On the other hand, you do raise a fair point that if folks on the CI/CD side do care about Node, the plugin shouldn't be frustrating them in their efforts to set it. I'm looking into this, and wish I had more time to devote to it. I'd definitely welcome a PR in the meantime, if you've got time to dig deeper. Thank you for your feedback!