diff --git a/.github/workflows/pr-labeler.yml b/.github/workflows/pr-labeler.yml index c44d7f10..0567b1a2 100644 --- a/.github/workflows/pr-labeler.yml +++ b/.github/workflows/pr-labeler.yml @@ -17,7 +17,7 @@ jobs: pull-requests: write runs-on: ubuntu-latest steps: - - uses: actions/github-script@v6 + - uses: actions/github-script@v7 with: github-token: ${{secrets.GITHUB_TOKEN}} script: | diff --git a/aws_lambda_builders/__init__.py b/aws_lambda_builders/__init__.py index 047943ec..c35fe4e1 100644 --- a/aws_lambda_builders/__init__.py +++ b/aws_lambda_builders/__init__.py @@ -5,5 +5,5 @@ # Changing version will trigger a new release! # Please make the version change as the last step of your development. -__version__ = "1.42.0" +__version__ = "1.43.0" RPC_PROTOCOL_VERSION = "0.3" diff --git a/aws_lambda_builders/actions.py b/aws_lambda_builders/actions.py index ae3a8542..491bcc5a 100644 --- a/aws_lambda_builders/actions.py +++ b/aws_lambda_builders/actions.py @@ -57,7 +57,7 @@ class _ActionMetaClass(type): def __new__(mcs, name, bases, class_dict): cls = type.__new__(mcs, name, bases, class_dict) - if cls.__name__ == "BaseAction": + if cls.__name__ in ["BaseAction", "NodejsNpmInstallOrUpdateBaseAction"]: return cls # Validate class variables @@ -156,7 +156,12 @@ def __init__(self, source: Union[str, os.PathLike], dest: Union[str, os.PathLike self._dest = dest def execute(self): + source_path = Path(self._source) destination_path = Path(self._dest) + if not source_path.exists(): + # Source path doesn't exist, nothing to symlink + LOG.debug("Source path %s does not exist, skipping generating symlink", source_path) + return if not destination_path.exists(): os.makedirs(destination_path.parent, exist_ok=True) utils.create_symlink_or_copy(str(self._source), str(destination_path)) diff --git a/aws_lambda_builders/utils.py b/aws_lambda_builders/utils.py index 9f227ea0..4bcd43dc 100644 --- a/aws_lambda_builders/utils.py +++ b/aws_lambda_builders/utils.py @@ -198,6 +198,10 @@ def create_symlink_or_copy(source: str, destination: str) -> None: """Tries to create symlink, if it fails it will copy source into destination""" LOG.debug("Creating symlink; source: %s, destination: %s", source, destination) try: + if Path(destination).exists() and Path(destination).is_symlink(): + # The symlink is already in place, don't try re-creating it + LOG.debug("Symlink between %s and %s already exists, skipping generating symlink", source, destination) + return os.symlink(Path(source).absolute(), Path(destination).absolute()) except OSError as ex: LOG.warning( diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index ffabac3b..bb45fb1e 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -72,17 +72,14 @@ def execute(self): raise ActionFailedError(str(ex)) -class NodejsNpmInstallAction(BaseAction): - +class NodejsNpmInstallOrUpdateBaseAction(BaseAction): """ - A Lambda Builder Action that installs NPM project dependencies + A base Lambda Builder Action that is used for installs or updating NPM project dependencies """ - NAME = "NpmInstall" - DESCRIPTION = "Installing dependencies from NPM" PURPOSE = Purpose.RESOLVE_DEPENDENCIES - def __init__(self, install_dir: str, subprocess_npm: SubprocessNpm, install_links: Optional[bool] = False): + def __init__(self, install_dir: str, subprocess_npm: SubprocessNpm): """ Parameters ---------- @@ -90,14 +87,20 @@ def __init__(self, install_dir: str, subprocess_npm: SubprocessNpm, install_link Dependencies will be installed in this directory. subprocess_npm : SubprocessNpm An instance of the NPM process wrapper - install_links : Optional[bool] - Uses the --install-links npm option if True, by default False """ - super(NodejsNpmInstallAction, self).__init__() + super().__init__() self.install_dir = install_dir self.subprocess_npm = subprocess_npm - self.install_links = install_links + + +class NodejsNpmInstallAction(NodejsNpmInstallOrUpdateBaseAction): + """ + A Lambda Builder Action that installs NPM project dependencies + """ + + NAME = "NpmInstall" + DESCRIPTION = "Installing dependencies from NPM" def execute(self): """ @@ -109,9 +112,38 @@ def execute(self): LOG.debug("NODEJS installing in: %s", self.install_dir) command = ["install", "-q", "--no-audit", "--no-save", "--unsafe-perm", "--production"] - if self.install_links: - command.append("--install-links") + self.subprocess_npm.run(command, cwd=self.install_dir) + + except NpmExecutionError as ex: + raise ActionFailedError(str(ex)) + + +class NodejsNpmUpdateAction(NodejsNpmInstallOrUpdateBaseAction): + """ + A Lambda Builder Action that installs NPM project dependencies + """ + NAME = "NpmUpdate" + DESCRIPTION = "Updating dependencies from NPM" + + def execute(self): + """ + Runs the action. + + :raises lambda_builders.actions.ActionFailedError: when NPM execution fails + """ + try: + LOG.debug("NODEJS updating in: %s", self.install_dir) + + command = [ + "update", + "--no-audit", + "--no-save", + "--unsafe-perm", + "--production", + "--no-package-lock", + "--install-links", + ] self.subprocess_npm.run(command, cwd=self.install_dir) except NpmExecutionError as ex: diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 2f1fa127..cd111b53 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -22,6 +22,7 @@ NodejsNpmPackAction, NodejsNpmrcAndLockfileCopyAction, NodejsNpmrcCleanUpAction, + NodejsNpmUpdateAction, ) from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils @@ -119,7 +120,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim subprocess_npm=subprocess_npm, osutils=osutils, build_options=self.options, - install_links=is_building_in_source, + is_building_in_source=is_building_in_source, ) ) @@ -211,7 +212,7 @@ def get_install_action( subprocess_npm: SubprocessNpm, osutils: OSUtils, build_options: Optional[dict], - install_links: Optional[bool] = False, + is_building_in_source: Optional[bool] = False, ): """ Get the install action used to install dependencies. @@ -228,8 +229,8 @@ def get_install_action( An instance of OS Utilities for file manipulation build_options : Optional[dict] Object containing build options configurations - install_links : Optional[bool] - Uses the --install-links npm option if True, by default False + is_building_in_source : Optional[bool] + States whether --build-in-source flag is set or not Returns ------- @@ -245,12 +246,13 @@ def get_install_action( if (osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path)) and npm_ci_option: return NodejsNpmCIAction( - install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=install_links + install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=is_building_in_source ) - return NodejsNpmInstallAction( - install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=install_links - ) + if is_building_in_source: + return NodejsNpmUpdateAction(install_dir=install_dir, subprocess_npm=subprocess_npm) + + return NodejsNpmInstallAction(install_dir=install_dir, subprocess_npm=subprocess_npm) @staticmethod def can_use_install_links(npm_process: SubprocessNpm) -> bool: diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py index 1ea3e898..ae3ade1c 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py @@ -118,7 +118,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim subprocess_npm=self.subprocess_npm, osutils=self.osutils, build_options=self.options, - install_links=is_building_in_source, + is_building_in_source=is_building_in_source, ) ) diff --git a/requirements/dev.txt b/requirements/dev.txt index 457b4ecc..20af74f1 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -13,5 +13,5 @@ pyelftools~=0.30 # Used to verify the generated Go binary architecture in integr # formatter black==22.6.0; python_version < "3.8" -black==23.10.1; python_version >= "3.8" -ruff==0.1.4 +black==23.11.0; python_version >= "3.8" +ruff==0.1.5 diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index e34e66d9..279f6e19 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -315,6 +315,46 @@ def test_build_in_source_with_download_dependencies(self, runtime): output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",), ("nodejs20.x",)]) + def test_build_in_source_with_removed_dependencies(self, runtime): + # run a build with default requirements and confirm dependencies are downloaded + source_dir = os.path.join(self.temp_testdata_dir, "npm-deps") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=runtime, + build_in_source=True, + ) + + # dependencies installed in source folder + source_node_modules = os.path.join(source_dir, "node_modules") + self.assertTrue(os.path.isdir(source_node_modules)) + expected_node_modules_contents = {"minimal-request-promise", ".package-lock.json"} + self.assertEqual(set(os.listdir(source_node_modules)), expected_node_modules_contents) + + # update package.json with empty one and re-run the build then confirm node_modules are cleared up + shutil.copy2( + os.path.join(self.temp_testdata_dir, "no-deps", "package.json"), + os.path.join(self.temp_testdata_dir, "npm-deps", "package.json"), + ) + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=runtime, + build_in_source=True, + ) + # dependencies installed in source folder + source_node_modules = os.path.join(source_dir, "node_modules") + self.assertTrue(os.path.isdir(source_node_modules)) + self.assertIn(".package-lock.json", set(os.listdir(source_node_modules))) + self.assertNotIn("minimal-request-promise", set(os.listdir(source_node_modules))) + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",), ("nodejs20.x",)]) def test_build_in_source_with_download_dependencies_local_dependency(self, runtime): source_dir = os.path.join(self.temp_testdata_dir, "with-local-dependency") diff --git a/tests/unit/test_actions.py b/tests/unit/test_actions.py index 1314c5f6..e85b1277 100644 --- a/tests/unit/test_actions.py +++ b/tests/unit/test_actions.py @@ -12,6 +12,7 @@ MoveDependenciesAction, CleanUpAction, DependencyManager, + LinkSinglePathAction, ) @@ -262,3 +263,15 @@ def test_excludes_dependencies_from_source( @staticmethod def _convert_strings_to_paths(source_dest_list): return map(lambda item: (Path(item[0]), Path(item[1])), source_dest_list) + + +class TestLinkSinglePathAction(TestCase): + @patch("aws_lambda_builders.actions.os.makedirs") + @patch("aws_lambda_builders.utils.create_symlink_or_copy") + def test_skips_non_existent_source(self, mock_create_symlink_or_copy, mock_makedirs): + src = "src/path" + dest = "dest/path" + + LinkSinglePathAction(source=src, dest=dest).execute() + mock_create_symlink_or_copy.assert_not_called() + mock_makedirs.assert_not_called() diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index ba95185a..aecb8679 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,24 +1,27 @@ import platform +from pathlib import Path from unittest import TestCase -from unittest.mock import patch +from unittest.mock import patch, Mock, MagicMock from aws_lambda_builders import utils from aws_lambda_builders.utils import decode class Test_create_symlink_or_copy(TestCase): - @patch("aws_lambda_builders.utils.Path") @patch("aws_lambda_builders.utils.os") @patch("aws_lambda_builders.utils.copytree") - def test_must_create_symlink_with_absolute_path(self, patched_copy_tree, pathced_os, patched_path): + def test_must_create_symlink_with_absolute_path(self, patched_copy_tree, patched_os): source_path = "source/path" destination_path = "destination/path" - utils.create_symlink_or_copy(source_path, destination_path) - pathced_os.symlink.assert_called_with( - patched_path(source_path).absolute(), patched_path(destination_path).absolute() - ) + p = MagicMock() + p.return_value = False + + with patch("aws_lambda_builders.utils.Path.is_symlink", p): + utils.create_symlink_or_copy(source_path, destination_path) + + patched_os.symlink.assert_called_with(Path(source_path).absolute(), Path(destination_path).absolute()) patched_copy_tree.assert_not_called() @patch("aws_lambda_builders.utils.Path") @@ -34,6 +37,17 @@ def test_must_copy_if_symlink_fails(self, patched_copy_tree, pathced_os, patched pathced_os.symlink.assert_called_once() patched_copy_tree.assert_called_with(source_path, destination_path) + @patch("aws_lambda_builders.utils.Path") + @patch("aws_lambda_builders.utils.os") + @patch("aws_lambda_builders.utils.copytree") + def test_must_copy_if_symlink_fails(self, patched_copy_tree, pathced_os, patched_path): + source_path = "source/path" + destination_path = "destination/path" + utils.create_symlink_or_copy(source_path, destination_path) + + pathced_os.symlink.assert_not_called() + patched_copy_tree.assert_not_called() + class TestDecode(TestCase): def test_does_not_crash_non_utf8_encoding(self): diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index b95b2bfc..832ce53b 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -20,6 +20,7 @@ NodejsNpmrcCleanUpAction, NodejsNpmLockFileCleanUpAction, NodejsNpmCIAction, + NodejsNpmUpdateAction, ) @@ -107,7 +108,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende self.assertIsInstance(workflow.actions[3], CopySourceAction) self.assertEqual(workflow.actions[3].source_dir, "source") self.assertEqual(workflow.actions[3].dest_dir, "artifacts") - self.assertIsInstance(workflow.actions[4], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[4], NodejsNpmUpdateAction) self.assertEqual(workflow.actions[4].install_dir, "not_source") self.assertIsInstance(workflow.actions[5], LinkSinglePathAction) self.assertEqual(workflow.actions[5]._source, os.path.join("not_source", "node_modules")) @@ -331,7 +332,7 @@ def test_build_in_source_with_download_dependencies(self, can_use_links_mock): self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) - self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[3], NodejsNpmUpdateAction) self.assertEqual(workflow.actions[3].install_dir, source_dir) self.assertIsInstance(workflow.actions[4], LinkSinglePathAction) self.assertEqual(workflow.actions[4]._source, os.path.join(source_dir, "node_modules")) @@ -358,7 +359,7 @@ def test_build_in_source_with_download_dependencies_and_dependencies_dir(self, c self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) - self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[3], NodejsNpmUpdateAction) self.assertEqual(workflow.actions[3].install_dir, source_dir) self.assertIsInstance(workflow.actions[4], LinkSinglePathAction) self.assertEqual(workflow.actions[4]._source, os.path.join(source_dir, "node_modules")) @@ -442,5 +443,5 @@ def test_workflow_revert_build_in_source(self, install_action_mock, install_link subprocess_npm=ANY, osutils=ANY, build_options=ANY, - install_links=False, + is_building_in_source=False, ) diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py index a851e7bc..6773c45f 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py @@ -12,7 +12,11 @@ LinkSinglePathAction, ) from aws_lambda_builders.architecture import ARM64 -from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmInstallAction, NodejsNpmCIAction +from aws_lambda_builders.workflows.nodejs_npm.actions import ( + NodejsNpmInstallAction, + NodejsNpmCIAction, + NodejsNpmUpdateAction, +) from aws_lambda_builders.workflows.nodejs_npm_esbuild import NodejsNpmEsbuildWorkflow from aws_lambda_builders.workflows.nodejs_npm_esbuild.actions import EsbuildBundleAction from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import SubprocessEsbuild @@ -313,7 +317,7 @@ def test_workflow_uses_production_npm_version(self, get_workflow_mock): subprocess_npm=ANY, osutils=ANY, build_options=None, - install_links=False, + is_building_in_source=False, ) @patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.workflow.NodejsNpmEsbuildWorkflow._get_esbuild_subprocess") @@ -360,7 +364,7 @@ def test_build_in_source(self, install_links_mock): self.assertEqual(len(workflow.actions), 2) - self.assertIsInstance(workflow.actions[0], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[0], NodejsNpmUpdateAction) self.assertEqual(workflow.actions[0].install_dir, source_dir) self.assertIsInstance(workflow.actions[1], EsbuildBundleAction) self.assertEqual(workflow.actions[1]._working_directory, source_dir) @@ -405,7 +409,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende self.assertEqual(len(workflow.actions), 3) - self.assertIsInstance(workflow.actions[0], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[0], NodejsNpmUpdateAction) self.assertEqual(workflow.actions[0].install_dir, "not_source") self.assertIsInstance(workflow.actions[1], LinkSinglePathAction) self.assertEqual(workflow.actions[1]._source, os.path.join("not_source", "node_modules")) @@ -439,7 +443,7 @@ def test_workflow_revert_build_in_source(self, install_action_mock, install_link subprocess_npm=ANY, osutils=ANY, build_options=ANY, - install_links=False, + is_building_in_source=False, ) @parameterized.expand(