Extend Node.js NPM support for .npmrc configuration (#53)

* adding support for .npmrc

* updating design document

* update design document

* correcting to OSError

* minor cleanup

* adding a test when .npmrc doesn't exist

* renaming the global cleanup name to specific npmrc cleanup

Author: simalexan

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 

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))

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)
 

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)
         ]

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")

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")

tests/integration/workflows/nodejs_npm/testdata/npmrc/.npmrc

@@ -0,0 +1 @@
+optional=false

tests/integration/workflows/nodejs_npm/testdata/npmrc/excluded.js

@@ -0,0 +1,2 @@
+//excluded
+const x = 1;

tests/integration/workflows/nodejs_npm/testdata/npmrc/included.js

@@ -0,0 +1,2 @@
+//included
+const x = 1;

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

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()

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)