Skip to content

Commit 7ca7a35

Browse files
committed
chore: improve stability of tests by copying SSM Parameter Value
`{Fn::GetAtt}` on an `AWS::SSM::Parameter` is eventually consistent. If an SSM Parameter was just created in a stack deployment, the `{Fn::GetAtt}` may sometimes fail with a "Parameter not found" error. This causes ~2 canary failures per week (about 1% failure rate). We don't actually need to `{Fn::GetAtt}` the value as we know what it is (it is static, after all). Copy the same literal value into both places to reduce the canary failure rate by a little bit, and add a test to make sure the values don't accidentally drift apart. Internal reference D259904064.
1 parent 7ad2e9c commit 7ca7a35

File tree

3 files changed

+29
-19
lines changed

3 files changed

+29
-19
lines changed

packages/@aws-cdk/toolkit-lib/test/api/bootstrap/external-id.test.ts renamed to packages/@aws-cdk/toolkit-lib/test/api/bootstrap/bootstrap-template.test.ts

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,25 @@ import { Bootstrapper } from '../../../lib/api/bootstrap';
22
import type { IIoHost } from '../../../lib/api/io';
33
import { asIoHelper } from '../../../lib/api/io/private';
44

5-
describe('ExternalId Protection Integration Test', () => {
6-
let ioHost: IIoHost;
7-
let ioHelper: any;
5+
let ioHost: IIoHost;
6+
let ioHelper: any;
7+
let template: any;
88

9-
beforeEach(() => {
10-
ioHost = {
11-
notify: jest.fn(),
12-
requestResponse: jest.fn(),
13-
};
14-
ioHelper = asIoHelper(ioHost, 'bootstrap');
15-
});
16-
17-
test('bootstrap template denies AssumeRole with ExternalId by default', async () => {
18-
// GIVEN
19-
const bootstrapper = new Bootstrapper({ source: 'default' }, ioHelper);
9+
beforeEach(async () => {
10+
ioHost = {
11+
notify: jest.fn(),
12+
requestResponse: jest.fn(),
13+
};
14+
ioHelper = asIoHelper(ioHost, 'bootstrap');
2015

21-
// WHEN
22-
const template = await (bootstrapper as any).loadTemplate();
16+
// GIVEN
17+
const bootstrapper = new Bootstrapper({ source: 'default' }, ioHelper);
18+
// WHEN
19+
template = await (bootstrapper as any).loadTemplate();
20+
});
2321

22+
describe('bootstrap template', () => {
23+
test('denies AssumeRole with ExternalId by default', async () => {
2424
// THEN
2525
// Verify the parameter exists
2626
expect(template.Parameters.DenyExternalId).toMatchObject({
@@ -70,4 +70,8 @@ describe('ExternalId Protection Integration Test', () => {
7070
expect(stmt.Condition).toBeUndefined();
7171
}
7272
});
73+
74+
test('has the same values for BootstrapVersion Parameter and Output', async () => {
75+
expect(template.Outputs.BootstrapVersion.Value).toEqual(template.Resources.CdkBootstrapVersion.Properties.Value);
76+
});
7377
});

packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -844,5 +844,8 @@ Outputs:
844844
BootstrapVersion:
845845
Description: The version of the bootstrap resources that are currently mastered
846846
in this stack
847-
Value:
848-
Fn::GetAtt: [CdkBootstrapVersion, Value]
847+
# This value is purposely duplicated here from the AWS::SSM::Parameter value we define above.
848+
# {Fn::GetAtt} on an SSM Parameter is eventually consistent, and can fail with "parameter
849+
# doesn't exist" even after just having been created. To reduce our deploy failure rate, we
850+
# duplicate the value here and use a build-time test to ensure the two values are the same.
851+
Value: '29'

packages/aws-cdk/test/commands/bootstrap.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
import * as fs from 'fs/promises';
2+
import * as path from 'path';
13
import { Bootstrapper } from '../../lib/api/bootstrap';
24
import { exec } from '../../lib/cli/cli';
5+
import { deserializeStructure } from '@aws-cdk/toolkit-lib/lib/util';
36

47
beforeEach(() => {
58
jest.clearAllMocks();
@@ -87,4 +90,4 @@ describe('cdk bootstrap --show-template', () => {
8790
await exec(['bootstrap', '--show-template']);
8891
expect(stdoutSpy).toHaveBeenCalledWith(expect.stringContaining('ShouldDenyExternalId'));
8992
});
90-
});
93+
});

0 commit comments

Comments
 (0)