diff --git a/packages/@aws-cdk/aws-lambda/test/integ.bundling.ts b/packages/@aws-cdk/aws-lambda/test/integ.bundling.ts index 56c92cfbefbf6..200ca72f204b6 100644 --- a/packages/@aws-cdk/aws-lambda/test/integ.bundling.ts +++ b/packages/@aws-cdk/aws-lambda/test/integ.bundling.ts @@ -1,4 +1,3 @@ -/// !cdk-integ pragma:ignore-assets import * as path from 'path'; import { App, CfnOutput, Construct, Stack, StackProps } from '@aws-cdk/core'; import * as lambda from '../lib'; diff --git a/packages/@aws-cdk/aws-s3-assets/test/integ.assets.bundling.lit.ts b/packages/@aws-cdk/aws-s3-assets/test/integ.assets.bundling.lit.ts index abcf8c9598e2a..1ba1ad26deee2 100644 --- a/packages/@aws-cdk/aws-s3-assets/test/integ.assets.bundling.lit.ts +++ b/packages/@aws-cdk/aws-s3-assets/test/integ.assets.bundling.lit.ts @@ -1,4 +1,3 @@ -/// !cdk-integ pragma:ignore-assets import * as path from 'path'; import * as iam from '@aws-cdk/aws-iam'; import { App, BundlingDockerImage, Construct, Stack, StackProps } from '@aws-cdk/core'; diff --git a/packages/@aws-cdk/core/lib/asset-staging.ts b/packages/@aws-cdk/core/lib/asset-staging.ts index 17a26e1b058ca..92a2b6b9bdf97 100644 --- a/packages/@aws-cdk/core/lib/asset-staging.ts +++ b/packages/@aws-cdk/core/lib/asset-staging.ts @@ -88,23 +88,12 @@ export class AssetStaging extends Construct { this.sourcePath = props.sourcePath; this.fingerprintOptions = props; - // Determine the hash type based on the props as props.assetHashType is - // optional from a caller perspective. - const hashType = this.determineHashType(props.assetHashType, props.assetHash); - if (props.bundling) { - // Determine the source hash in advance of bundling if the asset hash type - // is source so that the bundler can opt to re-use its bundle dir. - const sourceHash = hashType === AssetHashType.SOURCE - ? this.calculateHash(hashType, props.assetHash, props.bundling) - : undefined; - - this.bundleDir = this.bundle(props.bundling, sourceHash); - this.assetHash = sourceHash ?? this.calculateHash(hashType, props.assetHash, props.bundling); - } else { - this.assetHash = this.calculateHash(hashType, props.assetHash); + this.bundleDir = this.bundle(props.bundling); } + this.assetHash = this.calculateHash(props); + const stagingDisabled = this.node.tryGetContext(cxapi.DISABLE_ASSET_STAGING_CONTEXT); if (stagingDisabled) { this.stagedPath = this.bundleDir ?? this.sourcePath; @@ -148,37 +137,15 @@ export class AssetStaging extends Construct { } } - /** - * @Property sourceHash The source hash of the asset. If specified, the bundler - * will attempt to re-use the bundling directory, which is based on a hash of - * both the sourceHash and options. If bundling finds a pre-existing directory, - * the bundler will return it as-is and won't regenerate the bundle. - */ - private bundle(options: BundlingOptions, sourceHash?: string): string { + private bundle(options: BundlingOptions): string { // Temp staging directory in the working directory const stagingTmp = path.join('.', STAGING_TMP); fs.ensureDirSync(stagingTmp); - let bundleDir: string; - if (sourceHash) { - // When an asset hash is known in advance of bundling, bundling is done into a dedicated staging directory. - bundleDir = path.resolve(path.join(stagingTmp, 'asset-bundle-hash-' + sourceHash)); - - if (fs.existsSync(bundleDir)) { - // Pre-existing bundle directory. The bundle has already been generated once before, so lets provide it - // as-is to the caller. - return bundleDir; - } - - fs.ensureDirSync(bundleDir); - } else { - // When the asset hash isn't known in advance, bundling is done into a temporary staging directory. - - // Create temp directory for bundling inside the temp staging directory - bundleDir = path.resolve(fs.mkdtempSync(path.join(stagingTmp, 'asset-bundle-temp-'))); - // Chmod the bundleDir to full access. - fs.chmodSync(bundleDir, 0o777); - } + // Create temp directory for bundling inside the temp staging directory + const bundleDir = path.resolve(fs.mkdtempSync(path.join(stagingTmp, 'asset-bundle-'))); + // Chmod the bundleDir to full access. + fs.chmodSync(bundleDir, 0o777); let user: string; if (options.user) { @@ -214,18 +181,7 @@ export class AssetStaging extends Construct { workingDirectory: options.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR, }); } catch (err) { - // When bundling fails, keep the bundle output for diagnosability, but - // rename it out of the way so that the next run doesn't assume it has a - // valid bundleDir. - - const bundleErrorDir = bundleDir + '-error'; - if (fs.existsSync(bundleErrorDir)) { - // Remove the last bundleErrorDir. - fs.removeSync(bundleErrorDir); - } - - fs.renameSync(bundleDir, bundleErrorDir); - throw new Error(`Failed to run bundling Docker image for asset ${this.node.path}, bundle output is located at ${bundleErrorDir}: ${err}`); + throw new Error(`Failed to run bundling Docker image for asset ${this.node.path}: ${err}`); } if (FileSystem.isEmpty(bundleDir)) { @@ -235,49 +191,35 @@ export class AssetStaging extends Construct { return bundleDir; } - /** - * Determines the hash type from user-given prop values. - * - * @param assetHashType Asset hash type construct prop - * @param assetHash Asset hash given in the construct props - */ - private determineHashType(assetHashType?: AssetHashType, assetHash?: string) { - if (assetHash) { - if (assetHashType && assetHashType !== AssetHashType.CUSTOM) { - throw new Error(`Cannot specify \`${assetHashType}\` for \`assetHashType\` when \`assetHash\` is specified. Use \`CUSTOM\` or leave \`undefined\`.`); + private calculateHash(props: AssetStagingProps): string { + let hashType: AssetHashType; + + if (props.assetHash) { + if (props.assetHashType && props.assetHashType !== AssetHashType.CUSTOM) { + throw new Error(`Cannot specify \`${props.assetHashType}\` for \`assetHashType\` when \`assetHash\` is specified. Use \`CUSTOM\` or leave \`undefined\`.`); } - return AssetHashType.CUSTOM; - } else if (assetHashType) { - return assetHashType; + hashType = AssetHashType.CUSTOM; + } else if (props.assetHashType) { + hashType = props.assetHashType; } else { - return AssetHashType.SOURCE; + hashType = AssetHashType.SOURCE; } - } - private calculateHash(hashType: AssetHashType, assetHash?: string, bundling?: BundlingOptions): string { switch (hashType) { case AssetHashType.SOURCE: - const sourceHash = FileSystem.fingerprint(this.sourcePath, this.fingerprintOptions); - if (bundling) { - return crypto.createHash('sha256') - .update(JSON.stringify(bundling)) - .update(sourceHash) - .digest('hex'); - } else { - return sourceHash; - } + return FileSystem.fingerprint(this.sourcePath, this.fingerprintOptions); case AssetHashType.BUNDLE: if (!this.bundleDir) { throw new Error('Cannot use `AssetHashType.BUNDLE` when `bundling` is not specified.'); } return FileSystem.fingerprint(this.bundleDir, this.fingerprintOptions); case AssetHashType.CUSTOM: - if (!assetHash) { + if (!props.assetHash) { throw new Error('`assetHash` must be specified when `assetHashType` is set to `AssetHashType.CUSTOM`.'); } // Hash the hash to make sure we can use it in a file/directory name. // The resulting hash will also have the same length as for the other hash types. - return crypto.createHash('sha256').update(assetHash).digest('hex'); + return crypto.createHash('sha256').update(props.assetHash).digest('hex'); default: throw new Error('Unknown asset hash type.'); } diff --git a/packages/@aws-cdk/core/test/docker-stub.sh b/packages/@aws-cdk/core/test/docker-stub.sh index fe48e93d4a207..45a78ef881ebd 100755 --- a/packages/@aws-cdk/core/test/docker-stub.sh +++ b/packages/@aws-cdk/core/test/docker-stub.sh @@ -6,7 +6,6 @@ set -euo pipefail # `/tmp/docker-stub.input` and accepts one of 3 commands that impact it's # behavior. -echo "$@" >> /tmp/docker-stub.input.concat echo "$@" > /tmp/docker-stub.input if echo "$@" | grep "DOCKER_STUB_SUCCESS_NO_OUTPUT"; then diff --git a/packages/@aws-cdk/core/test/test.staging.ts b/packages/@aws-cdk/core/test/test.staging.ts index f0553ba7b5944..bff677342bf7f 100644 --- a/packages/@aws-cdk/core/test/test.staging.ts +++ b/packages/@aws-cdk/core/test/test.staging.ts @@ -7,8 +7,6 @@ import * as sinon from 'sinon'; import { App, AssetHashType, AssetStaging, BundlingDockerImage, Stack } from '../lib'; const STUB_INPUT_FILE = '/tmp/docker-stub.input'; -const STUB_INPUT_CONCAT_FILE = '/tmp/docker-stub.input.concat'; -const STAGING_TMP_DIRECTORY = path.join('.', '.cdk.staging'); enum DockerStubCommand { SUCCESS = 'DOCKER_STUB_SUCCESS', @@ -28,12 +26,6 @@ export = { if (fs.existsSync(STUB_INPUT_FILE)) { fs.unlinkSync(STUB_INPUT_FILE); } - if (fs.existsSync(STUB_INPUT_CONCAT_FILE)) { - fs.unlinkSync(STUB_INPUT_CONCAT_FILE); - } - if (fs.existsSync(STAGING_TMP_DIRECTORY)) { - fs.removeSync(STAGING_TMP_DIRECTORY); - } cb(); sinon.restore(); }, @@ -114,6 +106,8 @@ export = { const stack = new Stack(app, 'stack'); const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); const ensureDirSyncSpy = sinon.spy(fs, 'ensureDirSync'); + const mkdtempSyncSpy = sinon.spy(fs, 'mkdtempSync'); + const chmodSyncSpy = sinon.spy(fs, 'chmodSync'); const consoleErrorSpy = sinon.spy(console, 'error'); // WHEN @@ -132,7 +126,7 @@ export = { `run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`, ); test.deepEqual(fs.readdirSync(assembly.directory), [ - 'asset.dec215520dfd57a87aa1362e9e15131938583cd6e5a2bd9a45d38fe5dc5ab3d7', + 'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00', 'cdk.out', 'manifest.json', 'stack.template.json', @@ -140,8 +134,10 @@ export = { ]); // asset is bundled in a directory inside .cdk.staging - test.ok(ensureDirSyncSpy.calledWith(STAGING_TMP_DIRECTORY)); - test.ok(ensureDirSyncSpy.calledWith(path.resolve(path.join(STAGING_TMP_DIRECTORY, 'asset-bundle-hash-dec215520dfd57a87aa1362e9e15131938583cd6e5a2bd9a45d38fe5dc5ab3d7')))); + const stagingTmp = path.join('.', '.cdk.staging'); + test.ok(ensureDirSyncSpy.calledWith(stagingTmp)); + test.ok(mkdtempSyncSpy.calledWith(sinon.match(path.join(stagingTmp, 'asset-bundle-')))); + test.ok(chmodSyncSpy.calledWith(sinon.match(path.join(stagingTmp, 'asset-bundle-')), 0o777)); // shows a message before bundling test.ok(consoleErrorSpy.calledWith('Bundling asset stack/Asset...')); @@ -149,148 +145,6 @@ export = { test.done(); }, - 'bundler reuses its output when it can'(test: Test) { - // GIVEN - const app = new App(); - const stack = new Stack(app, 'stack'); - const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); - const ensureDirSyncSpy = sinon.spy(fs, 'ensureDirSync'); - - // WHEN - new AssetStaging(stack, 'Asset', { - sourcePath: directory, - bundling: { - image: BundlingDockerImage.fromRegistry('alpine'), - command: [ DockerStubCommand.SUCCESS ], - }, - }); - - new AssetStaging(stack, 'AssetDuplicate', { - sourcePath: directory, - bundling: { - image: BundlingDockerImage.fromRegistry('alpine'), - command: [ DockerStubCommand.SUCCESS ], - }, - }); - - // THEN - app.synth(); - - // We're testing that docker was run exactly once even though there are two bundling assets. - test.deepEqual( - readDockerStubInputConcat(), - `run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`, - ); - - // asset is bundled in a directory inside .cdk.staging - test.ok(ensureDirSyncSpy.calledWith(STAGING_TMP_DIRECTORY)); - test.ok(ensureDirSyncSpy.calledWith(path.resolve(path.join(STAGING_TMP_DIRECTORY, 'asset-bundle-hash-dec215520dfd57a87aa1362e9e15131938583cd6e5a2bd9a45d38fe5dc5ab3d7')))); - - test.done(); - }, - - 'bundler considers its options when reusing bundle output'(test: Test) { - // GIVEN - const app = new App(); - const stack = new Stack(app, 'stack'); - const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); - const ensureDirSyncSpy = sinon.spy(fs, 'ensureDirSync'); - - // WHEN - new AssetStaging(stack, 'Asset', { - sourcePath: directory, - bundling: { - image: BundlingDockerImage.fromRegistry('alpine'), - command: [ DockerStubCommand.SUCCESS ], - }, - }); - - new AssetStaging(stack, 'AssetWithDifferentBundlingOptions', { - sourcePath: directory, - bundling: { - image: BundlingDockerImage.fromRegistry('alpine'), - command: [ DockerStubCommand.SUCCESS ], - environment: { - UNIQUE_ENV_VAR: 'SOMEVALUE', - }, - }, - }); - - // THEN - const assembly = app.synth(); - - // We're testing that docker was run twice - once for each set of bundler options - // operating on the same source asset. - test.deepEqual( - readDockerStubInputConcat(), - `run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS\n` + - `run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated --env UNIQUE_ENV_VAR=SOMEVALUE -w /asset-input alpine DOCKER_STUB_SUCCESS`, - ); - - // asset is bundled in a directory inside .cdk.staging - test.ok(ensureDirSyncSpy.calledWith(STAGING_TMP_DIRECTORY)); - - test.deepEqual(fs.readdirSync(assembly.directory), [ - 'asset.a33245f0209379d58d125d89906c2b47d38382ae745375f25697760a8c475c6b', // 'Asset' - 'asset.dec215520dfd57a87aa1362e9e15131938583cd6e5a2bd9a45d38fe5dc5ab3d7', // 'AssetWithDifferentBundlingOptions' - 'cdk.out', - 'manifest.json', - 'stack.template.json', - 'tree.json', - ]); - - test.done(); - }, - - 'bundler outputs to a temp dir when using bundle asset type'(test: Test) { - // GIVEN - const app = new App(); - const stack = new Stack(app, 'stack'); - const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); - const mkdtempSyncSpy = sinon.spy(fs, 'mkdtempSync'); - const chmodSyncSpy = sinon.spy(fs, 'chmodSync'); - - // WHEN - new AssetStaging(stack, 'Asset', { - sourcePath: directory, - assetHashType: AssetHashType.BUNDLE, - bundling: { - image: BundlingDockerImage.fromRegistry('alpine'), - command: [ DockerStubCommand.SUCCESS ], - }, - }); - - // THEN - test.ok(mkdtempSyncSpy.calledWith(sinon.match(path.join(STAGING_TMP_DIRECTORY, 'asset-bundle-temp-')))); - test.ok(chmodSyncSpy.calledWith(sinon.match(path.join(STAGING_TMP_DIRECTORY, 'asset-bundle-temp-')), 0o777)); - - test.done(); - }, - - 'bundling failure preserves the bundleDir for diagnosability'(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.FAIL ], - }, - }), /Failed to run bundling.*asset-bundle-hash.*-error/); - - // THEN - test.ok(!fs.existsSync(path.resolve(path.join(STAGING_TMP_DIRECTORY, - 'asset-bundle-hash-e40b2b1537234d458e9e524494dc0a7a364079d457a2886a44b1f3c28a956469')))); - test.ok(fs.existsSync(path.join(STAGING_TMP_DIRECTORY, - 'asset-bundle-hash-e40b2b1537234d458e9e524494dc0a7a364079d457a2886a44b1f3c28a956469-error'))); - - test.done(); - }, - 'bundling throws when /asset-ouput is empty'(test: Test) { // GIVEN const app = new App(); @@ -374,6 +228,10 @@ export = { assetHash: 'my-custom-hash', assetHashType: AssetHashType.BUNDLE, }), /Cannot specify `bundle` for `assetHashType`/); + test.equal( + readDockerStubInput(), + `run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`, + ); test.done(); }, @@ -433,20 +291,9 @@ export = { }, }; -// Reads a docker stub and cleans the volume paths out of the stub. -function readAndCleanDockerStubInput(file: string) { - return fs - .readFileSync(file, 'utf-8') - .trim() - .replace(/-v ([^:]+):\/asset-input/g, '-v /input:/asset-input') - .replace(/-v ([^:]+):\/asset-output/g, '-v /output:/asset-output'); -} - -// Last docker input since last teardown function readDockerStubInput() { - return readAndCleanDockerStubInput(STUB_INPUT_FILE); -} -// Concatenated docker inputs since last teardown -function readDockerStubInputConcat() { - return readAndCleanDockerStubInput(STUB_INPUT_CONCAT_FILE); + const out = fs.readFileSync(STUB_INPUT_FILE, 'utf-8').trim(); + return out + .replace(/-v ([^:]+):\/asset-input/, '-v /input:/asset-input') + .replace(/-v ([^:]+):\/asset-output/, '-v /output:/asset-output'); }