Merge branch 'master' into corymhall/integ-runner/gaps

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

@@ -3,7 +3,7 @@ on:
     # Cron format: min hr day month dow
     - cron: "0 0 * * *"
 jobs:
-  rix0rrr/close-stale-prs:
+  close-stale-prs:
     permissions:
       pull-requests: write
     runs-on: ubuntu-latest

deprecated_apis.txt

@@ -27,7 +27,6 @@
 @aws-cdk/core.DefaultStackSynthesizerProps#fileAssetKeyArnExportName
 @aws-cdk/core.DockerImageAssetSource#repositoryName
 @aws-cdk/core.Duration#toISOString
-@aws-cdk/core.FileAssetLocation#kmsKeyArn
 @aws-cdk/core.FileAssetLocation#s3Url
 @aws-cdk/core.ITemplateOptions#transform
 @aws-cdk/core.Lazy#anyValue

package.json

@@ -21,10 +21,10 @@
     "fs-extra": "^9.1.0",
     "graceful-fs": "^4.2.10",
     "jest-junit": "^13.1.0",
-    "jsii-diff": "^1.56.0",
-    "jsii-pacmak": "^1.56.0",
-    "jsii-reflect": "^1.56.0",
-    "jsii-rosetta": "^1.56.0",
+    "jsii-diff": "^1.57.0",
+    "jsii-pacmak": "^1.57.0",
+    "jsii-reflect": "^1.57.0",
+    "jsii-rosetta": "^1.57.0",
     "lerna": "^4.0.0",
     "patch-package": "^6.4.7",
     "semver": "^6.3.0",

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

@@ -274,21 +274,14 @@ export interface FileAssetLocation {
   readonly s3ObjectUrl: string;
 
   /**
-   * The ARN of the KMS key used to encrypt the file asset bucket, if any
+   * The ARN of the KMS key used to encrypt the file asset bucket, if any.
    *
-   * If so, the consuming role should be given "kms:Decrypt" permissions in its
-   * identity policy.
+   * The CDK bootstrap stack comes with a key policy that does not require
+   * setting this property, so you only need to set this property if you
+   * have customized the bootstrap stack to require it.
    *
-   * It's the responsibility of they key's creator to make sure that all
-   * consumers that the key's key policy is configured such that the key can be used
-   * by all consumers that need it.
-   *
-   * The default bootstrap stack provisioned by the CDK CLI ensures this, and
-   * can be used as an example for how to configure the key properly.
-   *
-   * @default - Asset bucket is not encrypted
-   * @deprecated Since bootstrap bucket v4, the key policy properly allows use of the
-   * key via the bucket and no additional parameters have to be granted anymore.
+   * @default - Asset bucket is not encrypted, or decryption permissions are
+   * defined by a Key Policy.
    */
   readonly kmsKeyArn?: string;
 

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

@@ -1,4 +1,4 @@
-# integ-runner
+# integ-tests
 
 <!--BEGIN STABILITY BANNER-->
 

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

@@ -21,7 +21,7 @@ describe(IntegManifestWriter, () => {
   };
 
   beforeEach(() => {
-    tmpDir = fs.mkdtempSync(os.tmpdir());
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cdk-test'));
   });
 
   afterEach(() => {

packages/aws-cdk/lib/cli.ts

@@ -282,8 +282,7 @@ async function initCommandLine() {
 
   if (shouldDisplayNotices()) {
     void refreshNotices()
-      .then(_ => debug('Notices refreshed'))
-      .catch(e => debug(`Notices refresh failed: ${e}`));
+      .catch(e => debug(`Could not refresh notices: ${e}`));
   }
 
   const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({

packages/aws-cdk/lib/notices.ts

@@ -107,9 +107,7 @@ export interface NoticeDataSource {
 export class WebsiteNoticeDataSource implements NoticeDataSource {
   fetch(): Promise<Notice[]> {
     const timeout = 3000;
-
-    return new Promise((resolve) => {
-      setTimeout(() => resolve([]), timeout);
+    return new Promise((resolve, reject) => {
       try {
         const req = https.get('https://cli.cdk.dev-tools.aws.dev/notices.json',
           { timeout },
@@ -123,29 +121,39 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
               res.on('end', () => {
                 try {
                   const data = JSON.parse(rawData).notices as Notice[];
+                  if (!data) {
+                    throw new Error("'notices' key is missing");
+                  }
+                  debug('Notices refreshed');
                   resolve(data ?? []);
                 } catch (e) {
-                  debug(`Failed to parse notices: ${e}`);
-                  resolve([]);
+                  reject(new Error(`Failed to parse notices: ${e.message}`));
                 }
               });
               res.on('error', e => {
-                debug(`Failed to fetch notices: ${e}`);
-                resolve([]);
+                reject(new Error(`Failed to fetch notices: ${e.message}`));
               });
             } else {
-              debug(`Failed to fetch notices. Status code: ${res.statusCode}`);
-              resolve([]);
+              reject(new Error(`Failed to fetch notices. Status code: ${res.statusCode}`));
             }
           });
-        req.on('error', e => {
-          debug(`Error on request: ${e}`);
-          resolve([]);
+        req.on('error', reject);
+        req.on('timeout', () => {
+          // The 'timeout' event doesn't stop anything by itself, it just
+          // notifies that it has been long time since we saw bytes.
+          // In our case, we want to give up.
+          req.destroy(new Error('Request timed out'));
         });
-        req.on('timeout', _ => resolve([]));
+
+        // It's not like I don't *trust* the 'timeout' event... but I don't trust it.
+        // Add a backup timer that will destroy the request after all.
+        // (This is at least necessary to make the tests pass, but that's probably because of 'nock'.
+        // It's not clear whether users will hit this).
+        setTimeout(() => {
+          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'));
+        }, timeout + 200);
       } catch (e) {
-        debug(`HTTPS 'get' call threw an error: ${e}`);
-        resolve([]);
+        reject(new Error(`HTTPS 'get' call threw an error: ${e.message}`));
       }
     });
   }
@@ -156,7 +164,8 @@ interface CachedNotices {
   notices: Notice[],
 }
 
-const TIME_TO_LIVE = 60 * 60 * 1000; // 1 hour
+const TIME_TO_LIVE_SUCCESS = 60 * 60 * 1000; // 1 hour
+const TIME_TO_LIVE_ERROR = 1 * 60 * 1000; // 1 minute
 
 export class CachedDataSource implements NoticeDataSource {
   constructor(
@@ -171,17 +180,30 @@ export class CachedDataSource implements NoticeDataSource {
     const expiration = cachedData.expiration ?? 0;
 
     if (Date.now() > expiration || this.skipCache) {
-      const freshData = {
-        expiration: Date.now() + TIME_TO_LIVE,
-        notices: await this.dataSource.fetch(),
-      };
+      const freshData = await this.fetchInner();
       await this.save(freshData);
       return freshData.notices;
     } else {
+      debug(`Reading cached notices from ${this.fileName}`);
       return data;
     }
   }
 
+  private async fetchInner(): Promise<CachedNotices> {
+    try {
+      return {
+        expiration: Date.now() + TIME_TO_LIVE_SUCCESS,
+        notices: await this.dataSource.fetch(),
+      };
+    } catch (e) {
+      debug(`Could not refresh notices: ${e}`);
+      return {
+        expiration: Date.now() + TIME_TO_LIVE_ERROR,
+        notices: [],
+      };
+    }
+  }
+
   private async load(): Promise<CachedNotices> {
     const defaultValue = {
       expiration: 0,

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

@@ -191,60 +191,60 @@ describe('cli notices', () => {
       expect(result).toEqual([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]);
     });
 
-    test('returns empty array when the server returns an unexpected status code', async () => {
-      const result = await mockCall(500, {
+    test('returns appropriate error when the server returns an unexpected status code', async () => {
+      const result = mockCall(500, {
         notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE],
       });
 
-      expect(result).toEqual([]);
+      await expect(result).rejects.toThrow(/500/);
     });
 
-    test('returns empty array when the server returns an unexpected structure', async () => {
-      const result = await mockCall(200, {
+    test('returns appropriate error when the server returns an unexpected structure', async () => {
+      const result = mockCall(200, {
         foo: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE],
       });
 
-      expect(result).toEqual([]);
+      await expect(result).rejects.toThrow(/key is missing/);
     });
 
-    test('returns empty array when the server returns invalid json', async () => {
-      const result = await mockCall(200, '-09aiskjkj838');
+    test('returns appropriate error when the server returns invalid json', async () => {
+      const result = mockCall(200, '-09aiskjkj838');
 
-      expect(result).toEqual([]);
+      await expect(result).rejects.toThrow(/Failed to parse/);
     });
 
-    test('returns empty array when HTTPS call throws', async () => {
+    test('returns appropriate error when HTTPS call throws', async () => {
       const mockGet = jest.spyOn(https, 'get')
         .mockImplementation(() => { throw new Error('No connection'); });
 
-      const result = await dataSource.fetch();
+      const result = dataSource.fetch();
 
-      expect(result).toEqual([]);
+      await expect(result).rejects.toThrow(/No connection/);
 
       mockGet.mockRestore();
     });
 
-    test('returns empty array when the request has an error', async () => {
+    test('returns appropriate error when the request has an error', async () => {
       nock('https://cli.cdk.dev-tools.aws.dev')
         .get('/notices.json')
         .replyWithError('DNS resolution failed');
 
-      const result = await dataSource.fetch();
+      const result = dataSource.fetch();
 
-      expect(result).toEqual([]);
+      await expect(result).rejects.toThrow(/DNS resolution failed/);
     });
 
-    test('returns empty array when the connection stays idle for  too long', async () => {
+    test('returns appropriate error when the connection stays idle for too long', async () => {
       nock('https://cli.cdk.dev-tools.aws.dev')
         .get('/notices.json')
         .delayConnection(3500)
         .reply(200, {
           notices: [BASIC_NOTICE],
         });
 
-      const result = await dataSource.fetch();
+      const result = dataSource.fetch();
 
-      expect(result).toEqual([]);
+      await expect(result).rejects.toThrow(/timed out/);
     });
 
     test('returns empty array when the request takes too long to finish', async () => {
@@ -255,9 +255,9 @@ describe('cli notices', () => {
           notices: [BASIC_NOTICE],
         });
 
-      const result = await dataSource.fetch();
+      const result = dataSource.fetch();
 
-      expect(result).toEqual([]);
+      await expect(result).rejects.toThrow(/timed out/);
     });
 
     function mockCall(statusCode: number, body: any): Promise<Notice[]> {
@@ -313,6 +313,10 @@ describe('cli notices', () => {
     test('retrieves data from the delegate when the file cannot be read', async () => {
       const debugSpy = jest.spyOn(logging, 'debug');
 
+      if (fs.existsSync('does-not-exist.json')) {
+        fs.unlinkSync('does-not-exist.json');
+      }
+
       const dataSource = dataSourceWithDelegateReturning(freshData, 'does-not-exist.json');
 
       const notices = await dataSource.fetch();
@@ -335,6 +339,20 @@ describe('cli notices', () => {
       expect(notices).toEqual(freshData);
     });
 
+    test('error in delegate gets turned into empty result by cached source', async () => {
+      // GIVEN
+      const delegate = {
+        fetch: jest.fn().mockRejectedValue(new Error('fetching failed')),
+      };
+      const dataSource = new CachedDataSource(fileName, delegate, true);
+
+      // WHEN
+      const notices = await dataSource.fetch();
+
+      // THEN
+      expect(notices).toEqual([]);
+    });
+
     function dataSourceWithDelegateReturning(notices: Notice[], file: string = fileName, ignoreCache: boolean = false) {
       const delegate = {
         fetch: jest.fn(),

packages/awslint/package.json

@@ -18,11 +18,11 @@
     "awslint": "bin/awslint"
   },
   "dependencies": {
-    "@jsii/spec": "^1.56.0",
+    "@jsii/spec": "^1.57.0",
     "camelcase": "^6.3.0",
     "chalk": "^4",
     "fs-extra": "^9.1.0",
-    "jsii-reflect": "^1.56.0",
+    "jsii-reflect": "^1.57.0",
     "yargs": "^16.2.0"
   },
   "devDependencies": {

scripts/find-latest-release.js

@@ -30,7 +30,6 @@ async function main(matchRange) {
 
       // 60 requests/hour, so we need to sleep for a full minute to get any kind of response
       const sleepTime = Math.floor(Math.random() * 60 * Math.pow(2, attempt));
-      console.log('Sleeping for', sleepTime, 'seconds');
       await sleep(sleepTime * 1000);
       continue;
     }

tools/@aws-cdk/cdk-build-tools/package.json

@@ -57,9 +57,9 @@
     "fs-extra": "^9.1.0",
     "jest": "^27.5.1",
     "jest-junit": "^13.1.0",
-    "jsii": "^1.56.0",
-    "jsii-pacmak": "^1.56.0",
-    "jsii-reflect": "^1.56.0",
+    "jsii": "^1.57.0",
+    "jsii-pacmak": "^1.57.0",
+    "jsii-reflect": "^1.57.0",
     "markdownlint-cli": "^0.31.1",
     "nyc": "^15.1.0",
     "semver": "^7.3.6",