fix(aws-lambda-nodejs): use closest lockfile when autodetecting (aws#…

…16629)

fixes aws#15847

A bug in the automatic lockfile finding logic causes lockfiles higher in the directory tree to be used over lower/closer ones. This is because the code traverses the tree once per lockfile type in series, stopping when it finds one: https://github.com/aws/aws-cdk/blob/58fda9104ad884026d578dc0602f7d64dd533f6d/packages/%40aws-cdk/aws-lambda-nodejs/lib/function.ts#L137-L139

This updates the code to traverse the tree once looking for all the lockfile types at the same time and stop when one or more is found. If multiple are found at the same level, an error is thrown (per aws#15847 (comment)).

----

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

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

@@ -5,7 +5,7 @@ import { Architecture } from '@aws-cdk/aws-lambda';
 import { Bundling } from './bundling';
 import { PackageManager } from './package-manager';
 import { BundlingOptions } from './types';
-import { callsites, findUp } from './util';
+import { callsites, findUpMultiple } from './util';
 
 // keep this import separate from other imports to reduce chance for merge conflicts with v2-main
 // eslint-disable-next-line no-duplicate-imports, import/order
@@ -137,15 +137,20 @@ function findLockFile(depsLockFilePath?: string): string {
     return path.resolve(depsLockFilePath);
   }
 
-  const lockFile = findUp(PackageManager.PNPM.lockFile)
-    ?? findUp(PackageManager.YARN.lockFile)
-    ?? findUp(PackageManager.NPM.lockFile);
+  const lockFiles = findUpMultiple([
+    PackageManager.PNPM.lockFile,
+    PackageManager.YARN.lockFile,
+    PackageManager.NPM.lockFile,
+  ]);
 
-  if (!lockFile) {
+  if (lockFiles.length === 0) {
     throw new Error('Cannot find a package lock file (`pnpm-lock.yaml`, `yarn.lock` or `package-lock.json`). Please specify it with `depsFileLockPath`.');
   }
+  if (lockFiles.length > 1) {
+    throw new Error(`Multiple package lock files found: ${lockFiles.join(', ')}. Please specify the desired one with \`depsFileLockPath\`.`);
+  }
 
-  return lockFile;
+  return lockFiles[0];
 }
 
 /**

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

@@ -35,19 +35,34 @@ export function callsites(): CallSite[] {
  * Find a file by walking up parent directories
  */
 export function findUp(name: string, directory: string = process.cwd()): string | undefined {
+  return findUpMultiple([name], directory)[0];
+}
+
+/**
+ * Find the lowest of multiple files by walking up parent directories. If
+ * multiple files exist at the same level, they will all be returned.
+ */
+export function findUpMultiple(names: string[], directory: string = process.cwd()): string[] {
   const absoluteDirectory = path.resolve(directory);
 
-  const file = path.join(directory, name);
-  if (fs.existsSync(file)) {
-    return file;
+  const files = [];
+  for (const name of names) {
+    const file = path.join(directory, name);
+    if (fs.existsSync(file)) {
+      files.push(file);
+    }
+  }
+
+  if (files.length > 0) {
+    return files;
   }
 
   const { root } = path.parse(absoluteDirectory);
   if (absoluteDirectory === root) {
-    return undefined;
+    return [];
   }
 
-  return findUp(name, path.dirname(absoluteDirectory));
+  return findUpMultiple(names, path.dirname(absoluteDirectory));
 }
 
 /**

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

@@ -1,7 +1,7 @@
 import * as child_process from 'child_process';
 import * as fs from 'fs';
 import * as path from 'path';
-import { callsites, exec, extractDependencies, findUp } from '../lib/util';
+import { callsites, exec, extractDependencies, findUp, findUpMultiple } from '../lib/util';
 
 beforeEach(() => {
   jest.clearAllMocks();
@@ -33,6 +33,49 @@ describe('findUp', () => {
   });
 });
 
+describe('findUpMultiple', () => {
+  test('Starting at process.cwd()', () => {
+    const files = findUpMultiple(['README.md', 'package.json']);
+    expect(files).toHaveLength(2);
+    expect(files[0]).toMatch(/aws-lambda-nodejs\/README\.md$/);
+    expect(files[1]).toMatch(/aws-lambda-nodejs\/package\.json$/);
+  });
+
+  test('Non existing files', () => {
+    expect(findUpMultiple(['non-existing-file.unknown', 'non-existing-file.unknown2'])).toEqual([]);
+  });
+
+  test('Existing and non existing files', () => {
+    const files = findUpMultiple(['non-existing-file.unknown', 'README.md']);
+    expect(files).toHaveLength(1);
+    expect(files[0]).toMatch(/aws-lambda-nodejs\/README\.md$/);
+  });
+
+  test('Starting at a specific path', () => {
+    const files = findUpMultiple(['util.test.ts', 'function.test.ts'], path.join(__dirname, 'integ-handlers'));
+    expect(files).toHaveLength(2);
+    expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/);
+    expect(files[1]).toMatch(/aws-lambda-nodejs\/test\/function\.test\.ts$/);
+  });
+
+  test('Non existing files starting at a non existing relative path', () => {
+    expect(findUpMultiple(['not-to-be-found.txt', 'not-to-be-found2.txt'], 'non-existing/relative/path')).toEqual([]);
+  });
+
+  test('Starting at a relative path', () => {
+    const files = findUpMultiple(['util.test.ts', 'function.test.ts'], 'test/integ-handlers');
+    expect(files).toHaveLength(2);
+    expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/);
+    expect(files[1]).toMatch(/aws-lambda-nodejs\/test\/function\.test\.ts$/);
+  });
+
+  test('Files on multiple levels', () => {
+    const files = findUpMultiple(['README.md', 'util.test.ts'], path.join(__dirname, 'integ-handlers'));
+    expect(files).toHaveLength(1);
+    expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/);
+  });
+});
+
 describe('exec', () => {
   test('normal execution', () => {
     const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({