Skip to content

Commit 19f3b5e

Browse files
mrgrainrix0rrr
andauthored
fix(toolkit-lib): make default environment resolution optional (#689)
This PR makes the default environment resolution optional in the toolkit-lib. It allows avoiding unnecessary STS calls when the environment is explicitly specified or when local actions are performed without internet access. Then uses the new feature in `integ-runner` to disable the default env lookup in the toolkit-lib engine. The existing engine doesn't provide the env variables either, so it's clearly not needed. We don't want to add to this contract at this time and also resolving the account adds an unnecessary delay. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license Co-authored-by: Rico Hermans <rix0rrr@gmail.com>
1 parent 099787b commit 19f3b5e

File tree

5 files changed

+150
-21
lines changed

5 files changed

+150
-21
lines changed

packages/@aws-cdk/integ-runner/lib/engines/toolkit-lib.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export class ToolkitLibRunnerEngine implements ICdk {
8484
outdir: options.output ? path.join(this.options.workingDirectory, options.output) : undefined,
8585
contextStore: new MemoryContext(options.context),
8686
lookups: false,
87+
resolveDefaultEnvironment: false,
8788
env: {
8889
...this.options.env,
8990
...options.env,
@@ -209,6 +210,7 @@ export class ToolkitLibRunnerEngine implements ICdk {
209210
workingDirectory: this.options.workingDirectory,
210211
outdir,
211212
lookups: options.lookups,
213+
resolveDefaultEnvironment: false, // not part of the integ-runner contract
212214
contextStore: new MemoryContext(options.context),
213215
env: this.options.env,
214216
synthOptions: {

packages/@aws-cdk/integ-runner/test/engines/toolkit-lib.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ describe('ToolkitLibRunnerEngine', () => {
7373
contextStore: expect.any(Object),
7474
lookups: false,
7575
env: { TEST: 'true' },
76+
resolveDefaultEnvironment: false,
7677
synthOptions: {
7778
versionReporting: false,
7879
pathMetadata: false,

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

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@ import type { Context, Env } from '../environment';
2020
import { prepareDefaultEnvironment, spaceAvailableForContext, guessExecutable, synthParametersFromSettings } from '../environment';
2121
import type { AppSynthOptions, LoadAssemblyOptions } from '../source-builder';
2222

23+
export interface ExecutionEnvironmentOptions {
24+
/**
25+
* The directory the cloud assembly will be written to.
26+
*
27+
* @default - use a temporary directory as output directory, this will be cleaned up when this object is disposed
28+
*/
29+
readonly outdir?: string;
30+
31+
/**
32+
* Resolve and add environment variables for the app's default environment.
33+
*
34+
* This will make a call to STS, which is not always desirable e.g. if the env is explicitly specified.
35+
*/
36+
readonly resolveDefaultAppEnv: boolean;
37+
}
38+
2339
export class ExecutionEnvironment implements AsyncDisposable {
2440
/**
2541
* Create an ExecutionEnvironment
@@ -33,18 +49,39 @@ export class ExecutionEnvironment implements AsyncDisposable {
3349
* If `markSuccessful()` is called, the writer lock is converted to a reader lock
3450
* and temporary directories will not be cleaned up anymore.
3551
*/
36-
public static async create(services: ToolkitServices, props: { outdir?: string } = {}) {
37-
let tempDir = false;
38-
let dir = props.outdir;
52+
public static async create(services: ToolkitServices, options: ExecutionEnvironmentOptions) {
53+
let outDirIsTemporary = false;
54+
let dir = options.outdir;
3955
if (!dir) {
40-
tempDir = true;
56+
outDirIsTemporary = true;
4157
dir = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'cdk.out'));
4258
}
43-
4459
const lock = await new RWLock(dir).acquireWrite();
45-
return new ExecutionEnvironment(services, dir, tempDir, lock);
60+
61+
const opts = {
62+
outdir: dir,
63+
resolveDefaultAppEnv: options.resolveDefaultAppEnv,
64+
};
65+
66+
return new ExecutionEnvironment(services, opts, {
67+
lock,
68+
outDirIsTemporary,
69+
});
70+
}
71+
72+
/**
73+
* Should the outdir be disposed of.
74+
*/
75+
public get shouldDisposeOutDir(): boolean {
76+
return this.shouldClean;
4677
}
4778

79+
/**
80+
* The directory the cloud assembly will be written to.
81+
*/
82+
public readonly outdir: string;
83+
84+
private readonly options: Required<ExecutionEnvironmentOptions>;
4885
private readonly ioHelper: IoHelper;
4986
private readonly sdkProvider: SdkProvider;
5087
private readonly debugFn: (msg: string) => Promise<void>;
@@ -53,21 +90,25 @@ export class ExecutionEnvironment implements AsyncDisposable {
5390

5491
private constructor(
5592
services: ToolkitServices,
56-
public readonly outdir: string,
57-
public readonly outDirIsTemporary: boolean,
58-
lock: IWriteLock,
93+
options: Required<ExecutionEnvironmentOptions>,
94+
{ lock, outDirIsTemporary }: {
95+
readonly outDirIsTemporary: boolean;
96+
readonly lock: IWriteLock;
97+
},
5998
) {
6099
this.ioHelper = services.ioHelper;
61100
this.sdkProvider = services.sdkProvider;
62101
this.debugFn = (msg: string) => this.ioHelper.defaults.debug(msg);
63102
this.lock = lock;
64103
this.shouldClean = outDirIsTemporary;
104+
this.outdir = options.outdir;
105+
this.options = options;
65106
}
66107

67108
public async [Symbol.asyncDispose]() {
68109
await this.lock?.release();
69110

70-
if (this.shouldClean) {
111+
if (this.shouldDisposeOutDir) {
71112
await fs.rm(this.outdir, { recursive: true, force: true });
72113
}
73114
}
@@ -136,7 +177,7 @@ export class ExecutionEnvironment implements AsyncDisposable {
136177
*/
137178
public async defaultEnvVars(): Promise<Env> {
138179
const debugFn = (msg: string) => this.ioHelper.notify(IO.CDK_ASSEMBLY_I0010.msg(msg));
139-
const env = await prepareDefaultEnvironment(this.sdkProvider, debugFn);
180+
const env = this.options.resolveDefaultAppEnv ? await prepareDefaultEnvironment(this.sdkProvider, debugFn) : {};
140181

141182
env[cxapi.OUTDIR_ENV] = this.outdir;
142183
await debugFn(format('outdir:', this.outdir));

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,18 @@ export interface AssemblySourceProps {
118118
* @default - `true` if `outdir` is not given, `false` otherwise
119119
*/
120120
readonly disposeOutdir?: boolean;
121+
122+
/**
123+
* Resolve the current default environment an provide as environment variables to the app.
124+
*
125+
* This will make a (cached) call to STS to resolve the current account using
126+
* base credentials. The behavior is not always desirable and can add
127+
* unnecessary delays, e.g. when an app specifies an environment explicitly
128+
* or when local actions are be performed without internet access.
129+
*
130+
* @default true
131+
*/
132+
readonly resolveDefaultEnvironment?: boolean;
121133
}
122134

123135
/**
@@ -324,7 +336,10 @@ export abstract class CloudAssemblySourceBuilder {
324336
return new ContextAwareCloudAssemblySource(
325337
{
326338
produce: async () => {
327-
await using execution = await ExecutionEnvironment.create(services, { outdir });
339+
await using execution = await ExecutionEnvironment.create(services, {
340+
outdir,
341+
resolveDefaultAppEnv: props.resolveDefaultEnvironment ?? true,
342+
});
328343

329344
const synthParams = parametersFromSynthOptions(props.synthOptions);
330345

@@ -368,7 +383,7 @@ export abstract class CloudAssemblySourceBuilder {
368383
: await assemblyFromDirectory(assembly.directory, services.ioHelper, props.loadAssemblyOptions);
369384

370385
const success = await execution.markSuccessful();
371-
const deleteOnDispose = props.disposeOutdir ?? execution.outDirIsTemporary;
386+
const deleteOnDispose = props.disposeOutdir ?? execution.shouldDisposeOutDir;
372387
return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose });
373388
},
374389
},
@@ -462,19 +477,16 @@ export abstract class CloudAssemblySourceBuilder {
462477
return new ContextAwareCloudAssemblySource(
463478
{
464479
produce: async () => {
465-
// @todo build
466-
// const build = this.props.configuration.settings.get(['build']);
467-
// if (build) {
468-
// await execInChildProcess(build, { cwd: props.workingDirectory });
469-
// }
470-
471480
try {
472481
fs.mkdirpSync(outdir);
473482
} catch (e: any) {
474483
throw new ToolkitError(`Could not create output directory at '${outdir}' (${e.message}).`);
475484
}
476485

477-
await using execution = await ExecutionEnvironment.create(services, { outdir });
486+
await using execution = await ExecutionEnvironment.create(services, {
487+
outdir,
488+
resolveDefaultAppEnv: props.resolveDefaultEnvironment ?? true,
489+
});
478490

479491
const commandLine = await execution.guessExecutable(app);
480492

@@ -521,7 +533,7 @@ export abstract class CloudAssemblySourceBuilder {
521533
const asm = await assemblyFromDirectory(outdir, services.ioHelper, props.loadAssemblyOptions);
522534

523535
const success = await execution.markSuccessful();
524-
const deleteOnDispose = props.disposeOutdir ?? execution.outDirIsTemporary;
536+
const deleteOnDispose = props.disposeOutdir ?? execution.shouldDisposeOutDir;
525537
return new ReadableCloudAssembly(asm, success.readLock, { deleteOnDispose });
526538
},
527539
},

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as path from 'path';
22
import { App } from 'aws-cdk-lib/core';
33
import * as fs from 'fs-extra';
44
import { MemoryContext } from '../../../lib';
5+
import * as environment from '../../../lib/api/cloud-assembly/environment';
56
import { RWLock } from '../../../lib/api/rwlock';
67
import * as contextproviders from '../../../lib/context-providers';
78
import { Toolkit } from '../../../lib/toolkit/toolkit';
@@ -157,6 +158,39 @@ describe('fromAssemblyBuilder', () => {
157158
expect(await (lock! as any)._currentWriter()).toBeUndefined();
158159
expect(await (lock! as any)._currentReaders()).toEqual([]);
159160
});
161+
162+
describe('resolveDefaultEnvironment', () => {
163+
let prepareDefaultEnvironmentSpy: jest.SpyInstance;
164+
beforeEach(() => {
165+
// Mock the prepareDefaultEnvironment function to avoid actual STS calls
166+
prepareDefaultEnvironmentSpy = jest.spyOn(environment, 'prepareDefaultEnvironment')
167+
.mockImplementation(async () => {
168+
return {
169+
CDK_DEFAULT_ACCOUNT: '123456789012',
170+
CDK_DEFAULT_REGION: 'us-east-1',
171+
};
172+
});
173+
});
174+
175+
afterEach(() => {
176+
prepareDefaultEnvironmentSpy.mockRestore();
177+
});
178+
179+
test.each([[true, 1], [false, 0], [undefined, 1]])('respects resolveDefaultEnvironment=%s', async (resolveDefaultEnvironment, callCount) => {
180+
// WHEN
181+
const cx = await toolkit.fromAssemblyBuilder(async () => {
182+
const app = new App();
183+
return app.synth();
184+
}, {
185+
resolveDefaultEnvironment,
186+
});
187+
188+
await using _ = await cx.produce();
189+
190+
// THEN
191+
expect(prepareDefaultEnvironmentSpy).toHaveBeenCalledTimes(callCount);
192+
});
193+
});
160194
});
161195

162196
describe('fromCdkApp', () => {
@@ -290,6 +324,45 @@ describe('fromCdkApp', () => {
290324
expect(await (lock! as any)._currentWriter()).toBeUndefined();
291325
expect(await (lock! as any)._currentReaders()).toEqual([]);
292326
});
327+
328+
describe('resolveDefaultEnvironment', () => {
329+
let prepareDefaultEnvironmentSpy: jest.SpyInstance;
330+
beforeEach(() => {
331+
// Mock the prepareDefaultEnvironment function to avoid actual STS calls
332+
prepareDefaultEnvironmentSpy = jest.spyOn(environment, 'prepareDefaultEnvironment')
333+
.mockImplementation(async () => {
334+
return {
335+
CDK_DEFAULT_ACCOUNT: '123456789012',
336+
CDK_DEFAULT_REGION: 'us-east-1',
337+
};
338+
});
339+
});
340+
341+
afterEach(() => {
342+
prepareDefaultEnvironmentSpy.mockRestore();
343+
});
344+
345+
test.each([[true, 1], [false, 0], [undefined, 1]])('respects resolveDefaultEnvironment=%s', async (resolveDefaultEnvironment, callCount) => {
346+
// GIVEN
347+
await using synthDir = autoCleanOutDir();
348+
349+
// WHEN
350+
const cx = await toolkit.fromCdkApp('node -e "console.log(JSON.stringify(process.env))"', {
351+
workingDirectory: process.cwd(),
352+
outdir: synthDir.dir,
353+
resolveDefaultEnvironment,
354+
});
355+
356+
try {
357+
await using _ = await cx.produce();
358+
} catch {
359+
// we expect this app to throw
360+
}
361+
362+
// THEN
363+
expect(prepareDefaultEnvironmentSpy).toHaveBeenCalledTimes(callCount);
364+
});
365+
});
293366
});
294367

295368
describe('fromAssemblyDirectory', () => {

0 commit comments

Comments
 (0)