From d7b1c6a4b723a5c71112219fc804c5c39fc87cf0 Mon Sep 17 00:00:00 2001 From: Fritz Kunstler Date: Fri, 19 Jan 2024 20:08:13 +0000 Subject: [PATCH 1/5] Implemented local asset bundling, which will be used by default (instead of Docker) when possible. --- lib/layer/index.ts | 43 ++++++++++++++++++++ lib/shared/shared-asset-bundler.ts | 64 +++++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/lib/layer/index.ts b/lib/layer/index.ts index 9505b293c..793f06d60 100644 --- a/lib/layer/index.ts +++ b/lib/layer/index.ts @@ -1,6 +1,8 @@ import * as cdk from "aws-cdk-lib"; import * as lambda from "aws-cdk-lib/aws-lambda"; import * as s3assets from "aws-cdk-lib/aws-s3-assets"; +import * as lpath from "path"; +import { execSync } from "child_process"; import { Construct } from "constructs"; interface LayerProps { @@ -26,6 +28,47 @@ export class Layer extends Construct { const layerAsset = new s3assets.Asset(this, "LayerAsset", { path, bundling: { + local: { + /* implements a local method of bundling that does not depend on Docker. Local + bundling is preferred over DIND for performance and security reasons. + see https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.ILocalBundling.html and + https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3_assets-readme.html#asset-bundling */ + tryBundle(outputDir: string, options: cdk.BundlingOptions) { + let canRunLocal = false; + let python = props.runtime.name; + + try { + // check if pip is available locally + const testCommand = `${python} -m pip -V` + console.log(`Checking for pip: ${testCommand}`) + // without the stdio arg no output is printed to console + execSync(testCommand, { stdio: 'inherit' }); + // no exception means command executed successfully + canRunLocal = true; + } catch { + // execSync throws Error in case return value of child process is non-zero. + // Actual output should be printed to the console. + console.warn(`Unable to do local bundling! ${python} with pip must be on path.`); + } + + if (canRunLocal) { + const command = `${python} -m pip install -r ${lpath.posix.join(path, "requirements.txt")} -t ${outputDir} ${autoUpgrade ? '-U' : ''}`; + try { + console.debug(`Local bundling: ${command}`); + // this is where the work gets done + execSync(command, { stdio: 'inherit' }); + return true; + } catch (ex) { + // execSync throws Error in case return value of child process + // is non-zero. It'll be printed to the console because of the + // stdio argument. + console.log(`Local bundling attempt failed: ${ex}`) + } + } + // if we get here then Docker will be used as configured below + return false; + } + }, image: runtime.bundlingImage, platform: architecture.dockerPlatform, command: [ diff --git a/lib/shared/shared-asset-bundler.ts b/lib/shared/shared-asset-bundler.ts index 838e9d62a..196c11a26 100644 --- a/lib/shared/shared-asset-bundler.ts +++ b/lib/shared/shared-asset-bundler.ts @@ -3,6 +3,7 @@ import { BundlingOutput, DockerImage, aws_s3_assets, + BundlingOptions } from "aws-cdk-lib"; import { Code, S3Code } from "aws-cdk-lib/aws-lambda"; import { Asset } from "aws-cdk-lib/aws-s3-assets"; @@ -10,6 +11,7 @@ import { md5hash } from "aws-cdk-lib/core/lib/helpers-internal"; import { Construct } from "constructs"; import * as path from "path"; import * as fs from "fs"; +import { execSync } from "child_process"; function calculateHash(paths: string[]): string { return paths.reduce((mh, p) => { @@ -33,6 +35,8 @@ function calculateHash(paths: string[]): string { export class SharedAssetBundler extends Construct { private readonly sharedAssets: string[]; private readonly WORKING_PATH = "/asset-input/"; + private readonly container_image: DockerImage; + private useLocalBundler: boolean = false; /** * Instantiate a new SharedAssetBundler. You then invoke `bundleWithAsset(pathToAsset)` to * bundle your asset code with the common code. @@ -47,17 +51,75 @@ export class SharedAssetBundler extends Construct { constructor(scope: Construct, id: string, sharedAssets: string[]) { super(scope, id); this.sharedAssets = sharedAssets; + // Check if we can do local bundling + if (!this.localBundlerTest()) { + // if not, then build Apline from local definition + this.container_image = DockerImage.fromBuild(path.posix.join(__dirname, "alpine-zip")); + } else { + // if yes, then don't build the container. https://hub.docker.com/_/scratch/ + this.container_image = DockerImage.fromRegistry("scratch"); + } + } + + /** + * Check if possible to use local bundling instead of Docker. Sets this.useLocalBundler to + * true if local environment supports bundling. See below in method bundleWithAsset(...). + */ + private localBundlerTest(): boolean { + const command = "zip -v"; + console.log(`Checking for zip: ${command}`); + // check if zip is available locally + try { + // without stdio option command output does not appear in console + execSync(command, {stdio: 'inherit'}); + // no exception means command executed successfully + this.useLocalBundler = true; + } catch { + // execSync throws Error in case return value of child process + // is non-zero. Actual output should be printed to the console. + console.warn("Unable to do local bundling! Is zip installed?"); + } + return this.useLocalBundler; } bundleWithAsset(assetPath: string): Asset { console.log(`Bundling asset ${assetPath}`); + + // necessary for access from anonymous class + const runLocal = this.useLocalBundler; + const asset = new aws_s3_assets.Asset( this, md5hash(assetPath).slice(0, 6), { path: assetPath, bundling: { - image: DockerImage.fromBuild(path.posix.join(__dirname, "alpine-zip")), + local: { + /* implements a local method of bundling that does not depend on Docker. Local + bundling is preferred over DIND for performance and security reasons. + see https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.ILocalBundling.html and + https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3_assets-readme.html#asset-bundling */ + tryBundle(outputDir: string, options: BundlingOptions) { + if (runLocal) { + const command = `zip -r ${path.posix.join(outputDir, "asset.zip")} ${assetPath}`; + try { + console.debug(`Local bundling: ${command}`); + // this is where the work gets done + execSync(command, {stdio: 'inherit'}); + // no exception means command executed successfully + return true; + } catch (ex) { + // execSync throws Error in case return value of child process + // is non-zero. It'll be printed to the console because of the + // stdio argument. + console.log(`local bundling attempt failed: ${ex}`) + } + } + // if we get here then Docker will be used as configured below + return false; + } + }, + image: this.container_image, command: ["zip", "-r", path.posix.join("/asset-output", "asset.zip"), "."], volumes: this.sharedAssets.map((f) => ({ containerPath: path.posix.join(this.WORKING_PATH, path.basename(f)), From fe579099b0b6e4027f1cf63800fbf97bffe9eb41 Mon Sep 17 00:00:00 2001 From: Fritz Kunstler Date: Mon, 22 Jan 2024 16:15:41 +0000 Subject: [PATCH 2/5] fixed two typos discovered in code review. --- lib/shared/shared-asset-bundler.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/shared/shared-asset-bundler.ts b/lib/shared/shared-asset-bundler.ts index 196c11a26..35f9542b7 100644 --- a/lib/shared/shared-asset-bundler.ts +++ b/lib/shared/shared-asset-bundler.ts @@ -35,7 +35,7 @@ function calculateHash(paths: string[]): string { export class SharedAssetBundler extends Construct { private readonly sharedAssets: string[]; private readonly WORKING_PATH = "/asset-input/"; - private readonly container_image: DockerImage; + private readonly containerImage: DockerImage; private useLocalBundler: boolean = false; /** * Instantiate a new SharedAssetBundler. You then invoke `bundleWithAsset(pathToAsset)` to @@ -53,11 +53,11 @@ export class SharedAssetBundler extends Construct { this.sharedAssets = sharedAssets; // Check if we can do local bundling if (!this.localBundlerTest()) { - // if not, then build Apline from local definition - this.container_image = DockerImage.fromBuild(path.posix.join(__dirname, "alpine-zip")); + // if not, then build Alpine from local definition + this.containerImage = DockerImage.fromBuild(path.posix.join(__dirname, "alpine-zip")); } else { // if yes, then don't build the container. https://hub.docker.com/_/scratch/ - this.container_image = DockerImage.fromRegistry("scratch"); + this.containerImage = DockerImage.fromRegistry("scratch"); } } @@ -119,7 +119,7 @@ export class SharedAssetBundler extends Construct { return false; } }, - image: this.container_image, + image: this.containerImage, command: ["zip", "-r", path.posix.join("/asset-output", "asset.zip"), "."], volumes: this.sharedAssets.map((f) => ({ containerPath: path.posix.join(this.WORKING_PATH, path.basename(f)), From 5358fed59ad60dcf79dcee8b891ce204f6b79a78 Mon Sep 17 00:00:00 2001 From: Fritz Kunstler Date: Mon, 22 Jan 2024 17:53:22 +0000 Subject: [PATCH 3/5] Skip local bundling of Layer if local machine architecture different from target Lambda runtime architecture. --- lib/layer/index.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/layer/index.ts b/lib/layer/index.ts index 793f06d60..9a16b7255 100644 --- a/lib/layer/index.ts +++ b/lib/layer/index.ts @@ -37,6 +37,16 @@ export class Layer extends Construct { let canRunLocal = false; let python = props.runtime.name; + /* check if local machine architecture matches lambda runtime architecture. annoyingly, + Node refers to x86_64 CPUs as x64 instead of using the POSIX standard name. + https://nodejs.org/docs/latest-v18.x/api/process.html#processarch + https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda.Architecture.html */ + if (!((process.arch == 'x64' && architecture.name == 'x86_64') || (process.arch == architecture.name))) { + console.log(`Can't do local bundling because local arch != target arch (${process.arch} != ${architecture.name})`); + // Local bundling is pointless if architectures don't match + return false; + } + try { // check if pip is available locally const testCommand = `${python} -m pip -V` From 12deaef5b9ad2a3a847e98dccda8976882ab5058 Mon Sep 17 00:00:00 2001 From: Fritz Kunstler Date: Wed, 24 Jan 2024 14:30:02 +0000 Subject: [PATCH 4/5] Made container setup a static operation. Fixed zip command to cd to parent dir of asset before execution, instead of including full paths in archive (only applies to local bundling). --- lib/shared/shared-asset-bundler.ts | 98 ++++++++++++++++++------------ 1 file changed, 59 insertions(+), 39 deletions(-) diff --git a/lib/shared/shared-asset-bundler.ts b/lib/shared/shared-asset-bundler.ts index 35f9542b7..e13c71079 100644 --- a/lib/shared/shared-asset-bundler.ts +++ b/lib/shared/shared-asset-bundler.ts @@ -35,8 +35,47 @@ function calculateHash(paths: string[]): string { export class SharedAssetBundler extends Construct { private readonly sharedAssets: string[]; private readonly WORKING_PATH = "/asset-input/"; - private readonly containerImage: DockerImage; - private useLocalBundler: boolean = false; + // see static init block below + private static useLocalBundler: boolean = false; + /** The container image we'll use if Local Bundling is not possible. */ + private static containerImage: DockerImage; + + /** + * Check if possible to use local bundling instead of Docker. Sets `useLocalBundler` to + * true if local environment supports bundling. Referenced below in method bundleWithAsset(...). + */ + static { + const command = "zip -v"; + console.log(`Checking for zip: ${command}`); + // check if zip is available locally + try { + // without stdio option command output does not appear in console + execSync(command, { stdio: 'inherit' }); + // no exception means command executed successfully + this.useLocalBundler = true; + } catch { + /* execSync throws Error in case return value of child process + is non-zero. Actual output should be printed to the console. */ + console.warn("`zip` is required for local bundling; falling back to default method."); + } + + try { + /** Build Alpine image from local definition. */ + this.containerImage = DockerImage.fromBuild(path.posix.join(__dirname, "alpine-zip")); + } catch (erx) { + // this will result in an exception if Docker is unavailable + if (this.useLocalBundler) { + /* we don't actually need the container if local bundling succeeds, but + it is a required parameter in the method below. + https://hub.docker.com/_/scratch/ */ + this.containerImage = DockerImage.fromRegistry("scratch"); + } else { + // Build will fail anyway so no point suppressing the exception + throw erx; + } + } + } + /** * Instantiate a new SharedAssetBundler. You then invoke `bundleWithAsset(pathToAsset)` to * bundle your asset code with the common code. @@ -51,42 +90,12 @@ export class SharedAssetBundler extends Construct { constructor(scope: Construct, id: string, sharedAssets: string[]) { super(scope, id); this.sharedAssets = sharedAssets; - // Check if we can do local bundling - if (!this.localBundlerTest()) { - // if not, then build Alpine from local definition - this.containerImage = DockerImage.fromBuild(path.posix.join(__dirname, "alpine-zip")); - } else { - // if yes, then don't build the container. https://hub.docker.com/_/scratch/ - this.containerImage = DockerImage.fromRegistry("scratch"); - } - } - - /** - * Check if possible to use local bundling instead of Docker. Sets this.useLocalBundler to - * true if local environment supports bundling. See below in method bundleWithAsset(...). - */ - private localBundlerTest(): boolean { - const command = "zip -v"; - console.log(`Checking for zip: ${command}`); - // check if zip is available locally - try { - // without stdio option command output does not appear in console - execSync(command, {stdio: 'inherit'}); - // no exception means command executed successfully - this.useLocalBundler = true; - } catch { - // execSync throws Error in case return value of child process - // is non-zero. Actual output should be printed to the console. - console.warn("Unable to do local bundling! Is zip installed?"); - } - return this.useLocalBundler; } bundleWithAsset(assetPath: string): Asset { console.log(`Bundling asset ${assetPath}`); - // necessary for access from anonymous class - const runLocal = this.useLocalBundler; + const thisAssets = this.sharedAssets; const asset = new aws_s3_assets.Asset( this, @@ -100,12 +109,22 @@ export class SharedAssetBundler extends Construct { see https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.ILocalBundling.html and https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3_assets-readme.html#asset-bundling */ tryBundle(outputDir: string, options: BundlingOptions) { - if (runLocal) { - const command = `zip -r ${path.posix.join(outputDir, "asset.zip")} ${assetPath}`; + if (SharedAssetBundler.useLocalBundler) { + // base command to execute + const command = `zip -r ${path.posix.join(outputDir, "asset.zip")} . `; + try { - console.debug(`Local bundling: ${command}`); - // this is where the work gets done - execSync(command, {stdio: 'inherit'}); + console.debug(`Local bundling: ${assetPath}`); + // cd to dir of current asset and zip contents + execSync(`cd ${assetPath} && `.concat(command), {stdio: 'inherit'}); + // do the same for each dir in shared assets array + thisAssets.forEach((a)=>{ + /* Complete the command for this specific shared asset path; for example: + `cd ${assetPath}/.. && ${command} -i ${assetPath.basename}/*` */ + const cx = `cd ${path.posix.join(a, '..')} && `.concat(command).concat(`-i "${path.basename(a)}/*"`); + //execute the command in child process + execSync(cx, {stdio: 'inherit'}); + }); // no exception means command executed successfully return true; } catch (ex) { @@ -119,7 +138,7 @@ export class SharedAssetBundler extends Construct { return false; } }, - image: this.containerImage, + image: SharedAssetBundler.containerImage, command: ["zip", "-r", path.posix.join("/asset-output", "asset.zip"), "."], volumes: this.sharedAssets.map((f) => ({ containerPath: path.posix.join(this.WORKING_PATH, path.basename(f)), @@ -132,6 +151,7 @@ export class SharedAssetBundler extends Construct { assetHashType: AssetHashType.CUSTOM, } ); + console.log(`Successfully bundled ${asset.toString()} shared assets for ${assetPath} as ${asset.s3ObjectKey}.`); return asset; } From dec68b62334167fb9c8a99b45388060d319556e0 Mon Sep 17 00:00:00 2001 From: Fritz Kunstler Date: Wed, 24 Jan 2024 19:53:29 +0000 Subject: [PATCH 5/5] Install python packages for common layer into `python` sub-directory instead of root output dir (applies only to local bundling). --- lib/layer/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/layer/index.ts b/lib/layer/index.ts index 9a16b7255..ba9469661 100644 --- a/lib/layer/index.ts +++ b/lib/layer/index.ts @@ -62,7 +62,8 @@ export class Layer extends Construct { } if (canRunLocal) { - const command = `${python} -m pip install -r ${lpath.posix.join(path, "requirements.txt")} -t ${outputDir} ${autoUpgrade ? '-U' : ''}`; + const pkgDir = lpath.posix.join(outputDir, "python"); + const command = `${python} -m pip install -r ${lpath.posix.join(path, "requirements.txt")} -t ${pkgDir} ${autoUpgrade ? '-U' : ''}`; try { console.debug(`Local bundling: ${command}`); // this is where the work gets done