Skip to content

Commit 46b2a32

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 e1a9030 commit 46b2a32

File tree

4 files changed

+73
-23
lines changed

4 files changed

+73
-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+
* `deleteOutdir: 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.deleteOutdir ?? 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+
* `deleteOutdir: 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) {
@@ -185,13 +194,14 @@ export abstract class CloudAssemblySourceBuilder {
185194
}
186195
},
187196
extraEnv: envWithContext,
188-
cwd: props.workingDirectory,
197+
cwd: workingDirectory,
189198
});
190199

191200
const asm = await assemblyFromDirectory(outdir, services.ioHelper, props.loadAssemblyOptions);
192201

193202
const success = await execution.markSuccessful();
194-
return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose: execution.outDirIsTemporary });
203+
const deleteOnDispose = props.deleteOutdir ?? execution.outDirIsTemporary;
204+
return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose });
195205
});
196206
},
197207
},

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 deleteOutdir?: boolean;
8390
}
8491

8592
/**

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

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,32 @@ import type { CloudAssemblySourceBuilder } from '../../lib/api/cloud-assembly/pr
88
export * from './test-cloud-assembly-source';
99
export * from './test-io-host';
1010

11-
function fixturePath(...parts: string[]): string {
12-
return path.normalize(path.join(__dirname, '..', '_fixtures', ...parts));
11+
function appFixturePath(...parts: string[]): string {
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, context?: { [key: string]: any }) {
16-
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, {
19+
/**
20+
* Return config we can send into `fromCdkApp` to execute a given app fixture
21+
*/
22+
export function appFixtureCommand(name: string) {
23+
const appPath = appFixturePath(name, 'app.js');
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, context?: { [key: string]: any }) {
31+
const app = appFixtureCommand(name);
32+
return toolkit.fromCdkApp(app.app, {
33+
workingDirectory: app.workingDirectory,
2334
outdir: tmpOutdir(),
24-
context,
35+
deleteOutdir: true,
36+
...options,
2537
});
2638
}
2739

@@ -30,6 +42,7 @@ export function builderFixture(toolkit: CloudAssemblySourceBuilder, name: string
3042
const builder = require(path.join(__dirname, '..', '_fixtures', name)).default;
3143
return toolkit.fromAssemblyBuilder(builder, {
3244
outdir: tmpOutdir(),
45+
deleteOutdir: true,
3346
context,
3447
});
3548
}

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import { promises as fs } from 'fs';
2+
import * as path from 'path';
13
import { RWLock } from '../../../lib/api/rwlock';
24
import { contextproviders } from '../../../lib/api/shared-private';
35
import { ToolkitError } from '../../../lib/api/shared-public';
46
import { Toolkit } from '../../../lib/toolkit/toolkit';
5-
import { appFixture, autoCleanOutDir, builderFixture, cdkOutFixture, TestIoHost } from '../../_helpers';
7+
import { appFixture, appFixtureCommand, autoCleanOutDir, builderFixture, cdkOutFixture, TestIoHost } from '../../_helpers';
68

79
// these tests often run a bit longer than the default
810
jest.setTimeout(10_000);
@@ -114,6 +116,24 @@ describe('fromCdkApp', () => {
114116
expect(assembly.cloudAssembly.stacksRecursively.map(s => s.hierarchicalId)).toEqual(['Stack1', 'Stack2']);
115117
});
116118

119+
test('synth succeeds when working directory is given and outdir is relative', async () => {
120+
// WHEN
121+
const app = await appFixtureCommand('two-empty-stacks');
122+
const cx = await toolkit.fromCdkApp(app.app, {
123+
workingDirectory: app.workingDirectory,
124+
outdir: 'relative.dir',
125+
});
126+
127+
try {
128+
await using assembly = await cx.produce();
129+
130+
// THEN - synth succeeds
131+
expect(assembly.cloudAssembly.stacksRecursively.map(s => s.hierarchicalId)).toEqual(['Stack1', 'Stack2']);
132+
} finally {
133+
await fs.rm(path.join(app.workingDirectory, 'relative.dir'), { force: true, recursive: true });
134+
}
135+
});
136+
117137
test('can provide context', async () => {
118138
// WHEN
119139
const cx = await appFixture(toolkit, 'external-context', {

0 commit comments

Comments
 (0)