Skip to content

Commit 61240bd

Browse files
mergify[bot]mergify[bot]
authored
Merge branch 'master' into corymhall/integ-runner/gaps
2 parents 7439fa9 + 2fbea60 commit 61240bd

14 files changed

Lines changed: 200 additions & 91 deletions

File tree

‎.github/workflows/close-stale-prs.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ on:
33
# Cron format: min hr day month dow
44
- cron: "0 0 * * *"
55
jobs:
6-
rix0rrr/close-stale-prs:
6+
close-stale-prs:
77
permissions:
88
pull-requests: write
99
runs-on: ubuntu-latest

‎deprecated_apis.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
@aws-cdk/core.DefaultStackSynthesizerProps#fileAssetKeyArnExportName
2828
@aws-cdk/core.DockerImageAssetSource#repositoryName
2929
@aws-cdk/core.Duration#toISOString
30-
@aws-cdk/core.FileAssetLocation#kmsKeyArn
3130
@aws-cdk/core.FileAssetLocation#s3Url
3231
@aws-cdk/core.ITemplateOptions#transform
3332
@aws-cdk/core.Lazy#anyValue

‎package.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@
2121
"fs-extra": "^9.1.0",
2222
"graceful-fs": "^4.2.10",
2323
"jest-junit": "^13.1.0",
24-
"jsii-diff": "^1.56.0",
25-
"jsii-pacmak": "^1.56.0",
26-
"jsii-reflect": "^1.56.0",
27-
"jsii-rosetta": "^1.56.0",
24+
"jsii-diff": "^1.57.0",
25+
"jsii-pacmak": "^1.57.0",
26+
"jsii-reflect": "^1.57.0",
27+
"jsii-rosetta": "^1.57.0",
2828
"lerna": "^4.0.0",
2929
"patch-package": "^6.4.7",
3030
"semver": "^6.3.0",

‎packages/@aws-cdk/cfnspec/spec-source/cfn-docs/cfn-docs.json‎

Lines changed: 3 additions & 3 deletions
Large diffs are not rendered by default.

‎packages/@aws-cdk/core/lib/assets.ts‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -274,21 +274,14 @@ export interface FileAssetLocation {
274274
readonly s3ObjectUrl: string;
275275

276276
/**
277-
* The ARN of the KMS key used to encrypt the file asset bucket, if any
277+
* The ARN of the KMS key used to encrypt the file asset bucket, if any.
278278
*
279-
* If so, the consuming role should be given "kms:Decrypt" permissions in its
280-
* identity policy.
279+
* The CDK bootstrap stack comes with a key policy that does not require
280+
* setting this property, so you only need to set this property if you
281+
* have customized the bootstrap stack to require it.
281282
*
282-
* It's the responsibility of they key's creator to make sure that all
283-
* consumers that the key's key policy is configured such that the key can be used
284-
* by all consumers that need it.
285-
*
286-
* The default bootstrap stack provisioned by the CDK CLI ensures this, and
287-
* can be used as an example for how to configure the key properly.
288-
*
289-
* @default - Asset bucket is not encrypted
290-
* @deprecated Since bootstrap bucket v4, the key policy properly allows use of the
291-
* key via the bucket and no additional parameters have to be granted anymore.
283+
* @default - Asset bucket is not encrypted, or decryption permissions are
284+
* defined by a Key Policy.
292285
*/
293286
readonly kmsKeyArn?: string;
294287

‎packages/@aws-cdk/integ-tests/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# integ-runner
1+
# integ-tests
22

33
<!--BEGIN STABILITY BANNER-->
44

‎packages/@aws-cdk/integ-tests/test/manifest-writer.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe(IntegManifestWriter, () => {
2121
};
2222

2323
beforeEach(() => {
24-
tmpDir = fs.mkdtempSync(os.tmpdir());
24+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cdk-test'));
2525
});
2626

2727
afterEach(() => {

‎packages/aws-cdk/lib/cli.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,7 @@ async function initCommandLine() {
282282

283283
if (shouldDisplayNotices()) {
284284
void refreshNotices()
285-
.then(_ => debug('Notices refreshed'))
286-
.catch(e => debug(`Notices refresh failed: ${e}`));
285+
.catch(e => debug(`Could not refresh notices: ${e}`));
287286
}
288287

289288
const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({

‎packages/aws-cdk/lib/notices.ts‎

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ export interface NoticeDataSource {
107107
export class WebsiteNoticeDataSource implements NoticeDataSource {
108108
fetch(): Promise<Notice[]> {
109109
const timeout = 3000;
110-
111-
return new Promise((resolve) => {
112-
setTimeout(() => resolve([]), timeout);
110+
return new Promise((resolve, reject) => {
113111
try {
114112
const req = https.get('https://cli.cdk.dev-tools.aws.dev/notices.json',
115113
{ timeout },
@@ -123,29 +121,39 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
123121
res.on('end', () => {
124122
try {
125123
const data = JSON.parse(rawData).notices as Notice[];
124+
if (!data) {
125+
throw new Error("'notices' key is missing");
126+
}
127+
debug('Notices refreshed');
126128
resolve(data ?? []);
127129
} catch (e) {
128-
debug(`Failed to parse notices: ${e}`);
129-
resolve([]);
130+
reject(new Error(`Failed to parse notices: ${e.message}`));
130131
}
131132
});
132133
res.on('error', e => {
133-
debug(`Failed to fetch notices: ${e}`);
134-
resolve([]);
134+
reject(new Error(`Failed to fetch notices: ${e.message}`));
135135
});
136136
} else {
137-
debug(`Failed to fetch notices. Status code: ${res.statusCode}`);
138-
resolve([]);
137+
reject(new Error(`Failed to fetch notices. Status code: ${res.statusCode}`));
139138
}
140139
});
141-
req.on('error', e => {
142-
debug(`Error on request: ${e}`);
143-
resolve([]);
140+
req.on('error', reject);
141+
req.on('timeout', () => {
142+
// The 'timeout' event doesn't stop anything by itself, it just
143+
// notifies that it has been long time since we saw bytes.
144+
// In our case, we want to give up.
145+
req.destroy(new Error('Request timed out'));
144146
});
145-
req.on('timeout', _ => resolve([]));
147+
148+
// It's not like I don't *trust* the 'timeout' event... but I don't trust it.
149+
// Add a backup timer that will destroy the request after all.
150+
// (This is at least necessary to make the tests pass, but that's probably because of 'nock'.
151+
// It's not clear whether users will hit this).
152+
setTimeout(() => {
153+
req.destroy(new Error('Request timed out. You should never see this message; if you do, please let us know at https://github.com/aws/aws-cdk/issues'));
154+
}, timeout + 200);
146155
} catch (e) {
147-
debug(`HTTPS 'get' call threw an error: ${e}`);
148-
resolve([]);
156+
reject(new Error(`HTTPS 'get' call threw an error: ${e.message}`));
149157
}
150158
});
151159
}
@@ -156,7 +164,8 @@ interface CachedNotices {
156164
notices: Notice[],
157165
}
158166

159-
const TIME_TO_LIVE = 60 * 60 * 1000; // 1 hour
167+
const TIME_TO_LIVE_SUCCESS = 60 * 60 * 1000; // 1 hour
168+
const TIME_TO_LIVE_ERROR = 1 * 60 * 1000; // 1 minute
160169

161170
export class CachedDataSource implements NoticeDataSource {
162171
constructor(
@@ -171,17 +180,30 @@ export class CachedDataSource implements NoticeDataSource {
171180
const expiration = cachedData.expiration ?? 0;
172181

173182
if (Date.now() > expiration || this.skipCache) {
174-
const freshData = {
175-
expiration: Date.now() + TIME_TO_LIVE,
176-
notices: await this.dataSource.fetch(),
177-
};
183+
const freshData = await this.fetchInner();
178184
await this.save(freshData);
179185
return freshData.notices;
180186
} else {
187+
debug(`Reading cached notices from ${this.fileName}`);
181188
return data;
182189
}
183190
}
184191

192+
private async fetchInner(): Promise<CachedNotices> {
193+
try {
194+
return {
195+
expiration: Date.now() + TIME_TO_LIVE_SUCCESS,
196+
notices: await this.dataSource.fetch(),
197+
};
198+
} catch (e) {
199+
debug(`Could not refresh notices: ${e}`);
200+
return {
201+
expiration: Date.now() + TIME_TO_LIVE_ERROR,
202+
notices: [],
203+
};
204+
}
205+
}
206+
185207
private async load(): Promise<CachedNotices> {
186208
const defaultValue = {
187209
expiration: 0,

‎packages/aws-cdk/test/notices.test.ts‎

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -191,60 +191,60 @@ describe('cli notices', () => {
191191
expect(result).toEqual([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]);
192192
});
193193

194-
test('returns empty array when the server returns an unexpected status code', async () => {
195-
const result = await mockCall(500, {
194+
test('returns appropriate error when the server returns an unexpected status code', async () => {
195+
const result = mockCall(500, {
196196
notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE],
197197
});
198198

199-
expect(result).toEqual([]);
199+
await expect(result).rejects.toThrow(/500/);
200200
});
201201

202-
test('returns empty array when the server returns an unexpected structure', async () => {
203-
const result = await mockCall(200, {
202+
test('returns appropriate error when the server returns an unexpected structure', async () => {
203+
const result = mockCall(200, {
204204
foo: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE],
205205
});
206206

207-
expect(result).toEqual([]);
207+
await expect(result).rejects.toThrow(/key is missing/);
208208
});
209209

210-
test('returns empty array when the server returns invalid json', async () => {
211-
const result = await mockCall(200, '-09aiskjkj838');
210+
test('returns appropriate error when the server returns invalid json', async () => {
211+
const result = mockCall(200, '-09aiskjkj838');
212212

213-
expect(result).toEqual([]);
213+
await expect(result).rejects.toThrow(/Failed to parse/);
214214
});
215215

216-
test('returns empty array when HTTPS call throws', async () => {
216+
test('returns appropriate error when HTTPS call throws', async () => {
217217
const mockGet = jest.spyOn(https, 'get')
218218
.mockImplementation(() => { throw new Error('No connection'); });
219219

220-
const result = await dataSource.fetch();
220+
const result = dataSource.fetch();
221221

222-
expect(result).toEqual([]);
222+
await expect(result).rejects.toThrow(/No connection/);
223223

224224
mockGet.mockRestore();
225225
});
226226

227-
test('returns empty array when the request has an error', async () => {
227+
test('returns appropriate error when the request has an error', async () => {
228228
nock('https://cli.cdk.dev-tools.aws.dev')
229229
.get('/notices.json')
230230
.replyWithError('DNS resolution failed');
231231

232-
const result = await dataSource.fetch();
232+
const result = dataSource.fetch();
233233

234-
expect(result).toEqual([]);
234+
await expect(result).rejects.toThrow(/DNS resolution failed/);
235235
});
236236

237-
test('returns empty array when the connection stays idle for too long', async () => {
237+
test('returns appropriate error when the connection stays idle for too long', async () => {
238238
nock('https://cli.cdk.dev-tools.aws.dev')
239239
.get('/notices.json')
240240
.delayConnection(3500)
241241
.reply(200, {
242242
notices: [BASIC_NOTICE],
243243
});
244244

245-
const result = await dataSource.fetch();
245+
const result = dataSource.fetch();
246246

247-
expect(result).toEqual([]);
247+
await expect(result).rejects.toThrow(/timed out/);
248248
});
249249

250250
test('returns empty array when the request takes too long to finish', async () => {
@@ -255,9 +255,9 @@ describe('cli notices', () => {
255255
notices: [BASIC_NOTICE],
256256
});
257257

258-
const result = await dataSource.fetch();
258+
const result = dataSource.fetch();
259259

260-
expect(result).toEqual([]);
260+
await expect(result).rejects.toThrow(/timed out/);
261261
});
262262

263263
function mockCall(statusCode: number, body: any): Promise<Notice[]> {
@@ -313,6 +313,10 @@ describe('cli notices', () => {
313313
test('retrieves data from the delegate when the file cannot be read', async () => {
314314
const debugSpy = jest.spyOn(logging, 'debug');
315315

316+
if (fs.existsSync('does-not-exist.json')) {
317+
fs.unlinkSync('does-not-exist.json');
318+
}
319+
316320
const dataSource = dataSourceWithDelegateReturning(freshData, 'does-not-exist.json');
317321

318322
const notices = await dataSource.fetch();
@@ -335,6 +339,20 @@ describe('cli notices', () => {
335339
expect(notices).toEqual(freshData);
336340
});
337341

342+
test('error in delegate gets turned into empty result by cached source', async () => {
343+
// GIVEN
344+
const delegate = {
345+
fetch: jest.fn().mockRejectedValue(new Error('fetching failed')),
346+
};
347+
const dataSource = new CachedDataSource(fileName, delegate, true);
348+
349+
// WHEN
350+
const notices = await dataSource.fetch();
351+
352+
// THEN
353+
expect(notices).toEqual([]);
354+
});
355+
338356
function dataSourceWithDelegateReturning(notices: Notice[], file: string = fileName, ignoreCache: boolean = false) {
339357
const delegate = {
340358
fetch: jest.fn(),

0 commit comments

Comments
 (0)