fix(lambda-nodejs): logLevel property of BundlingOptions is ignored when nodeModules are defined (#18456)

In this pull request I managed to propagate the effect of the `logLevel` property downstream to the `PackageManager` class.

I had to refactor the static approach for constructing handler objects for the various package managers as we need to pass a new optional property in the `fromLockFile(lockFilePath: string)` static builder method.

The value for `installCommand` attribute in the constructor now depends on the log level and the approach varies between package managers: while `npm ci` supports a flexible `--loglevel` command option matching each of the `LogLevel` enum existing values, `yarn install` only supports a `--silent` option and similarly `pnpm install` supports a `--reporter=silent` which suppresses the command output except for errors and warnings. For `yarn` and `pnpm` my idea is to include the silent options whenever a log level is specified and its value is different from the default `LogLevel.INFO`.

I also adapted existing unit tests and added new ones taking into account the property's value. I have no experience with yarn and pnpm and I am not sure that any tests exist using bundling with yarn- and pnpm-based lambda function handlers, so I cannot guarantee that my changes work for that too.

Fixes #18383

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: flavioleggio

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

@@ -86,7 +86,7 @@ export class Bundling implements cdk.BundlingOptions {
   private readonly packageManager: PackageManager;
 
   constructor(private readonly props: BundlingProps) {
-    this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath);
+    this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath, props.logLevel);
 
     Bundling.esbuildInstallation = Bundling.esbuildInstallation ?? PackageInstallation.detect('esbuild');
     Bundling.tscInstallation = Bundling.tscInstallation ?? PackageInstallation.detect('tsc');

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

@@ -3,7 +3,7 @@ import * as path from 'path';
 import * as lambda from '@aws-cdk/aws-lambda';
 import { Architecture } from '@aws-cdk/aws-lambda';
 import { Bundling } from './bundling';
-import { PackageManager } from './package-manager';
+import { LockFile } from './package-manager';
 import { BundlingOptions } from './types';
 import { callsites, findUpMultiple } from './util';
 
@@ -138,9 +138,9 @@ function findLockFile(depsLockFilePath?: string): string {
   }
 
   const lockFiles = findUpMultiple([
-    PackageManager.PNPM.lockFile,
-    PackageManager.YARN.lockFile,
-    PackageManager.NPM.lockFile,
+    LockFile.PNPM,
+    LockFile.YARN,
+    LockFile.NPM,
   ]);
 
   if (lockFiles.length === 0) {

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

@@ -1,48 +1,54 @@
 import * as os from 'os';
 import * as path from 'path';
+import { LogLevel } from './types';
 
 interface PackageManagerProps {
   readonly lockFile: string;
   readonly installCommand: string[];
   readonly runCommand: string[];
-  readonly argsSeparator?: string
+  readonly argsSeparator?: string;
+}
+
+export enum LockFile {
+  NPM = 'package-lock.json',
+  YARN = 'yarn.lock',
+  PNPM = 'pnpm-lock.yaml',
 }
 
 /**
  * A node package manager
  */
 export class PackageManager {
-  public static NPM = new PackageManager({
-    lockFile: 'package-lock.json',
-    installCommand: ['npm', 'ci'],
-    runCommand: ['npx', '--no-install'],
-  });
-
-  public static YARN = new PackageManager({
-    lockFile: 'yarn.lock',
-    installCommand: ['yarn', 'install', '--no-immutable'],
-    runCommand: ['yarn', 'run'],
-  });
-
-  public static PNPM = new PackageManager({
-    lockFile: 'pnpm-lock.yaml',
-    installCommand: ['pnpm', 'install'],
-    runCommand: ['pnpm', 'exec'],
-    argsSeparator: '--',
-  });
-
-  public static fromLockFile(lockFilePath: string): PackageManager {
+  /**
+   * Use a lock file path to determine the package manager to use. Optionally, specify a log level to
+   * control its verbosity.
+   * @param lockFilePath Path of the lock file
+   * @param logLevel optional log level @default LogLevel.INFO
+   * @returns the right PackageManager for that lock file
+   */
+  public static fromLockFile(lockFilePath: string, logLevel?: LogLevel): PackageManager {
     const lockFile = path.basename(lockFilePath);
 
     switch (lockFile) {
-      case PackageManager.NPM.lockFile:
-        return PackageManager.NPM;
-      case PackageManager.YARN.lockFile:
-        return PackageManager.YARN;
-      case PackageManager.PNPM.lockFile:
-        return PackageManager.PNPM;
+      case LockFile.YARN:
+        return new PackageManager({
+          lockFile: LockFile.YARN,
+          installCommand: logLevel && logLevel !== LogLevel.INFO ? ['yarn', 'install', '--no-immutable', '--silent'] : ['yarn', 'install', '--no-immutable'],
+          runCommand: ['yarn', 'run'],
+        });
+      case LockFile.PNPM:
+        return new PackageManager({
+          lockFile: LockFile.PNPM,
+          installCommand: logLevel && logLevel !== LogLevel.INFO ? ['pnpm', 'install', '--reporter', 'silent'] : ['pnpm', 'install'],
+          runCommand: ['pnpm', 'exec'],
+          argsSeparator: '--',
+        });
       default:
-        return PackageManager.NPM;
+        return new PackageManager({
+          lockFile: LockFile.NPM,
+          installCommand: logLevel ? ['npm', 'ci', '--loglevel', logLevel] : ['npm', 'ci'],
+          runCommand: ['npx', '--no-install'],
+        });
     }
   }
 

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

@@ -59,7 +59,8 @@ export interface BundlingOptions {
   readonly loader?: { [ext: string]: string };
 
   /**
-   * Log level for esbuild
+   * Log level for esbuild. This is also propagated to the package manager and
+   * applies to its specific install command.
    *
    * @default LogLevel.WARNING
    */
@@ -340,7 +341,7 @@ export interface ICommandHooks {
 }
 
 /**
- * Log level for esbuild
+ * Log levels for esbuild and package managers' install commands.
  */
 export enum LogLevel {
   /** Show everything */

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

@@ -422,6 +422,7 @@ test('Local bundling', () => {
     environment: {
       KEY: 'value',
     },
+    logLevel: LogLevel.ERROR,
   });
 
   expect(bundler.local).toBeDefined();

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

@@ -1,36 +1,62 @@
 import * as os from 'os';
-import { PackageManager } from '../lib/package-manager';
+import { LogLevel } from '../lib';
+import { LockFile, PackageManager } from '../lib/package-manager';
 
 test('from a package-lock.json', () => {
   const packageManager = PackageManager.fromLockFile('/path/to/package-lock.json');
-  expect(packageManager).toEqual(PackageManager.NPM);
+  expect(packageManager.lockFile).toEqual(LockFile.NPM);
+  expect(packageManager.argsSeparator).toBeUndefined();
+  expect(packageManager.installCommand).toEqual(['npm', 'ci']);
+  expect(packageManager.runCommand).toEqual(['npx', '--no-install']);
 
   expect(packageManager.runBinCommand('my-bin')).toBe('npx --no-install my-bin');
 });
 
+test('from a package-lock.json with LogLevel.ERROR', () => {
+  const logLevel = LogLevel.ERROR;
+  const packageManager = PackageManager.fromLockFile('/path/to/package-lock.json', logLevel);
+  expect(packageManager.installCommand).toEqual(['npm', 'ci', '--loglevel', logLevel]);
+});
+
 test('from a yarn.lock', () => {
   const packageManager = PackageManager.fromLockFile('/path/to/yarn.lock');
-  expect(packageManager).toEqual(PackageManager.YARN);
+  expect(packageManager.lockFile).toEqual(LockFile.YARN);
+  expect(packageManager.argsSeparator).toBeUndefined();
+  expect(packageManager.installCommand).toEqual(['yarn', 'install', '--no-immutable']);
+  expect(packageManager.runCommand).toEqual(['yarn', 'run']);
 
   expect(packageManager.runBinCommand('my-bin')).toBe('yarn run my-bin');
 });
 
+test('from a yarn.lock with LogLevel.ERROR', () => {
+  const packageManager = PackageManager.fromLockFile('/path/to/yarn.lock', LogLevel.ERROR);
+  expect(packageManager.installCommand).toEqual(['yarn', 'install', '--no-immutable', '--silent']);
+});
+
 test('from a pnpm-lock.yaml', () => {
   const packageManager = PackageManager.fromLockFile('/path/to/pnpm-lock.yaml');
-  expect(packageManager).toEqual(PackageManager.PNPM);
+  expect(packageManager.lockFile).toEqual(LockFile.PNPM);
+  expect(packageManager.argsSeparator).toEqual('--');
+  expect(packageManager.installCommand).toEqual(['pnpm', 'install']);
+  expect(packageManager.runCommand).toEqual(['pnpm', 'exec']);
 
   expect(packageManager.runBinCommand('my-bin')).toBe('pnpm exec -- my-bin');
 });
 
+test('from a pnpm-lock.yaml with LogLevel.ERROR', () => {
+  const packageManager = PackageManager.fromLockFile('/path/to/pnpm-lock.yaml', LogLevel.ERROR);
+  expect(packageManager.installCommand).toEqual(['pnpm', 'install', '--reporter', 'silent']);
+});
+
 test('defaults to NPM', () => {
   const packageManager = PackageManager.fromLockFile('/path/to/other.lock');
-  expect(packageManager).toEqual(PackageManager.NPM);
+  expect(packageManager.lockFile).toEqual(LockFile.NPM);
 });
 
 test('Windows', () => {
   const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32');
 
-  const packageManager = PackageManager.NPM;
+  const packageManager = PackageManager.fromLockFile('/path/to/whatever');
   expect(packageManager.runBinCommand('my-bin')).toEqual('npx.cmd --no-install my-bin');
 
   osPlatformMock.mockRestore();