diff --git a/packages/cdktn/src/private/fs.ts b/packages/cdktn/src/private/fs.ts index 4f86ca102..44520181a 100644 --- a/packages/cdktn/src/private/fs.ts +++ b/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, + }); +} diff --git a/packages/cdktn/src/terraform-asset.ts b/packages/cdktn/src/terraform-asset.ts index 01b6c3497..3a742ce53 100644 --- a/packages/cdktn/src/terraform-asset.ts +++ b/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); } } diff --git a/packages/cdktn/test/canonical-asset-hash.test.ts b/packages/cdktn/test/canonical-asset-hash.test.ts index 4f99657c9..d66d9a61c 100644 --- a/packages/cdktn/test/canonical-asset-hash.test.ts +++ b/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;