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): command hooks #11583

Merged
merged 7 commits into from
Nov 29, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
26 changes: 26 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,29 @@ $ yarn add --dev esbuild@0
To force bundling in a Docker container, set the `forceDockerBundling` prop to `true`. This
is useful if your function relies on node modules that should be installed (`nodeModules` prop, see [above](#install-modules)) in a Lambda compatible environment. This is usually the
case with modules using native dependencies.

### Command hooks
It is possible to run additional commands by specifying the `commandHooks` prop:
eladb marked this conversation as resolved.
Show resolved Hide resolved

```ts
new lambda.NodejsFunction(this, 'my-handler-with-commands', {
commandHooks: {
// Copy a file so that it will be included in the bundled asset
afterBundling(inputDir: string, outputDir: string): string[] {
return [`cp ${inputDir}/my-binary.node ${outputDir}`];
}
}
});
```

The following hooks are available:
- `beforeBundling`: runs before all bundling commands
- `beforeInstall`: runs before node modules installation
- `afterBundling`: runs after all bundling commands

They all receive the directory containing the lock file (`inputDir`) and the
directory where the bundled asset will be output (`outputDir`). They must return
an array of commands to run. Commands are chained with `&&`.

The commands will run in the environment in which bundling occurs: inside the
container for Docker bundling or on the host OS for local bundling.
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,13 @@ export class Bundling implements cdk.BundlingOptions {
]);
}

return chain([esbuildCommand, depsCommand]);
return chain([
...this.props.commandHooks?.beforeBundling?.(inputDir, outputDir) ?? [],
esbuildCommand,
...(this.props.nodeModules && this.props.commandHooks?.beforeInstall?.(inputDir, outputDir)) ?? [],
depsCommand,
...this.props.commandHooks?.afterBundling?.(inputDir, outputDir) ?? [],
]);
}
}

Expand Down
49 changes: 49 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,53 @@ export interface BundlingOptions {
* @default - use the Docker image provided by @aws-cdk/aws-lambda-nodejs
*/
readonly bundlingDockerImage?: BundlingDockerImage;

/**
* Command hooks
*
* @default - do not run additional commands
*/
readonly commandHooks?: ICommandHooks;
}

/**
* Command hooks
eladb marked this conversation as resolved.
Show resolved Hide resolved
*
* These commands will run in the environment in which bundling occurs: inside
* the container for Docker bundling or on the host OS for local bundling.
*
* Commands are chained with `&&`.
*
* @example
* {
* // Copy a file from the input directory to the output directory
* // to include it in the bundled asset
* afterBundling(inputDir: string, outputDir: string): string[] {
* return [`cp ${inputDir}/my-binary.node ${outputDir}`];
* }
* }
*/
export interface ICommandHooks {
/**
* Returns commands to run before bundling.
*
* Commands are chained with `&&`.
*/
beforeBundling?(inputDir: string, outputDir: string): string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Does these optional methods work with jsii?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It builds but in the .jsii I don't see any optional: true added to the method's type definition.
Also this pattern is nowhere to be seen in the codebase currently.

Does it mean that it will work for TS but other languages will have to define the 3 methods if commandHooks is specified? Can we live with that?

Copy link
Contributor

Choose a reason for hiding this comment

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

@RomainMuller thought?

Copy link
Contributor

Choose a reason for hiding this comment

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

Optional methods are not a thing in most languages, so we can't really support that. This is a TypeScript syntax I hadn't seen before... so I guess this is why the compiler does not complain...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eladb should we make the methods mandatory then or leave it like that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make them mandatory so all languages will have a similar experience.


/**
* Returns commands to run before installing node modules.
*
* This hook only runs when node modules are installed.
*
* Commands are chained with `&&`.
*/
beforeInstall?(inputDir: string, outputDir: string): string[];

/**
* Returns commands to run after bundling.
*
* Commands are chained with `&&`.
*/
afterBundling?(inputDir: string, outputDir: string): string[];
}
30 changes: 30 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,33 @@ test('Custom bundling docker image', () => {
}),
});
});

test('with command hooks', () => {
Bundling.bundle({
entry,
depsLockFilePath,
runtime: Runtime.NODEJS_12_X,
commandHooks: {
beforeBundling(inputDir: string, outputDir: string): string[] {
return [
`echo hello > ${inputDir}/a.txt`,
`cp ${inputDir}/a.txt ${outputDir}`,
];
},
afterBundling(inputDir: string, outputDir: string): string[] {
return [`cp ${inputDir}/b.txt ${outputDir}/txt`];
},
},
forceDockerBundling: true,
});

expect(Code.fromAsset).toHaveBeenCalledWith(path.dirname(depsLockFilePath), {
assetHashType: AssetHashType.OUTPUT,
bundling: expect.objectContaining({
command: [
'bash', '-c',
expect.stringMatching(/^echo hello > \/asset-input\/a.txt && cp \/asset-input\/a.txt \/asset-output && .+ && cp \/asset-input\/b.txt \/asset-output\/txt$/),
],
}),
});
});