fix(toolkit-lib): fromCdkApp fails if working directory is given and outdir is relative (#444)

If a working directory is given and outdir is relative, then the CDK app
synths relative to the supplied working directory, but the toolkit tries
to load the assembly relative to the process' `cwd` and fails.

Also, the documentation of `fromCdkApp` falsely claimed that by default
it will create a temporary directory and clean it up. It does not, by
default it emits into a relative `cdk.out` directory which won't get
cleaned up. The `cdk.out` directory also was not entirely consistently
evaluated, so we specify that it must be evaluated against
`workingDirectory`.

While we're at it, immediately add a flag that allows explicit
directories to be cleaned up as well so that our tests (which explicitly
specify temporary directories and so they don't get cleaned up) don't
spam all the way into the system's tempdir.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

Author: rix0rrr

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/source-builder.ts

@@ -1,3 +1,4 @@
+import * as path from 'path';
 import * as cxapi from '@aws-cdk/cx-api';
 import * as fs from 'fs-extra';
 import type { AssemblyDirectoryProps, AssemblySourceProps, ICloudAssemblySource } from '../';
@@ -25,9 +26,10 @@ export abstract class CloudAssemblySourceBuilder {
   /**
    * Create a Cloud Assembly from a Cloud Assembly builder function.
    *
-   * A temporary output directory will be created if no output directory is
-   * explicitly given. This directory will be cleaned up if synthesis fails, or
-   * when the Cloud Assembly produced by this source is disposed.
+   * The output directory will be evaluated with respect to the working
+   * directory if relative. If not given, it will synthesize into a temporary
+   * system directory. The temporary directory will be cleaned up, unless
+   * `disposeOutdir: false`.
    *
    * A write lock will be acquired on the output directory for the duration of
    * the CDK app synthesis (which means that no two apps can synthesize at the
@@ -51,10 +53,13 @@ export abstract class CloudAssemblySourceBuilder {
       lookups: props.lookups,
     };
 
+    const workingDirectory = props.workingDirectory ?? process.cwd();
+    const outdir = props.outdir ? path.resolve(workingDirectory, props.outdir) : undefined;
+
     return new ContextAwareCloudAssemblySource(
       {
         produce: async () => {
-          await using execution = await ExecutionEnvironment.create(services, { outdir: props.outdir });
+          await using execution = await ExecutionEnvironment.create(services, { outdir });
 
           const env = await execution.defaultEnvVars();
           const assembly = await execution.changeDir(async () =>
@@ -74,15 +79,16 @@ export abstract class CloudAssemblySourceBuilder {
                   throw AssemblyError.withCause('Assembly builder failed', error);
                 }
               }),
-            ), props.workingDirectory);
+            ), workingDirectory);
 
           // Convert what we got to the definitely correct type we're expecting, a cxapi.CloudAssembly
           const asm = cxapi.CloudAssembly.isCloudAssembly(assembly)
             ? assembly
             : await assemblyFromDirectory(assembly.directory, services.ioHelper, props.loadAssemblyOptions);
 
           const success = await execution.markSuccessful();
-          return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose: execution.outDirIsTemporary });
+          const deleteOnDispose = props.disposeOutdir ?? execution.outDirIsTemporary;
+          return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose });
         },
       },
       contextAssemblyProps,
@@ -129,9 +135,10 @@ export abstract class CloudAssemblySourceBuilder {
   /**
    * Use a directory containing an AWS CDK app as source.
    *
-   * A temporary output directory will be created if no output directory is
-   * explicitly given. This directory will be cleaned up if synthesis fails, or
-   * when the Cloud Assembly produced by this source is disposed.
+   * The output directory will be evaluated with respect to the working
+   * directory if relative. If not given, it will synthesize into a `cdk.out`
+   * subdirectory. This directory will not be cleaned up, unless
+   * `disposeOutdir: true`.
    *
    * A write lock will be acquired on the output directory for the duration of
    * the CDK app synthesis (which means that no two apps can synthesize at the
@@ -152,6 +159,9 @@ export abstract class CloudAssemblySourceBuilder {
       lookups: props.lookups,
     };
 
+    const workingDirectory = props.workingDirectory ?? process.cwd();
+    const outdir = path.resolve(workingDirectory, props.outdir ?? 'cdk.out');
+
     return new ContextAwareCloudAssemblySource(
       {
         produce: async () => {
@@ -161,7 +171,6 @@ export abstract class CloudAssemblySourceBuilder {
           //   await execInChildProcess(build, { cwd: props.workingDirectory });
           // }
 
-          const outdir = props.outdir ?? 'cdk.out';
           try {
             fs.mkdirpSync(outdir);
           } catch (e: any) {
@@ -188,13 +197,14 @@ export abstract class CloudAssemblySourceBuilder {
                 }
               },
               extraEnv: envWithContext,
-              cwd: props.workingDirectory,
+              cwd: workingDirectory,
             });
 
             const asm = await assemblyFromDirectory(outdir, services.ioHelper, props.loadAssemblyOptions);
 
             const success = await execution.markSuccessful();
-            return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose: execution.outDirIsTemporary });
+            const deleteOnDispose = props.disposeOutdir ?? execution.outDirIsTemporary;
+            return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose });
           });
         },
       },

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/source-builder.ts

@@ -80,6 +80,13 @@ export interface AssemblySourceProps {
    * Options to configure loading of the assembly after it has been synthesized
    */
   readonly loadAssemblyOptions?: LoadAssemblyOptions;
+
+  /**
+   * Delete the `outdir` when the assembly is disposed
+   *
+   * @default - `true` if `outdir` is not given, `false` otherwise
+   */
+  readonly disposeOutdir?: boolean;
 }
 
 /**

packages/@aws-cdk/toolkit-lib/test/_helpers/index.ts

@@ -1,35 +1,55 @@
 import * as fs from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
-import type { AssemblyDirectoryProps, FromCdkAppOptions, ICloudAssemblySource } from '../../lib';
+import type { AssemblyBuilder, AssemblyDirectoryProps, FromCdkAppOptions, ICloudAssemblySource } from '../../lib';
 import { ToolkitError } from '../../lib';
 import type { CloudAssemblySourceBuilder } from '../../lib/api/cloud-assembly/private';
 
 export * from './test-cloud-assembly-source';
 export * from './test-io-host';
 
 function fixturePath(...parts: string[]): string {
-  return path.normalize(path.join(__dirname, '..', '_fixtures', ...parts));
+  const ret = path.normalize(path.join(__dirname, '..', '_fixtures', ...parts));
+  if (!fs.existsSync(ret)) {
+    throw new ToolkitError(`App Fixture not found: ${ret}`);
+  }
+  return ret;
 }
 
-export async function appFixture(toolkit: CloudAssemblySourceBuilder, name: string, options?: Omit<FromCdkAppOptions, 'workingDirectory' | 'outdir'>) {
+/**
+ * Return config we can send into `fromCdkApp` to execute a given app fixture
+ */
+export function appFixtureConfig(name: string) {
   const appPath = fixturePath(name, 'app.js');
-  if (!fs.existsSync(appPath)) {
-    throw new ToolkitError(`App Fixture ${name} does not exist in ${appPath}`);
-  }
-  const app = `cat ${appPath} | node --input-type=module`;
-  return toolkit.fromCdkApp(app, {
+  return {
+    app: `cat ${appPath} | node --input-type=module`,
     workingDirectory: path.join(__dirname, '..', '..'),
+  };
+}
+
+export async function appFixture(toolkit: CloudAssemblySourceBuilder, name: string, options?: Omit<FromCdkAppOptions, 'workingDirectory' | 'outdir'>) {
+  const app = appFixtureConfig(name);
+  return toolkit.fromCdkApp(app.app, {
+    workingDirectory: app.workingDirectory,
     outdir: tmpOutdir(),
+    disposeOutdir: true,
     ...options,
   });
 }
 
-export function builderFixture(toolkit: CloudAssemblySourceBuilder, name: string, context?: { [key: string]: any }) {
+/**
+ * Loads a builder from a directory that contains an 'index.js' with a default export
+ */
+export function builderFunctionFromFixture(builderName: string): AssemblyBuilder {
   // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const builder = require(path.join(__dirname, '..', '_fixtures', name)).default;
+  return require(path.join(__dirname, '..', '_fixtures', builderName)).default;
+}
+
+export function builderFixture(toolkit: CloudAssemblySourceBuilder, name: string, context?: { [key: string]: any }) {
+  const builder = builderFunctionFromFixture(name);
   return toolkit.fromAssemblyBuilder(builder, {
     outdir: tmpOutdir(),
+    disposeOutdir: true,
     context,
   });
 }

packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/source-builder.test.ts

@@ -1,8 +1,9 @@
+import * as fs from 'fs-extra';
 import { RWLock } from '../../../lib/api/rwlock';
 import { contextproviders } from '../../../lib/api/shared-private';
 import { ToolkitError } from '../../../lib/api/shared-public';
 import { Toolkit } from '../../../lib/toolkit/toolkit';
-import { appFixture, autoCleanOutDir, builderFixture, cdkOutFixture, TestIoHost } from '../../_helpers';
+import { appFixture, appFixtureConfig, autoCleanOutDir, builderFixture, builderFunctionFromFixture, cdkOutFixture, TestIoHost } from '../../_helpers';
 
 // these tests often run a bit longer than the default
 jest.setTimeout(10_000);
@@ -60,6 +61,44 @@ describe('fromAssemblyBuilder', () => {
     }
   });
 
+  test('outdir is relative to workingDirectory parameter', async () => {
+    // GIVEN
+    await using synthDir = autoCleanOutDir();
+    const builder = builderFunctionFromFixture('two-empty-stacks');
+
+    // WHEN
+    const cx = await toolkit.fromAssemblyBuilder(builder, {
+      workingDirectory: synthDir.dir,
+      outdir: 'relative.dir',
+    });
+
+    await using asm = await toolkit.synth(cx);
+
+    // THEN - asm directory is relative to the temp dir
+    expect(asm.cloudAssembly.directory.startsWith(synthDir.dir));
+  });
+
+  test('disposeOutdir can be used to disappear explicit synth dir', async() => {
+    // GIVEN
+    await using synthDir = autoCleanOutDir();
+    const builder = builderFunctionFromFixture('two-empty-stacks');
+
+    // WHEN
+    const cx = await toolkit.fromAssemblyBuilder(builder, {
+      workingDirectory: synthDir.dir,
+      outdir: 'relative.dir',
+      disposeOutdir: true,
+    });
+
+    const asm = await toolkit.synth(cx);
+
+    expect(await fs.pathExists(asm.cloudAssembly.directory)).toBeTruthy();
+    await asm.dispose();
+
+    // THEN
+    expect(await fs.pathExists(asm.cloudAssembly.directory)).toBeFalsy();
+  });
+
   test('fromAssemblyBuilder can successfully loop', async () => {
     // GIVEN
     const provideContextValues = jest.spyOn(contextproviders, 'provideContextValues').mockImplementation(async (
@@ -114,6 +153,44 @@ describe('fromCdkApp', () => {
     expect(assembly.cloudAssembly.stacksRecursively.map(s => s.hierarchicalId)).toEqual(['Stack1', 'Stack2']);
   });
 
+  test('synth succeeds when working directory is given and outdir is relative', async () => {
+    // WHEN
+    const app = await appFixtureConfig('two-empty-stacks');
+    const cx = await toolkit.fromCdkApp(app.app, {
+      workingDirectory: app.workingDirectory,
+      outdir: 'relative.dir',
+      disposeOutdir: true,
+    });
+
+    await using assembly = await cx.produce();
+
+    // THEN - synth succeeds
+    expect(assembly.cloudAssembly.stacksRecursively.map(s => s.hierarchicalId)).toEqual(['Stack1', 'Stack2']);
+
+    // asm directory is relative to the dir containing the app
+    expect(assembly.cloudAssembly.directory.startsWith(app.workingDirectory));
+  });
+
+  test('disposeOutdir can be used to disappear explicit synth dir', async() => {
+    // GIVEN
+    const app = await appFixtureConfig('two-empty-stacks');
+
+    // WHEN
+    const cx = await toolkit.fromCdkApp(app.app, {
+      workingDirectory: app.workingDirectory,
+      outdir: 'relative.dir',
+      disposeOutdir: true,
+    });
+
+    const asm = await toolkit.synth(cx);
+
+    expect(await fs.pathExists(asm.cloudAssembly.directory)).toBeTruthy();
+    await asm.dispose();
+
+    // THEN
+    expect(await fs.pathExists(asm.cloudAssembly.directory)).toBeFalsy();
+  });
+
   test('can provide context', async () => {
     // WHEN
     const cx = await appFixture(toolkit, 'external-context', {