Skip to content

Commit 7c752db

Browse files
authored
fix(lambda-nodejs): unable to use nodeModules with pnpm (#21911)
Fixes #21910 By default, pnpm uses symlinks when installing dependencies, and modules only have access to dependencies in their `package.json`. This means when trying to bundle, dependencies of depedencies are not installed. This PR fixes this by using the ` --config.node_linker=hoisted` flag to create a flat `node_modules` structure. [Docs](https://pnpm.io/npmrc#node-linker). The second problem this PR fixes is when using pnpm workspaces. With workspaces, modules are installed within the workspace the command is run in. This means that when installing as part of bundling in a nested directory, no `node_modules` directory is produced at all. By creating a new `pnpm-workspace.yaml` file locally before installing, this essentially creates a new (nested) workspace, and `node_modules` is installed here. An empty file is enough for this to suffice. The PR also removes a `.modules.yaml` file from the `node_modules` after installing. This file contains a datetime of the last install, therefore it would cause the lambda code to change on each deployment if it was included in the bundle. I have tested this fix locally on both Mac and Windows. I have also included an integration test which failed before these changes, however I am not sure if it should be included due to the `node_modules` in the assets. ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [X] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent b195465 commit 7c752db

20 files changed

+921
-13
lines changed

packages/@aws-cdk/aws-lambda-nodejs/lib/Dockerfile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ RUN mkdir /tmp/yarn-cache && \
2727
chmod -R 777 /tmp/yarn-cache && \
2828
yarn config set cache-folder /tmp/yarn-cache
2929

30+
# Ensure all users can write to pnpm cache
31+
RUN mkdir /tmp/pnpm-cache && \
32+
chmod -R 777 /tmp/pnpm-cache && \
33+
pnpm config --global set store-dir /tmp/pnpm-cache
34+
3035
# Disable npm update notifications
3136
RUN npm config --global set update-notifier false
3237

packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as path from 'path';
33
import { Architecture, AssetCode, Code, Runtime } from '@aws-cdk/aws-lambda';
44
import * as cdk from '@aws-cdk/core';
55
import { PackageInstallation } from './package-installation';
6-
import { PackageManager } from './package-manager';
6+
import { LockFile, PackageManager } from './package-manager';
77
import { BundlingOptions, OutputFormat, SourceMapMode } from './types';
88
import { exec, extractDependencies, findUp, getTsconfigCompilerOptions } from './util';
99

@@ -229,12 +229,16 @@ export class Bundling implements cdk.BundlingOptions {
229229

230230
const lockFilePath = pathJoin(options.inputDir, this.relativeDepsLockFilePath ?? this.packageManager.lockFile);
231231

232+
const isPnpm = this.packageManager.lockFile === LockFile.PNPM;
233+
232234
// Create dummy package.json, copy lock file if any and then install
233235
depsCommand = chain([
236+
isPnpm ? osCommand.write(pathJoin(options.outputDir, 'pnpm-workspace.yaml'), ''): '', // Ensure node_modules directory is installed locally by creating local 'pnpm-workspace.yaml' file
234237
osCommand.writeJson(pathJoin(options.outputDir, 'package.json'), { dependencies }),
235238
osCommand.copy(lockFilePath, pathJoin(options.outputDir, this.packageManager.lockFile)),
236239
osCommand.changeDirectory(options.outputDir),
237240
this.packageManager.installCommand.join(' '),
241+
isPnpm ? osCommand.remove(pathJoin(options.outputDir, 'node_modules', '.modules.yaml')) : '', // Remove '.modules.yaml' file which changes on each deployment
238242
]);
239243
}
240244

@@ -310,13 +314,20 @@ interface BundlingCommandOptions {
310314
class OsCommand {
311315
constructor(private readonly osPlatform: NodeJS.Platform) {}
312316

313-
public writeJson(filePath: string, data: any): string {
314-
const stringifiedData = JSON.stringify(data);
317+
public write(filePath: string, data: string): string {
315318
if (this.osPlatform === 'win32') {
316-
return `echo ^${stringifiedData}^ > "${filePath}"`;
319+
if (!data) { // if `data` is empty, echo a blank line, otherwise the file will contain a `^` character
320+
return `echo. > "${filePath}"`;
321+
}
322+
return `echo ^${data}^ > "${filePath}"`;
317323
}
318324

319-
return `echo '${stringifiedData}' > "${filePath}"`;
325+
return `echo '${data}' > "${filePath}"`;
326+
}
327+
328+
public writeJson(filePath: string, data: any): string {
329+
const stringifiedData = JSON.stringify(data);
330+
return this.write(filePath, stringifiedData);
320331
}
321332

322333
public copy(src: string, dest: string): string {
@@ -330,6 +341,14 @@ class OsCommand {
330341
public changeDirectory(dir: string): string {
331342
return `cd "${dir}"`;
332343
}
344+
345+
public remove(filePath: string): string {
346+
if (this.osPlatform === 'win32') {
347+
return `del "${filePath}"`;
348+
}
349+
350+
return `rm "${filePath}"`;
351+
}
333352
}
334353

335354
/**

packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,16 @@ export interface NodejsFunctionProps extends lambda.FunctionOptions {
5151
readonly awsSdkConnectionReuse?: boolean;
5252

5353
/**
54-
* The path to the dependencies lock file (`yarn.lock` or `package-lock.json`).
54+
* The path to the dependencies lock file (`yarn.lock`, `pnpm-lock.yaml` or `package-lock.json`).
5555
*
5656
* This will be used as the source for the volume mounted in the Docker
5757
* container.
5858
*
5959
* Modules specified in `nodeModules` will be installed using the right
60-
* installer (`npm` or `yarn`) along with this lock file.
60+
* installer (`yarn`, `pnpm` or `npm`) along with this lock file.
6161
*
6262
* @default - the path is found by walking up parent directories searching for
63-
* a `yarn.lock` or `package-lock.json` file
63+
* a `yarn.lock`, `pnpm-lock.yaml` or `package-lock.json` file
6464
*/
6565
readonly depsLockFilePath?: string;
6666

packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ export class PackageManager {
3939
case LockFile.PNPM:
4040
return new PackageManager({
4141
lockFile: LockFile.PNPM,
42-
installCommand: logLevel && logLevel !== LogLevel.INFO ? ['pnpm', 'install', '--reporter', 'silent'] : ['pnpm', 'install'],
42+
installCommand: logLevel && logLevel !== LogLevel.INFO ? ['pnpm', 'install', '--reporter', 'silent', '--config.node-linker=hoisted', '--config.package-import-method=clone-or-copy'] : ['pnpm', 'install', '--config.node-linker=hoisted', '--config.package-import-method=clone-or-copy'],
43+
// --config.node-linker=hoisted to create flat node_modules without symlinks
44+
// --config.package-import-method=clone-or-copy to avoid hardlinking packages from the store
4345
runCommand: ['pnpm', 'exec'],
4446
argsSeparator: '--',
4547
});

packages/@aws-cdk/aws-lambda-nodejs/lib/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ export interface BundlingOptions extends DockerRunOptions {
247247
* A custom bundling Docker image.
248248
*
249249
* This image should have esbuild installed globally. If you plan to use `nodeModules`
250-
* it should also have `npm` or `yarn` depending on the lock file you're using.
250+
* it should also have `npm`, `yarn` or `pnpm` depending on the lock file you're using.
251251
*
252252
* See https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-lambda-nodejs/lib/Dockerfile
253253
* for the default image provided by @aws-cdk/aws-lambda-nodejs.

packages/@aws-cdk/aws-lambda-nodejs/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
"@aws-cdk/integ-runner": "0.0.0",
7979
"@aws-cdk/integ-tests": "0.0.0",
8080
"@aws-cdk/pkglint": "0.0.0",
81+
"@aws-cdk/triggers": "0.0.0",
8182
"@types/jest": "^27.5.2",
8283
"delay": "5.0.0",
8384
"esbuild": "^0.16.6"

packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ test('Detects pnpm-lock.yaml', () => {
408408
assetHashType: AssetHashType.OUTPUT,
409409
bundling: expect.objectContaining({
410410
command: expect.arrayContaining([
411-
expect.stringMatching(/pnpm-lock\.yaml.+pnpm install/),
411+
expect.stringMatching(/echo '' > "\/asset-output\/pnpm-workspace.yaml\".+pnpm-lock\.yaml.+pnpm install --config.node-linker=hoisted --config.package-import-method=clone-or-copy && rm "\/asset-output\/node_modules\/.modules.yaml"/),
412412
]),
413413
}),
414414
});

packages/@aws-cdk/aws-lambda-nodejs/test/docker.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ test('cache folders have the right permissions', () => {
5858
'bash', '-c', [
5959
'stat -c \'%a\' /tmp/npm-cache',
6060
'stat -c \'%a\' /tmp/yarn-cache',
61+
'stat -c \'%a\' /tmp/pnpm-cache',
6162
].join(' && '),
6263
]);
6364
expect(proc.stdout.toString()).toMatch('777\n777');
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// eslint-disable-next-line import/no-extraneous-dependencies
2+
import axios from 'axios';
3+
4+
export async function handler() {
5+
await axios.get('https://www.google.com');
6+
}

packages/@aws-cdk/aws-lambda-nodejs/test/integ-handlers/pnpm/pnpm-lock.yaml

Lines changed: 70 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"version": "22.0.0",
3+
"files": {
4+
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
5+
"source": {
6+
"path": "PnpmTestDefaultTestDeployAssert397EDF83.template.json",
7+
"packaging": "file"
8+
},
9+
"destinations": {
10+
"current_account-current_region": {
11+
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
12+
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
13+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
14+
}
15+
}
16+
}
17+
},
18+
"dockerImages": {}
19+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
"Parameters": {
3+
"BootstrapVersion": {
4+
"Type": "AWS::SSM::Parameter::Value<String>",
5+
"Default": "/cdk-bootstrap/hnb659fds/version",
6+
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
7+
}
8+
},
9+
"Rules": {
10+
"CheckBootstrapVersion": {
11+
"Assertions": [
12+
{
13+
"Assert": {
14+
"Fn::Not": [
15+
{
16+
"Fn::Contains": [
17+
[
18+
"1",
19+
"2",
20+
"3",
21+
"4",
22+
"5"
23+
],
24+
{
25+
"Ref": "BootstrapVersion"
26+
}
27+
]
28+
}
29+
]
30+
},
31+
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
32+
}
33+
]
34+
}
35+
}
36+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
{
2+
"version": "22.0.0",
3+
"files": {
4+
"58d2406e0a74fd42ac31e584a098bb581709c3de7bff389451c2a08db50c4383": {
5+
"source": {
6+
"path": "asset.58d2406e0a74fd42ac31e584a098bb581709c3de7bff389451c2a08db50c4383",
7+
"packaging": "zip"
8+
},
9+
"destinations": {
10+
"current_account-current_region": {
11+
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
12+
"objectKey": "58d2406e0a74fd42ac31e584a098bb581709c3de7bff389451c2a08db50c4383.zip",
13+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
14+
}
15+
}
16+
},
17+
"44fdff138ed916c94d14f276217febea5c4a2e1746518f7f620c37b22a6675b8": {
18+
"source": {
19+
"path": "asset.44fdff138ed916c94d14f276217febea5c4a2e1746518f7f620c37b22a6675b8",
20+
"packaging": "zip"
21+
},
22+
"destinations": {
23+
"current_account-current_region": {
24+
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
25+
"objectKey": "44fdff138ed916c94d14f276217febea5c4a2e1746518f7f620c37b22a6675b8.zip",
26+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
27+
}
28+
}
29+
},
30+
"c51cd09452e746f11796192b2e9025f8c82de145d16aeb2e2c66c2197ecbae26": {
31+
"source": {
32+
"path": "TestStack.template.json",
33+
"packaging": "file"
34+
},
35+
"destinations": {
36+
"current_account-current_region": {
37+
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
38+
"objectKey": "c51cd09452e746f11796192b2e9025f8c82de145d16aeb2e2c66c2197ecbae26.json",
39+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
40+
}
41+
}
42+
}
43+
},
44+
"dockerImages": {}
45+
}

0 commit comments

Comments
 (0)