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(lambda-nodejs): use docker instead of npm for parcel-bundler #7169

Merged
merged 32 commits into from
May 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ea0e032
feat(lambda-nodejs): use docker instead of npm package for parcel-bun…
AlexCheema Apr 3, 2020
3f9db92
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 3, 2020
4490601
require version ^1 of parcel-bundler in Dockerfile
AlexCheema Apr 4, 2020
157281e
update README: docker requirement
AlexCheema Apr 4, 2020
74383e2
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 4, 2020
e744f9c
comment in Dockerfile, make node version a build ARG
AlexCheema Apr 6, 2020
e1428ae
add test for when docker not installed
AlexCheema Apr 6, 2020
9a5cb5c
add optional nodeDockerTag to props
AlexCheema Apr 6, 2020
370778d
Merge branch 'master' into alexcheema/docker-parcel-bundler
AlexCheema Apr 6, 2020
d36a005
Expose nodeDockerTag in NodejsFunctionProps and pass it to the Builder
AlexCheema Apr 6, 2020
23ffd92
Merge branch 'alexcheema/docker-parcel-bundler' of https://github.com…
AlexCheema Apr 6, 2020
459e587
cdk-build pre
AlexCheema Apr 6, 2020
b64686d
Merge branch 'master' into alexcheema/docker-parcel-bundler
AlexCheema Apr 6, 2020
5fc915d
Merge branch 'master' into alexcheema/docker-parcel-bundler
AlexCheema Apr 7, 2020
9842450
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 14, 2020
34afc14
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 15, 2020
6f8fd3c
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 15, 2020
0404baa
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 22, 2020
ebb464f
chore(lambda-nodejs): remove dockerd pre directive
AlexCheema Apr 24, 2020
5f9d9ab
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 24, 2020
eaf48b2
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 26, 2020
86ee7d2
add missing closing parenthesis
Apr 27, 2020
2f63cab
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 29, 2020
a78b7e0
fix linter errors
AlexCheema Apr 29, 2020
24d9e8d
fix linter error
AlexCheema Apr 29, 2020
5fa1e63
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema Apr 29, 2020
c7852e9
start docker daemon inside the container
Apr 30, 2020
9f85d87
Update buildspec.yaml
Apr 30, 2020
d3c79d4
Fix docker location.
Apr 30, 2020
feadb67
Update buildspec.yaml
Apr 30, 2020
711b58e
Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…
AlexCheema May 1, 2020
8de8246
Merge branch 'master' into alexcheema/docker-parcel-bundler
mergify[bot] May 1, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions buildspec-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ version: 0.2
phases:
install:
commands:
# Start docker daemon inside the container
- nohup /usr/bin/dockerd --host=unix:///var/run/docker.sock --host=tcp://127.0.0.1:2375 --storage-driver=overlay2&
- timeout 15 sh -c "until docker info; do echo .; sleep 1; done"

# Install yarn if it wasn't already present in the image
- yarn --version || npm -g install yarn
build:
Expand Down
4 changes: 4 additions & 0 deletions buildspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ version: 0.2
phases:
install:
commands:
# Start docker daemon inside the container
- nohup /usr/bin/dockerd --host=unix:///var/run/docker.sock --host=tcp://127.0.0.1:2375 --storage-driver=overlay2&
- timeout 15 sh -c "until docker info; do echo .; sleep 1; done"

# Install yarn if it wasn't already present in the image
- yarn --version || npm -g install yarn
pre_build:
Expand Down
9 changes: 1 addition & 8 deletions packages/@aws-cdk/aws-lambda-nodejs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,7 @@

This library provides constructs for Node.js Lambda functions.

To use this module, you will need to add a dependency on `parcel-bundler` in your
AlexCheema marked this conversation as resolved.
Show resolved Hide resolved
`package.json`:

```
yarn add parcel-bundler@^1
# or
npm install parcel-bundler@^1
```
To use this module, you will need to have Docker installed.

### Node.js Function
Define a `NodejsFunction`:
Expand Down
55 changes: 30 additions & 25 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,29 +41,20 @@ export interface BuilderOptions {
* The node version to use as target for Babel
*/
readonly nodeVersion?: string;

/**
* The docker tag of the node base image to use in the parcel-bundler docker image
*
* @see https://hub.docker.com/_/node/?tab=tags
*/
readonly nodeDockerTag: string;
}

/**
* Builder
*/
export class Builder {
private readonly parcelBinPath: string;

constructor(private readonly options: BuilderOptions) {
let parcelPkgPath: string;
try {
parcelPkgPath = require.resolve('parcel-bundler/package.json'); // This will throw if `parcel-bundler` cannot be found
} catch (err) {
throw new Error('It looks like parcel-bundler is not installed. Please install v1.x of parcel-bundler with yarn or npm.');
}
const parcelDir = path.dirname(parcelPkgPath);
const parcelPkg = JSON.parse(fs.readFileSync(parcelPkgPath, 'utf8'));

if (!parcelPkg.version || !/^1\./.test(parcelPkg.version)) { // Peer dependency on parcel v1.x
throw new Error(`This module has a peer dependency on parcel-bundler v1.x. Got v${parcelPkg.version}.`);
}

this.parcelBinPath = path.join(parcelDir, parcelPkg.bin.parcel);
}

public build(): void {
Expand All @@ -78,29 +69,43 @@ export class Builder {
});
}

const args = [
'build', this.options.entry,
'--out-dir', this.options.outDir,
const dockerBuildArgs = [
'build', '--build-arg', `NODE_TAG=${this.options.nodeDockerTag}`, '-t', 'parcel-bundler', path.join(__dirname, '../parcel-bundler'),
];
const dockerRunArgs = [
'run', '--rm',
'-v', `${path.dirname(path.resolve(this.options.entry))}:/entry`,
'-v', `${path.resolve(this.options.outDir)}:/out`,
...(this.options.cacheDir ? ['-v', `${path.resolve(this.options.cacheDir)}:/cache`] : []),
'parcel-bundler',
];
const parcelArgs = [
'parcel', 'build', `/entry/${path.basename(this.options.entry)}`,
'--out-dir', '/out',
'--out-file', 'index.js',
'--global', this.options.global,
'--target', 'node',
'--bundle-node-modules',
'--log-level', '2',
!this.options.minify && '--no-minify',
!this.options.sourceMaps && '--no-source-maps',
...this.options.cacheDir
? ['--cache-dir', this.options.cacheDir]
: [],
...(this.options.cacheDir ? ['--cache-dir', '/cache'] : []),
].filter(Boolean) as string[];

const parcel = spawnSync(this.parcelBinPath, args);
const build = spawnSync('docker', dockerBuildArgs);
if (build.error) {
throw build.error;
}
if (build.status !== 0) {
throw new Error(`[Status ${build.status}] stdout: ${build.stdout?.toString().trim()}\n\n\nstderr: ${build.stderr?.toString().trim()}`);
}

const parcel = spawnSync('docker', [...dockerRunArgs, ...parcelArgs]);
if (parcel.error) {
throw parcel.error;
}

if (parcel.status !== 0) {
throw new Error(parcel.stdout.toString().trim());
throw new Error(`[Status ${parcel.status}] stdout: ${parcel.stdout?.toString().trim()}\n\n\nstderr: ${parcel.stderr?.toString().trim()}`);
}
} catch (err) {
throw new Error(`Failed to build file at ${this.options.entry}: ${err}`);
Expand Down
11 changes: 11 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ export interface NodejsFunctionProps extends lambda.FunctionOptions {
* @default - `.cache` in the root directory
*/
readonly cacheDir?: string;

/**
* The docker tag of the node base image to use in the parcel-bundler docker image
*
* @see https://hub.docker.com/_/node/?tab=tags
*
* @default - 13.8.0-alpine3.11
*/
readonly nodeDockerTag?: string;
}

/**
Expand All @@ -84,6 +93,7 @@ export class NodejsFunction extends lambda.Function {
? lambda.Runtime.NODEJS_12_X
: lambda.Runtime.NODEJS_10_X;
const runtime = props.runtime || defaultRunTime;
const nodeDockerTag = props.nodeDockerTag || '13.8.0-alpine3.11';

// Build with Parcel
const builder = new Builder({
Expand All @@ -94,6 +104,7 @@ export class NodejsFunction extends lambda.Function {
sourceMaps: props.sourceMaps,
cacheDir: props.cacheDir,
nodeVersion: extractVersion(runtime),
nodeDockerTag,
});
builder.build();

Expand Down
9 changes: 8 additions & 1 deletion packages/@aws-cdk/aws-lambda-nodejs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@
"build+test": "npm run build && npm test",
"compat": "cdk-compat"
},
"cdk-build": {
"eslint": {
"ignore-pattern": [
"test/function.test.handler2.js",
"test/integ-handlers/js-handler.js"
]
}
},
"keywords": [
"aws",
"cdk",
Expand Down Expand Up @@ -80,7 +88,6 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"fs-extra": "^8.1.0",
"parcel-bundler": "^1.12.4",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/parcel-bundler/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# runs the parcel-bundler npm package to package and install dependencies of nodejs lambda functions
ARG NODE_TAG
FROM node:${NODE_TAG}

RUN yarn global add parcel-bundler@^1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there not a prebuilt Parcel image up on DockerHub that we can use?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really... also the idea is to maybe customize this image to address this #6323 and support externals (like aws-sdk).


CMD [ "parcel" ]
63 changes: 34 additions & 29 deletions packages/@aws-cdk/aws-lambda-nodejs/test/builder.test.ts
Original file line number Diff line number Diff line change
@@ -1,60 +1,66 @@
import { spawnSync } from 'child_process';
import * as fs from 'fs';
import * as path from 'path';
import { Builder } from '../lib/builder';

let parcelPkgPath: string;
let parcelPkg: Buffer;
beforeAll(() => {
parcelPkgPath = require.resolve('parcel-bundler/package.json');
parcelPkg = fs.readFileSync(parcelPkgPath);
});

afterEach(() => {
fs.writeFileSync(parcelPkgPath, parcelPkg);
});

jest.mock('child_process', () => ({
spawnSync: jest.fn((_cmd: string, args: string[]) => {
if (args[1] === 'error') {
if (args.includes('/entry/error')) {
return { error: 'parcel-error' };
}

if (args[1] === 'status') {
if (args.includes('/entry/status')) {
return { status: 1, stdout: Buffer.from('status-error') };
}

if (args.includes('/entry/no-docker')) {
return { error: 'Error: spawnSync docker ENOENT' };
}

return { error: null, status: 0 };
}),
}));

test('calls parcel with the correct args', () => {
test('calls docker with the correct args', () => {
const builder = new Builder({
entry: 'entry',
global: 'handler',
outDir: 'out-dir',
cacheDir: 'cache-dir',
nodeDockerTag: '13.8.0-alpine3.11',
});
builder.build();

expect(spawnSync).toHaveBeenCalledWith(expect.stringContaining('parcel-bundler'), expect.arrayContaining([
'build', 'entry',
'--out-dir', 'out-dir',
// docker build
expect(spawnSync).toHaveBeenNthCalledWith(1, 'docker', [
'build', '--build-arg', 'NODE_TAG=13.8.0-alpine3.11', '-t', 'parcel-bundler', path.join(__dirname, '../parcel-bundler'),
]);

// docker run
expect(spawnSync).toHaveBeenNthCalledWith(2, 'docker', [
'run', '--rm',
'-v', `${path.join(__dirname, '..')}:/entry`,
'-v', `${path.join(__dirname, '../out-dir')}:/out`,
'-v', `${path.join(__dirname, '../cache-dir')}:/cache`,
'parcel-bundler',
'parcel', 'build', '/entry/entry',
'--out-dir', '/out',
'--out-file', 'index.js',
'--global', 'handler',
'--target', 'node',
'--bundle-node-modules',
'--log-level', '2',
'--no-minify',
'--no-source-maps',
'--cache-dir', 'cache-dir',
]));
'--cache-dir', '/cache',
]);
});

test('throws in case of error', () => {
const builder = new Builder({
entry: 'error',
global: 'handler',
outDir: 'out-dir',
nodeDockerTag: '13.8.0-alpine3.11',
});
expect(() => builder.build()).toThrow('parcel-error');
});
Expand All @@ -64,18 +70,17 @@ test('throws if status is not 0', () => {
entry: 'status',
global: 'handler',
outDir: 'out-dir',
nodeDockerTag: '13.8.0-alpine3.11',
});
expect(() => builder.build()).toThrow('status-error');
});

test('throws when parcel-bundler is not 1.x', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a test for when docker is not installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

fs.writeFileSync(parcelPkgPath, JSON.stringify({
...JSON.parse(parcelPkg.toString()),
version: '2.3.4',
}));
expect(() => new Builder({
entry: 'entry',
test('throws if docker is not installed', () => {
const builder = new Builder({
entry: 'no-docker',
global: 'handler',
outDir: 'out-dur',
})).toThrow(/This module has a peer dependency on parcel-bundler v1.x. Got v2.3.4./);
outDir: 'out-dir',
nodeDockerTag: '13.8.0-alpine3.11',
});
expect(() => builder.build()).toThrow('Error: spawnSync docker ENOENT');
});
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"Properties": {
"Code": {
"S3Bucket": {
"Ref": "AssetParametersa736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543S3BucketD096DC83"
"Ref": "AssetParametersfdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88eS3Bucket9E60CFAE"
},
"S3Key": {
"Fn::Join": [
Expand All @@ -49,7 +49,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParametersa736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543S3VersionKeyA76287A5"
"Ref": "AssetParametersfdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88eS3VersionKey74429E96"
}
]
}
Expand All @@ -62,7 +62,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParametersa736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543S3VersionKeyA76287A5"
"Ref": "AssetParametersfdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88eS3VersionKey74429E96"
}
]
}
Expand Down Expand Up @@ -172,17 +172,17 @@
}
},
"Parameters": {
"AssetParametersa736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543S3BucketD096DC83": {
"AssetParametersfdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88eS3Bucket9E60CFAE": {
"Type": "String",
"Description": "S3 bucket for asset \"a736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543\""
"Description": "S3 bucket for asset \"fdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88e\""
},
"AssetParametersa736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543S3VersionKeyA76287A5": {
"AssetParametersfdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88eS3VersionKey74429E96": {
"Type": "String",
"Description": "S3 key for asset version \"a736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543\""
"Description": "S3 key for asset version \"fdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88e\""
},
"AssetParametersa736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543ArtifactHashB7337532": {
"AssetParametersfdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88eArtifactHashAA5D4787": {
"Type": "String",
"Description": "Artifact hash for asset \"a736580e4382e6d67a7acaedbbd271f559ed918382808ae1431364ae70286543\""
"Description": "Artifact hash for asset \"fdefbd5f0d90231e11e34991218a3674ec3ac1a3a99948038164c6d7a8efa88e\""
},
"AssetParameters0d445d366e339b4344917ba638a4cb2dcddfd0e063b10fb909340fd1cc51c278S3BucketDBD288E6": {
"Type": "String",
Expand Down