Skip to content
This repository was archived by the owner on Apr 22, 2025. It is now read-only.

Commit eaf5eb7

Browse files
authored
fix(cli-integ): some tests are failing when executing the suite with atmosphere (#72)
Inline code comments provide the necessary justifications. Also in this PR: Actually enable atmosphere so subsequent (not this one) PRs start using it. **Draft until we get atmosphere prod fully launched with enough environments**
1 parent f84826f commit eaf5eb7

File tree

9 files changed

+109
-27
lines changed

9 files changed

+109
-27
lines changed

.github/workflows/integ.yml

Lines changed: 5 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.projenrc.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,11 @@ new CdkCliIntegTestsWorkflow(repo, {
240240
testEnvironment: TEST_ENVIRONMENT,
241241
testRunsOn: TEST_RUNNER,
242242
localPackages: [cliInteg.name],
243+
enableAtmosphere: {
244+
oidcRoleArn: '${{ vars.CDK_ATMOSPHERE_PROD_OIDC_ROLE }}',
245+
endpoint: '${{ vars.CDK_ATMOSPHERE_PROD_ENDPOINT }}',
246+
pool: '${{ vars.CDK_INTEG_ATMOSPHERE_POOL }}'
247+
}
243248
});
244249

245250
repo.synth();

package.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/cli-integ/.projen/tasks.json

Lines changed: 5 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,20 @@ export class AwsClients {
8080
return (await stsClient.send(new GetCallerIdentityCommand({}))).Account!;
8181
}
8282

83+
/**
84+
* If the clients already has an established identity (via atmosphere for example),
85+
* return an environment variable map activating it.
86+
*
87+
* Otherwise, returns undefined.
88+
*/
89+
public identityEnv(): Record<string, string> | undefined {
90+
return this.identity ? {
91+
AWS_ACCESS_KEY_ID: this.identity.accessKeyId,
92+
AWS_SECRET_ACCESS_KEY: this.identity.secretAccessKey,
93+
AWS_SESSION_TOKEN: this.identity.sessionToken!,
94+
} : undefined;
95+
}
96+
8397
/**
8498
* Resolve the current identity or identity provider to credentials
8599
*/

packages/@aws-cdk-testing/cli-integ/lib/shell.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,13 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom
6060
if (interaction) {
6161

6262
if (interaction.prompt.test(lastLine.get())) {
63-
// subprocess expects a user input now
64-
child.writeStdin(interaction.input + (interaction.end ?? os.EOL));
65-
remainingInteractions.shift();
63+
// subprocess expects a user input now.
64+
// we have to write the input AFTER the child has started
65+
// reading, so we do this with a small delay.
66+
setTimeout(() => {
67+
child.writeStdin(interaction.input + (interaction.end ?? os.EOL));
68+
remainingInteractions.shift();
69+
}, 500);
6670
}
6771

6872
}
@@ -218,6 +222,12 @@ export class ShellHelper {
218222
outputs: [this._output],
219223
cwd: this._cwd,
220224
...options,
225+
modEnv: {
226+
// give every shell its own docker config directory
227+
// so that parallel runs don't interfere with each other.
228+
DOCKER_CONFIG: path.join(this._cwd, '.docker'),
229+
...options.modEnv,
230+
},
221231
});
222232
}
223233
}

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as fs from 'fs';
44
import * as os from 'os';
55
import * as path from 'path';
66
import { DescribeStacksCommand, Stack } from '@aws-sdk/client-cloudformation';
7-
import { outputFromStack, AwsClients } from './aws';
7+
import { outputFromStack, AwsClients, sleep } from './aws';
88
import { TestContext } from './integ-test';
99
import { findYarnPackages } from './package-sources/repo-source';
1010
import { IPackageSource } from './package-sources/source';
@@ -525,11 +525,7 @@ export class TestFixture extends ShellHelper {
525525
public cdkShellEnv() {
526526
// if tests are using an explicit aws identity already (i.e creds)
527527
// force every cdk command to use the same identity.
528-
const awsCreds: Record<string, string> = this.aws.identity ? {
529-
AWS_ACCESS_KEY_ID: this.aws.identity.accessKeyId,
530-
AWS_SECRET_ACCESS_KEY: this.aws.identity.secretAccessKey,
531-
AWS_SESSION_TOKEN: this.aws.identity.sessionToken!,
532-
} : {};
528+
const awsCreds = this.aws.identityEnv() ?? {};
533529

534530
return {
535531
AWS_REGION: this.aws.region,
@@ -693,17 +689,61 @@ export async function ensureBootstrapped(fixture: TestFixture) {
693689
const envSpecifier = `aws://${await fixture.aws.account()}/${fixture.aws.region}`;
694690
if (ALREADY_BOOTSTRAPPED_IN_THIS_RUN.has(envSpecifier)) { return; }
695691

696-
await fixture.cdk(['bootstrap', envSpecifier], {
692+
if (atmosphereEnabled()) {
693+
// when atmosphere is enabled, each test starts with an empty environment
694+
// and needs to deploy the bootstrap stack. in case environments are recylced too quickly,
695+
// cloudformation may think the bootstrap bucket still exists even though it doesnt (because of s3 eventual consistency).
696+
// so we retry on the specific error for a while.
697+
await bootstrapWithRetryOnBucketExists(envSpecifier, fixture);
698+
} else {
699+
await doBootstrap(envSpecifier, fixture, false);
700+
}
701+
702+
// when using the atmosphere service, every test needs to bootstrap
703+
// its own environment.
704+
if (!atmosphereEnabled()) {
705+
ALREADY_BOOTSTRAPPED_IN_THIS_RUN.add(envSpecifier);
706+
}
707+
}
708+
709+
async function doBootstrap(envSpecifier: string, fixture: TestFixture, allowErrExit: boolean) {
710+
return fixture.cdk(['bootstrap', envSpecifier], {
697711
modEnv: {
698712
// Even for v1, use new bootstrap
699713
CDK_NEW_BOOTSTRAP: '1',
714+
// when allowing error exit, we probably want to inspect
715+
// and compare output, which is better done without color characters.
716+
...(allowErrExit ? { FORCE_COLOR: '0' } : {}),
700717
},
718+
allowErrExit,
701719
});
720+
}
702721

703-
// when using the atmosphere service, every test needs to bootstrap
704-
// its own environment.
705-
if (!atmosphereEnabled()) {
706-
ALREADY_BOOTSTRAPPED_IN_THIS_RUN.add(envSpecifier);
722+
async function bootstrapWithRetryOnBucketExists(envSpecifier: string, fixture: TestFixture) {
723+
724+
const account = await fixture.aws.account();
725+
const retryAfterSeconds = 30;
726+
const bootstrapBucket = `cdk-hnb659fds-assets-${account}-${fixture.aws.region}`;
727+
728+
// s3 says that a bucket deletion can take up to an hour to be fully visible.
729+
// empirically we see that a few minutes is enough though. lets give 10 to be on the safe(r) side.
730+
const timeoutMinutes = 10;
731+
732+
const timeoutDate = new Date(Date.now() + timeoutMinutes * 60 * 1000)
733+
while (true) {
734+
const out = await doBootstrap(envSpecifier, fixture, true);
735+
if (out.includes(`Environment ${envSpecifier} bootstrapped`)) {
736+
break;
737+
}
738+
if (out.includes(`${bootstrapBucket} already exists`)) {
739+
// might be an s3 eventualy consistency issue due to recycled environments.
740+
if (Date.now() < timeoutDate.getTime()) {
741+
fixture.log(`Bootstrap of ${envSpecifier} failed due to bucket existence check. Retrying in ${retryAfterSeconds} seconds...`);
742+
await sleep(retryAfterSeconds * 1000)
743+
continue;
744+
}
745+
}
746+
throw new Error(`Failed bootstrapping ${envSpecifier}`);
707747
}
708748
}
709749

packages/@aws-cdk-testing/cli-integ/tests/tool-integrations/amplify.integtest.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,12 @@ integTest('amplify integration', withToolContext(async (context) => {
2828
await shell.shell(['npm', 'create', '-y', 'amplify']);
2929
await shell.shell(['npx', 'ampx', 'configure', 'telemetry', 'disable']);
3030

31+
const awsCreds = context.aws.identityEnv();
32+
3133
await shell.shell(['npx', 'ampx', 'sandbox', '--once'], {
3234
modEnv: {
3335
AWS_REGION: context.aws.region,
36+
...awsCreds
3437
},
3538
});
3639
try {
@@ -42,6 +45,7 @@ integTest('amplify integration', withToolContext(async (context) => {
4245
await shell.shell(['npx', 'ampx', 'sandbox', 'delete', '--yes'], {
4346
modEnv: {
4447
AWS_REGION: context.aws.region,
48+
...awsCreds
4549
},
4650
});
4751
}

yarn.lock

Lines changed: 11 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)