From 367a055688c97ca3b01aff19d6d91ed5b1b86e1e Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 17 Feb 2021 20:20:05 +0100 Subject: [PATCH] feat(core): customize bundling output packaging (#13076) If the bundling output contains a single archive file (zip or jar), upload it as-is to S3 without zipping it. Allow to customize this behavior with `bundling.packaging`: * `ALWAYS_ZIP`: The bundling output will always be zipped and uploaded to S3. * `NEVER_ZIP`: The bundling output will not be zipped. Bundling will fail if the bundling output doesn't contain a single file. * `AUTO`: If the bundling output contains a single archive file (zip or jar) it will not be zipped. Otherwise it will be zipped. Closes #10776 Closes https://github.com/aws/aws-cdk/issues/12651 ---- *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-s3-assets/README.md | 21 ++++ packages/@aws-cdk/aws-s3-assets/lib/asset.ts | 30 +---- packages/@aws-cdk/core/lib/asset-staging.ts | 90 ++++++++++++- packages/@aws-cdk/core/lib/bundling.ts | 35 ++++++ .../@aws-cdk/core/test/archive/archive.zip | 0 packages/@aws-cdk/core/test/docker-stub.sh | 15 ++- packages/@aws-cdk/core/test/staging.test.ts | 118 +++++++++++++++++- 7 files changed, 274 insertions(+), 35 deletions(-) create mode 100644 packages/@aws-cdk/core/test/archive/archive.zip diff --git a/packages/@aws-cdk/aws-s3-assets/README.md b/packages/@aws-cdk/aws-s3-assets/README.md index aab4c46d9c44d..f2583b7c10a24 100644 --- a/packages/@aws-cdk/aws-s3-assets/README.md +++ b/packages/@aws-cdk/aws-s3-assets/README.md @@ -124,6 +124,27 @@ new assets.Asset(this, 'BundledAsset', { Although optional, it's recommended to provide a local bundling method which can greatly improve performance. +If the bundling output contains a single archive file (zip or jar) it will be +uploaded to S3 as-is and will not be zipped. Otherwise the contents of the +output directory will be zipped and the zip file will be uploaded to S3. This +is the default behavior for `bundling.outputType` (`BundlingOutput.AUTO_DISCOVER`). + +Use `BundlingOutput.NOT_ARCHIVED` if the bundling output must always be zipped: + +```ts +const asset = new assets.Asset(this, 'BundledAsset', { + path: '/path/to/asset', + bundling: { + image: BundlingDockerImage.fromRegistry('alpine'), + command: ['command-that-produces-an-archive.sh'], + outputType: BundlingOutput.NOT_ARCHIVED, // Bundling output will be zipped even though it produces a single archive file. + }, +}); +``` + +Use `BundlingOutput.ARCHIVED` if the bundling output contains a single archive file and +you don't want it to be zippped. + ## CloudFormation Resource Metadata > NOTE: This section is relevant for authors of AWS Resource Constructs. diff --git a/packages/@aws-cdk/aws-s3-assets/lib/asset.ts b/packages/@aws-cdk/aws-s3-assets/lib/asset.ts index 938778d1381f4..510834a61c634 100644 --- a/packages/@aws-cdk/aws-s3-assets/lib/asset.ts +++ b/packages/@aws-cdk/aws-s3-assets/lib/asset.ts @@ -1,4 +1,3 @@ -import * as fs from 'fs'; import * as path from 'path'; import * as assets from '@aws-cdk/assets'; import * as iam from '@aws-cdk/aws-iam'; @@ -13,8 +12,6 @@ import { toSymlinkFollow } from './compat'; // eslint-disable-next-line no-duplicate-imports, import/order import { Construct as CoreConstruct } from '@aws-cdk/core'; -const ARCHIVE_EXTENSIONS = ['.zip', '.jar']; - export interface AssetOptions extends assets.CopyOptions, cdk.AssetOptions { /** * A list of principals that should be able to read this asset from S3. @@ -139,17 +136,12 @@ export class Asset extends CoreConstruct implements cdk.IAsset { this.assetPath = staging.relativeStagedPath(stack); - const packaging = determinePackaging(staging.sourcePath); - - this.isFile = packaging === cdk.FileAssetPackaging.FILE; + this.isFile = staging.packaging === cdk.FileAssetPackaging.FILE; - // sets isZipArchive based on the type of packaging and file extension - this.isZipArchive = packaging === cdk.FileAssetPackaging.ZIP_DIRECTORY - ? true - : ARCHIVE_EXTENSIONS.some(ext => staging.sourcePath.toLowerCase().endsWith(ext)); + this.isZipArchive = staging.isArchive; const location = stack.synthesizer.addFileAsset({ - packaging, + packaging: staging.packaging, sourceHash: this.sourceHash, fileName: this.assetPath, }); @@ -210,19 +202,3 @@ export class Asset extends CoreConstruct implements cdk.IAsset { this.bucket.grantRead(grantee); } } - -function determinePackaging(assetPath: string): cdk.FileAssetPackaging { - if (!fs.existsSync(assetPath)) { - throw new Error(`Cannot find asset at ${assetPath}`); - } - - if (fs.statSync(assetPath).isDirectory()) { - return cdk.FileAssetPackaging.ZIP_DIRECTORY; - } - - if (fs.statSync(assetPath).isFile()) { - return cdk.FileAssetPackaging.FILE; - } - - throw new Error(`Asset ${assetPath} is expected to be either a directory or a regular file`); -} diff --git a/packages/@aws-cdk/core/lib/asset-staging.ts b/packages/@aws-cdk/core/lib/asset-staging.ts index 66c65e3d14864..17fd89781004a 100644 --- a/packages/@aws-cdk/core/lib/asset-staging.ts +++ b/packages/@aws-cdk/core/lib/asset-staging.ts @@ -5,8 +5,8 @@ import * as cxapi from '@aws-cdk/cx-api'; import { Construct } from 'constructs'; import * as fs from 'fs-extra'; import * as minimatch from 'minimatch'; -import { AssetHashType, AssetOptions } from './assets'; -import { BundlingOptions } from './bundling'; +import { AssetHashType, AssetOptions, FileAssetPackaging } from './assets'; +import { BundlingOptions, BundlingOutput } from './bundling'; import { FileSystem, FingerprintOptions } from './fs'; import { Names } from './names'; import { Cache } from './private/cache'; @@ -17,6 +17,8 @@ import { Stage } from './stage'; // eslint-disable-next-line import { Construct as CoreConstruct } from './construct-compat'; +const ARCHIVE_EXTENSIONS = ['.zip', '.jar']; + /** * A previously staged asset */ @@ -138,6 +140,9 @@ export class AssetStaging extends CoreConstruct { private readonly cacheKey: string; + private _packaging = FileAssetPackaging.ZIP_DIRECTORY; + private _isArchive = true; + constructor(scope: Construct, id: string, props: AssetStagingProps) { super(scope, id); @@ -203,6 +208,20 @@ export class AssetStaging extends CoreConstruct { return this.assetHash; } + /** + * How this asset should be packaged. + */ + public get packaging(): FileAssetPackaging { + return this._packaging; + } + + /** + * Whether this asset is an archive (zip or jar). + */ + public get isArchive(): boolean { + return this._isArchive; + } + /** * Return the path to the staged asset, relative to the Cloud Assembly (manifest) directory of the given stack * @@ -281,11 +300,16 @@ export class AssetStaging extends CoreConstruct { const bundleDir = this.determineBundleDir(this.assetOutdir, assetHash); this.bundle(bundling, bundleDir); + // Check bundling output content and determine if we will need to archive + const bundlingOutputType = bundling.outputType ?? BundlingOutput.AUTO_DISCOVER; + const bundledAsset = determineBundledAsset(bundleDir, bundlingOutputType); + this._packaging = bundledAsset.packaging; + // Calculate assetHash afterwards if we still must - assetHash = assetHash ?? this.calculateHash(this.hashType, bundling, bundleDir); - const stagedPath = path.resolve(this.assetOutdir, renderAssetFilename(assetHash)); + assetHash = assetHash ?? this.calculateHash(this.hashType, bundling, bundledAsset.path); + const stagedPath = path.resolve(this.assetOutdir, renderAssetFilename(assetHash, bundledAsset.extension)); - this.stageAsset(bundleDir, stagedPath, 'move'); + this.stageAsset(bundledAsset.path, stagedPath, 'move'); return { assetHash, stagedPath }; } @@ -323,6 +347,8 @@ export class AssetStaging extends CoreConstruct { const stat = fs.statSync(sourcePath); if (stat.isFile()) { fs.copyFileSync(sourcePath, targetPath); + this._packaging = FileAssetPackaging.FILE; + this._isArchive = ARCHIVE_EXTENSIONS.includes(path.extname(sourcePath).toLowerCase()); } else if (stat.isDirectory()) { fs.mkdirSync(targetPath); FileSystem.copyDirectory(sourcePath, targetPath, this.fingerprintOptions); @@ -502,3 +528,57 @@ function sortObject(object: { [key: string]: any }): { [key: string]: any } { } return ret; } + +/** + * Returns the single archive file of a directory or undefined + */ +function singleArchiveFile(directory: string): string | undefined { + if (!fs.existsSync(directory)) { + throw new Error(`Directory ${directory} does not exist.`); + } + + if (!fs.statSync(directory).isDirectory()) { + throw new Error(`${directory} is not a directory.`); + } + + const content = fs.readdirSync(directory); + if (content.length === 1) { + const file = path.join(directory, content[0]); + const extension = path.extname(content[0]).toLowerCase(); + if (fs.statSync(file).isFile() && ARCHIVE_EXTENSIONS.includes(extension)) { + return file; + } + } + + return undefined; +} + +interface BundledAsset { + path: string, + packaging: FileAssetPackaging, + extension?: string +} + +/** + * Returns the bundled asset to use based on the content of the bundle directory + * and the type of output. + */ +function determineBundledAsset(bundleDir: string, outputType: BundlingOutput): BundledAsset { + const archiveFile = singleArchiveFile(bundleDir); + + // auto-discover means that if there is an archive file, we take it as the + // bundle, otherwise, we will archive here. + if (outputType === BundlingOutput.AUTO_DISCOVER) { + outputType = archiveFile ? BundlingOutput.ARCHIVED : BundlingOutput.NOT_ARCHIVED; + } + + switch (outputType) { + case BundlingOutput.NOT_ARCHIVED: + return { path: bundleDir, packaging: FileAssetPackaging.ZIP_DIRECTORY }; + case BundlingOutput.ARCHIVED: + if (!archiveFile) { + throw new Error('Bundling output directory is expected to include only a single .zip or .jar file when `output` is set to `ARCHIVED`'); + } + return { path: archiveFile, packaging: FileAssetPackaging.FILE, extension: path.extname(archiveFile) }; + } +} diff --git a/packages/@aws-cdk/core/lib/bundling.ts b/packages/@aws-cdk/core/lib/bundling.ts index b1247fd913ea0..57885ceeca05c 100644 --- a/packages/@aws-cdk/core/lib/bundling.ts +++ b/packages/@aws-cdk/core/lib/bundling.ts @@ -79,6 +79,41 @@ export interface BundlingOptions { * @experimental */ readonly local?: ILocalBundling; + + /** + * The type of output that this bundling operation is producing. + * + * @default BundlingOutput.AUTO_DISCOVER + * + * @experimental + */ + readonly outputType?: BundlingOutput; +} + +/** + * The type of output that a bundling operation is producing. + * + * @experimental + */ +export enum BundlingOutput { + /** + * The bundling output directory includes a single .zip or .jar file which + * will be used as the final bundle. If the output directory does not + * include exactly a single archive, bundling will fail. + */ + ARCHIVED = 'archived', + + /** + * The bundling output directory contains one or more files which will be + * archived and uploaded as a .zip file to S3. + */ + NOT_ARCHIVED = 'not-archived', + + /** + * If the bundling output directory contains a single archive file (zip or jar) + * it will not be zipped. Otherwise the bundling output will be zipped. + */ + AUTO_DISCOVER = 'auto-discover', } /** diff --git a/packages/@aws-cdk/core/test/archive/archive.zip b/packages/@aws-cdk/core/test/archive/archive.zip new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/packages/@aws-cdk/core/test/docker-stub.sh b/packages/@aws-cdk/core/test/docker-stub.sh index fe48e93d4a207..94f806f69a120 100755 --- a/packages/@aws-cdk/core/test/docker-stub.sh +++ b/packages/@aws-cdk/core/test/docker-stub.sh @@ -24,5 +24,18 @@ if echo "$@" | grep "DOCKER_STUB_SUCCESS"; then exit 0 fi -echo "Docker mock only supports one of the following commands: DOCKER_STUB_SUCCESS_NO_OUTPUT,DOCKER_STUB_FAIL,DOCKER_STUB_SUCCESS" +if echo "$@" | grep "DOCKER_STUB_MULTIPLE_FILES"; then + outdir=$(echo "$@" | xargs -n1 | grep "/asset-output" | head -n1 | cut -d":" -f1) + touch ${outdir}/test1.txt + touch ${outdir}/test2.txt + exit 0 +fi + +if echo "$@" | grep "DOCKER_STUB_SINGLE_ARCHIVE"; then + outdir=$(echo "$@" | xargs -n1 | grep "/asset-output" | head -n1 | cut -d":" -f1) + touch ${outdir}/test.zip + exit 0 +fi + +echo "Docker mock only supports one of the following commands: DOCKER_STUB_SUCCESS_NO_OUTPUT,DOCKER_STUB_FAIL,DOCKER_STUB_SUCCESS,DOCKER_STUB_MULTIPLE_FILES,DOCKER_SINGLE_ARCHIVE" exit 1 diff --git a/packages/@aws-cdk/core/test/staging.test.ts b/packages/@aws-cdk/core/test/staging.test.ts index 347c5fcea3b63..76bf74f9cffd7 100644 --- a/packages/@aws-cdk/core/test/staging.test.ts +++ b/packages/@aws-cdk/core/test/staging.test.ts @@ -1,10 +1,11 @@ import * as os from 'os'; import * as path from 'path'; +import { FileAssetPackaging } from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import * as fs from 'fs-extra'; import { nodeunitShim, Test } from 'nodeunit-shim'; import * as sinon from 'sinon'; -import { App, AssetHashType, AssetStaging, BundlingDockerImage, BundlingOptions, FileSystem, Stack, Stage } from '../lib'; +import { App, AssetHashType, AssetStaging, BundlingDockerImage, BundlingOptions, BundlingOutput, FileSystem, Stack, Stage } from '../lib'; const STUB_INPUT_FILE = '/tmp/docker-stub.input'; const STUB_INPUT_CONCAT_FILE = '/tmp/docker-stub.input.concat'; @@ -12,7 +13,9 @@ const STUB_INPUT_CONCAT_FILE = '/tmp/docker-stub.input.concat'; enum DockerStubCommand { SUCCESS = 'DOCKER_STUB_SUCCESS', FAIL = 'DOCKER_STUB_FAIL', - SUCCESS_NO_OUTPUT = 'DOCKER_STUB_SUCCESS_NO_OUTPUT' + SUCCESS_NO_OUTPUT = 'DOCKER_STUB_SUCCESS_NO_OUTPUT', + MULTIPLE_FILES = 'DOCKER_STUB_MULTIPLE_FILES', + SINGLE_ARCHIVE = 'DOCKER_STUB_SINGLE_ARCHIVE', } const FIXTURE_TEST1_DIR = path.join(__dirname, 'fs', 'fixtures', 'test1'); @@ -50,6 +53,34 @@ nodeunitShim({ test.deepEqual(staging.sourcePath, sourcePath); test.deepEqual(path.basename(staging.stagedPath), 'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00'); test.deepEqual(path.basename(staging.relativeStagedPath(stack)), 'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00'); + test.deepEqual(staging.packaging, FileAssetPackaging.ZIP_DIRECTORY); + test.deepEqual(staging.isArchive, true); + test.done(); + }, + + 'staging of an archive file correctly sets packaging and isArchive'(test: Test) { + // GIVEN + const stack = new Stack(); + const sourcePath = path.join(__dirname, 'archive', 'archive.zip'); + + // WHEN + const staging = new AssetStaging(stack, 's1', { sourcePath }); + + test.deepEqual(staging.packaging, FileAssetPackaging.FILE); + test.deepEqual(staging.isArchive, true); + test.done(); + }, + + 'staging of a non-archive file correctly sets packaging and isArchive'(test: Test) { + // GIVEN + const stack = new Stack(); + const sourcePath = __filename; + + // WHEN + const staging = new AssetStaging(stack, 's1', { sourcePath }); + + test.deepEqual(staging.packaging, FileAssetPackaging.FILE); + test.deepEqual(staging.isArchive, false); test.done(); }, @@ -785,6 +816,89 @@ nodeunitShim({ ); test.equal(asset.assetHash, '33cbf2cae5432438e0f046bc45ba8c3cef7b6afcf47b59d1c183775c1918fb1f'); // hash of MyStack/Asset + test.done(); + }, + + 'bundling that produces a single archive file is autodiscovered'(test: Test) { + // GIVEN + const app = new App(); + const stack = new Stack(app, 'stack'); + const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); + + // WHEN + const staging = new AssetStaging(stack, 'Asset', { + sourcePath: directory, + bundling: { + image: BundlingDockerImage.fromRegistry('alpine'), + command: [DockerStubCommand.SINGLE_ARCHIVE], + }, + }); + + // THEN + const assembly = app.synth(); + test.deepEqual(fs.readdirSync(assembly.directory), [ + 'asset.f43148c61174f444925231b5849b468f21e93b5d1469cd07c53625ffd039ef48', // this is the bundle dir but it's empty + 'asset.f43148c61174f444925231b5849b468f21e93b5d1469cd07c53625ffd039ef48.zip', + 'cdk.out', + 'manifest.json', + 'stack.template.json', + 'tree.json', + ]); + test.equal(fs.readdirSync(path.join(assembly.directory, 'asset.f43148c61174f444925231b5849b468f21e93b5d1469cd07c53625ffd039ef48')).length, 0); // empty bundle dir + test.deepEqual(staging.packaging, FileAssetPackaging.FILE); + test.deepEqual(staging.isArchive, true); + + test.done(); + }, + + 'bundling that produces a single archive file with NOT_ARCHIVED'(test: Test) { + // GIVEN + const app = new App(); + const stack = new Stack(app, 'stack'); + const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); + + // WHEN + const staging = new AssetStaging(stack, 'Asset', { + sourcePath: directory, + bundling: { + image: BundlingDockerImage.fromRegistry('alpine'), + command: [DockerStubCommand.SINGLE_ARCHIVE], + outputType: BundlingOutput.NOT_ARCHIVED, + }, + }); + + // THEN + const assembly = app.synth(); + test.deepEqual(fs.readdirSync(assembly.directory), [ + 'asset.86ec07746e1d859290cfd8b9c648e581555649c75f51f741f11e22cab6775abc', + 'cdk.out', + 'manifest.json', + 'stack.template.json', + 'tree.json', + ]); + test.deepEqual(staging.packaging, FileAssetPackaging.ZIP_DIRECTORY); + test.deepEqual(staging.isArchive, true); + + test.done(); + }, + + 'throws with ARCHIVED and bundling that does not produce a single archive file'(test: Test) { + // GIVEN + const app = new App(); + const stack = new Stack(app, 'stack'); + const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); + + // WHEN + test.throws(() => new AssetStaging(stack, 'Asset', { + sourcePath: directory, + bundling: { + image: BundlingDockerImage.fromRegistry('alpine'), + command: [DockerStubCommand.MULTIPLE_FILES], + outputType: BundlingOutput.ARCHIVED, + }, + }), /Bundling output directory is expected to include only a single .zip or .jar file when `output` is set to `ARCHIVED`/); + + test.done(); }, });