chore(core): allow the bundler to re-use pre-existing bundler output

packages/@aws-cdk/core/lib/asset-staging.ts

@@ -88,12 +88,23 @@ 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) {
-      this.bundleDir = this.bundle(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)
+        : undefined;
+
+      this.bundleDir = this.bundle(props.bundling, sourceHash);
+      this.assetHash = sourceHash ?? this.calculateHash(hashType, props.assetHash);
+    } else {
+      this.assetHash = this.calculateHash(hashType, props.assetHash);
     }
 
-    this.assetHash = this.calculateHash(props);
-
     const stagingDisabled = this.node.tryGetContext(cxapi.DISABLE_ASSET_STAGING_CONTEXT);
     if (stagingDisabled) {
       this.stagedPath = this.bundleDir ?? this.sourcePath;
@@ -137,15 +148,40 @@ export class AssetStaging extends Construct {
     }
   }
 
-  private bundle(options: BundlingOptions): string {
+  /**
+   * @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 {
     // Temp staging directory in the working directory
     const stagingTmp = path.join('.', STAGING_TMP);
     fs.ensureDirSync(stagingTmp);
 
-    // 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 bundleDir: string;
+    if (sourceHash) {
+      // Calculate a hash that considers the source hash as well as the bundling options.
+      const bundleHash = this.calculateBundleHash(options, 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-' + bundleHash));
+
+      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);
+    }
 
     let user: string;
     if (options.user) {
@@ -179,7 +215,18 @@ export class AssetStaging extends Construct {
         workingDirectory: options.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR,
       });
     } catch (err) {
-      throw new Error(`Failed to run bundling Docker image for asset ${this.node.path}: ${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}`);
     }
 
     if (FileSystem.isEmpty(bundleDir)) {
@@ -189,20 +236,40 @@ export class AssetStaging extends Construct {
     return bundleDir;
   }
 
-  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\`.`);
+  /**
+   * 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\`.`);
       }
-      hashType = AssetHashType.CUSTOM;
-    } else if (props.assetHashType) {
-      hashType = props.assetHashType;
+      return AssetHashType.CUSTOM;
+    } else if (assetHashType) {
+      return assetHashType;
     } else {
-      hashType = AssetHashType.SOURCE;
+      return AssetHashType.SOURCE;
     }
+  }
+
+  /**
+   * Calculates a hash for bundle directories which combines the bundler options
+   * and source hash.
+   *
+   * @param options Bundling options considered for hashing purposes
+   * @param sourceHash The source asset hash
+   */
+  private calculateBundleHash(options: BundlingOptions, sourceHash: string) {
+    return crypto.createHash('sha256')
+      .update(JSON.stringify(options))
+      .update(sourceHash)
+      .digest('hex');
+  }
 
+  private calculateHash(hashType: AssetHashType, assetHash?: string): string {
     switch (hashType) {
       case AssetHashType.SOURCE:
         return FileSystem.fingerprint(this.sourcePath, this.fingerprintOptions);
@@ -212,12 +279,12 @@ export class AssetStaging extends Construct {
         }
         return FileSystem.fingerprint(this.bundleDir, this.fingerprintOptions);
       case AssetHashType.CUSTOM:
-        if (!props.assetHash) {
+        if (!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(props.assetHash).digest('hex');
+        return crypto.createHash('sha256').update(assetHash).digest('hex');
       default:
         throw new Error('Unknown asset hash type.');
     }

packages/@aws-cdk/core/test/docker-stub.sh

@@ -6,6 +6,7 @@ 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

packages/@aws-cdk/core/test/test.staging.ts

@@ -7,6 +7,8 @@ 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',
@@ -26,6 +28,12 @@ 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();
   },
@@ -106,8 +114,6 @@ 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');
 
     // WHEN
     new AssetStaging(stack, 'Asset', {
@@ -133,10 +139,144 @@ export = {
     ]);
 
     // asset is bundled in a directory inside .cdk.staging
-    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));
+    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 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
+    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));
+    // 'Asset'
+    test.ok(ensureDirSyncSpy.calledWith(path.resolve(path.join(STAGING_TMP_DIRECTORY, 'asset-bundle-hash-dec215520dfd57a87aa1362e9e15131938583cd6e5a2bd9a45d38fe5dc5ab3d7'))));
+    // 'AssetWithDifferentBundlingOptions'
+    test.ok(ensureDirSyncSpy.calledWith(path.resolve(path.join(STAGING_TMP_DIRECTORY, 'asset-bundle-hash-a33245f0209379d58d125d89906c2b47d38382ae745375f25697760a8c475c6b'))));
+
+    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();
   },
@@ -224,10 +364,6 @@ 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();
   },
@@ -287,9 +423,20 @@ 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() {
-  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');
+  return readAndCleanDockerStubInput(STUB_INPUT_FILE);
+}
+// Concatenated docker inputs since last teardown
+function readDockerStubInputConcat() {
+  return readAndCleanDockerStubInput(STUB_INPUT_CONCAT_FILE);
 }