From 79fb4f36dc679ec701532ac0c7d377fc6ff061fa Mon Sep 17 00:00:00 2001 From: AWS CDK Team Date: Wed, 19 May 2021 09:01:11 +0000 Subject: [PATCH 1/5] chore(release): 1.105.0 --- CHANGELOG.md | 25 +++++++++++++++++++++++++ version.v1.json | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30f83c5d5d399..6354de8d8c529 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,31 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [1.105.0](https://github.com/aws/aws-cdk/compare/v1.104.0...v1.105.0) (2021-05-19) + + +### ⚠ BREAKING CHANGES TO EXPERIMENTAL FEATURES + +* **lambda-nodejs:** using `banner` and `footer` now requires `esbuild` >= 0.9.0 + +### Features + +* **apigatewayv2:** http api - lambda authorizer ([#13181](https://github.com/aws/aws-cdk/issues/13181)) ([4da78f6](https://github.com/aws/aws-cdk/commit/4da78f6ba2036f4a94d0e47c8581131b9bc23e14)), closes [#10534](https://github.com/aws/aws-cdk/issues/10534) [/github.com/aws/aws-cdk/issues/10534#issuecomment-780271588](https://github.com/aws//github.com/aws/aws-cdk/issues/10534/issues/issuecomment-780271588) +* **custom-resources:** restrict output of AwsCustomResource to list of paths ([#14041](https://github.com/aws/aws-cdk/issues/14041)) ([773ca8c](https://github.com/aws/aws-cdk/commit/773ca8c5d2a845f392f530d7710020075b884c72)), closes [/github.com/aws/aws-cdk/issues/2825#issuecomment-814999890](https://github.com/aws//github.com/aws/aws-cdk/issues/2825/issues/issuecomment-814999890) +* **stepfunctions:** Add support for ResultSelector ([#14648](https://github.com/aws/aws-cdk/issues/14648)) ([50d486a](https://github.com/aws/aws-cdk/commit/50d486ad4e7d175dfac048dbb4abf5e4084ce4fe)), closes [#9904](https://github.com/aws/aws-cdk/issues/9904) + + +### Bug Fixes + +* **cli:** Updated typo user to uses ([#14357](https://github.com/aws/aws-cdk/issues/14357)) ([7fe329c](https://github.com/aws/aws-cdk/commit/7fe329cd17502cf04c451153f6d19955621952dc)) +* **core:** cannot determine packaging when bundling that produces an archive is skipped ([#14372](https://github.com/aws/aws-cdk/issues/14372)) ([163e812](https://github.com/aws/aws-cdk/commit/163e8122db994d0bea7077f025876dbeac490ead)), closes [#14369](https://github.com/aws/aws-cdk/issues/14369) +* **ecr:** add validations for ECR repository names ([#12613](https://github.com/aws/aws-cdk/issues/12613)) ([396dca9](https://github.com/aws/aws-cdk/commit/396dca965b56bfbe8a7aedb2bcaddb196b5560c4)), closes [#9877](https://github.com/aws/aws-cdk/issues/9877) +* **lambda:** unable to access SingletonFunction vpc connections ([#14533](https://github.com/aws/aws-cdk/issues/14533)) ([49d18ab](https://github.com/aws/aws-cdk/commit/49d18ab8e8f55f8b36584f7fb95427106139a140)), closes [#6261](https://github.com/aws/aws-cdk/issues/6261) +* **lambda-nodejs:** banner and footer values not escaped ([#14743](https://github.com/aws/aws-cdk/issues/14743)) ([81aa612](https://github.com/aws/aws-cdk/commit/81aa61213b4f5e3bd9cbbc155264252bd64d0f5b)), closes [#13576](https://github.com/aws/aws-cdk/issues/13576) +* **pipelines:** self-mutating builds cannot be run in privileged mode ([#14655](https://github.com/aws/aws-cdk/issues/14655)) ([73b9b4a](https://github.com/aws/aws-cdk/commit/73b9b4a89078d1425f4acdf50a6e9b5275b7e555)), closes [#11425](https://github.com/aws/aws-cdk/issues/11425) +* **pipelines:** stackOutput generates names too long to be used in useOutputs ([#14680](https://github.com/aws/aws-cdk/issues/14680)) ([d81e06d](https://github.com/aws/aws-cdk/commit/d81e06d5a5651cf332614d73e27bf6ed95d083a3)), closes [#13552](https://github.com/aws/aws-cdk/issues/13552) +* **pipelines:** synth fails if 'aws-cdk' is not in `package.json` ([#14745](https://github.com/aws/aws-cdk/issues/14745)) ([0b8ee97](https://github.com/aws/aws-cdk/commit/0b8ee97b7c029c5195de694a1d2eea309c343f61)), closes [#14658](https://github.com/aws/aws-cdk/issues/14658) + ## [1.104.0](https://github.com/aws/aws-cdk/compare/v1.103.0...v1.104.0) (2021-05-14) diff --git a/version.v1.json b/version.v1.json index 9076251f5032f..b99e8c208bdc0 100644 --- a/version.v1.json +++ b/version.v1.json @@ -1,3 +1,3 @@ { - "version": "1.104.0" + "version": "1.105.0" } From 0855649cf0c830f19653e6a127f2aca8cb38dcb3 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Wed, 19 May 2021 10:05:53 +0100 Subject: [PATCH 2/5] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6354de8d8c529..e601ac973e2c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,8 @@ All notable changes to this project will be documented in this file. See [standa ### Features -* **apigatewayv2:** http api - lambda authorizer ([#13181](https://github.com/aws/aws-cdk/issues/13181)) ([4da78f6](https://github.com/aws/aws-cdk/commit/4da78f6ba2036f4a94d0e47c8581131b9bc23e14)), closes [#10534](https://github.com/aws/aws-cdk/issues/10534) [/github.com/aws/aws-cdk/issues/10534#issuecomment-780271588](https://github.com/aws//github.com/aws/aws-cdk/issues/10534/issues/issuecomment-780271588) -* **custom-resources:** restrict output of AwsCustomResource to list of paths ([#14041](https://github.com/aws/aws-cdk/issues/14041)) ([773ca8c](https://github.com/aws/aws-cdk/commit/773ca8c5d2a845f392f530d7710020075b884c72)), closes [/github.com/aws/aws-cdk/issues/2825#issuecomment-814999890](https://github.com/aws//github.com/aws/aws-cdk/issues/2825/issues/issuecomment-814999890) +* **apigatewayv2:** http api - lambda authorizer ([#13181](https://github.com/aws/aws-cdk/issues/13181)) ([4da78f6](https://github.com/aws/aws-cdk/commit/4da78f6ba2036f4a94d0e47c8581131b9bc23e14)), closes [#10534](https://github.com/aws/aws-cdk/issues/10534) +* **custom-resources:** restrict output of AwsCustomResource to list of paths ([#14041](https://github.com/aws/aws-cdk/issues/14041)) ([773ca8c](https://github.com/aws/aws-cdk/commit/773ca8c5d2a845f392f530d7710020075b884c72)), closes [#2825](https://github.com/aws/aws-cdk/issues/2825) * **stepfunctions:** Add support for ResultSelector ([#14648](https://github.com/aws/aws-cdk/issues/14648)) ([50d486a](https://github.com/aws/aws-cdk/commit/50d486ad4e7d175dfac048dbb4abf5e4084ce4fe)), closes [#9904](https://github.com/aws/aws-cdk/issues/9904) From 5c84696a88f9319af1b2782b747e10f408c4c8fb Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 19 May 2021 12:39:19 +0200 Subject: [PATCH 3/5] fix(lambda-nodejs): esbuild detection with Yarn 2 in PnP mode (#14739) --- .../aws-lambda-nodejs/lib/bundling.ts | 170 ++++++++++-------- .../lib/esbuild-installation.ts | 35 ++++ .../aws-lambda-nodejs/lib/function.ts | 5 +- .../aws-lambda-nodejs/lib/package-manager.ts | 57 ++++++ .../@aws-cdk/aws-lambda-nodejs/lib/util.ts | 46 ++--- .../aws-lambda-nodejs/test/bundling.test.ts | 46 +++-- .../test/esbuild-installation.test.ts | 52 ++++++ .../test/package-manager.test.ts | 30 ++++ .../aws-lambda-nodejs/test/util.test.ts | 70 +------- 9 files changed, 317 insertions(+), 194 deletions(-) create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts index d28f43ca874c1..240114fbfa43d 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts @@ -2,10 +2,12 @@ import * as os from 'os'; import * as path from 'path'; import { AssetCode, Code, Runtime } from '@aws-cdk/aws-lambda'; import * as cdk from '@aws-cdk/core'; +import { EsbuildInstallation } from './esbuild-installation'; +import { PackageManager } from './package-manager'; import { BundlingOptions } from './types'; -import { exec, extractDependencies, findUp, getEsBuildVersion, LockFile } from './util'; +import { exec, extractDependencies, findUp } from './util'; -const ESBUILD_VERSION = '0'; +const ESBUILD_MAJOR_VERSION = '0'; /** * Bundling properties @@ -41,11 +43,11 @@ export class Bundling implements cdk.BundlingOptions { }); } - public static clearRunsLocallyCache(): void { - this.runsLocally = undefined; + public static clearEsbuildInstallationCache(): void { + this.esbuildInstallation = undefined; } - private static runsLocally?: boolean; + private static esbuildInstallation?: EsbuildInstallation; // Core bundling options public readonly image: cdk.DockerImage; @@ -54,20 +56,22 @@ export class Bundling implements cdk.BundlingOptions { public readonly workingDirectory: string; public readonly local?: cdk.ILocalBundling; + private readonly projectRoot: string; private readonly relativeEntryPath: string; private readonly relativeTsconfigPath?: string; private readonly externals: string[]; + private readonly packageManager: PackageManager; constructor(private readonly props: BundlingProps) { - Bundling.runsLocally = Bundling.runsLocally - ?? getEsBuildVersion()?.startsWith(ESBUILD_VERSION) - ?? false; + this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath); - const projectRoot = path.dirname(props.depsLockFilePath); - this.relativeEntryPath = path.relative(projectRoot, path.resolve(props.entry)); + Bundling.esbuildInstallation = Bundling.esbuildInstallation ?? EsbuildInstallation.detect(); + + this.projectRoot = path.dirname(props.depsLockFilePath); + this.relativeEntryPath = path.relative(this.projectRoot, path.resolve(props.entry)); if (props.tsconfig) { - this.relativeTsconfigPath = path.relative(projectRoot, path.resolve(props.tsconfig)); + this.relativeTsconfigPath = path.relative(this.projectRoot, path.resolve(props.tsconfig)); } this.externals = [ @@ -76,18 +80,23 @@ export class Bundling implements cdk.BundlingOptions { ]; // Docker bundling - const shouldBuildImage = props.forceDockerBundling || !Bundling.runsLocally; + const shouldBuildImage = props.forceDockerBundling || !Bundling.esbuildInstallation; this.image = shouldBuildImage ? props.dockerImage ?? cdk.DockerImage.fromBuild(path.join(__dirname, '../lib'), { buildArgs: { ...props.buildArgs ?? {}, - IMAGE: props.runtime.bundlingDockerImage.image, - ESBUILD_VERSION: props.esbuildVersion ?? ESBUILD_VERSION, + IMAGE: props.runtime.bundlingImage.image, + ESBUILD_VERSION: props.esbuildVersion ?? ESBUILD_MAJOR_VERSION, }, }) : cdk.DockerImage.fromRegistry('dummy'); // Do not build if we don't need to - const bundlingCommand = this.createBundlingCommand(cdk.AssetStaging.BUNDLING_INPUT_DIR, cdk.AssetStaging.BUNDLING_OUTPUT_DIR); + const bundlingCommand = this.createBundlingCommand({ + inputDir: cdk.AssetStaging.BUNDLING_INPUT_DIR, + outputDir: cdk.AssetStaging.BUNDLING_OUTPUT_DIR, + esbuildRunner: 'esbuild', // esbuild is installed globally in the docker image + osPlatform: 'linux', // linux docker image + }); this.command = ['bash', '-c', bundlingCommand]; this.environment = props.environment; // Bundling sets the working directory to cdk.AssetStaging.BUNDLING_INPUT_DIR @@ -96,54 +105,22 @@ export class Bundling implements cdk.BundlingOptions { // Local bundling if (!props.forceDockerBundling) { // only if Docker is not forced - const osPlatform = os.platform(); - const createLocalCommand = (outputDir: string) => this.createBundlingCommand(projectRoot, outputDir, osPlatform); - - this.local = { - tryBundle(outputDir: string) { - if (Bundling.runsLocally === false) { - process.stderr.write('esbuild cannot run locally. Switching to Docker bundling.\n'); - return false; - } - - const localCommand = createLocalCommand(outputDir); - - exec( - osPlatform === 'win32' ? 'cmd' : 'bash', - [ - osPlatform === 'win32' ? '/c' : '-c', - localCommand, - ], - { - env: { ...process.env, ...props.environment ?? {} }, - stdio: [ // show output - 'ignore', // ignore stdio - process.stderr, // redirect stdout to stderr - 'inherit', // inherit stderr - ], - cwd: path.dirname(props.entry), - windowsVerbatimArguments: osPlatform === 'win32', - }); - - return true; - }, - }; + this.local = this.getLocalBundlingProvider(); } } - public createBundlingCommand(inputDir: string, outputDir: string, osPlatform: NodeJS.Platform = 'linux'): string { - const pathJoin = osPathJoin(osPlatform); + private createBundlingCommand(options: BundlingCommandOptions): string { + const pathJoin = osPathJoin(options.osPlatform); - const npx = osPlatform === 'win32' ? 'npx.cmd' : 'npx'; const loaders = Object.entries(this.props.loader ?? {}); const defines = Object.entries(this.props.define ?? {}); - const esbuildCommand: string = [ - npx, 'esbuild', - '--bundle', `"${pathJoin(inputDir, this.relativeEntryPath)}"`, + const esbuildCommand: string[] = [ + options.esbuildRunner, + '--bundle', `"${pathJoin(options.inputDir, this.relativeEntryPath)}"`, `--target=${this.props.target ?? toTarget(this.props.runtime)}`, '--platform=node', - `--outfile="${pathJoin(outputDir, 'index.js')}"`, + `--outfile="${pathJoin(options.outputDir, 'index.js')}"`, ...this.props.minify ? ['--minify'] : [], ...this.props.sourceMap ? ['--sourcemap'] : [], ...this.externals.map(external => `--external:${external}`), @@ -151,11 +128,11 @@ export class Bundling implements cdk.BundlingOptions { ...defines.map(([key, value]) => `--define:${key}=${JSON.stringify(value)}`), ...this.props.logLevel ? [`--log-level=${this.props.logLevel}`] : [], ...this.props.keepNames ? ['--keep-names'] : [], - ...this.relativeTsconfigPath ? [`--tsconfig=${pathJoin(inputDir, this.relativeTsconfigPath)}`] : [], - ...this.props.metafile ? [`--metafile=${pathJoin(outputDir, 'index.meta.json')}`] : [], + ...this.relativeTsconfigPath ? [`--tsconfig=${pathJoin(options.inputDir, this.relativeTsconfigPath)}`] : [], + ...this.props.metafile ? [`--metafile=${pathJoin(options.outputDir, 'index.meta.json')}`] : [], ...this.props.banner ? [`--banner:js=${JSON.stringify(this.props.banner)}`] : [], ...this.props.footer ? [`--footer:js=${JSON.stringify(this.props.footer)}`] : [], - ].join(' '); + ]; let depsCommand = ''; if (this.props.nodeModules) { @@ -168,37 +145,78 @@ export class Bundling implements cdk.BundlingOptions { // Determine dependencies versions, lock file and installer const dependencies = extractDependencies(pkgPath, this.props.nodeModules); - let installer = Installer.NPM; - let lockFile = LockFile.NPM; - if (this.props.depsLockFilePath.endsWith(LockFile.YARN)) { - lockFile = LockFile.YARN; - installer = Installer.YARN; - } - - const osCommand = new OsCommand(osPlatform); + const osCommand = new OsCommand(options.osPlatform); // Create dummy package.json, copy lock file if any and then install depsCommand = chain([ - osCommand.writeJson(pathJoin(outputDir, 'package.json'), { dependencies }), - osCommand.copy(pathJoin(inputDir, lockFile), pathJoin(outputDir, lockFile)), - osCommand.changeDirectory(outputDir), - `${installer} install`, + osCommand.writeJson(pathJoin(options.outputDir, 'package.json'), { dependencies }), + osCommand.copy(pathJoin(options.inputDir, this.packageManager.lockFile), pathJoin(options.outputDir, this.packageManager.lockFile)), + osCommand.changeDirectory(options.outputDir), + this.packageManager.installCommand.join(' '), ]); } return chain([ - ...this.props.commandHooks?.beforeBundling(inputDir, outputDir) ?? [], - esbuildCommand, - ...(this.props.nodeModules && this.props.commandHooks?.beforeInstall(inputDir, outputDir)) ?? [], + ...this.props.commandHooks?.beforeBundling(options.inputDir, options.outputDir) ?? [], + esbuildCommand.join(' '), + ...(this.props.nodeModules && this.props.commandHooks?.beforeInstall(options.inputDir, options.outputDir)) ?? [], depsCommand, - ...this.props.commandHooks?.afterBundling(inputDir, outputDir) ?? [], + ...this.props.commandHooks?.afterBundling(options.inputDir, options.outputDir) ?? [], ]); } + + private getLocalBundlingProvider(): cdk.ILocalBundling { + const osPlatform = os.platform(); + const createLocalCommand = (outputDir: string, esbuild: EsbuildInstallation) => this.createBundlingCommand({ + inputDir: this.projectRoot, + outputDir, + esbuildRunner: esbuild.isLocal ? this.packageManager.runBinCommand('esbuild') : 'esbuild', + osPlatform, + }); + const environment = this.props.environment ?? {}; + const cwd = path.dirname(this.props.entry); + + return { + tryBundle(outputDir: string) { + if (!Bundling.esbuildInstallation) { + process.stderr.write('esbuild cannot run locally. Switching to Docker bundling.\n'); + return false; + } + + if (!Bundling.esbuildInstallation.version.startsWith(`${ESBUILD_MAJOR_VERSION}.`)) { + throw new Error(`Expected esbuild version ${ESBUILD_MAJOR_VERSION}.x but got ${Bundling.esbuildInstallation.version}`); + } + + const localCommand = createLocalCommand(outputDir, Bundling.esbuildInstallation); + + exec( + osPlatform === 'win32' ? 'cmd' : 'bash', + [ + osPlatform === 'win32' ? '/c' : '-c', + localCommand, + ], + { + env: { ...process.env, ...environment }, + stdio: [ // show output + 'ignore', // ignore stdio + process.stderr, // redirect stdout to stderr + 'inherit', // inherit stderr + ], + cwd, + windowsVerbatimArguments: osPlatform === 'win32', + }); + + return true; + }, + }; + } } -enum Installer { - NPM = 'npm', - YARN = 'yarn', +interface BundlingCommandOptions { + readonly inputDir: string; + readonly outputDir: string; + readonly esbuildRunner: string; + readonly osPlatform: NodeJS.Platform; } /** diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts new file mode 100644 index 0000000000000..8ef2e8dbb23d9 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts @@ -0,0 +1,35 @@ +import { spawnSync } from 'child_process'; +import { tryGetModuleVersion } from './util'; + +/** + * An esbuild installation + */ +export abstract class EsbuildInstallation { + public static detect(): EsbuildInstallation | undefined { + try { + // Check local version first + const version = tryGetModuleVersion('esbuild'); + if (version) { + return { + isLocal: true, + version, + }; + } + + // Fallback to a global version + const esbuild = spawnSync('esbuild', ['--version']); + if (esbuild.status === 0 && !esbuild.error) { + return { + isLocal: false, + version: esbuild.stdout.toString().trim(), + }; + } + return undefined; + } catch (err) { + return undefined; + } + } + + public abstract readonly isLocal: boolean; + public abstract readonly version: string; +} diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts index 00e5315654796..476c1e45b595c 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts @@ -2,8 +2,9 @@ import * as fs from 'fs'; import * as path from 'path'; import * as lambda from '@aws-cdk/aws-lambda'; import { Bundling } from './bundling'; +import { PackageManager } from './package-manager'; import { BundlingOptions } from './types'; -import { callsites, findUp, LockFile } from './util'; +import { callsites, findUp } 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 @@ -94,7 +95,7 @@ export class NodejsFunction extends lambda.Function { } depsLockFilePath = path.resolve(props.depsLockFilePath); } else { - const lockFile = findUp(LockFile.YARN) ?? findUp(LockFile.NPM); + const lockFile = findUp(PackageManager.YARN.lockFile) ?? findUp(PackageManager.NPM.lockFile); if (!lockFile) { throw new Error('Cannot find a package lock file (`yarn.lock` or `package-lock.json`). Please specify it with `depsFileLockPath`.'); } diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts new file mode 100644 index 0000000000000..dd5260f87838c --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts @@ -0,0 +1,57 @@ +import * as os from 'os'; +import * as path from 'path'; + +interface PackageManagerProps { + readonly lockFile: string; + readonly installCommand: string[]; + readonly runCommand: string[]; +} + +/** + * A node package manager + */ +export class PackageManager { + public static NPM = new PackageManager({ + lockFile: 'package-lock.json', + installCommand: ['npm', 'install'], + runCommand: ['npx', '--no-install'], + }); + + public static YARN = new PackageManager({ + lockFile: 'yarn.lock', + installCommand: ['yarn', 'install'], + runCommand: ['yarn', 'run'], + }); + + public static fromLockFile(lockFilePath: string): PackageManager { + const lockFile = path.basename(lockFilePath); + + switch (lockFile) { + case PackageManager.NPM.lockFile: + return PackageManager.NPM; + case PackageManager.YARN.lockFile: + return PackageManager.YARN; + default: + return PackageManager.NPM; + } + } + + public readonly lockFile: string; + public readonly installCommand: string[]; + public readonly runCommand: string[]; + + constructor(props: PackageManagerProps) { + this.lockFile = props.lockFile; + this.installCommand = props.installCommand; + this.runCommand = props.runCommand; + } + + public runBinCommand(bin: string): string { + const [runCommand, ...runArgs] = this.runCommand; + return [ + os.platform() === 'win32' ? `${runCommand}.cmd` : runCommand, + ...runArgs, + bin, + ].join(' '); + } +} diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts index 206941f71f66d..5ead91793e93b 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts @@ -1,6 +1,5 @@ import { spawnSync, SpawnSyncOptions } from 'child_process'; import * as fs from 'fs'; -import * as os from 'os'; import * as path from 'path'; export interface CallSite { @@ -71,6 +70,18 @@ export function exec(cmd: string, args: string[], options?: SpawnSyncOptions) { return proc; } +/** + * Returns a module version by requiring its package.json file + */ +export function tryGetModuleVersion(mod: string): string | undefined { + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + return require(`${mod}/package.json`).version; + } catch (err) { + return undefined; + } +} + /** * Extract versions for a list of modules. * @@ -90,39 +101,12 @@ export function extractDependencies(pkgPath: string, modules: string[]): { [key: }; for (const mod of modules) { - try { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const version = pkgDependencies[mod] ?? require(`${mod}/package.json`).version; - dependencies[mod] = version; - } catch (err) { + const version = pkgDependencies[mod] ?? tryGetModuleVersion(mod); + if (!version) { throw new Error(`Cannot extract version for module '${mod}'. Check that it's referenced in your package.json or installed.`); } + dependencies[mod] = version; } return dependencies; } - -/** - * Returns the installed esbuild version - */ -export function getEsBuildVersion(): string | undefined { - try { - // --no-install ensures that we are checking for an installed version - // (either locally or globally) - const npx = os.platform() === 'win32' ? 'npx.cmd' : 'npx'; - const esbuild = spawnSync(npx, ['--no-install', 'esbuild', '--version']); - - if (esbuild.status !== 0 || esbuild.error) { - return undefined; - } - - return esbuild.stdout.toString().trim(); - } catch (err) { - return undefined; - } -} - -export enum LockFile { - NPM = 'package-lock.json', - YARN = 'yarn.lock' -} diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts index e5441cb5aeadb..7d79365da5077 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -5,20 +5,27 @@ import { Code, Runtime } from '@aws-cdk/aws-lambda'; import { AssetHashType, DockerImage } from '@aws-cdk/core'; import { version as delayVersion } from 'delay/package.json'; import { Bundling } from '../lib/bundling'; +import { EsbuildInstallation } from '../lib/esbuild-installation'; import { LogLevel } from '../lib/types'; import * as util from '../lib/util'; jest.mock('@aws-cdk/aws-lambda'); -// Mock BundlingDockerImage.fromAsset() to avoid building the image -let fromAssetMock = jest.spyOn(DockerImage, 'fromBuild'); -let getEsBuildVersionMock = jest.spyOn(util, 'getEsBuildVersion'); +// Mock DockerImage.fromAsset() to avoid building the image +let fromBuildMock: jest.SpyInstance; +let detectEsbuildMock: jest.SpyInstance; beforeEach(() => { jest.clearAllMocks(); jest.resetAllMocks(); - Bundling.clearRunsLocallyCache(); - getEsBuildVersionMock.mockReturnValue('0.8.8'); - fromAssetMock.mockReturnValue({ + jest.restoreAllMocks(); + Bundling.clearEsbuildInstallationCache(); + + detectEsbuildMock = jest.spyOn(EsbuildInstallation, 'detect').mockReturnValue({ + isLocal: true, + version: '0.8.8', + }); + + fromBuildMock = jest.spyOn(DockerImage, 'fromBuild').mockReturnValue({ image: 'built-image', cp: () => 'dest-path', run: () => {}, @@ -53,7 +60,7 @@ test('esbuild bundling in Docker', () => { }, command: [ 'bash', '-c', - 'npx esbuild --bundle "/asset-input/lib/handler.ts" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk --loader:.png=dataurl', + 'esbuild --bundle "/asset-input/lib/handler.ts" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk --loader:.png=dataurl', ], workingDirectory: '/', }), @@ -74,7 +81,7 @@ test('esbuild bundling with handler named index.ts', () => { bundling: expect.objectContaining({ command: [ 'bash', '-c', - 'npx esbuild --bundle "/asset-input/lib/index.ts" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk', + 'esbuild --bundle "/asset-input/lib/index.ts" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk', ], }), }); @@ -94,7 +101,7 @@ test('esbuild bundling with tsx handler', () => { bundling: expect.objectContaining({ command: [ 'bash', '-c', - 'npx esbuild --bundle "/asset-input/lib/handler.tsx" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk', + 'esbuild --bundle "/asset-input/lib/handler.tsx" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk', ], }), }); @@ -102,6 +109,9 @@ test('esbuild bundling with tsx handler', () => { test('esbuild with Windows paths', () => { const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32'); + // Mock path.basename() because it cannot extract the basename of a Windows + // path when running on Linux + jest.spyOn(path, 'basename').mockReturnValueOnce('package-lock.json'); Bundling.bundle({ entry: 'C:\\my-project\\lib\\entry.ts', @@ -139,7 +149,7 @@ test('esbuild bundling with externals and dependencies', () => { command: [ 'bash', '-c', [ - 'npx esbuild --bundle "/asset-input/test/bundling.test.js" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:abc --external:delay', + 'esbuild --bundle "/asset-input/test/bundling.test.js" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:abc --external:delay', `echo \'{\"dependencies\":{\"delay\":\"${delayVersion}\"}}\' > /asset-output/package.json`, 'cp /asset-input/package-lock.json /asset-output/package-lock.json', 'cd /asset-output', @@ -184,7 +194,7 @@ test('esbuild bundling with esbuild options', () => { command: [ 'bash', '-c', [ - 'npx esbuild --bundle "/asset-input/lib/handler.ts"', + 'esbuild --bundle "/asset-input/lib/handler.ts"', '--target=es2020 --platform=node --outfile="/asset-output/index.js"', '--minify --sourcemap --external:aws-sdk --loader:.png=dataurl', defineInstructions, @@ -232,7 +242,7 @@ test('with Docker build args', () => { forceDockerBundling: true, }); - expect(fromAssetMock).toHaveBeenCalledWith(expect.stringMatching(/lib$/), expect.objectContaining({ + expect(fromBuildMock).toHaveBeenCalledWith(expect.stringMatching(/lib$/), expect.objectContaining({ buildArgs: expect.objectContaining({ HELLO: 'WORLD', }), @@ -273,14 +283,17 @@ test('Local bundling', () => { ); // Docker image is not built - expect(fromAssetMock).not.toHaveBeenCalled(); + expect(fromBuildMock).not.toHaveBeenCalled(); spawnSyncMock.mockRestore(); }); test('Incorrect esbuild version', () => { - getEsBuildVersionMock.mockReturnValueOnce('3.4.5'); + detectEsbuildMock.mockReturnValueOnce({ + isLocal: true, + version: '3.4.5', + }); const bundler = new Bundling({ entry, @@ -288,8 +301,9 @@ test('Incorrect esbuild version', () => { runtime: Runtime.NODEJS_12_X, }); - const tryBundle = bundler.local?.tryBundle('/outdir', { image: Runtime.NODEJS_12_X.bundlingDockerImage }); - expect(tryBundle).toBe(false); + expect(() => bundler.local?.tryBundle('/outdir', { + image: Runtime.NODEJS_12_X.bundlingImage, + })).toThrow(/Expected esbuild version 0.x but got 3.4.5/); }); test('Custom bundling docker image', () => { diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts new file mode 100644 index 0000000000000..24b9b512c98bc --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts @@ -0,0 +1,52 @@ +import * as child_process from 'child_process'; +import { EsbuildInstallation } from '../lib/esbuild-installation'; +import * as util from '../lib/util'; + +// eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies +const version = require('esbuild/package.json').version; + +test('detects local version', () => { + expect(EsbuildInstallation.detect()).toEqual({ + isLocal: true, + version, + }); +}); + +test('checks global version if local detection fails', () => { + const getModuleVersionMock = jest.spyOn(util, 'tryGetModuleVersion').mockReturnValue(undefined); + const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ + status: 0, + stderr: Buffer.from('stderr'), + stdout: Buffer.from('global-version'), + pid: 123, + output: ['stdout', 'stderr'], + signal: null, + }); + + expect(EsbuildInstallation.detect()).toEqual({ + isLocal: false, + version: 'global-version', + }); + + spawnSyncMock.mockRestore(); + getModuleVersionMock.mockRestore(); +}); + +test('returns undefined on error', () => { + const getModuleVersionMock = jest.spyOn(util, 'tryGetModuleVersion').mockReturnValue(undefined); + const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ + error: new Error('bad error'), + status: 0, + stderr: Buffer.from('stderr'), + stdout: Buffer.from('stdout'), + pid: 123, + output: ['stdout', 'stderr'], + signal: null, + }); + + expect(EsbuildInstallation.detect()).toBeUndefined(); + + spawnSyncMock.mockRestore(); + getModuleVersionMock.mockRestore(); +}); + diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts new file mode 100644 index 0000000000000..f561bce592f12 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts @@ -0,0 +1,30 @@ +import * as os from 'os'; +import { 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.runBinCommand('my-bin')).toBe('npx --no-install my-bin'); +}); + +test('from a yarn.lock', () => { + const packageManager = PackageManager.fromLockFile('/path/to/yarn.lock'); + expect(packageManager).toEqual(PackageManager.YARN); + + expect(packageManager.runBinCommand('my-bin')).toBe('yarn run my-bin'); +}); + +test('defaults to NPM', () => { + const packageManager = PackageManager.fromLockFile('/path/to/pnpm-lock.yaml'); + expect(packageManager).toEqual(PackageManager.NPM); +}); + +test('Windows', () => { + const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32'); + + const packageManager = PackageManager.NPM; + expect(packageManager.runBinCommand('my-bin')).toEqual('npx.cmd --no-install my-bin'); + + osPlatformMock.mockRestore(); +}); diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts index df91c4433f153..4962ed203b31f 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts @@ -1,7 +1,6 @@ import * as child_process from 'child_process'; -import * as os from 'os'; import * as path from 'path'; -import { callsites, exec, extractDependencies, findUp, getEsBuildVersion } from '../lib/util'; +import { callsites, exec, extractDependencies, findUp } from '../lib/util'; beforeEach(() => { jest.clearAllMocks(); @@ -121,70 +120,3 @@ describe('extractDependencies', () => { )).toThrow(/Cannot extract version for module 'unknown'/); }); }); - -describe('getEsBuildVersion', () => { - test('returns the version', () => { - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - status: 0, - stderr: Buffer.from('stderr'), - stdout: Buffer.from('version'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - - expect(getEsBuildVersion()).toBe('version'); - expect(spawnSyncMock).toHaveBeenCalledWith('npx', ['--no-install', 'esbuild', '--version']); - - spawnSyncMock.mockRestore(); - }); - - test('returns undefined on non zero status', () => { - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - status: 127, // status error - stderr: Buffer.from('stderr'), - stdout: Buffer.from('stdout'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - - expect(getEsBuildVersion()).toBeUndefined(); - - spawnSyncMock.mockRestore(); - }); - - test('returns undefined on error', () => { - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - error: new Error('bad error'), - status: 0, - stderr: Buffer.from('stderr'), - stdout: Buffer.from('stdout'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - - expect(getEsBuildVersion()).toBeUndefined(); - - spawnSyncMock.mockRestore(); - }); - - test('Windows', () => { - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - status: 0, - stderr: Buffer.from('stderr'), - stdout: Buffer.from('version'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32'); - - expect(getEsBuildVersion()).toBe('version'); - expect(spawnSyncMock).toHaveBeenCalledWith('npx.cmd', expect.any(Array)); - - spawnSyncMock.mockRestore(); - osPlatformMock.mockRestore(); - }); -}); From 7013f5884f298ab217044dd104e27236e53c57c0 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Wed, 19 May 2021 14:59:05 +0100 Subject: [PATCH 4/5] chore: permissions for github actions (#14767) By default, all Github actions have read permissions via the standard `GITHUB_TOKEN`. For jobs that require write permission, explicitly add the necessary permissions. In the case of the 'Yarn Upgrade' Github action, separated the 'upgrade' step and the 'pull request' step into separate jobs to build a better security boundary between the two. Inspired from: https://github.com/projen/projen/blob/a4f875d07b57f8f8247b9352e34c3c83759afe82/.github/workflows/upgrade-dependencies.yml --- .github/workflows/auto-approve.yml | 2 ++ .github/workflows/close-stale-issues.yml | 2 ++ .github/workflows/closed-issue-message.yml | 30 ++++++++-------- .github/workflows/issue-label-assign.yml | 2 ++ .github/workflows/pr-linter.yml | 2 ++ .github/workflows/v2-pull-request.yml | 3 ++ .github/workflows/yarn-upgrade.yml | 41 +++++++++++++++++++--- 7 files changed, 63 insertions(+), 19 deletions(-) diff --git a/.github/workflows/auto-approve.yml b/.github/workflows/auto-approve.yml index 9b185ec0fcd47..cf881f5bf20f2 100644 --- a/.github/workflows/auto-approve.yml +++ b/.github/workflows/auto-approve.yml @@ -13,6 +13,8 @@ jobs: || github.event.pull_request.user.login == 'dependabot[bot]' || github.event.pull_request.user.login == 'dependabot-preview[bot]') runs-on: ubuntu-latest + permissions: + pull-requests: write steps: - uses: hmarr/auto-approve-action@v2.1.0 with: diff --git a/.github/workflows/close-stale-issues.yml b/.github/workflows/close-stale-issues.yml index df8e9f3d5ffb5..db75a56aa7f8b 100644 --- a/.github/workflows/close-stale-issues.yml +++ b/.github/workflows/close-stale-issues.yml @@ -7,6 +7,8 @@ on: jobs: cleanup: + permissions: + issues: write runs-on: ubuntu-latest name: Stale issue job steps: diff --git a/.github/workflows/closed-issue-message.yml b/.github/workflows/closed-issue-message.yml index 3340afb1f3b11..f1a0a3b0fc7b1 100644 --- a/.github/workflows/closed-issue-message.yml +++ b/.github/workflows/closed-issue-message.yml @@ -1,17 +1,19 @@ name: Closed Issue Message on: - issues: - types: [closed] + issues: + types: [closed] jobs: - auto_comment: - runs-on: ubuntu-latest - steps: - - uses: aws-actions/closed-issue-message@v1 - with: - # These inputs are both required - repo-token: "${{ secrets.GITHUB_TOKEN }}" - message: | - ### ⚠️COMMENT VISIBILITY WARNING⚠️ - Comments on closed issues are hard for our team to see. - If you need more assistance, please either tag a team member or open a new issue that references this one. - If you wish to keep having a conversation with other community members under this issue feel free to do so. + auto_comment: + permissions: + issues: write + runs-on: ubuntu-latest + steps: + - uses: aws-actions/closed-issue-message@v1 + with: + # These inputs are both required + repo-token: "${{ secrets.GITHUB_TOKEN }}" + message: | + ### ⚠️COMMENT VISIBILITY WARNING⚠️ + Comments on closed issues are hard for our team to see. + If you need more assistance, please either tag a team member or open a new issue that references this one. + If you wish to keep having a conversation with other community members under this issue feel free to do so. diff --git a/.github/workflows/issue-label-assign.yml b/.github/workflows/issue-label-assign.yml index 237e44deefc36..70b40968a4289 100644 --- a/.github/workflows/issue-label-assign.yml +++ b/.github/workflows/issue-label-assign.yml @@ -9,6 +9,8 @@ on: jobs: test: + permissions: + issues: write runs-on: ubuntu-latest steps: - uses: Naturalclar/issue-action@v2.0.2 diff --git a/.github/workflows/pr-linter.yml b/.github/workflows/pr-linter.yml index 5702b254d4a0b..80137c4068020 100644 --- a/.github/workflows/pr-linter.yml +++ b/.github/workflows/pr-linter.yml @@ -14,6 +14,8 @@ on: jobs: validate-pr: + permissions: + pull-requests: read runs-on: ubuntu-latest steps: diff --git a/.github/workflows/v2-pull-request.yml b/.github/workflows/v2-pull-request.yml index e820647947705..351ee2c8c427f 100644 --- a/.github/workflows/v2-pull-request.yml +++ b/.github/workflows/v2-pull-request.yml @@ -16,6 +16,9 @@ on: jobs: # Run yarn pkglint on merge forward PRs and commit the results pkglint: + permissions: + pull-requests: write + contents: write if: contains(github.event.pull_request.labels.*.name, 'pr/forward-merge') runs-on: ubuntu-latest steps: diff --git a/.github/workflows/yarn-upgrade.yml b/.github/workflows/yarn-upgrade.yml index c0df170ff30ef..dd3bec734d944 100644 --- a/.github/workflows/yarn-upgrade.yml +++ b/.github/workflows/yarn-upgrade.yml @@ -1,10 +1,9 @@ name: Yarn Upgrade on: - # Disable this workflow - #schedule: + schedule: # Every wednesday at 13:37 UTC - #- cron: 37 13 * * 3 + - cron: 37 13 * * 3 workflow_dispatch: {} jobs: @@ -69,6 +68,39 @@ jobs: # also - jest-enviroment-jsdom doesnt actually require 16.5.1 (https://github.com/facebook/jest/blob/master/packages/jest-environment-jsdom/package.json#L23) run: yarn upgrade --pattern '!(jsdom)' + # Next, create and upload the changes as a patch file. This will later be downloaded to create a pull request + # Creating a pull request requires write permissions and it's best to keep write privileges isolated. + - name: Create Patch + run: |- + git add . + git diff --patch --staged > ${{ runner.temp }}/upgrade.patch + - name: Upload Patch + uses: actions/upload-artifact@v2 + with: + name: upgrade.patch + path: ${{ runner.temp }}/upgrade.patch + + pr: + name: Create Pull Request + needs: upgrade + permissions: + contents: write + pull-requests: write + runs-on: ubuntu-latest + steps: + - name: Check Out + uses: actions/checkout@v2 + + - name: Download patch + uses: actions/download-artifact@v2 + with: + name: upgrade.patch + path: ${{ runner.temp }} + + - name: Apply patch + run: '[ -s ${{ runner.temp }}/upgrade.patch ] && git apply ${{ runner.temp + }}/upgrade.patch || echo "Empty patch. Skipping."' + - name: Make Pull Request uses: peter-evans/create-pull-request@v3 with: @@ -83,5 +115,4 @@ jobs: Ran npm-check-updates and yarn upgrade to keep the `yarn.lock` file up-to-date. labels: contribution/core,dependencies,pr/auto-approve team-reviewers: aws-cdk-team - # Privileged token so automated PR validation happens - token: ${{ secrets.AUTOMATION_GITHUB_TOKEN }} + token: ${{ secrets.GITHUB_TOKEN }} From b816ef902b8e4285f8c2476d839f6fc98db6f557 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Wed, 19 May 2021 15:14:25 +0100 Subject: [PATCH 5/5] chore: switch back to privileged token in upgrade github action (#14775) The previous commit incorrectly removed this token and used the default Github token. Github prevents subsequent Github actions to be triggered if the default token is used. Switch it back. --- .github/workflows/yarn-upgrade.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/yarn-upgrade.yml b/.github/workflows/yarn-upgrade.yml index dd3bec734d944..473e9423051cb 100644 --- a/.github/workflows/yarn-upgrade.yml +++ b/.github/workflows/yarn-upgrade.yml @@ -9,6 +9,8 @@ on: jobs: upgrade: name: Yarn Upgrade + permissions: + contents: read runs-on: ubuntu-latest steps: @@ -115,4 +117,6 @@ jobs: Ran npm-check-updates and yarn upgrade to keep the `yarn.lock` file up-to-date. labels: contribution/core,dependencies,pr/auto-approve team-reviewers: aws-cdk-team - token: ${{ secrets.GITHUB_TOKEN }} + # Github prevents further Github actions to be run if the default Github token is used. + # Instead use a privileged token here, so further GH actions can be triggered on this PR. + token: ${{ secrets.AUTOMATION_GITHUB_TOKEN }}