fix(lib): reuse a bundled asset's artifact when its hash is unchanged

packages/cdktn/src/private/fs.ts

@@ -443,3 +443,46 @@ export function findFileAboveCwd(
 
   return null;
 }
+
+const RETRY_ATTEMPTS = 5;
+const RETRY_BACKOFF_MS = 50;
+const RENAME_RETRY_CODES = ["EPERM", "EACCES", "EBUSY"];
+
+/**
+ * Rename `src` to `dest`, retrying with a short backoff while Windows refuses
+ * because a file inside is briefly open (antivirus, search indexer).
+ * @param src - path to rename
+ * @param dest - new path
+ */
+export function renameSyncWithRetry(src: string, dest: string): void {
+  for (let attempt = 1; ; attempt++) {
+    try {
+      fs.renameSync(src, dest);
+      return;
+    } catch (err: any) {
+      if (attempt >= RETRY_ATTEMPTS || !RENAME_RETRY_CODES.includes(err.code)) {
+        throw err;
+      }
+      Atomics.wait(
+        new Int32Array(new SharedArrayBuffer(4)),
+        0,
+        0,
+        RETRY_BACKOFF_MS * 2 ** (attempt - 1),
+      );
+    }
+  }
+}
+
+/**
+ * Remove `target` recursively, letting Node retry while Windows briefly holds
+ * a file inside open.
+ * @param target - path to remove
+ */
+export function rmSyncWithRetry(target: string): void {
+  fs.rmSync(target, {
+    recursive: true,
+    force: true,
+    maxRetries: RETRY_ATTEMPTS - 1,
+    retryDelay: RETRY_BACKOFF_MS,
+  });
+}

packages/cdktn/src/terraform-asset.ts

@@ -12,7 +12,11 @@ import {
 } from "./assets";
 import { AssetStaging } from "./asset-staging";
 import { IIgnoreStrategy } from "./ignore-strategy";
-import { findFileAboveCwd } from "./private/fs";
+import {
+  findFileAboveCwd,
+  renameSyncWithRetry,
+  rmSyncWithRetry,
+} from "./private/fs";
 import { ISynthesisSession } from "./synthesize";
 import { addCustomSynthesis } from "./synthesize/synthesizer";
 import { TerraformStack } from "./terraform-stack";
@@ -74,6 +78,12 @@ export interface TerraformAssetConfig {
    * Extra information to fold into the hash (e.g. build instructions and
    * other inputs).
    *
+   * A bundled asset reuses the artifact an earlier synth staged for the same
+   * hash, so set this for build inputs the hash cannot see: files outside
+   * `path`, a mutable Docker tag such as `node:20`, or which bundler is used
+   * and its options when it has no `bundlerKey`. It is ignored when
+   * `assetHash` is set; change `assetHash` instead.
+   *
    * @default - no extra hash
    */
   readonly extraHash?: string;
@@ -87,6 +97,10 @@ export interface TerraformAssetConfig {
    * The bundler's output shape must match `type`. See
    * `AssetStagingOptions.bundler`.
    *
+   * Under `SOURCE` hashing or a fixed `assetHash`, a synth keeps the artifact
+   * already staged for the same hash and `type`. See `extraHash` for inputs
+   * the hash cannot see; deleting the output directory also forces a rebuild.
+   *
    * @default - the source is staged verbatim, with no build step
    */
   readonly bundler?: IAssetBundler;
@@ -136,6 +150,8 @@ export class TerraformAsset extends Construct implements IAsset {
   private readonly staging: AssetStaging;
   // set when a bundler builds the artifact; names a single-file result
   private readonly bundler?: IAssetBundler;
+  // true when the hash is known before the build, so a staged artifact can be reused
+  private readonly reusesStagedArtifact: boolean;
 
   /**
    * A Terraform Asset takes a file or directory outside of the CDK Terrain context and moves it into it.
@@ -177,6 +193,8 @@ export class TerraformAsset extends Construct implements IAsset {
     }
 
     this.bundler = config.bundler;
+    this.reusesStagedArtifact =
+      !!config.bundler && config.assetHashType !== AssetHashType.OUTPUT;
 
     const outputFileName = config.bundler?.outputFileName;
     if (outputFileName !== undefined) {
@@ -270,21 +288,72 @@ export class TerraformAsset extends Construct implements IAsset {
       "..",
     );
 
-    // Cleanup existing assets
-    const previousVersionsFolder = path.join(basePath, this.namedFolder);
-    if (fs.existsSync(previousVersionsFolder)) {
-      fs.rmSync(previousVersionsFolder, { recursive: true });
+    // Cleanup assets staged for previous hashes
+    const namedFolder = path.join(basePath, this.namedFolder);
+    const hashFolder = path.join(namedFolder, this.assetHash);
+    // Records how the artifact in the hash folder was packaged, which the
+    // hash does not cover: ARCHIVE and DIRECTORY share a hash.
+    const packagingFile = `${hashFolder}.json`;
+    if (fs.existsSync(namedFolder)) {
+      for (const entry of fs.readdirSync(namedFolder)) {
+        if (entry !== this.assetHash && entry !== `${this.assetHash}.json`) {
+          rmSyncWithRetry(path.join(namedFolder, entry));
+        }
+      }
     }
 
+    // SOURCE and CUSTOM hashes are known before the build, so an artifact
+    // already staged for this hash and packaging is what the bundler would
+    // build again.
     const targetPath = path.join(basePath, this.path);
-    const packaging = this.packaging;
+    // A directory artifact has no file name, so moving its source does not
+    // rebuild it.
+    const stagedPackaging = JSON.stringify({
+      type: AssetType[this.type],
+      fileName: this.packaging.producesDirectory ? undefined : this.fileName,
+    });
+    if (
+      this.reusesStagedArtifact &&
+      fs.existsSync(targetPath) &&
+      fs.existsSync(packagingFile) &&
+      fs.readFileSync(packagingFile, "utf8") === stagedPackaging
+    ) {
+      return;
+    }
+    fs.rmSync(packagingFile, { force: true });
 
-    if (packaging.producesDirectory) {
-      fs.mkdirSync(targetPath, { recursive: true });
-    } else {
-      fs.mkdirSync(path.dirname(targetPath), { recursive: true });
+    if (!this.reusesStagedArtifact) {
+      fs.rmSync(hashFolder, { recursive: true, force: true });
+      if (this.packaging.producesDirectory) {
+        fs.mkdirSync(targetPath, { recursive: true });
+      } else {
+        fs.mkdirSync(path.dirname(targetPath), { recursive: true });
+      }
+      this.staging.stage(targetPath);
+      return;
     }
 
-    this.staging.stage(targetPath);
+    // Stage next to the hash folder and move it into place only once staging
+    // succeeded, so a failed or interrupted build never leaves a folder that
+    // a later synth would reuse.
+    const stagingFolder = `${hashFolder}.staging`;
+    fs.mkdirSync(stagingFolder, { recursive: true });
+    try {
+      this.staging.stage(
+        this.packaging.producesDirectory
+          ? stagingFolder
+          : path.join(stagingFolder, this.fileName),
+      );
+      rmSyncWithRetry(hashFolder);
+      renameSyncWithRetry(stagingFolder, hashFolder);
+    } catch (err) {
+      try {
+        rmSyncWithRetry(stagingFolder);
+      } catch {
+        // The next synth sweeps it; the build error is the one to report.
+      }
+      throw err;
+    }
+    fs.writeFileSync(packagingFile, stagedPackaging);
   }
 }

packages/cdktn/test/canonical-asset-hash.test.ts

@@ -14,6 +14,7 @@ import {
   AssetHashType,
   BundleResult,
   IAsset,
+  IAssetBundler,
 } from "../src";
 import { CANONICAL_ASSET_HASHES } from "../src/features";
 import { TerraformModuleAsset } from "../src/terraform-module-asset";
@@ -899,6 +900,197 @@ describe("TerraformAsset stages inside the stack's own directory (#380)", () =>
   });
 });
 
+describe("TerraformAsset reuses a bundled artifact across synths (#582)", () => {
+  let srcDir: string;
+  let outdir: string;
+
+  beforeEach(() => {
+    srcDir = createTempDir();
+    fs.writeFileSync(path.join(srcDir, "a.txt"), "content");
+    outdir = createTempDir();
+  });
+
+  afterEach(() => {
+    fs.rmSync(srcDir, { recursive: true, force: true });
+    fs.rmSync(outdir, { recursive: true, force: true });
+  });
+
+  // Each synth gets a fresh App writing to the same outdir, as consecutive
+  // `cdktn synth` runs do.
+  const build = (type: AssetType, bundler: IAssetBundler) => {
+    const app = Testing.app({
+      outdir,
+      context: { [CANONICAL_ASSET_HASHES]: "true" },
+    });
+    const s = new TerraformStack(app, "s");
+    const asset = new TerraformAsset(s, "asset", {
+      path: srcDir,
+      type,
+      bundler,
+    });
+    return { app, asset, stackDir: path.join(outdir, "stacks", s.node.id) };
+  };
+
+  const synth = (type: AssetType, bundler: IAssetBundler) => {
+    const built = build(type, bundler);
+    built.app.synth();
+    return built;
+  };
+
+  const countingBundler = (bundlerKey?: string) => {
+    const bundler = {
+      runs: 0,
+      bundlerKey,
+      bundle: (opts: { outputDir: string }) => {
+        bundler.runs++;
+        fs.writeFileSync(path.join(opts.outputDir, "index.txt"), "built");
+        return BundleResult.directory(opts.outputDir);
+      },
+    };
+    return bundler;
+  };
+
+  const fileBundler: IAssetBundler = {
+    outputFileName: "out.txt",
+    bundle: (opts) => {
+      const file = path.join(opts.outputDir, "out.txt");
+      fs.writeFileSync(file, "built");
+      return BundleResult.file(file);
+    },
+  };
+
+  test("a second synth with the same hash does not rerun the bundler", () => {
+    const bundler = countingBundler();
+
+    const first = synth(AssetType.ARCHIVE, bundler);
+    const second = synth(AssetType.ARCHIVE, bundler);
+
+    expect(second.asset.assetHash).toBe(first.asset.assetHash);
+    expect(bundler.runs).toBe(1);
+    expect(fs.existsSync(path.join(second.stackDir, second.asset.path))).toBe(
+      true,
+    );
+  });
+
+  test("a changed hash reruns the bundler and removes the old artifact", () => {
+    const v1 = countingBundler("v1");
+    const v2 = countingBundler("v2");
+
+    const first = synth(AssetType.ARCHIVE, v1);
+    const second = synth(AssetType.ARCHIVE, v2);
+
+    expect(second.asset.assetHash).not.toBe(first.asset.assetHash);
+    expect(v2.runs).toBe(1);
+    const namedFolder = path.dirname(
+      path.dirname(path.join(second.stackDir, second.asset.path)),
+    );
+    expect(fs.readdirSync(namedFolder)).toEqual([
+      second.asset.assetHash,
+      `${second.asset.assetHash}.json`,
+    ]);
+  });
+
+  test.each([
+    ["ARCHIVE", AssetType.ARCHIVE, countingBundler()],
+    ["FILE", AssetType.FILE, fileBundler],
+  ])(
+    "changing the type from %s to DIRECTORY rebuilds the artifact",
+    (_, type, previous) => {
+      const first = synth(type, previous);
+      const bundler = countingBundler();
+      const second = synth(AssetType.DIRECTORY, bundler);
+
+      expect(second.asset.assetHash).toBe(first.asset.assetHash);
+      expect(bundler.runs).toBe(1);
+      expect(
+        fs.readdirSync(path.join(second.stackDir, second.asset.path)),
+      ).toEqual(["index.txt"]);
+    },
+  );
+
+  test("a failed build leaves no artifact for the next synth to reuse", () => {
+    const failing = {
+      bundle: (opts: { outputDir: string }) => {
+        fs.writeFileSync(path.join(opts.outputDir, "partial.txt"), "partial");
+        throw new Error("build failed");
+      },
+    };
+
+    const failed = build(AssetType.DIRECTORY, failing);
+    expect(() => failed.app.synth()).toThrow(/build failed/);
+    expect(fs.existsSync(path.join(failed.stackDir, failed.asset.path))).toBe(
+      false,
+    );
+
+    const bundler = countingBundler();
+    const { asset, stackDir } = synth(AssetType.DIRECTORY, bundler);
+    expect(bundler.runs).toBe(1);
+    expect(fs.readdirSync(path.join(stackDir, asset.path))).toEqual([
+      "index.txt",
+    ]);
+  });
+
+  test("a failed cleanup does not hide the build error", () => {
+    const failing = {
+      bundle: () => {
+        throw new Error("build failed");
+      },
+    };
+    const rmSync = fs.rmSync;
+    const spy = jest.spyOn(fs, "rmSync").mockImplementation((p, opts) => {
+      if (String(p).endsWith(".staging")) {
+        throw Object.assign(new Error("resource busy or locked"), {
+          code: "EBUSY",
+        });
+      }
+      rmSync(p, opts);
+    });
+    try {
+      const failed = build(AssetType.DIRECTORY, failing);
+      expect(() => failed.app.synth()).toThrow(/build failed/);
+      const hashFolder = path.join(failed.stackDir, failed.asset.path);
+      expect(fs.existsSync(`${hashFolder}.staging`)).toBe(true);
+    } finally {
+      spy.mockRestore();
+    }
+
+    const bundler = countingBundler();
+    const { asset, stackDir } = synth(AssetType.DIRECTORY, bundler);
+    expect(bundler.runs).toBe(1);
+    expect(
+      fs.readdirSync(path.dirname(path.join(stackDir, asset.path))),
+    ).toEqual([asset.assetHash, `${asset.assetHash}.json`]);
+  });
+
+  test("moving a directory asset's source does not rebuild it", () => {
+    const bundler = countingBundler();
+    const first = synth(AssetType.DIRECTORY, bundler);
+    const moved = `${srcDir}-moved`;
+    fs.renameSync(srcDir, moved);
+    srcDir = moved;
+    const second = synth(AssetType.DIRECTORY, bundler);
+
+    expect(second.asset.assetHash).toBe(first.asset.assetHash);
+    expect(bundler.runs).toBe(1);
+  });
+
+  test("a rename refused with EPERM is retried", () => {
+    const spy = jest.spyOn(fs, "renameSync").mockImplementationOnce(() => {
+      throw Object.assign(new Error("operation not permitted"), {
+        code: "EPERM",
+      });
+    });
+    try {
+      const { asset, stackDir } = synth(AssetType.DIRECTORY, countingBundler());
+      expect(fs.readdirSync(path.join(stackDir, asset.path))).toEqual([
+        "index.txt",
+      ]);
+    } finally {
+      spy.mockRestore();
+    }
+  });
+});
+
 describe("TerraformModuleAsset with the canonicalAssetHashes flag", () => {
   let rootDir: string;
   let moduleA: string;