fix: respect lockfile when building Node code

aws_lambda_builders/workflows/nodejs_npm/actions.py

@@ -162,14 +162,14 @@ def execute(self):
             raise ActionFailedError(str(ex))
 
 
-class NodejsNpmrcCopyAction(BaseAction):
+class NodejsNpmrcAndLockfileCopyAction(BaseAction):
 
     """
-    A Lambda Builder Action that copies NPM config file .npmrc
+    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):
@@ -185,7 +185,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
@@ -194,14 +194,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"]:
+                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

@@ -4,7 +4,6 @@
 
 import logging
 import json
-from typing import List
 
 from aws_lambda_builders.path_resolver import PathResolver
 from aws_lambda_builders.workflow import BaseWorkflow, Capability
@@ -21,7 +20,7 @@
     NodejsNpmPackAction,
     NodejsNpmLockFileCleanUpAction,
     NodejsNpmInstallAction,
-    NodejsNpmrcCopyAction,
+    NodejsNpmrcAndLockfileCopyAction,
     NodejsNpmrcCleanUpAction,
     NodejsNpmCIAction,
     EsbuildBundleAction,
@@ -105,17 +104,19 @@ def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manife
         npm_pack = NodejsNpmPackAction(
             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_npmrc_and_lockfile = NodejsNpmrcAndLockfileCopyAction(tar_package_dir, source_dir, osutils=osutils)
 
         actions = [
             npm_pack,
-            npm_copy_npmrc,
+            npm_copy_npmrc_and_lockfile,
             CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES),
         ]
 
         if self.download_dependencies:
             # installed the dependencies into artifact folder
-            actions.append(NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm))
+            install_action = NodejsNpmWorkflow.get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils)
+            actions.append(install_action)
 
             # if dependencies folder exists, copy or move dependencies from artifact folder to dependencies folder
             # depends on the combine_dependencies flag
@@ -169,19 +170,13 @@ def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutil
         :rtype: list
         :return: List of build actions to execute
         """
-        lockfile_path = osutils.joinpath(source_dir, "package-lock.json")
-        shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json")
         npm_bin_path = subprocess_npm.run(["bin"], cwd=source_dir)
         executable_search_paths = [npm_bin_path]
         if self.executable_search_paths is not None:
             executable_search_paths = executable_search_paths + self.executable_search_paths
         subprocess_esbuild = SubprocessEsbuild(osutils, executable_search_paths, which=which)
 
-        if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path):
-            install_action = NodejsNpmCIAction(source_dir, subprocess_npm=subprocess_npm)
-        else:
-            install_action = NodejsNpmInstallAction(source_dir, subprocess_npm=subprocess_npm, is_production=False)
-
+        install_action = NodejsNpmWorkflow.get_install_action(source_dir, source_dir, subprocess_npm, osutils, False)
         esbuild_action = EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild)
         return [install_action, esbuild_action]
 
@@ -213,3 +208,34 @@ def get_resolvers(self):
         specialized path resolver that just returns the list of executable for the runtime on the path.
         """
         return [PathResolver(runtime=self.runtime, binary="npm")]
+
+    @staticmethod
+    def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils, is_production=True):
+        """
+        Get the install action used to install dependencies at artifacts_dir
+
+        :type source_dir: str
+        :param source_dir: an existing (readable) directory containing source files
+
+        :type artifacts_dir: str
+        :param artifacts_dir: Dependencies will be installed in this directory.
+
+        :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils
+        :param osutils: An instance of OS Utilities for file manipulation
+
+        :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm
+        :param subprocess_npm: An instance of the NPM process wrapper
+
+        :type is_production: bool
+        :param is_production: NPM installation mode is production (eg --production=false to force dev dependencies)
+
+        :rtype: BaseAction
+        :return: Install action to use
+        """
+        lockfile_path = osutils.joinpath(source_dir, "package-lock.json")
+        shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json")
+
+        if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path):
+            return NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm)
+        else:
+            return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm, is_production=is_production)

tests/integration/workflows/nodejs_npm/test_nodejs_npm.py

@@ -6,6 +6,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
 
@@ -120,6 +122,31 @@ def test_builds_project_with_npmrc(self):
         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")

tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py

@@ -38,7 +38,7 @@ def test_invokes_old_builder_without_feature_flag(self):
             runtime=self.runtime,
         )
 
-        expected_files = {"included.js", "node_modules", "excluded.js", "package.json"}
+        expected_files = {"included.js", "node_modules", "excluded.js", "package.json", "package-lock.json"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 

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": "*" 
+  }
+}