fix: respect lockfile when building Node code

aws_lambda_builders/workflows/nodejs_npm/actions.py

@@ -112,14 +112,55 @@ def execute(self):
             raise ActionFailedError(str(ex))
 
 
-class NodejsNpmrcCopyAction(BaseAction):
+class NodejsNpmCIAction(BaseAction):
 
     """
-    A Lambda Builder Action that copies NPM config file .npmrc
+    A Lambda Builder Action that installs NPM project dependencies
+    using the CI method - which is faster and better reproducible
+    for CI environments, but requires a lockfile (package-lock.json
+    or npm-shrinkwrap.json)
+    """
+
+    NAME = "NpmCI"
+    DESCRIPTION = "Installing dependencies from NPM using the CI method"
+    PURPOSE = Purpose.RESOLVE_DEPENDENCIES
+
+    def __init__(self, artifacts_dir, subprocess_npm):
+        """
+        :type artifacts_dir: str
+        :param artifacts_dir: an existing (writable) directory with project source files.
+            Dependencies will be installed in this directory.
+        :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm
+        :param subprocess_npm: An instance of the NPM process wrapper
+        """
+
+        super(NodejsNpmCIAction, self).__init__()
+        self.artifacts_dir = artifacts_dir
+        self.subprocess_npm = subprocess_npm
+
+    def execute(self):
+        """
+        Runs the action.
+        :raises lambda_builders.actions.ActionFailedError: when NPM execution fails
+        """
+
+        try:
+            LOG.debug("NODEJS installing ci in: %s", self.artifacts_dir)
+
+            self.subprocess_npm.run(["ci"], cwd=self.artifacts_dir)
+
+        except NpmExecutionError as ex:
+            raise ActionFailedError(str(ex))
+
+
+class NodejsNpmrcAndLockfileCopyAction(BaseAction):
+
+    """
+    A Lambda Builder Action that copies lockfile and NPM config file .npmrc
     """
 
-    NAME = "CopyNpmrc"
-    DESCRIPTION = "Copying configuration from .npmrc"
+    NAME = "CopyNpmrcAndLockfile"
+    DESCRIPTION = "Copying configuration from .npmrc and dependencies from lockfile"
     PURPOSE = Purpose.COPY_SOURCE
 
     def __init__(self, artifacts_dir, source_dir, osutils):
@@ -135,7 +176,7 @@ def __init__(self, artifacts_dir, source_dir, osutils):
         :param osutils: An instance of OS Utilities for file manipulation
         """
 
-        super(NodejsNpmrcCopyAction, self).__init__()
+        super(NodejsNpmrcAndLockfileCopyAction, self).__init__()
         self.artifacts_dir = artifacts_dir
         self.source_dir = source_dir
         self.osutils = osutils
@@ -144,14 +185,15 @@ def execute(self):
         """
         Runs the action.
 
-        :raises lambda_builders.actions.ActionFailedError: when .npmrc copying fails
+        :raises lambda_builders.actions.ActionFailedError: when copying fails
         """
 
         try:
-            npmrc_path = self.osutils.joinpath(self.source_dir, ".npmrc")
-            if self.osutils.file_exists(npmrc_path):
-                LOG.debug(".npmrc copying in: %s", self.artifacts_dir)
-                self.osutils.copy_file(npmrc_path, self.artifacts_dir)
+            for filename in [".npmrc", "package-lock.json", "npm-shrinkwrap.json"]:
+                file_path = self.osutils.joinpath(self.source_dir, filename)
+                if self.osutils.file_exists(file_path):
+                    LOG.debug("%s copying in: %s", filename, self.artifacts_dir)
+                    self.osutils.copy_file(file_path, self.artifacts_dir)
 
         except OSError as ex:
             raise ActionFailedError(str(ex))

aws_lambda_builders/workflows/nodejs_npm/workflow.py

@@ -6,7 +6,13 @@
 from aws_lambda_builders.path_resolver import PathResolver
 from aws_lambda_builders.workflow import BaseWorkflow, Capability
 from aws_lambda_builders.actions import CopySourceAction, CopyDependenciesAction, MoveDependenciesAction, CleanUpAction
-from .actions import NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction
+from .actions import (
+    NodejsNpmPackAction,
+    NodejsNpmInstallAction,
+    NodejsNpmrcAndLockfileCopyAction,
+    NodejsNpmrcCleanUpAction,
+    NodejsNpmCIAction,
+)
 from .utils import OSUtils
 from .npm import SubprocessNpm
 
@@ -26,6 +32,7 @@ class NodejsNpmWorkflow(BaseWorkflow):
 
     EXCLUDED_FILES = (".aws-sam", ".git")
 
+    # pylint: disable=too-many-statements
     def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs):
 
         super(NodejsNpmWorkflow, self).__init__(
@@ -49,17 +56,23 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
             tar_dest_dir, scratch_dir, manifest_path, osutils=osutils, subprocess_npm=subprocess_npm
         )
 
-        npm_copy_npmrc = NodejsNpmrcCopyAction(tar_package_dir, source_dir, osutils=osutils)
+        npm_copy = NodejsNpmrcAndLockfileCopyAction(tar_package_dir, source_dir, osutils=osutils)
 
         self.actions = [
             npm_pack,
-            npm_copy_npmrc,
+            npm_copy,
             CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES),
         ]
 
         if self.download_dependencies:
+            lockfile_path = osutils.joinpath(source_dir, "package-lock.json")
+            shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json")
+
             # installed the dependencies into artifact folder
-            self.actions.append(NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm))
+            if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path):
+                self.actions.append(NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm))
+            else:
+                self.actions.append(NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm))
 
             # if dependencies folder exists, copy or move dependencies from artifact folder to dependencies folder
             # depends on the combine_dependencies flag

tests/integration/workflows/nodejs_npm/test_nodejs_npm.py

@@ -7,6 +7,8 @@
 from unittest import TestCase
 
 import mock
+from parameterized import parameterized
+
 from aws_lambda_builders.builder import LambdaBuilder
 from aws_lambda_builders.exceptions import WorkflowFailedError
 
@@ -97,7 +99,7 @@ def test_builds_project_with_remote_dependencies(self):
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
-        expected_modules = {"minimal-request-promise"}
+        expected_modules = {"minimal-request-promise", ".package-lock.json"}
         output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules")))
         self.assertEqual(expected_modules, output_modules)
 
@@ -117,10 +119,35 @@ def test_builds_project_with_npmrc(self):
 
         self.assertEqual(expected_files, output_files)
 
-        expected_modules = {"fake-http-request"}
+        expected_modules = {"fake-http-request", ".package-lock.json"}
         output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules")))
         self.assertEqual(expected_modules, output_modules)
 
+    @parameterized.expand(["package-lock", "shrinkwrap", "package-lock-and-shrinkwrap"])
+    def test_builds_project_with_lockfile(self, dir_name):
+        expected_files_common = {"package.json", "included.js", "node_modules"}
+        expected_files_by_dir_name = {
+            "package-lock": {"package-lock.json"},
+            "shrinkwrap": {"npm-shrinkwrap.json"},
+            "package-lock-and-shrinkwrap": {"package-lock.json", "npm-shrinkwrap.json"},
+        }
+
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, dir_name)
+
+        self.builder.build(
+            source_dir,
+            self.artifacts_dir,
+            self.scratch_dir,
+            os.path.join(source_dir, "package.json"),
+            runtime=self.runtime,
+        )
+
+        expected_files = expected_files_common.union(expected_files_by_dir_name[dir_name])
+
+        output_files = set(os.listdir(self.artifacts_dir))
+
+        self.assertEqual(expected_files, output_files)
+
     def test_fails_if_npm_cannot_resolve_dependencies(self):
 
         source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-deps")
@@ -185,11 +212,11 @@ def test_builds_project_with_remote_dependencies_with_download_dependencies_and_
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
-        expected_modules = {"minimal-request-promise"}
+        expected_modules = {"minimal-request-promise", ".package-lock.json"}
         output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules")))
         self.assertEqual(expected_modules, output_modules)
 
-        expected_modules = {"minimal-request-promise"}
+        expected_modules = {"minimal-request-promise", ".package-lock.json"}
         output_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules")))
         self.assertEqual(expected_modules, output_modules)
 

tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/excluded.js

@@ -0,0 +1,2 @@
+//excluded
+const x = 1;

tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/included.js

@@ -0,0 +1,2 @@
+//included
+const x = 1;

tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "package-lock-and-shrinkwrap",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["included.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "minimal-request-promise": "*" 
+  }
+}

tests/integration/workflows/nodejs_npm/testdata/package-lock/excluded.js

@@ -0,0 +1,2 @@
+//excluded
+const x = 1;

tests/integration/workflows/nodejs_npm/testdata/package-lock/included.js

@@ -0,0 +1,2 @@
+//included
+const x = 1;

tests/integration/workflows/nodejs_npm/testdata/package-lock/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "package-lock",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["included.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "minimal-request-promise": "*" 
+  }
+}

tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/excluded.js

@@ -0,0 +1,2 @@
+//excluded
+const x = 1;

tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/included.js

@@ -0,0 +1,2 @@
+//included
+const x = 1;

tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "shrinkwrap",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["included.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "minimal-request-promise": "*" 
+  }
+}