Skip to content

Commit 24a3953

Browse files
committed
fix(integ-runner): region is not passed to CDK App
When using the `toolkit-lib` deployment engine, the selected region was not being passed to the CDK app. In the old engine, we could set the `AWS_REGION` environment variable and the CDK CLI would respect that, but the toolkit lib was passing that environment variable directly to the CDK app which was not respecting that. Fix that by threading the region through as an explicit parameter and adding an override parameter to the `AwsCliCompatible` base credentials: we don't necessarily want to read this only from the INI files. Additionally: add a validation to make sure that the CDK app respects the region we are requesting. CDK integ tests should either not pass an environment, or read `$CDK_DEFAULT_REGION` to determine the current region. It was too easy to just override the region with a hardcoded one, but that breaks the integ runner's attempts to isolate tests across regions. Guard against that by validating. Also in this PR: disable the update workflow. I've lost 2 hours debugging why my deployment was failing with nonsenical values and the answer was: the checked-in snapshot was nonsensical, and the update workflow will first deploy the checked-in snapshot and only then deploy the current snapshot. Good idea, bad execution: if existing snapshots are not region-agnostic they are guaranteed to fail. We should probably resynthesize them from the previous git commit instead. Whatever it is, what we do right now doesn't work so I'm disabling it.
1 parent 29a6178 commit 24a3953

File tree

10 files changed

+89
-20
lines changed

10 files changed

+89
-20
lines changed
Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
#!/usr/bin/env node
2-
const { cli } = require('../lib');
3-
cli();
2+
require('../lib/run-cli');
43

packages/@aws-cdk/integ-runner/lib/cli.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export function parseCliArgs(args: string[] = []) {
4040
.option('strict', { type: 'boolean', default: false, desc: 'Fail if any specified tests are not found' })
4141
.options('from-file', { type: 'string', desc: 'Read TEST names from a file (one TEST per line)' })
4242
.option('inspect-failures', { type: 'boolean', desc: 'Keep the integ test cloud assembly if a failure occurs for inspection', default: false })
43-
.option('disable-update-workflow', { type: 'boolean', default: false, desc: 'If this is "true" then the stack update workflow will be disabled' })
43+
.option('disable-update-workflow', { type: 'boolean', default: true, desc: 'DEPRECATED. Update workflow has been temporarily disabled until we can make it work correctly.' })
4444
.option('language', {
4545
alias: 'l',
4646
default: ['javascript', 'typescript', 'python', 'go'],
@@ -100,7 +100,7 @@ export function parseCliArgs(args: string[] = []) {
100100
clean: argv.clean as boolean,
101101
force: argv.force as boolean,
102102
dryRun: argv['dry-run'] as boolean,
103-
disableUpdateWorkflow: argv['disable-update-workflow'] as boolean,
103+
disableUpdateWorkflow: true,
104104
language: arrayFromYargs(argv.language),
105105
watch: argv.watch as boolean,
106106
strict: argv.strict as boolean,
@@ -186,7 +186,7 @@ async function run(options: ReturnType<typeof parseCliArgs>, { engine }: EngineO
186186
clean: options.clean,
187187
dryRun: options.dryRun,
188188
verbosity: options.verbosity,
189-
updateWorkflow: !options.disableUpdateWorkflow,
189+
updateWorkflow: false,
190190
watch: options.watch,
191191
});
192192
testsSucceeded = success;

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

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import * as path from 'node:path';
22
import type { DeployOptions, ICdk, ListOptions, SynthFastOptions, SynthOptions, WatchEvents } from '@aws-cdk/cdk-cli-wrapper';
33
import type { DefaultCdkOptions, DestroyOptions } from '@aws-cdk/cloud-assembly-schema/lib/integ-tests';
4-
import type { DeploymentMethod, ICloudAssemblySource, IIoHost, IoMessage, IoRequest, NonInteractiveIoHostProps, StackSelector } from '@aws-cdk/toolkit-lib';
5-
import { ExpandStackSelection, MemoryContext, NonInteractiveIoHost, StackSelectionStrategy, Toolkit } from '@aws-cdk/toolkit-lib';
4+
import { UNKNOWN_REGION } from '@aws-cdk/cx-api';
5+
import type { DeploymentMethod, ICloudAssemblySource, IIoHost, IoMessage, IoRequest, IReadableCloudAssembly, NonInteractiveIoHostProps, StackSelector } from '@aws-cdk/toolkit-lib';
6+
import { BaseCredentials, ExpandStackSelection, MemoryContext, NonInteractiveIoHost, StackSelectionStrategy, Toolkit } from '@aws-cdk/toolkit-lib';
67
import * as chalk from 'chalk';
78
import * as fs from 'fs-extra';
89

@@ -27,6 +28,11 @@ export interface ToolkitLibEngineOptions {
2728
* @default false
2829
*/
2930
readonly showOutput?: boolean;
31+
32+
/**
33+
* The region the CDK app should synthesize itself for
34+
*/
35+
readonly region: string;
3036
}
3137

3238
/**
@@ -42,7 +48,12 @@ export class ToolkitLibRunnerEngine implements ICdk {
4248
this.showOutput = options.showOutput ?? false;
4349

4450
this.toolkit = new Toolkit({
45-
ioHost: this.showOutput? new IntegRunnerIoHost() : new NoopIoHost(),
51+
ioHost: this.showOutput ? new IntegRunnerIoHost() : new NoopIoHost(),
52+
sdkConfig: {
53+
baseCredentials: BaseCredentials.awsCliCompatible({
54+
defaultRegion: options.region,
55+
}),
56+
},
4657
// @TODO - these options are currently available on the action calls
4758
// but toolkit-lib needs them at the constructor level.
4859
// Need to decide what to do with them.
@@ -73,6 +84,7 @@ export class ToolkitLibRunnerEngine implements ICdk {
7384
stacks: this.stackSelector(options),
7485
validateStacks: options.validation,
7586
});
87+
await this.validateRegion(lock);
7688
await lock.dispose();
7789
}
7890

@@ -100,6 +112,7 @@ export class ToolkitLibRunnerEngine implements ICdk {
100112
try {
101113
// @TODO - use produce to mimic the current behavior more closely
102114
const lock = await cx.produce();
115+
await this.validateRegion(lock);
103116
await lock.dispose();
104117
// We should fix this once we have stabilized toolkit-lib as engine.
105118
// What we really should do is this:
@@ -217,7 +230,6 @@ export class ToolkitLibRunnerEngine implements ICdk {
217230
workingDirectory: this.options.workingDirectory,
218231
outdir,
219232
lookups: options.lookups,
220-
resolveDefaultEnvironment: false, // not part of the integ-runner contract
221233
contextStore: new MemoryContext(options.context),
222234
env: this.options.env,
223235
synthOptions: {
@@ -256,6 +268,37 @@ export class ToolkitLibRunnerEngine implements ICdk {
256268
method: options.deploymentMethod ?? 'change-set',
257269
};
258270
}
271+
272+
/**
273+
* Check that the regions for the stacks in the CloudAssembly match the regions requested on the engine
274+
*
275+
* This prevents misconfiguration of the integ test app. People tend to put:
276+
*
277+
* ```ts
278+
* new Stack(app, 'Stack', {
279+
* env: {
280+
* region: 'some-region-that-suits-me',
281+
* }
282+
* });
283+
* ```
284+
*
285+
* Into their integ tests, instead of:
286+
*
287+
* ```ts
288+
* {
289+
* region: process.env.CDK_DEFAULT_REGION,
290+
* }
291+
* ```
292+
*
293+
* This catches that misconfiguration.
294+
*/
295+
private async validateRegion(asm: IReadableCloudAssembly) {
296+
for (const stack of asm.cloudAssembly.stacksRecursively) {
297+
if (stack.environment.region !== this.options.region && stack.environment.region !== UNKNOWN_REGION) {
298+
throw new Error(`Stack ${stack.displayName} synthesizes for region ${stack.environment.region}, even though ${this.options.region} was requested. Please configure \`{ env: { region: process.env.CDK_DEFAULT_REGION, account: process.env.CDK_DEFAULT_ACCOUNT } }\`, or use no env at all. Do not hardcode a region or account.`);
299+
}
300+
}
301+
}
259302
}
260303

261304
/**
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import { cli } from './cli';
2+
cli();

packages/@aws-cdk/integ-runner/lib/runner/engine.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,8 @@ export function makeEngine(options: IntegRunnerOptions): ICdk {
1818
return new ToolkitLibRunnerEngine({
1919
workingDirectory: options.test.directory,
2020
showOutput: options.showOutput,
21-
env: {
22-
...options.env,
23-
},
21+
env: options.env,
22+
region: options.region,
2423
});
2524
case 'cli-wrapper':
2625
default:
@@ -29,6 +28,8 @@ export function makeEngine(options: IntegRunnerOptions): ICdk {
2928
showOutput: options.showOutput,
3029
env: {
3130
...options.env,
31+
// The CDK CLI will interpret this and use it usefully
32+
AWS_REGION: options.region,
3233
},
3334
});
3435
}

packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ export interface IntegRunnerOptions extends EngineOptions {
2626
*/
2727
readonly test: IntegTest;
2828

29+
/**
30+
* The region where the test should be deployed
31+
*/
32+
readonly region: string;
33+
2934
/**
3035
* The AWS profile to use when invoking the CDK CLI
3136
*

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@ interface SnapshotAssembly {
3434
* the validation of the integration test snapshots
3535
*/
3636
export class IntegSnapshotRunner extends IntegRunner {
37-
constructor(options: IntegRunnerOptions) {
38-
super(options);
37+
constructor(options: Omit<IntegRunnerOptions, 'region'>) {
38+
super({
39+
...options,
40+
region: 'unused',
41+
});
3942
}
4043

4144
/**

packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ export async function integTestWorker(request: IntegTestBatchRequest): Promise<I
3131
engine: request.engine,
3232
test,
3333
profile: request.profile,
34+
region: request.region,
3435
env: {
35-
AWS_REGION: request.region,
3636
CDK_DOCKER: process.env.CDK_DOCKER ?? 'docker',
3737
},
3838
showOutput: verbosity >= 2,
@@ -99,8 +99,8 @@ export async function watchTestWorker(options: IntegWatchOptions): Promise<void>
9999
engine: options.engine,
100100
test,
101101
profile: options.profile,
102+
region: options.region,
102103
env: {
103-
AWS_REGION: options.region,
104104
CDK_DOCKER: process.env.CDK_DOCKER ?? 'docker',
105105
},
106106
showOutput: verbosity >= 2,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,9 @@ describe('ToolkitLibRunnerEngine', () => {
223223
showOutput: true,
224224
});
225225

226-
expect(MockedToolkit).toHaveBeenCalledWith({
226+
expect(MockedToolkit).toHaveBeenCalledWith(expect.objectContaining({
227227
ioHost: expect.any(Object),
228-
});
228+
}));
229229
});
230230

231231
it('should throw error when no app is provided', async () => {

packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/base-credentials.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,14 @@ export class BaseCredentials {
9292
*/
9393
public static awsCliCompatible(options: AwsCliCompatibleOptions = {}): IBaseCredentialsProvider {
9494
return new class implements IBaseCredentialsProvider {
95-
public sdkBaseConfig(ioHost: IActionAwareIoHost, clientConfig: SdkBaseClientConfig) {
95+
public async sdkBaseConfig(ioHost: IActionAwareIoHost, clientConfig: SdkBaseClientConfig) {
9696
const ioHelper = IoHelper.fromActionAwareIoHost(ioHost);
9797
const awsCli = new AwsCliCompatible(ioHelper, clientConfig.requestHandler ?? {}, new IoHostSdkLogger(ioHelper));
98-
return awsCli.baseConfig(options.profile);
98+
99+
const ret = await awsCli.baseConfig(options.profile);
100+
return options.defaultRegion
101+
? { ...ret, defaultRegion: options.defaultRegion }
102+
: ret;
99103
}
100104

101105
public toString() {
@@ -140,6 +144,18 @@ export interface AwsCliCompatibleOptions {
140144
* @default - Use environment variable if set.
141145
*/
142146
readonly profile?: string;
147+
148+
/**
149+
* Use a different default region than the one in the profile
150+
*
151+
* If not supplied the environment variable AWS_REGION will be used, or
152+
* whatever region is set in the indicated profile in `~/.aws/config`.
153+
* If no region is set in the profile the region in `[default]` will
154+
* be used.
155+
*
156+
* @default - Use region from `~/.aws/config`.
157+
*/
158+
readonly defaultRegion?: string;
143159
}
144160

145161
export interface CustomBaseCredentialsOption {

0 commit comments

Comments
 (0)