diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md index 1a7746756..803052ed7 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md +++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md @@ -122,9 +122,8 @@ the local `node_modules` subdirectory. This has to be executed in the directory a clean copy of the source files. Note that NPM can be configured to use proxies or local company repositories using -a local file, `.npmrc`. The packaging process from step 1 normally excludes this file, so it may -need to be copied additionally before dependency installation, and then removed. -_(out of scope for the current version)_ +a local file, `.npmrc`. The packaging process from step 1 normally excludes this file, so it needs +to be copied before dependency installation, and then removed. Some users may want to exclude optional dependencies, or even include development dependencies. To avoid incompatible flags in the `sam` CLI, the packager should allow users to specify diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index e3e9aad43..ad6e7a927 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -111,3 +111,87 @@ def execute(self): except NpmExecutionError as ex: raise ActionFailedError(str(ex)) + +class NodejsNpmrcCopyAction(BaseAction): + + """ + A Lambda Builder Action that copies NPM config file .npmrc + """ + + NAME = 'CopyNpmrc' + DESCRIPTION = "Copying configuration from .npmrc" + PURPOSE = Purpose.COPY_SOURCE + + def __init__(self, artifacts_dir, source_dir, osutils): + """ + :type artifacts_dir: str + :param artifacts_dir: an existing (writable) directory with project source files. + Dependencies will be installed in this directory. + + :type source_dir: str + :param source_dir: directory containing project source files. + + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + """ + + super(NodejsNpmrcCopyAction, self).__init__() + self.artifacts_dir = artifacts_dir + self.source_dir = source_dir + self.osutils = osutils + + def execute(self): + """ + Runs the action. + + :raises lambda_builders.actions.ActionFailedError: when .npmrc 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) + + except OSError as ex: + raise ActionFailedError(str(ex)) + +class NodejsNpmrcCleanUpAction(BaseAction): + + """ + A Lambda Builder Action that cleans NPM config file .npmrc + """ + + NAME = 'CleanUpNpmrc' + DESCRIPTION = "Cleans artifacts dir" + PURPOSE = Purpose.COPY_SOURCE + + def __init__(self, artifacts_dir, osutils): + """ + :type artifacts_dir: str + :param artifacts_dir: an existing (writable) directory with project source files. + 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 + """ + + super(NodejsNpmrcCleanUpAction, self).__init__() + self.artifacts_dir = artifacts_dir + self.osutils = osutils + + def execute(self): + """ + Runs the action. + + :raises lambda_builders.actions.ActionFailedError: when .npmrc copying fails + """ + + try: + npmrc_path = self.osutils.joinpath(self.artifacts_dir, ".npmrc") + if self.osutils.file_exists(npmrc_path): + LOG.debug(".npmrc cleanup in: %s", self.artifacts_dir) + self.osutils.remove_file(npmrc_path) + + except OSError as ex: + raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py index dcff97272..b34863788 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/utils.py +++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py @@ -6,6 +6,7 @@ import platform import tarfile import subprocess +import shutil class OSUtils(object): @@ -15,10 +16,16 @@ class OSUtils(object): unit test actions in memory """ + def copy_file(self, file_path, destination_path): + return shutil.copy2(file_path, destination_path) + def extract_tarfile(self, tarfile_path, unpack_dir): with tarfile.open(tarfile_path, 'r:*') as tar: tar.extractall(unpack_dir) + def file_exists(self, filename): + return os.path.isfile(filename) + def joinpath(self, *args): return os.path.join(*args) @@ -33,6 +40,9 @@ def pipe(self): def dirname(self, path): return os.path.dirname(path) + def remove_file(self, filename): + return os.remove(filename) + def abspath(self, path): return os.path.abspath(path) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index b55bc9cc1..57396cbc0 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -4,7 +4,7 @@ from aws_lambda_builders.workflow import BaseWorkflow, Capability from aws_lambda_builders.actions import CopySourceAction -from .actions import NodejsNpmPackAction, NodejsNpmInstallAction +from .actions import NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction from .utils import OSUtils from .npm import SubprocessNpm @@ -55,8 +55,13 @@ def __init__(self, npm_install = NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm) + + npm_copy_npmrc = NodejsNpmrcCopyAction(tar_package_dir, source_dir, osutils=osutils) + self.actions = [ npm_pack, + npm_copy_npmrc, CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), npm_install, + NodejsNpmrcCleanUpAction(artifacts_dir, osutils=osutils) ] diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py index b84bc1e8f..01fd52f01 100644 --- a/tests/functional/workflows/nodejs_npm/test_utils.py +++ b/tests/functional/workflows/nodejs_npm/test_utils.py @@ -14,6 +14,58 @@ def setUp(self): self.osutils = utils.OSUtils() + def test_copy_file_copies_existing_file_into_a_dir(self): + + test_file = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz") + + test_dir = tempfile.mkdtemp() + + self.osutils.copy_file(test_file, test_dir) + + output_files = set(os.listdir(test_dir)) + + shutil.rmtree(test_dir) + + self.assertEqual({"test.tgz"}, output_files) + + def test_copy_file_copies_existing_file_into_a_file(self): + + test_file = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz") + + test_dir = tempfile.mkdtemp() + + self.osutils.copy_file(test_file, os.path.join(test_dir, "copied_test.tgz")) + + output_files = set(os.listdir(test_dir)) + + shutil.rmtree(test_dir) + + self.assertEqual({"copied_test.tgz"}, output_files) + + def test_remove_file_removes_existing_file(self): + + test_file = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz") + + test_dir = tempfile.mkdtemp() + + copied_file = os.path.join(test_dir, "copied_test.tgz") + + shutil.copy(test_file, copied_file) + + self.osutils.remove_file(copied_file) + + self.assertFalse(os.path.isfile(copied_file)) + + def test_file_exists_checking_if_file_exists_in_a_dir(self): + + existing_file = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz") + + nonexisting_file = os.path.join(os.path.dirname(__file__), "test_data", "nonexisting.tgz") + + self.assertTrue(self.osutils.file_exists(existing_file)) + + self.assertFalse(self.osutils.file_exists(nonexisting_file)) + def test_extract_tarfile_unpacks_a_tar(self): test_tar = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz") diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index 980281e16..a8533d5ce 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -56,6 +56,22 @@ def test_builds_project_with_remote_dependencies(self): output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) self.assertEquals(expected_modules, output_modules) + def test_builds_project_with_npmrc(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "npmrc") + + self.builder.build(source_dir, self.artifacts_dir, self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime) + + expected_files = {"package.json", "included.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + + self.assertEquals(expected_files, output_files) + + expected_modules = {"fake-http-request"} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertEquals(expected_modules, output_modules) + def test_fails_if_npm_cannot_resolve_dependencies(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-deps") diff --git a/tests/integration/workflows/nodejs_npm/testdata/npmrc/.npmrc b/tests/integration/workflows/nodejs_npm/testdata/npmrc/.npmrc new file mode 100644 index 000000000..8f1b27978 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/npmrc/.npmrc @@ -0,0 +1 @@ +optional=false \ No newline at end of file diff --git a/tests/integration/workflows/nodejs_npm/testdata/npmrc/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/npmrc/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/npmrc/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/npmrc/included.js b/tests/integration/workflows/nodejs_npm/testdata/npmrc/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/npmrc/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/npmrc/package.json b/tests/integration/workflows/nodejs_npm/testdata/npmrc/package.json new file mode 100644 index 000000000..549f8165b --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/npmrc/package.json @@ -0,0 +1,15 @@ +{ + "name": "npmdeps", + "version": "1.0.0", + "description": "", + "files": ["included.js"], + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "fake-http-request": "*" + }, + "optionalDependencies": { + "minimal-request-promise": "*" + } +} diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index f6d0a46fb..612ccb189 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -2,7 +2,8 @@ from mock import patch from aws_lambda_builders.actions import ActionFailedError -from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmPackAction, NodejsNpmInstallAction +from aws_lambda_builders.workflows.nodejs_npm.actions import \ + NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError @@ -78,3 +79,77 @@ def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): action.execute() self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") + + +class TestNodejsNpmrcCopyAction(TestCase): + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_copies_npmrc_into_a_project(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + + action = NodejsNpmrcCopyAction("artifacts", + "source", + osutils=osutils) + osutils.file_exists.side_effect = [True] + action.execute() + + osutils.file_exists.assert_called_with("source/.npmrc") + osutils.copy_file.assert_called_with("source/.npmrc", "artifacts") + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_skips_copying_npmrc_into_a_project_if_npmrc_doesnt_exist(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + + action = NodejsNpmrcCopyAction("artifacts", + "source", + osutils=osutils) + osutils.file_exists.side_effect = [False] + action.execute() + + osutils.file_exists.assert_called_with("source/.npmrc") + osutils.copy_file.assert_not_called() + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_raises_action_failed_when_copying_fails(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + + osutils.copy_file.side_effect = OSError() + + action = NodejsNpmrcCopyAction("artifacts", + "source", + osutils=osutils) + + with self.assertRaises(ActionFailedError): + action.execute() + + +class TestNodejsNpmrcCleanUpAction(TestCase): + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_removes_npmrc_if_npmrc_exists(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + + action = NodejsNpmrcCleanUpAction( + "artifacts", + osutils=osutils) + osutils.file_exists.side_effect = [True] + action.execute() + + osutils.remove_file.assert_called_with("artifacts/.npmrc") + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_skips_npmrc_removal_if_npmrc_doesnt_exist(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + + action = NodejsNpmrcCleanUpAction( + "artifacts", + osutils=osutils) + osutils.file_exists.side_effect = [False] + action.execute() + + osutils.remove_file.assert_not_called() diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 1d8e0d63c..c2fe05be3 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -2,7 +2,8 @@ from aws_lambda_builders.actions import CopySourceAction from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow -from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmPackAction, NodejsNpmInstallAction +from aws_lambda_builders.workflows.nodejs_npm.actions import \ + NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction class TestNodejsNpmWorkflow(TestCase): @@ -16,10 +17,14 @@ def test_workflow_sets_up_npm_actions(self): workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest") - self.assertEqual(len(workflow.actions), 3) + self.assertEqual(len(workflow.actions), 5) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) - self.assertIsInstance(workflow.actions[1], CopySourceAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) - self.assertIsInstance(workflow.actions[2], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[2], CopySourceAction) + + self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) + + self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction)