Build should error if package.json and package-lock.json are out of sync

[ryanmoran]

The buildpack doesn't realize that it needs to run npm install instead of npm ci in the situation where the developer has forgotten to update their package-lock.json file.

Replication steps

1. Create a Node.js app with the following package.json

   {
     "name": "myapp",
     "devDependencies": {
       "chalk-cli": "~4.1.0"
     },
     "scripts": {
       "start": "chalk red bold 'Unicorns & Rainbows'"
     }
   }

2. Run npm install --package-lock-only in the source directory
3. Run pack build myapp --buildpack gcr.io/paketo-buildpacks/nodejs. You should see logs like the following:

   ===> DETECTING
   [detector] paketo-buildpacks/node-engine 0.0.262
   [detector] paketo-buildpacks/npm         0.1.80
   ===> ANALYZING
   [analyzer] Previous image with name "index.docker.io/library/myapp:latest" not found
   ===> RESTORING
   ===> BUILDING
   [builder] Node Engine Buildpack 0.0.262
   [builder]   Resolving Node Engine version
   [builder]     Candidate version sources (in priority order):
   [builder]        -> "*"
   [builder]
   [builder]     Selected Node Engine version (using ): 10.22.0
   [builder]
   [builder]   Executing build process
   [builder]     Installing Node Engine 10.22.0
   [builder]       Completed in 8.517s
   [builder]
   [builder]   Configuring environment
   [builder]     NODE_ENV     -> "production"
   [builder]     NODE_HOME    -> "/layers/paketo-buildpacks_node-engine/node"
   [builder]     NODE_VERBOSE -> "false"
   [builder]
   [builder]     Writing profile.d/0_memory_available.sh
   [builder]       Calculates available memory based on container limits at launch time.
   [builder]       Made available in the MEMORY_AVAILABLE environment variable.
   [builder]
   [builder] NPM Buildpack 0.1.80
   [builder]   Resolving installation process
   [builder]     Process inputs:
   [builder]       node_modules      -> "Not found"
   [builder]       npm-cache         -> "Not found"
   [builder]       package-lock.json -> "Found"
   [builder]
   [builder]     Selected NPM build process: 'npm ci'
   [builder]
   [builder]   Executing build process
   [builder]     Running 'npm ci --unsafe-perm --cache /layers/paketo-buildpacks_npm/npm-cache'
   [builder]       Completed in 966ms
   [builder]
   [builder]   Configuring environment
   [builder]     NPM_CONFIG_LOGLEVEL   -> "error"
   [builder]     NPM_CONFIG_PRODUCTION -> "true"
   [builder]     PATH                  -> "$PATH:/layers/paketo-buildpacks_npm/modules/node_modules/.bin"
   ===> EXPORTING
   [exporter] Adding layer 'paketo-buildpacks/node-engine:node'
   [exporter] Adding layer 'paketo-buildpacks/npm:modules'
   [exporter] Adding 1/1 app layer(s)
   [exporter] Adding layer 'launcher'
   [exporter] Adding layer 'config'
   [exporter] Adding layer 'process-types'
   [exporter] Adding label 'io.buildpacks.lifecycle.metadata'
   [exporter] Adding label 'io.buildpacks.build.metadata'
   [exporter] Adding label 'io.buildpacks.project.metadata'
   [exporter] Setting default process type 'web'
   [exporter] *** Images (6153d789a581):
   [exporter]       index.docker.io/library/myapp:latest
   [exporter] Adding cache layer 'paketo-buildpacks/node-engine:node'
   Successfully built image myapp
   

4. Run docker run -it myapp. You should see that it cannot find the chalk command as that was in the devDependencies section of our package.json.

   > myapp@ start /workspace
   > chalk red bold 'Unicorns & Rainbows'
   
   sh: 1: chalk: not found
   npm ERR! code ELIFECYCLE
   npm ERR! syscall spawn
   npm ERR! file sh
   npm ERR! errno ENOENT
   npm ERR! myapp@ start: `chalk red bold 'Unicorns & Rainbows'`
   npm ERR! spawn ENOENT
   npm ERR!
   npm ERR! Failed at the myapp@ start script.
   npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
   
   npm ERR! A complete log of this run can be found in:
   npm ERR!     /home/cnb/.npm/_logs/2020-09-16T17_04_01_831Z-debug.log
   

5. Update the package.json to move the chalk-cli from devDependencies to dependencies. This should make it so that it is available on the $PATH and callable in our start command.

   {
     "name": "myapp",
     "dependencies": {
       "chalk-cli": "~4.1.0"
     },
     "scripts": {
       "start": "chalk red bold 'Unicorns & Rainbows'"
     }
   }

6. Run pack build myapp --buildpack gcr.io/paketo-buildpacks/nodejs. NOTE: we did not update the package-lock.json. Also note in the logs that the node_modules layer is reused. You should see logs like the following:

   ===> DETECTING
   [detector] paketo-buildpacks/node-engine 0.0.262
   [detector] paketo-buildpacks/npm         0.1.80
   ===> ANALYZING
   [analyzer] Restoring metadata for "paketo-buildpacks/node-engine:node" from app image
   [analyzer] Restoring metadata for "paketo-buildpacks/npm:modules" from app image
   ===> RESTORING
   [restorer] Restoring data for "paketo-buildpacks/node-engine:node" from cache
   ===> BUILDING
   [builder] Node Engine Buildpack 0.0.262
   [builder]   Resolving Node Engine version
   [builder]     Candidate version sources (in priority order):
   [builder]        -> "*"
   [builder]
   [builder]     Selected Node Engine version (using ): 10.22.0
   [builder]
   [builder]   Reusing cached layer /layers/paketo-buildpacks_node-engine/node
   [builder]
   [builder] NPM Buildpack 0.1.80
   [builder]   Resolving installation process
   [builder]     Process inputs:
   [builder]       node_modules      -> "Not found"
   [builder]       npm-cache         -> "Not found"
   [builder]       package-lock.json -> "Found"
   [builder]
   [builder]     Selected NPM build process: 'npm ci'
   [builder]
   [builder]   Reusing cached layer /layers/paketo-buildpacks_npm/modules
   ===> EXPORTING
   [exporter] Reusing layer 'paketo-buildpacks/node-engine:node'
   [exporter] Reusing layer 'paketo-buildpacks/npm:modules'
   [exporter] Adding 1/1 app layer(s)
   [exporter] Reusing layer 'launcher'
   [exporter] Reusing layer 'config'
   [exporter] Reusing layer 'process-types'
   [exporter] Adding label 'io.buildpacks.lifecycle.metadata'
   [exporter] Adding label 'io.buildpacks.build.metadata'
   [exporter] Adding label 'io.buildpacks.project.metadata'
   [exporter] Setting default process type 'web'
   [exporter] *** Images (bd9b56b816b1):
   [exporter]       index.docker.io/library/myapp:latest
   [exporter] Reusing cache layer 'paketo-buildpacks/node-engine:node'
   Successfully built image myapp
   

7. Again run docker run -it myapp and confirm that the chalk command is still not found on the $PATH:

   > myapp@ start /workspace
   > chalk red bold 'Unicorns & Rainbows'
   
   sh: 1: chalk: not found
   npm ERR! code ELIFECYCLE
   npm ERR! syscall spawn
   npm ERR! file sh
   npm ERR! errno ENOENT
   npm ERR! myapp@ start: `chalk red bold 'Unicorns & Rainbows'`
   npm ERR! spawn ENOENT
   npm ERR!
   npm ERR! Failed at the myapp@ start script.
   npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
   
   npm ERR! A complete log of this run can be found in:
   npm ERR!     /home/cnb/.npm/_logs/2020-09-16T17_10_52_761Z-debug.log
   

8. Run npm install --package-lock-only in the source directory. This will update the package-lock.json file.
9. Run pack build myapp --buildpack gcr.io/paketo-buildpacks/nodejs and see the following logs, noting that the node_modules layer is rebuilt this time:

   ===> DETECTING
   [detector] paketo-buildpacks/node-engine 0.0.262
   [detector] paketo-buildpacks/npm         0.1.80
   ===> ANALYZING
   [analyzer] Restoring metadata for "paketo-buildpacks/node-engine:node" from app image
   [analyzer] Restoring metadata for "paketo-buildpacks/npm:modules" from app image
   ===> RESTORING
   [restorer] Restoring data for "paketo-buildpacks/node-engine:node" from cache
   ===> BUILDING
   [builder] Node Engine Buildpack 0.0.262
   [builder]   Resolving Node Engine version
   [builder]     Candidate version sources (in priority order):
   [builder]        -> "*"
   [builder]
   [builder]     Selected Node Engine version (using ): 10.22.0
   [builder]
   [builder]   Reusing cached layer /layers/paketo-buildpacks_node-engine/node
   [builder]
   [builder] NPM Buildpack 0.1.80
   [builder]   Resolving installation process
   [builder]     Process inputs:
   [builder]       node_modules      -> "Not found"
   [builder]       npm-cache         -> "Not found"
   [builder]       package-lock.json -> "Found"
   [builder]
   [builder]     Selected NPM build process: 'npm ci'
   [builder]
   [builder]   Executing build process
   [builder]     Running 'npm ci --unsafe-perm --cache /layers/paketo-buildpacks_npm/npm-cache'
   [builder]       Completed in 2.559s
   [builder]
   [builder]   Configuring environment
   [builder]     NPM_CONFIG_LOGLEVEL   -> "error"
   [builder]     NPM_CONFIG_PRODUCTION -> "true"
   [builder]     PATH                  -> "$PATH:/layers/paketo-buildpacks_npm/modules/node_modules/.bin"
   ===> EXPORTING
   [exporter] Reusing layer 'paketo-buildpacks/node-engine:node'
   [exporter] Adding layer 'paketo-buildpacks/npm:modules'
   [exporter] Adding 1/1 app layer(s)
   [exporter] Reusing layer 'launcher'
   [exporter] Reusing layer 'config'
   [exporter] Reusing layer 'process-types'
   [exporter] Adding label 'io.buildpacks.lifecycle.metadata'
   [exporter] Adding label 'io.buildpacks.build.metadata'
   [exporter] Adding label 'io.buildpacks.project.metadata'
   [exporter] Setting default process type 'web'
   [exporter] *** Images (ddfb2f3a09c1):
   [exporter]       index.docker.io/library/myapp:latest
   [exporter] Reusing cache layer 'paketo-buildpacks/node-engine:node'
   [exporter] Adding cache layer 'paketo-buildpacks/npm:npm-cache'
   Successfully built image myapp
   

10. Run docker run -it myapp one last time, seeing the following output:

    > myapp@ start /workspace
    > chalk red bold 'Unicorns & Rainbows'
    
    Unicorns & Rainbows
    

Proposal

The buildpack should fail the build if the package.json has been updated without the package-lock.json also being updated. The buildpack should detect this situation and show a reasonable error message to the user suggesting that they may need to run npm install --package-lock-only in order to update their package-lock.json file. It looks like this is at least impacts the npm ci build process, but it may also impact other build processes. We should ensure we have coverage of this case for all chosen build processes if applicable.

[ForestEckhardt]

Would it be appropriate to just warn the user through build output that their app may be out of date. So we could take a checksum of package.json and if that changes between runs but we decide to reuse the layer we could print a message to the effect of The package.json has changed but node_module/npm-cache/package-lock.json has stayed the same this may result in unexpected behavior please regenerate those artifacts.?

[ryanmoran]

I think a warning here is too subtle. Several users have already reported bugs that had this root issue. It gets reported as "the buildpack isn't updating my dependency" and then we discover that the package-lock.json was not updated. It causes confusion and results in invalid bug reports showing up here. I think it merits a build failure so that we can inform the user that their source is not validly configured.

[fg-j]

From what I can see of the npm-ci docs,

If dependencies in the package lock do not match those in package.json, npm ci will exit with an error, instead of updating the package lock.

So I think if the npm ci build process decided it should still run even if the package-lock.json is unchanged, we can get a nice error message for free. Unfortunately, with about a bit more logic, we lose the performance benefit of reusing a valid cached node_modules since

If a node_modules is already present, it will be automatically removed before npm ci begins its install.