diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index f0b1fb90b..ffabac3b5 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/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__) diff --git a/aws_lambda_builders/workflows/nodejs_npm/exceptions.py b/aws_lambda_builders/workflows/nodejs_npm/exceptions.py new file mode 100644 index 000000000..ebce8621c --- /dev/null +++ b/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)) diff --git a/aws_lambda_builders/workflows/nodejs_npm/npm.py b/aws_lambda_builders/workflows/nodejs_npm/npm.py index 674b6bd9c..33b2598ef 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/npm.py +++ b/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. diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 2a2edebed..2f1fa1273 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -15,8 +15,7 @@ ) 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, @@ -24,11 +23,22 @@ 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) diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py index fdb07ce54..478b5dfc4 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py +++ b/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, diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 222cd7876..b95b2bfc2 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/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, + ) diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py index de4cfed15..d528072c4 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py +++ b/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, + )