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

revert: "feat(lambda-nodejs): use docker instead of npm for parcel-bundler" #7738

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 0 additions & 4 deletions buildspec-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ 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: 0 additions & 4 deletions buildspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ 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: 8 additions & 1 deletion packages/@aws-cdk/aws-lambda-nodejs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@

This library provides constructs for Node.js Lambda functions.

To use this module, you will need to have Docker installed.
To use this module, you will need to add a dependency on `parcel-bundler` in your
`package.json`:

```
yarn add parcel-bundler@^1
# or
npm install parcel-bundler@^1
```

### Node.js Function
Define a `NodejsFunction`:
Expand Down
55 changes: 25 additions & 30 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,29 @@ 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 @@ -69,43 +78,29 @@ export class Builder {
});
}

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',
const args = [
'build', this.options.entry,
'--out-dir', this.options.outDir,
'--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', '/cache'] : []),
...this.options.cacheDir
? ['--cache-dir', this.options.cacheDir]
: [],
].filter(Boolean) as string[];

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(this.parcelBinPath, args);

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

if (parcel.status !== 0) {
throw new Error(`[Status ${parcel.status}] stdout: ${parcel.stdout?.toString().trim()}\n\n\nstderr: ${parcel.stderr?.toString().trim()}`);
throw new Error(parcel.stdout.toString().trim());
}
} catch (err) {
throw new Error(`Failed to build file at ${this.options.entry}: ${err}`);
Expand Down
11 changes: 0 additions & 11 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,6 @@ 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 @@ -93,7 +84,6 @@ 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 @@ -104,7 +94,6 @@ export class NodejsFunction extends lambda.Function {
sourceMaps: props.sourceMaps,
cacheDir: props.cacheDir,
nodeVersion: extractVersion(runtime),
nodeDockerTag,
});
builder.build();

Expand Down
9 changes: 1 addition & 8 deletions packages/@aws-cdk/aws-lambda-nodejs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,6 @@
"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 @@ -88,6 +80,7 @@
"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: 0 additions & 7 deletions packages/@aws-cdk/aws-lambda-nodejs/parcel-bundler/Dockerfile

This file was deleted.

63 changes: 29 additions & 34 deletions packages/@aws-cdk/aws-lambda-nodejs/test/builder.test.ts
Original file line number Diff line number Diff line change
@@ -1,66 +1,60 @@
import { spawnSync } from 'child_process';
import * as path from 'path';
import * as fs from 'fs';
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.includes('/entry/error')) {
if (args[1] === 'error') {
return { error: 'parcel-error' };
}

if (args.includes('/entry/status')) {
if (args[1] === '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 docker with the correct args', () => {
test('calls parcel 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();

// 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',
expect(spawnSync).toHaveBeenCalledWith(expect.stringContaining('parcel-bundler'), expect.arrayContaining([
'build', 'entry',
'--out-dir', 'out-dir',
'--out-file', 'index.js',
'--global', 'handler',
'--target', 'node',
'--bundle-node-modules',
'--log-level', '2',
'--no-minify',
'--no-source-maps',
'--cache-dir', '/cache',
]);
'--cache-dir', 'cache-dir',
]));
});

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