feat: Validate minimum npm version required to use --install-links

aws_lambda_builders/workflows/nodejs_npm/actions.py

@@ -7,8 +7,7 @@
 
 from aws_lambda_builders.actions import ActionFailedError, BaseAction, Purpose
 from aws_lambda_builders.utils import extract_tarfile
-
-from .npm import NpmExecutionError, SubprocessNpm
+from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError, SubprocessNpm
 
 LOG = logging.getLogger(__name__)
 

aws_lambda_builders/workflows/nodejs_npm/exceptions.py

@@ -0,0 +1,18 @@
+"""
+Exceptions for the Node.js workflow
+"""
+
+
+from aws_lambda_builders.exceptions import LambdaBuilderError
+
+
+class NpmExecutionError(LambdaBuilderError):
+    """
+    Exception raised in case NPM execution fails.
+    It will pass on the standard error output from the NPM console.
+    """
+
+    MESSAGE = "NPM Failed: {message}"
+
+    def __init__(self, **kwargs):
+        Exception.__init__(self, self.MESSAGE.format(**kwargs))

aws_lambda_builders/workflows/nodejs_npm/npm.py

@@ -4,20 +4,9 @@
 
 import logging
 
-LOG = logging.getLogger(__name__)
-
-
-class NpmExecutionError(Exception):
-
-    """
-    Exception raised in case NPM execution fails.
-    It will pass on the standard error output from the NPM console.
-    """
+from aws_lambda_builders.workflows.nodejs_npm.exceptions import NpmExecutionError
 
-    MESSAGE = "NPM Failed: {message}"
-
-    def __init__(self, **kwargs):
-        Exception.__init__(self, self.MESSAGE.format(**kwargs))
+LOG = logging.getLogger(__name__)
 
 
 class SubprocessNpm(object):
@@ -59,7 +48,7 @@ def run(self, args, cwd=None):
         :rtype: str
         :return: text of the standard output from the command
 
-        :raises aws_lambda_builders.workflows.nodejs_npm.npm.NpmExecutionError:
+        :raises aws_lambda_builders.workflows.nodejs_npm.exceptions.NpmExecutionError:
             when the command executes with a non-zero return code. The exception will
             contain the text of the standard error output from the command.
 

aws_lambda_builders/workflows/nodejs_npm/workflow.py

@@ -15,20 +15,30 @@
 )
 from aws_lambda_builders.path_resolver import PathResolver
 from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, BuildInSourceSupport, Capability
-
-from .actions import (
+from aws_lambda_builders.workflows.nodejs_npm.actions import (
     NodejsNpmCIAction,
     NodejsNpmInstallAction,
     NodejsNpmLockFileCleanUpAction,
     NodejsNpmPackAction,
     NodejsNpmrcAndLockfileCopyAction,
     NodejsNpmrcCleanUpAction,
 )
-from .npm import SubprocessNpm
-from .utils import OSUtils
+from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm
+from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils
 
 LOG = logging.getLogger(__name__)
 
+# npm>=8.8.0 supports --install-links
+MINIMUM_NPM_VERSION_INSTALL_LINKS = (8, 8)
+UNSUPPORTED_NPM_VERSION_MESSAGE = (
+    "Building in source was enabled, however the "
+    "currently installed npm version does not support "
+    "--install-links. Please ensure that the npm "
+    "version is at least 8.8.0. Switching to build "
+    f"in outside of the source directory.{os.linesep}"
+    "https://docs.npmjs.com/cli/v8/using-npm/changelog#v880-2022-04-27"
+)
+
 
 class NodejsNpmWorkflow(BaseWorkflow):
 
@@ -89,6 +99,12 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
             self.actions.append(CopySourceAction(self.source_dir, artifacts_dir, excludes=self.EXCLUDED_FILES))
 
         if self.download_dependencies:
+            if is_building_in_source and not self.can_use_install_links(subprocess_npm):
+                LOG.warning(UNSUPPORTED_NPM_VERSION_MESSAGE)
+
+                is_building_in_source = False
+                self.build_dir = self._select_build_dir(build_in_source=False)
+
             self.actions.append(
                 NodejsNpmWorkflow.get_install_action(
                     source_dir=source_dir,
@@ -235,3 +251,40 @@ def get_install_action(
         return NodejsNpmInstallAction(
             install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=install_links
         )
+
+    @staticmethod
+    def can_use_install_links(npm_process: SubprocessNpm) -> bool:
+        """
+        Checks the version of npm that is currently installed to determine
+        whether or not --install-links can be used
+
+        Parameters
+        ----------
+        npm_process: SubprocessNpm
+            Object containing helper methods to call the npm process
+
+        Returns
+        -------
+        bool
+            True if the current npm version meets the minimum for --install-links
+        """
+        try:
+            current_version = npm_process.run(["--version"])
+
+            LOG.debug(f"Currently installed version of npm is: {current_version}")
+
+            current_version = current_version.split(".")
+
+            major_version = int(current_version[0])
+            minor_version = int(current_version[1])
+        except (ValueError, IndexError):
+            LOG.debug(f"Failed to parse {current_version} output from npm for --install-links validation")
+            return False
+
+        is_older_major_version = major_version < MINIMUM_NPM_VERSION_INSTALL_LINKS[0]
+        is_older_patch_version = (
+            major_version == MINIMUM_NPM_VERSION_INSTALL_LINKS[0]
+            and minor_version < MINIMUM_NPM_VERSION_INSTALL_LINKS[1]
+        )
+
+        return not (is_older_major_version or is_older_patch_version)

aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py

@@ -14,17 +14,17 @@
     LinkSourceAction,
     MoveDependenciesAction,
 )
+from aws_lambda_builders.path_resolver import PathResolver
 from aws_lambda_builders.utils import which
 from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, BuildInSourceSupport, Capability
-
-from ...path_resolver import PathResolver
-from ..nodejs_npm import NodejsNpmWorkflow
-from ..nodejs_npm.npm import SubprocessNpm
-from ..nodejs_npm.utils import OSUtils
-from .actions import (
+from aws_lambda_builders.workflows.nodejs_npm import NodejsNpmWorkflow
+from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm
+from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils
+from aws_lambda_builders.workflows.nodejs_npm.workflow import UNSUPPORTED_NPM_VERSION_MESSAGE
+from aws_lambda_builders.workflows.nodejs_npm_esbuild.actions import (
     EsbuildBundleAction,
 )
-from .esbuild import EsbuildExecutionError, SubprocessEsbuild
+from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import EsbuildExecutionError, SubprocessEsbuild
 
 LOG = logging.getLogger(__name__)
 
@@ -98,6 +98,12 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
             )
 
         if self.download_dependencies:
+            if is_building_in_source and not NodejsNpmWorkflow.can_use_install_links(self.subprocess_npm):
+                LOG.warning(UNSUPPORTED_NPM_VERSION_MESSAGE)
+
+                is_building_in_source = False
+                self.build_dir = self._select_build_dir(build_in_source=False)
+
             self.actions.append(
                 NodejsNpmWorkflow.get_install_action(
                     source_dir=source_dir,

tests/unit/workflows/nodejs_npm/test_workflow.py

@@ -1,6 +1,8 @@
 import os
 from unittest import TestCase
-from unittest.mock import patch, call
+from unittest.mock import ANY, patch, call, Mock
+
+from parameterized import parameterized
 
 from aws_lambda_builders.actions import (
     CopySourceAction,
@@ -83,9 +85,12 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende
         self.assertIsInstance(workflow.actions[5], NodejsNpmrcCleanUpAction)
         self.assertIsInstance(workflow.actions[6], NodejsNpmLockFileCleanUpAction)
 
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
     def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir_external_manifest_and_build_in_source(
-        self,
+        self, can_use_links_mock
     ):
+        can_use_links_mock.return_value = True
+
         self.osutils.dirname.return_value = "not_source"
         self.osutils.file_exists.return_value = True
 
@@ -285,7 +290,10 @@ def test_workflow_uses_npm_ci_if_lockfile_exists_and_npm_ci_enabled(self):
         self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction)
         self.osutils.file_exists.assert_has_calls([call("source/package-lock.json")])
 
-    def test_build_in_source_without_download_dependencies_and_without_dependencies_dir(self):
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
+    def test_build_in_source_without_download_dependencies_and_without_dependencies_dir(self, can_use_links_mock):
+        can_use_links_mock.return_value = True
+
         source_dir = "source"
         artifacts_dir = "artifacts"
         workflow = NodejsNpmWorkflow(
@@ -304,7 +312,10 @@ def test_build_in_source_without_download_dependencies_and_without_dependencies_
         self.assertIsInstance(workflow.actions[2], CopySourceAction)
         self.assertIsInstance(workflow.actions[3], NodejsNpmrcCleanUpAction)
 
-    def test_build_in_source_with_download_dependencies(self):
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
+    def test_build_in_source_with_download_dependencies(self, can_use_links_mock):
+        can_use_links_mock.return_value = True
+
         source_dir = "source"
         artifacts_dir = "artifacts"
         workflow = NodejsNpmWorkflow(
@@ -327,7 +338,10 @@ def test_build_in_source_with_download_dependencies(self):
         self.assertEqual(workflow.actions[4]._dest, os.path.join(artifacts_dir, "node_modules"))
         self.assertIsInstance(workflow.actions[5], NodejsNpmrcCleanUpAction)
 
-    def test_build_in_source_with_download_dependencies_and_dependencies_dir(self):
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
+    def test_build_in_source_with_download_dependencies_and_dependencies_dir(self, can_use_links_mock):
+        can_use_links_mock.return_value = True
+
         source_dir = "source"
         artifacts_dir = "artifacts"
         workflow = NodejsNpmWorkflow(
@@ -353,7 +367,10 @@ def test_build_in_source_with_download_dependencies_and_dependencies_dir(self):
         self.assertIsInstance(workflow.actions[6], CopyDependenciesAction)
         self.assertIsInstance(workflow.actions[7], NodejsNpmrcCleanUpAction)
 
-    def test_build_in_source_with_dependencies_dir(self):
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
+    def test_build_in_source_with_dependencies_dir(self, can_use_links_mock):
+        can_use_links_mock.return_value = True
+
         source_dir = "source"
         artifacts_dir = "artifacts"
         workflow = NodejsNpmWorkflow(
@@ -373,3 +390,57 @@ def test_build_in_source_with_dependencies_dir(self):
         self.assertIsInstance(workflow.actions[2], CopySourceAction)
         self.assertIsInstance(workflow.actions[3], CopySourceAction)
         self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction)
+
+    @parameterized.expand(
+        [
+            ("8.8.0", True),
+            ("8.9.0", True),
+            ("8.7.0", False),
+            ("7.9.0", False),
+            ("9.9.0", True),
+            ("1.2", False),
+            ("8.8", True),
+            ("foo", False),
+            ("foo.bar", False),
+            ("", False),
+        ]
+    )
+    def test_npm_version_validation(self, returned_npm_version, expected_result):
+        workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "source/manifest")
+
+        npm_subprocess = Mock()
+        npm_subprocess.run = Mock(return_value=returned_npm_version)
+
+        result = workflow.can_use_install_links(npm_subprocess)
+
+        self.assertEqual(result, expected_result)
+
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.get_install_action")
+    def test_workflow_revert_build_in_source(self, install_action_mock, install_links_mock):
+        # fake having bad npm version
+        install_links_mock.return_value = False
+
+        source_dir = "source"
+        artifacts_dir = "artifacts"
+        scratch_dir = "scratch_dir"
+        NodejsNpmWorkflow(
+            source_dir=source_dir,
+            artifacts_dir=artifacts_dir,
+            scratch_dir=scratch_dir,
+            manifest_path="source/manifest",
+            osutils=self.osutils,
+            build_in_source=True,
+            dependencies_dir="dep",
+        )
+
+        # expect no build in source and install dir is
+        # artifacts, not the source
+        install_action_mock.assert_called_with(
+            source_dir=source_dir,
+            install_dir=artifacts_dir,
+            subprocess_npm=ANY,
+            osutils=ANY,
+            build_options=ANY,
+            install_links=False,
+        )

tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py

@@ -343,7 +343,10 @@ def test_no_download_dependencies_and_no_dependencies_dir_fails(self):
                 download_dependencies=False,
             )
 
-    def test_build_in_source(self):
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
+    def test_build_in_source(self, install_links_mock):
+        install_links_mock.return_value = True
+
         source_dir = "source"
         workflow = NodejsNpmEsbuildWorkflow(
             source_dir=source_dir,
@@ -382,9 +385,12 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende
         self.assertEquals(workflow.actions[2].install_dir, "scratch_dir")
         self.assertIsInstance(workflow.actions[3], EsbuildBundleAction)
 
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
     def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir_external_manifest_and_build_in_source(
-        self,
+        self, install_links_mock
     ):
+        install_links_mock.return_value = True
+
         self.osutils.dirname.return_value = "not_source"
 
         workflow = NodejsNpmEsbuildWorkflow(
@@ -404,3 +410,33 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende
         self.assertEquals(workflow.actions[1]._source, os.path.join("not_source", "node_modules"))
         self.assertEquals(workflow.actions[1]._dest, os.path.join("source", "node_modules"))
         self.assertIsInstance(workflow.actions[2], EsbuildBundleAction)
+
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.can_use_install_links")
+    @patch("aws_lambda_builders.workflows.nodejs_npm.workflow.NodejsNpmWorkflow.get_install_action")
+    def test_workflow_revert_build_in_source(self, install_action_mock, install_links_mock):
+        # fake having bad npm version
+        install_links_mock.return_value = False
+
+        source_dir = "source"
+        artifacts_dir = "artifacts"
+        scratch_dir = "scratch_dir"
+        NodejsNpmEsbuildWorkflow(
+            source_dir=source_dir,
+            artifacts_dir=artifacts_dir,
+            scratch_dir=scratch_dir,
+            manifest_path="source/manifest",
+            osutils=self.osutils,
+            build_in_source=True,
+            dependencies_dir="dep",
+        )
+
+        # expect no build in source and install dir is
+        # scratch, not the source
+        install_action_mock.assert_called_with(
+            source_dir=source_dir,
+            install_dir=scratch_dir,
+            subprocess_npm=ANY,
+            osutils=ANY,
+            build_options=ANY,
+            install_links=False,
+        )