Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: nodejs_npm rewrite local deps #215

Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b89732c
proof of concept action updates working
cBiscuitSurprise Dec 10, 2020
8e5c836
moved dependency helpers to utils.py
cBiscuitSurprise Dec 10, 2020
e16d565
Removed unused args in utils.mkdir
cBiscuitSurprise Dec 10, 2020
d84e5ea
Expanded to recursive local modules
cBiscuitSurprise Dec 10, 2020
221135a
Updated baseline tests
cBiscuitSurprise Dec 10, 2020
988a6c4
black formatting
cBiscuitSurprise Dec 10, 2020
7697c3b
Added basic integration test for local deps
cBiscuitSurprise Dec 10, 2020
49374dd
Refactored common to single function
cBiscuitSurprise Dec 10, 2020
e378384
Python2: updated f"{sfuff}" to "{}".format(stuff)
cBiscuitSurprise Dec 10, 2020
b98b533
Updated tests
cBiscuitSurprise Dec 10, 2020
f5d1a2f
updated npmjs DESIGN.md
cBiscuitSurprise Dec 10, 2020
48ff701
Python2 regression.
cBiscuitSurprise Dec 10, 2020
21e5e40
Merge branch 'develop' into feature/nodejs_npm/step-2-rewrite-local-deps
mgrandis Jan 7, 2021
62b4f20
removed manifest backup during write
cBiscuitSurprise Jan 19, 2021
3fb1ea1
refactored npm-pack + extract into utility
cBiscuitSurprise Jan 19, 2021
a7c25b4
remove unecessary else in nodejs_npm utils
cBiscuitSurprise Jan 19, 2021
8c33ad6
Merge branch 'develop' into feature/nodejs_npm/step-2-rewrite-local-deps
mgrandis Jan 19, 2021
84cf9b5
refactor: Light refactor to do everything in one pass, restore the fi…
mgrandis Feb 17, 2021
7b7a394
Merge pull request #1 from mgrandis/feature/nodejs_npm/step-2-rewrite…
cBiscuitSurprise Feb 20, 2021
850e781
nodejs_npm Tests passing in python3
cBiscuitSurprise Feb 21, 2021
e1172ad
nodejs_npm Tests in python2 passing and formatter executed
cBiscuitSurprise Feb 21, 2021
18a203a
Merge branch 'develop' into feature/nodejs_npm/step-2-rewrite-local-deps
mgrandis Feb 23, 2021
36833dd
chore: Update the DESIGN.md doc now that we track paths
mgrandis Feb 25, 2021
a81b4d4
refactor: Removed OSUtils.filename (not used anymore) and added missi…
mgrandis Feb 25, 2021
da930a3
test: Fix test failing on Windows
mgrandis Feb 25, 2021
662383a
Merge branch 'develop' into feature/nodejs_npm/step-2-rewrite-local-deps
mgrandis Feb 26, 2021
f5dfd23
refactor: PR review items
mgrandis Mar 17, 2021
bfa2289
fix: Replaced open with io.open for python 2 compatibility
mgrandis Mar 17, 2021
ee6576f
fix: Encoding issues
mgrandis Mar 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 27 additions & 28 deletions aws_lambda_builders/workflows/nodejs_npm/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ production dependencies (normally the minimum required for correct execution).
All these dependency types are mixed in the same directory.

To speed up Lambda startup time and optimise usage costs, the correct thing to
do in most cases is just to package up production dependencies. During development
work we can expect that the local `node_modules` directory contains all the
do in most cases is just to package up production dependencies. During development
work we can expect that the local `node_modules` directory contains all the
various dependency types, and NPM does not provide a way to directly identify
just the ones relevant for production. To identify production dependencies,
just the ones relevant for production. To identify production dependencies,
this packager needs to copy the source to a clean temporary directory and re-run
dependency installation there.

A frequently used trick to speed up NodeJS Lambda deployment is to avoid
A frequently used trick to speed up NodeJS Lambda deployment is to avoid
bundling the `aws-sdk`, since it is already available on the Lambda VM.
This makes deployment significantly faster for single-file lambdas, for
example. Although this is not good from a consistency and compatibility
Expand All @@ -40,7 +40,7 @@ dependencies.

Other runtimes do not have this flexibility, so instead of adding a specific
parameter to the SAM CLI, the packager should support a flag to include or
exclude optional dependencies through environment variables.
exclude optional dependencies through environment variables.

NPM also provides support for running user-defined scripts as part of the build
process, so this packager needs to support standard NPM script execution.
Expand All @@ -63,9 +63,9 @@ versions, when using on several machines with different project layout it can
lead to uninstallable dependencies.

NPM dependencies are usually plain javascript libraries, but they may include
native binaries precompiled for a particular platform, or require some system
libraries to be installed. A notable example is `sharp`, a popular image
manipulation library, that uses symbolic links to system libraries. Another
native binaries precompiled for a particular platform, or require some system
libraries to be installed. A notable example is `sharp`, a popular image
manipulation library, that uses symbolic links to system libraries. Another
notable example is `puppeteer`, a library to control a headless Chrome browser,
that downloads a Chromium binary for the target platform during installation.

Expand All @@ -81,56 +81,55 @@ is as follows.
#### Step 1: Prepare a clean copy of the project source files

Execute `npm pack` to perform project-specific packaging using the supplied
`package.json` manifest, which will automatically exclude temporary files,
test resources and other source files unnecessary for running in a production
`package.json` manifest, which will automatically exclude temporary files,
test resources and other source files unnecessary for running in a production
environment.

This will produce a `tar` archive that needs to be unpacked into the artifacts
directory. Note that the archive will actually contain a `package`
subdirectory containing the files, so it's not enough to just directly unpack
files.
files.

#### Step 2: Rewrite local dependencies

_(out of scope for the current version)_

To optimise disk space and avoid including development dependencies from other
locally linked packages, inspect the `package.json` manifest looking for dependencies
referring to local file paths (can be identified as they start with `.` or `file:`),
then for each dependency recursively execute the packaging process
then for each dependency recursively execute the packaging process

Local dependencies may include other local dependencies themselves, this is a very
common way of sharing configuration or development utilities such as linting or testing
Local dependencies may include other local dependencies themselves, this is a very
common way of sharing configuration or development utilities such as linting or testing
tools. This means that for each packaged local dependency this packager needs to
recursively apply the packaging process. It also means that the packager needs to
track local paths and avoid re-packaging directories it already visited.
recursively apply the packaging process.

It also means that the packager needs to track local paths and avoid re-packaging directories it already visited.

NPM produces a `tar` archive while packaging that can be directly included as a
dependency. This will make NPM unpack and install a copy correctly. Once the
packager produces all `tar` archives required by local dependencies, rewrite
the manifest to point to `tar` files instead of the original location.

If the project contains a package lock file, this will cause NPM to ignore changes
to the package.json manifest. In this case, the packager will need to remove
`package-lock.json` so that dependency rewrites take effect.
_(out of scope for the current version)_
to the package.json manifest. In this case, the packager will need to remove
`package-lock.json` so that dependency rewrites take effect.

#### Step 3: Install dependencies

The packager should then run `npm install` to download an expand all dependencies to
the local `node_modules` subdirectory. This has to be executed in the directory with
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 needs
to be copied before dependency installation, and then removed.
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 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
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
options for the `npm install` command using an environment variable.

_(out of scope for the current version)_

To fully support dependencies that download or compile binaries for a target platform, this step
needs to be executed inside a Docker image compatible with AWS Lambda.
_(out of scope for the current version)_
needs to be executed inside a Docker image compatible with AWS Lambda.

_(out of scope for the current version)_
75 changes: 48 additions & 27 deletions aws_lambda_builders/workflows/nodejs_npm/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError
from .npm import NpmExecutionError
from .utils import DependencyUtils

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -47,25 +48,21 @@ def __init__(self, artifacts_dir, scratch_dir, manifest_path, osutils, subproces

def execute(self):
"""
Runs the action.
Runs the action

:raises lambda_builders.actions.ActionFailedError: when NPM packaging fails
Raises
------
ActionFailedError
When NPM packaging fails
"""
try:
package_path = "file:{}".format(self.osutils.abspath(self.osutils.dirname(self.manifest_path)))

LOG.debug("NODEJS packaging %s to %s", package_path, self.scratch_dir)

tarfile_name = self.subprocess_npm.run(["pack", "-q", package_path], cwd=self.scratch_dir).splitlines()[-1]

LOG.debug("NODEJS packed to %s", tarfile_name)

tarfile_path = self.osutils.joinpath(self.scratch_dir, tarfile_name)

LOG.debug("NODEJS extracting to %s", self.artifacts_dir)
tar_path = DependencyUtils.package_dependencies(
self.manifest_path, self.scratch_dir, {}, self.osutils, self.subprocess_npm
)

self.osutils.extract_tarfile(tarfile_path, self.artifacts_dir)
LOG.debug("NODEJS extracting final %s to artifacts dir %s", tar_path, self.artifacts_dir)

self.osutils.extract_tarfile(tar_path, self.artifacts_dir)
except NpmExecutionError as ex:
raise ActionFailedError(str(ex))

Expand Down Expand Up @@ -115,7 +112,7 @@ def execute(self):
class NodejsNpmrcCopyAction(BaseAction):

"""
A Lambda Builder Action that copies NPM config file .npmrc
Lambda Builder Action that copies NPM config file .npmrc
"""

NAME = "CopyNpmrc"
Expand Down Expand Up @@ -157,13 +154,14 @@ def execute(self):
raise ActionFailedError(str(ex))


class NodejsNpmrcCleanUpAction(BaseAction):

class NodejsCleanUpAction(BaseAction):
"""
A Lambda Builder Action that cleans NPM config file .npmrc
Lambda Builder Action that cleans up after install:
- Removes NPM config file .npmrc
- Restores modified package.json files from their backup
"""

NAME = "CleanUpNpmrc"
NAME = "CleanUp"
DESCRIPTION = "Cleans artifacts dir"
PURPOSE = Purpose.COPY_SOURCE

Expand All @@ -177,22 +175,45 @@ def __init__(self, artifacts_dir, osutils):
:param osutils: An instance of OS Utilities for file manipulation
"""

super(NodejsNpmrcCleanUpAction, self).__init__()
super(NodejsCleanUpAction, self).__init__()
self.artifacts_dir = artifacts_dir
self.osutils = osutils

def execute(self):
"""
Runs the action.
Runs the actions

:raises lambda_builders.actions.ActionFailedError: when .npmrc copying fails
Raises
------
ActionFailedError
When .npmrc removing or package.json restoring 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)

self._remove_npmrc()
self._restore_package_json()
except OSError as ex:
raise ActionFailedError(str(ex))

def _remove_npmrc(self):
"""
Removes the .npmrc file
"""
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)

def _restore_package_json(self):
"""
Restores the original package.json from its backup
"""
org_file = "package.json"
bak_file = org_file + ".bak"

for (root, _, files) in self.osutils.walk(self.artifacts_dir):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the walk necessary?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During the building process we update the package.json files to point to the new packed dependencies.
We make backups of the original files.
At the end of the process we walk the dirs to revert the files to their backup so that the files we upload are the original ones. (#187)

if bak_file in files:
bak_path = self.osutils.joinpath(root, bak_file)
self.osutils.copy_file(bak_path, self.osutils.joinpath(root, org_file))
self.osutils.remove_file(bak_path)
Loading