ods-quickstarters icon indicating copy to clipboard operation
ods-quickstarters copied to clipboard

Adding .npmrc and package-lock.json to .gitignore

Open sino92 opened this issue 4 years ago • 7 comments

Wrapping up the discussion from our last conversation.

For local development, we can inform developers to set up .npmrc to point to our private nexus instance. We can add this to README. At the moment, this information is missing for angular and ionic quickstarters. For deployment, we don't need the users to submit the nexus host info as node10-12 Jenkins agents already have the npm configuration baked into them. So that section in angular and ionic quickstarters can be removed. We just need to inform the users why its important not to push package-lock.json file as it will override and switch our private registry to global one. Thoughts? @cschweikert @ManuelFeller @rianet @michaelsauter

sino92 avatar Jun 19 '20 08:06 sino92

I'd think best practice would be to commit the lock file though? I don't know NPM, but I would always want to lock down what versions get installed (e.g. for stability, security, ...)?

michaelsauter avatar Jun 19 '20 13:06 michaelsauter

Regarding the package-lock.json we can not just say "do not push it". It has multiple purposes that we should not "throw away" - and also our company security team wants that file to be committed.

It contains

  • the exact version to be used
  • the URL where the package should be downloaded
  • the integrity checksum for the package
  • dependencies and their versions that were used when installing the package

The package.json "only" contains the package name and a string telling what version should be taken - and by default it is often set in a way that higher versions are OK, so we can run into different package resolving between client and server, making debugging harder.

To the point that Ionic does not need that this is also not the whole truth. While Angular based applications are fully built on our build agents the Ionic ones are built in our environment and in the Ionic build cloud as soon as the are compiled into a native app there. And since we have the package-lock.json with URL's pointing to our repository server / proxy the the .npmrc is required to allow their agent to connect to it.

I will try to visualize the "issue" and also add a suggestion soon ;-)

ManuelFeller avatar Jun 19 '20 13:06 ManuelFeller

In the below image I show which build environments a node based project has to go through. Terminology:

  • local .npmrc = the one in the code repository
  • global .npmrc = the one in the user home directory

The situation

The local machine can be any OS (Linux, macOS or Windows) and may lack the global .npmrc as well as the required environment variables for the content of the .npmrc in the repo (we should avoid to have the nexus credentials hardcoded in a .npmrc we check in to version control). But having a global .npmrc is not so common and needs to be created. We could do so for our work machines, but that means whenever you need to change / load from another repo (e.g. because you need to go on premise or back) you need to adjust that file - or a local .npmrc to override the global one. -> The local machine is the one that adds new packages, this one needs to be set up properly before you do any package adding, npm install or yarn install. Only this way you do not mess up the lockfiles. While a global .npmrc works it means also that you may have to adjust it when you do not get everything from one package source all the time. Here a local one could help and make it easy to catch up - and if you do not have the needed environment vars it would cause errors, hinting you that you need to set something up (which should be in some file like the README.md). We still have to make sure that we have a solution that works on all platforms; and similar as the global .npmrc with the "only one environment" issue we may have to look for something that sets the environment variables per project - and also works with yarn (?).

The build agent has everything, the correctly configured global .npmrc, the environment variables and the local .npmrc. Having the global one originates from https://github.com/opendevstack/ods-quickstarters/issues/275 and is made in a way that yarn also works (since I could not get yarn to work with environment variables inside the .npmrc). The intention was to make sure that whatever you build you have a default setting that points to the internal repo server - independent from the quickstarter. That global file there does not tackle the issue with the lockfiles when the y point to different serves; they would cause issues too if they point to a different location. But at least the first build works for sure if you are behind a proxy or in an isolated network.

In the "special case" Ionic we can not provide a global .npmrc, so we need to fall back to a local one. But we have environment variables that can be passed, and Ionic uses npm, not yarn, so a local .npmrc with environment variables works nicely.

So, what is my suggestion?

  • keep the global .npmrc in the build agent to have a safe fallback for builds without local .npmrc
  • keep the local .npmrc with environment variables to be able to check it in, have no credentials in there and be similar on all node based projects (we need to find out how this works best on all platforms and put it to the readme!)
  • do not use yarn if it can be avoided (or we need to find a way to make environment variables work with it)

I know this is not perfect, I hope we find something even better - or have to decide we make an exception for Ionic, have a specific setup of the developers machines, ... but I hope this helps to understand the current issue why we can not just remove one of the files in all node based project types.

ManuelFeller avatar Jun 19 '20 16:06 ManuelFeller

@ManuelFeller : big thanks for bringing light into this topic

regarding .npmrc files with environment variables or the yarn issue: what about (re-)creating the .npmrc file during builds like this:

echo "${NEXUS_HOST}/:_authToken=${NEXUS_TOKEN}" > ~/.npmrc

cschweikert avatar Jun 30 '20 13:06 cschweikert

@cschweikert I liked the idea. I can see that introduces two different build options in package.json scripts: local-build: sh generate-local-npmrc; npm install; tsc --watch vs build step in Jenkinsfile would be build-remote: npm install; tsc. I also notice we lack an env.template file where NEXUS_HOST and NEXUS_TOKEN could be added. For the README.md, we can notify the user with something like:

You can configure your local .npmrc for private Nexus Registry by running `npm run local-build`...

Thoughts? @ManuelFeller @cschweikert

sino92 avatar Jul 14 '20 10:07 sino92

Will a .env file be taken into consideration by npm or sh automatically? When it comes to nodejs you usually need to read out a .env file with an explicit call like require('dotenv').config(). And does sh also work on a Windows machine? What about providing some interactive nodejs script init-setup.js to run one-time for requesting those values from the developer?

cschweikert avatar Jul 22 '20 09:07 cschweikert

Hmmm, for Windows we need some testing (I have to admit until now I just assumed it will work; thanks for pointing to that @cschweikert), and .sh scripts would for sure need the Windows Subsystem for Linux installed. Can we expect that from every developer when nod development normally does not require that? I do not think so, it would be another layer / technology someone has to deal with. That's why I also like the idea of the init-setup.js since this would be a solution that runs with the framework that for sure is already present. I will try to check the windows behaviour in case of environment variables in a .npmrc and let you know soon(ish)...

ManuelFeller avatar Jul 23 '20 07:07 ManuelFeller