Skip to content

Commit dede397

Browse files
committed
fix(toolkit-lib): fromCdkApp fails if working directory is given and outdir is relative
The documentation of `fromCdkApp` falsely claimed that it would create a temporary directory and clean it up. It does not, it emits into `cdk.out` which won't get cleaned up. Update the docs.
1 parent 1d4fd87 commit dede397

File tree

4 files changed

+137
-23
lines changed

4 files changed

+137
-23
lines changed

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

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import * as path from 'path';
12
import * as cxapi from '@aws-cdk/cx-api';
23
import * as fs from 'fs-extra';
34
import type { AssemblyDirectoryProps, AssemblySourceProps, ICloudAssemblySource } from '../';
@@ -25,9 +26,10 @@ export abstract class CloudAssemblySourceBuilder {
2526
/**
2627
* Create a Cloud Assembly from a Cloud Assembly builder function.
2728
*
28-
* A temporary output directory will be created if no output directory is
29-
* explicitly given. This directory will be cleaned up if synthesis fails, or
30-
* when the Cloud Assembly produced by this source is disposed.
29+
* The output directory will be evaluated with respect to the working
30+
* directory if relative. If not given, it will synthesize into a temporary
31+
* system directory. The temporary directory will be cleaned up, unless
32+
* `disposeOutdir: false`.
3133
*
3234
* A write lock will be acquired on the output directory for the duration of
3335
* the CDK app synthesis (which means that no two apps can synthesize at the
@@ -51,10 +53,13 @@ export abstract class CloudAssemblySourceBuilder {
5153
lookups: props.lookups,
5254
};
5355

56+
const workingDirectory = props.workingDirectory ?? process.cwd();
57+
const outdir = props.outdir ? path.resolve(workingDirectory, props.outdir) : undefined;
58+
5459
return new ContextAwareCloudAssemblySource(
5560
{
5661
produce: async () => {
57-
await using execution = await ExecutionEnvironment.create(services, { outdir: props.outdir });
62+
await using execution = await ExecutionEnvironment.create(services, { outdir });
5863

5964
const env = await execution.defaultEnvVars();
6065
const assembly = await execution.changeDir(async () =>
@@ -74,15 +79,16 @@ export abstract class CloudAssemblySourceBuilder {
7479
throw AssemblyError.withCause('Assembly builder failed', error);
7580
}
7681
}),
77-
), props.workingDirectory);
82+
), workingDirectory);
7883

7984
// Convert what we got to the definitely correct type we're expecting, a cxapi.CloudAssembly
8085
const asm = cxapi.CloudAssembly.isCloudAssembly(assembly)
8186
? assembly
8287
: await assemblyFromDirectory(assembly.directory, services.ioHelper, props.loadAssemblyOptions);
8388

8489
const success = await execution.markSuccessful();
85-
return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose: execution.outDirIsTemporary });
90+
const deleteOnDispose = props.disposeOutdir ?? execution.outDirIsTemporary;
91+
return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose });
8692
},
8793
},
8894
contextAssemblyProps,
@@ -129,9 +135,10 @@ export abstract class CloudAssemblySourceBuilder {
129135
/**
130136
* Use a directory containing an AWS CDK app as source.
131137
*
132-
* A temporary output directory will be created if no output directory is
133-
* explicitly given. This directory will be cleaned up if synthesis fails, or
134-
* when the Cloud Assembly produced by this source is disposed.
138+
* The output directory will be evaluated with respect to the working
139+
* directory if relative. If not given, it will synthesize into a `cdk.out`
140+
* subdirectory. This directory will not be cleaned up, unless
141+
* `disposeOutdir: true`.
135142
*
136143
* A write lock will be acquired on the output directory for the duration of
137144
* the CDK app synthesis (which means that no two apps can synthesize at the
@@ -152,6 +159,9 @@ export abstract class CloudAssemblySourceBuilder {
152159
lookups: props.lookups,
153160
};
154161

162+
const workingDirectory = props.workingDirectory ?? process.cwd();
163+
const outdir = path.resolve(workingDirectory, props.outdir ?? 'cdk.out');
164+
155165
return new ContextAwareCloudAssemblySource(
156166
{
157167
produce: async () => {
@@ -161,7 +171,6 @@ export abstract class CloudAssemblySourceBuilder {
161171
// await execInChildProcess(build, { cwd: props.workingDirectory });
162172
// }
163173

164-
const outdir = props.outdir ?? 'cdk.out';
165174
try {
166175
fs.mkdirpSync(outdir);
167176
} catch (e: any) {
@@ -188,13 +197,14 @@ export abstract class CloudAssemblySourceBuilder {
188197
}
189198
},
190199
extraEnv: envWithContext,
191-
cwd: props.workingDirectory,
200+
cwd: workingDirectory,
192201
});
193202

194203
const asm = await assemblyFromDirectory(outdir, services.ioHelper, props.loadAssemblyOptions);
195204

196205
const success = await execution.markSuccessful();
197-
return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose: execution.outDirIsTemporary });
206+
const deleteOnDispose = props.disposeOutdir ?? execution.outDirIsTemporary;
207+
return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose });
198208
});
199209
},
200210
},

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ export interface AssemblySourceProps {
8080
* Options to configure loading of the assembly after it has been synthesized
8181
*/
8282
readonly loadAssemblyOptions?: LoadAssemblyOptions;
83+
84+
/**
85+
* Delete the `outdir` when the assembly is disposed
86+
*
87+
* @default - `true` if `outdir` is not given, `false` otherwise
88+
*/
89+
readonly disposeOutdir?: boolean;
8390
}
8491

8592
/**

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

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,55 @@
11
import * as fs from 'node:fs';
22
import * as os from 'node:os';
33
import * as path from 'node:path';
4-
import type { AssemblyDirectoryProps, FromCdkAppOptions, ICloudAssemblySource } from '../../lib';
4+
import type { AssemblyBuilder, AssemblyDirectoryProps, FromCdkAppOptions, ICloudAssemblySource } from '../../lib';
55
import { ToolkitError } from '../../lib';
66
import type { CloudAssemblySourceBuilder } from '../../lib/api/cloud-assembly/private';
77

88
export * from './test-cloud-assembly-source';
99
export * from './test-io-host';
1010

1111
function fixturePath(...parts: string[]): string {
12-
return path.normalize(path.join(__dirname, '..', '_fixtures', ...parts));
12+
const ret = path.normalize(path.join(__dirname, '..', '_fixtures', ...parts));
13+
if (!fs.existsSync(ret)) {
14+
throw new ToolkitError(`App Fixture not found: ${ret}`);
15+
}
16+
return ret;
1317
}
1418

15-
export async function appFixture(toolkit: CloudAssemblySourceBuilder, name: string, options?: Omit<FromCdkAppOptions, 'workingDirectory' | 'outdir'>) {
19+
/**
20+
* Return config we can send into `fromCdkApp` to execute a given app fixture
21+
*/
22+
export function appFixtureConfig(name: string) {
1623
const appPath = fixturePath(name, 'app.js');
17-
if (!fs.existsSync(appPath)) {
18-
throw new ToolkitError(`App Fixture ${name} does not exist in ${appPath}`);
19-
}
20-
const app = `cat ${appPath} | node --input-type=module`;
21-
return toolkit.fromCdkApp(app, {
24+
return {
25+
app: `cat ${appPath} | node --input-type=module`,
2226
workingDirectory: path.join(__dirname, '..', '..'),
27+
};
28+
}
29+
30+
export async function appFixture(toolkit: CloudAssemblySourceBuilder, name: string, options?: Omit<FromCdkAppOptions, 'workingDirectory' | 'outdir'>) {
31+
const app = appFixtureConfig(name);
32+
return toolkit.fromCdkApp(app.app, {
33+
workingDirectory: app.workingDirectory,
2334
outdir: tmpOutdir(),
35+
disposeOutdir: true,
2436
...options,
2537
});
2638
}
2739

28-
export function builderFixture(toolkit: CloudAssemblySourceBuilder, name: string, context?: { [key: string]: any }) {
40+
/**
41+
* Loads a builder from a directory that contains an 'index.js' with a default export
42+
*/
43+
export function builderFunctionFromFixture(builderName: string): AssemblyBuilder {
2944
// eslint-disable-next-line @typescript-eslint/no-require-imports
30-
const builder = require(path.join(__dirname, '..', '_fixtures', name)).default;
45+
return require(path.join(__dirname, '..', '_fixtures', builderName)).default;
46+
}
47+
48+
export function builderFixture(toolkit: CloudAssemblySourceBuilder, name: string, context?: { [key: string]: any }) {
49+
const builder = builderFunctionFromFixture(name);
3150
return toolkit.fromAssemblyBuilder(builder, {
3251
outdir: tmpOutdir(),
52+
disposeOutdir: true,
3353
context,
3454
});
3555
}

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

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
import * as fs from 'fs-extra';
12
import { RWLock } from '../../../lib/api/rwlock';
23
import { contextproviders } from '../../../lib/api/shared-private';
34
import { ToolkitError } from '../../../lib/api/shared-public';
45
import { Toolkit } from '../../../lib/toolkit/toolkit';
5-
import { appFixture, autoCleanOutDir, builderFixture, cdkOutFixture, TestIoHost } from '../../_helpers';
6+
import { appFixture, appFixtureConfig, autoCleanOutDir, builderFixture, builderFunctionFromFixture, cdkOutFixture, TestIoHost } from '../../_helpers';
67

78
// these tests often run a bit longer than the default
89
jest.setTimeout(10_000);
@@ -60,6 +61,44 @@ describe('fromAssemblyBuilder', () => {
6061
}
6162
});
6263

64+
test('outdir is relative to workingDirectory parameter', async () => {
65+
// GIVEN
66+
await using synthDir = autoCleanOutDir();
67+
const builder = builderFunctionFromFixture('two-empty-stacks');
68+
69+
// WHEN
70+
const cx = await toolkit.fromAssemblyBuilder(builder, {
71+
workingDirectory: synthDir.dir,
72+
outdir: 'relative.dir',
73+
});
74+
75+
await using asm = await toolkit.synth(cx);
76+
77+
// THEN - asm directory is relative to the temp dir
78+
expect(asm.cloudAssembly.directory.startsWith(synthDir.dir));
79+
});
80+
81+
test('disposeOutdir can be used to disappear explicit synth dir', async() => {
82+
// GIVEN
83+
await using synthDir = autoCleanOutDir();
84+
const builder = builderFunctionFromFixture('two-empty-stacks');
85+
86+
// WHEN
87+
const cx = await toolkit.fromAssemblyBuilder(builder, {
88+
workingDirectory: synthDir.dir,
89+
outdir: 'relative.dir',
90+
disposeOutdir: true,
91+
});
92+
93+
const asm = await toolkit.synth(cx);
94+
95+
expect(await fs.pathExists(asm.cloudAssembly.directory)).toBeTruthy();
96+
await asm.dispose();
97+
98+
// THEN
99+
expect(await fs.pathExists(asm.cloudAssembly.directory)).toBeFalsy();
100+
});
101+
63102
test('fromAssemblyBuilder can successfully loop', async () => {
64103
// GIVEN
65104
const provideContextValues = jest.spyOn(contextproviders, 'provideContextValues').mockImplementation(async (
@@ -114,6 +153,44 @@ describe('fromCdkApp', () => {
114153
expect(assembly.cloudAssembly.stacksRecursively.map(s => s.hierarchicalId)).toEqual(['Stack1', 'Stack2']);
115154
});
116155

156+
test('synth succeeds when working directory is given and outdir is relative', async () => {
157+
// WHEN
158+
const app = await appFixtureConfig('two-empty-stacks');
159+
const cx = await toolkit.fromCdkApp(app.app, {
160+
workingDirectory: app.workingDirectory,
161+
outdir: 'relative.dir',
162+
disposeOutdir: true,
163+
});
164+
165+
await using assembly = await cx.produce();
166+
167+
// THEN - synth succeeds
168+
expect(assembly.cloudAssembly.stacksRecursively.map(s => s.hierarchicalId)).toEqual(['Stack1', 'Stack2']);
169+
170+
// asm directory is relative to the dir containing the app
171+
expect(assembly.cloudAssembly.directory.startsWith(app.workingDirectory));
172+
});
173+
174+
test('disposeOutdir can be used to disappear explicit synth dir', async() => {
175+
// GIVEN
176+
const app = await appFixtureConfig('two-empty-stacks');
177+
178+
// WHEN
179+
const cx = await toolkit.fromCdkApp(app.app, {
180+
workingDirectory: app.workingDirectory,
181+
outdir: 'relative.dir',
182+
disposeOutdir: true,
183+
});
184+
185+
const asm = await toolkit.synth(cx);
186+
187+
expect(await fs.pathExists(asm.cloudAssembly.directory)).toBeTruthy();
188+
await asm.dispose();
189+
190+
// THEN
191+
expect(await fs.pathExists(asm.cloudAssembly.directory)).toBeFalsy();
192+
});
193+
117194
test('can provide context', async () => {
118195
// WHEN
119196
const cx = await appFixture(toolkit, 'external-context', {

0 commit comments

Comments
 (0)