From abb4a05b1d9d84e0aa1d218608172df66e8ab789 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Fri, 11 Feb 2022 11:58:09 +0000 Subject: [PATCH 01/28] feat(cli): support for notices --- packages/aws-cdk/bin/cdk.ts | 2 + packages/aws-cdk/lib/notices.ts | 118 ++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 packages/aws-cdk/lib/notices.ts diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 5b82a72eab5d2..852bd01e65098 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -17,6 +17,7 @@ import { CdkToolkit } from '../lib/cdk-toolkit'; import { RequireApproval } from '../lib/diff'; import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib/init'; import { data, debug, error, print, setLogLevel } from '../lib/logging'; +import { displayNotices } from '../lib/notices'; import { PluginHost } from '../lib/plugin'; import { serializeStructure } from '../lib/serialize'; import { Command, Configuration, Settings } from '../lib/settings'; @@ -288,6 +289,7 @@ async function initCommandLine() { } } finally { await version.displayVersionMessage(); + await displayNotices(); } async function main(command: string, args: any): Promise { diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts new file mode 100644 index 0000000000000..136afd8a022e3 --- /dev/null +++ b/packages/aws-cdk/lib/notices.ts @@ -0,0 +1,118 @@ +import * as https from 'https'; +import * as semver from 'semver'; +import { print } from './logging'; +import { versionNumber } from './version'; + +export async function displayNotices() { + + const dataSource = new WebsiteNoticeDataSource(); + const filter = new NoticeFilter({ + cliVersion: versionNumber(), + temporarilySuppressed: false, + permanentlySuppressed: false, + }); + const formatter = new FooNoticeFormatter(); + + const notices = await dataSource.fetch(); + const individualMessages = notices + .filter(notice => filter.apply(notice)) + .map(notice => formatter.apply(notice)); + + + if (notices.length > 0) { + const finalMessage = `NOTICES + +${individualMessages.join('\n\n')} + +If you don’t want to see an notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge ${notices[0].issueNumber}".`; + + print(finalMessage); + } +} + +export interface Component { + name: string; + version: string; +} + +export interface Notice { + title: string; + issueNumber: number; + overview: string; + components: Component[]; + schemaVersion: string; +} + +export interface NoticeDataSource { + fetch(): Promise, +} + +export class WebsiteNoticeDataSource implements NoticeDataSource { + fetch(): Promise { + return new Promise((resolve, reject) => { + https.get('https://cli.cdk.dev-tools.aws.dev/notices.json', res => { + if (res.statusCode === 200) { + res.setEncoding('utf8'); + let rawData = ''; + res.on('data', (chunk) => { + rawData += chunk; + }); + res.on('end', () => { + try { + const notices = JSON.parse(rawData).notices as Notice[]; + resolve(notices); + } catch (e) { + reject(e); + } + }); + } + }); + }); + } +} + +// TODO Caching can be implemented as a decorator around NoticeDataSource + +export interface NoticeFilterProps { + permanentlySuppressed: boolean, + temporarilySuppressed: boolean, + cliVersion: string, + acknowledgedIssueNumbers?: number[], +} + +export class NoticeFilter { + private readonly acknowledgedIssueNumbers: number[]; + + constructor(private readonly props: NoticeFilterProps) { + this.acknowledgedIssueNumbers = props.acknowledgedIssueNumbers ?? []; + } + + apply(notice: Notice): boolean { + if (this.props.permanentlySuppressed + || this.props.temporarilySuppressed + || this.acknowledgedIssueNumbers.includes(notice.issueNumber)) { + return false; + } + + const cliComponent = notice.components.find(component => component.name === 'cli'); + const affectedCliVersion = cliComponent?.version; + return affectedCliVersion != null && semver.satisfies(this.props.cliVersion, affectedCliVersion); + } +} + +export interface NoticeFormatter { + apply(notice: Notice): string, +} + +// TODO rename this +export class FooNoticeFormatter implements NoticeFormatter { + apply(notice: Notice): string { + let result = ''; + result += `${notice.issueNumber}\t${notice.title}\n\n`; + result += `\tOverview: ${notice.overview}\n\n`; + const componentsValue = notice.components.map(c => `${c.name}: ${c.version}`).join(', '); + result += `\tAffected versions: ${componentsValue}\n\n`; + result += `\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`; + return result; + } +} \ No newline at end of file From 7c873c32e43ea776327e8f9ed0980cf620236ade Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Fri, 11 Feb 2022 14:04:04 +0000 Subject: [PATCH 02/28] Added cache --- packages/aws-cdk/lib/notices.ts | 62 ++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 136afd8a022e3..a3c602525d3da 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -1,11 +1,13 @@ import * as https from 'https'; +import * as path from 'path'; +import * as fs from 'fs-extra'; import * as semver from 'semver'; -import { print } from './logging'; +import { print, debug } from './logging'; +import { cdkCacheDir } from './util/directories'; import { versionNumber } from './version'; export async function displayNotices() { - - const dataSource = new WebsiteNoticeDataSource(); + const dataSource = new CachedDataSource(path.join(cdkCacheDir(), 'notices.json'), new WebsiteNoticeDataSource()); const filter = new NoticeFilter({ cliVersion: versionNumber(), temporarilySuppressed: false, @@ -18,8 +20,7 @@ export async function displayNotices() { .filter(notice => filter.apply(notice)) .map(notice => formatter.apply(notice)); - - if (notices.length > 0) { + if (individualMessages.length > 0) { const finalMessage = `NOTICES ${individualMessages.join('\n\n')} @@ -66,12 +67,63 @@ export class WebsiteNoticeDataSource implements NoticeDataSource { } }); } + // Otherwise, we just ignore the result and move on }); }); } } +interface CachedNotices { + expiration: number, + notices: Notice[], +} + +const TIME_TO_LIVE = 60 * 60 * 1000; // 1 hour + // TODO Caching can be implemented as a decorator around NoticeDataSource +export class CachedDataSource implements NoticeDataSource { + constructor( + private readonly fileName: string, + private readonly dataSource: NoticeDataSource) { + } + + async fetch(): Promise { + const cachedData = await this.loadCachedData(); + const notices = cachedData.notices; + const expiration = cachedData.expiration ?? 0; + + if (Date.now() > expiration) { + const freshData = { + expiration: Date.now() + TIME_TO_LIVE, + notices: await this.dataSource.fetch(), + }; + await this.storeCachedData(freshData); + return freshData.notices; + } else { + return notices; + } + } + + private async loadCachedData(): Promise { + try { + return await fs.readJSON(this.fileName) as CachedNotices; + } catch (e) { + debug(`Failed to load notices from cached: ${e}`); + return { + expiration: 0, + notices: [], + }; + } + } + + private async storeCachedData(cached: CachedNotices): Promise { + try { + await fs.writeJSON(this.fileName, cached); + } catch (e) { + debug(`Failed to store notices in the cache: ${e}`); + } + } +} export interface NoticeFilterProps { permanentlySuppressed: boolean, From e280527541471a4f115c0942482fec5e7b7d7ea0 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Fri, 11 Feb 2022 14:21:18 +0000 Subject: [PATCH 03/28] Refactoring --- packages/aws-cdk/lib/notices.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index a3c602525d3da..4f1a4efbc008f 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -10,8 +10,8 @@ export async function displayNotices() { const dataSource = new CachedDataSource(path.join(cdkCacheDir(), 'notices.json'), new WebsiteNoticeDataSource()); const filter = new NoticeFilter({ cliVersion: versionNumber(), - temporarilySuppressed: false, - permanentlySuppressed: false, + temporarilySuppressed: false, // coming from a command line option + permanentlySuppressed: false, // coming from the cdk.json file }); const formatter = new FooNoticeFormatter(); @@ -80,7 +80,6 @@ interface CachedNotices { const TIME_TO_LIVE = 60 * 60 * 1000; // 1 hour -// TODO Caching can be implemented as a decorator around NoticeDataSource export class CachedDataSource implements NoticeDataSource { constructor( private readonly fileName: string, @@ -88,7 +87,7 @@ export class CachedDataSource implements NoticeDataSource { } async fetch(): Promise { - const cachedData = await this.loadCachedData(); + const cachedData = await this.load(); const notices = cachedData.notices; const expiration = cachedData.expiration ?? 0; @@ -97,14 +96,14 @@ export class CachedDataSource implements NoticeDataSource { expiration: Date.now() + TIME_TO_LIVE, notices: await this.dataSource.fetch(), }; - await this.storeCachedData(freshData); + await this.save(freshData); return freshData.notices; } else { return notices; } } - private async loadCachedData(): Promise { + private async load(): Promise { try { return await fs.readJSON(this.fileName) as CachedNotices; } catch (e) { @@ -116,7 +115,7 @@ export class CachedDataSource implements NoticeDataSource { } } - private async storeCachedData(cached: CachedNotices): Promise { + private async save(cached: CachedNotices): Promise { try { await fs.writeJSON(this.fileName, cached); } catch (e) { From 236f3a91f69f8d16fa5de18111f0fdd3887f20f0 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Tue, 15 Feb 2022 12:34:42 -0500 Subject: [PATCH 04/28] refactor to make unit testing feasible --- packages/aws-cdk/lib/notices.ts | 66 +++++++++++++++--------- packages/aws-cdk/test/notices.test.ts | 74 +++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 23 deletions(-) create mode 100644 packages/aws-cdk/test/notices.test.ts diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 4f1a4efbc008f..17e42e249401a 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -6,29 +6,49 @@ import { print, debug } from './logging'; import { cdkCacheDir } from './util/directories'; import { versionNumber } from './version'; -export async function displayNotices() { - const dataSource = new CachedDataSource(path.join(cdkCacheDir(), 'notices.json'), new WebsiteNoticeDataSource()); - const filter = new NoticeFilter({ - cliVersion: versionNumber(), - temporarilySuppressed: false, // coming from a command line option - permanentlySuppressed: false, // coming from the cdk.json file - }); - const formatter = new FooNoticeFormatter(); +interface DisplayNoticesOptions { + readonly dataSource?: NoticeDataSource; + readonly cacheFilePath?: string; +} + +export async function displayNotices(options: DisplayNoticesOptions = {}) { + const cacheFilePath = options.cacheFilePath ?? path.join(cdkCacheDir(), 'notices.json'); + const dataSource = new CachedDataSource(cacheFilePath, options.dataSource ?? new WebsiteNoticeDataSource()); const notices = await dataSource.fetch(); - const individualMessages = notices - .filter(notice => filter.apply(notice)) - .map(notice => formatter.apply(notice)); + const individualMessages = formatNotices(filterNotices(notices)); if (individualMessages.length > 0) { - const finalMessage = `NOTICES - -${individualMessages.join('\n\n')} + print(finalMessage(individualMessages, notices[0].issueNumber)); + } +} + +export function finalMessage(individualMessages: string[], exampleNumber: number): string { + return [ + 'NOTICES', + ...individualMessages, + `If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge ${exampleNumber}".`, + ].join('\n\n'); +} -If you don’t want to see an notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge ${notices[0].issueNumber}".`; +export interface FilterOptions { + readonly versionNumber?: string; + readonly temporarilySuppressed?: boolean; + readonly permanentlySuppressed?: boolean; +} - print(finalMessage); - } +export function filterNotices(notices: Notice[], options: FilterOptions = {}): Notice[] { + const filter = new NoticeFilter({ + cliVersion: options.versionNumber ?? versionNumber(), + temporarilySuppressed: options.temporarilySuppressed ?? false, // coming from a command line option + permanentlySuppressed: options.permanentlySuppressed ?? false, // coming from the cdk.json file + }); + return notices.filter(notice => filter.apply(notice)); +} + +export function formatNotices(notices: Notice[]): string[] { + const formatter = new FooNoticeFormatter(); + return notices.map(notice => formatter.apply(notice)); } export interface Component { @@ -158,12 +178,12 @@ export interface NoticeFormatter { // TODO rename this export class FooNoticeFormatter implements NoticeFormatter { apply(notice: Notice): string { - let result = ''; - result += `${notice.issueNumber}\t${notice.title}\n\n`; - result += `\tOverview: ${notice.overview}\n\n`; const componentsValue = notice.components.map(c => `${c.name}: ${c.version}`).join(', '); - result += `\tAffected versions: ${componentsValue}\n\n`; - result += `\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`; - return result; + return [ + `${notice.issueNumber}\t${notice.title}`, + `\tOverview: ${notice.overview}`, + `\tAffected versions: ${componentsValue}`, + `\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`, + ].join('\n\n'); } } \ No newline at end of file diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts new file mode 100644 index 0000000000000..ed98a68a27a3d --- /dev/null +++ b/packages/aws-cdk/test/notices.test.ts @@ -0,0 +1,74 @@ +import { formatNotices, filterNotices } from '../lib/notices'; + +const BASIC_NOTICE = { + title: 'Toggling off auto_delete_objects for Bucket empties the bucket', + issueNumber: 16603, + overview: 'If a stack is deployed with an S3 bucket with auto_delete_objects=True, and then re-deployed with auto_delete_objects=False, all the objects in the bucket will be deleted.', + components: [{ + name: 'cli', + version: '<=1.126.0', + }], + schemaVersion: '1', +}; + +const MULTIPLE_AFFECTED_VERSIONS_NOTICE = { + title: 'Error when building EKS cluster with monocdk import', + issueNumber: 17061, + overview: 'When using monocdk/aws-eks to build a stack containing an EKS cluster, error is thrown about missing lambda-layer-node-proxy-agent/layer/package.json.', + components: [{ + name: 'cli', + version: '<1.130.0 >=1.126.0', + }], + schemaVersion: '1', +}; + +describe('cli notices', () => { + describe(formatNotices, () => { + test('correct format', () => { + expect(formatNotices([BASIC_NOTICE])).toEqual([ + [ + `${BASIC_NOTICE.issueNumber}\t${BASIC_NOTICE.title}`, + `\tOverview: ${BASIC_NOTICE.overview}`, + '\tAffected versions: cli: <=1.126.0', + `\tMore information at: https://github.com/aws/aws-cdk/issues/${BASIC_NOTICE.issueNumber}`, + ].join('\n\n'), + ]); + }); + + test('multiple affect versions', () => { + expect(formatNotices([MULTIPLE_AFFECTED_VERSIONS_NOTICE])).toEqual([ + [ + `${MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber}\t${MULTIPLE_AFFECTED_VERSIONS_NOTICE.title}`, + `\tOverview: ${MULTIPLE_AFFECTED_VERSIONS_NOTICE.overview}`, + '\tAffected versions: cli: <1.130.0 >=1.126.0', + `\tMore information at: https://github.com/aws/aws-cdk/issues/${MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber}`, + ].join('\n\n'), + ]); + }); + }); + + describe(filterNotices, () => { + test('correctly filter notices on cli', () => { + const notices = [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]; + expect(filterNotices(notices, { + versionNumber: '1.0.0', + })).toEqual([BASIC_NOTICE]); + + expect(filterNotices(notices, { + versionNumber: '1.129.0', + })).toEqual([MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + + expect(filterNotices(notices, { + versionNumber: '1.126.0', + })).toEqual(notices); + + expect(filterNotices(notices, { + versionNumber: '1.130.0', + })).toEqual([]); + }); + + test('correctly filter notices on framework', () => { + // TODO + }); + }); +}); From f5c2962df3a5c1a2887b8e1ebcdafc51124e19dd Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Tue, 15 Feb 2022 15:17:11 -0500 Subject: [PATCH 05/28] support cdk acknowledge and cdk notices --- packages/aws-cdk/bin/cdk.ts | 17 ++++- packages/aws-cdk/lib/cdk-toolkit.ts | 5 ++ packages/aws-cdk/lib/notices.ts | 94 +++++++++++++++++++++------ packages/aws-cdk/test/notices.test.ts | 8 +-- 4 files changed, 98 insertions(+), 26 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 852bd01e65098..90b4044f3bd53 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -185,6 +185,8 @@ async function parseCommandLineArguments() { .option('security-only', { type: 'boolean', desc: 'Only diff for broadened security changes', default: false }) .option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff', default: false })) .command('metadata [STACK]', 'Returns all metadata associated with this stack') + .command('acknowledge [ID]', 'Acknowledge a notice so that it does not show up anymore') + .command('notices', 'Returns a list of relevant notices') .command('init [TEMPLATE]', 'Create a new, empty CDK project from a template.', yargs => yargs .option('language', { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: initTemplateLanguages }) .option('list', { type: 'boolean', desc: 'List the available templates' }) @@ -289,7 +291,6 @@ async function initCommandLine() { } } finally { await version.displayVersionMessage(); - await displayNotices(); } async function main(command: string, args: any): Promise { @@ -430,10 +431,16 @@ async function initCommandLine() { return cli.synth(args.STACKS, true, args.quiet, args.validation); } - case 'metadata': return cli.metadata(args.STACK); + case 'acknowledge': + return cli.acknowledge(args.ID); + + case 'notices': + SHOW_ALL_NOTICES = true; + return; + case 'init': const language = configuration.settings.get(['language']); if (args.list) { @@ -523,6 +530,7 @@ function yargsNegativeAlias { if (value == null) { return; } @@ -532,6 +540,11 @@ initCommandLine() process.exitCode = value; } }) + .then(async () => { + await displayNotices({ + acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? new Set() : undefined, + }); + }) .catch(err => { error(err.message); if (err.stack) { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 56c5302544a84..8b583c86dc577 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -16,6 +16,7 @@ import { CloudWatchLogEventMonitor } from './api/logs/logs-monitor'; import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; import { data, debug, error, highlight, print, success, warning } from './logging'; +import { suppressNotice } from './notices'; import { deserializeStructure } from './serialize'; import { Configuration, PROJECT_CONFIG } from './settings'; import { numberFromBool, partition } from './util'; @@ -79,6 +80,10 @@ export class CdkToolkit { return stacks.firstStack.manifest.metadata ?? {}; } + public async acknowledge(noticeId: string) { + return suppressNotice(Number(noticeId)); + } + public async diff(options: DiffOptions): Promise { const stacks = await this.selectStacksForDiff(options.stackNames, options.exclusively); diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 17e42e249401a..9e7b2f68fcafd 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -6,24 +6,43 @@ import { print, debug } from './logging'; import { cdkCacheDir } from './util/directories'; import { versionNumber } from './version'; -interface DisplayNoticesOptions { - readonly dataSource?: NoticeDataSource; - readonly cacheFilePath?: string; +const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json'); +const ACKNOWLEDGEMENTS_FILE_PATH = path.join(cdkCacheDir(), 'acks.json'); + +export async function suppressNotice(issueNumber: number) { + await new Acknowledgements().addIssueNumber(issueNumber); +} + +export interface DisplayNoticesOptions extends FilterOptions { +} + +export interface FilterOptions { + readonly temporarilySuppressed?: boolean; + readonly permanentlySuppressed?: boolean; + readonly cliVersion?: string; + readonly acknowledgedIssueNumbers?: Set; } export async function displayNotices(options: DisplayNoticesOptions = {}) { - const cacheFilePath = options.cacheFilePath ?? path.join(cdkCacheDir(), 'notices.json'); - const dataSource = new CachedDataSource(cacheFilePath, options.dataSource ?? new WebsiteNoticeDataSource()); + const dataSource = dataSourceReference(); const notices = await dataSource.fetch(); - const individualMessages = formatNotices(filterNotices(notices)); + const individualMessages = formatNotices(filterNotices(notices, { + temporarilySuppressed: options.temporarilySuppressed, + permanentlySuppressed: options.permanentlySuppressed, + acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? await new Acknowledgements().getIssueNumbers(), + })); if (individualMessages.length > 0) { print(finalMessage(individualMessages, notices[0].issueNumber)); } } -export function finalMessage(individualMessages: string[], exampleNumber: number): string { +function dataSourceReference(): CachedDataSource { + return new CachedDataSource(CACHE_FILE_PATH, new WebsiteNoticeDataSource()); +} + +function finalMessage(individualMessages: string[], exampleNumber: number): string { return [ 'NOTICES', ...individualMessages, @@ -31,17 +50,12 @@ export function finalMessage(individualMessages: string[], exampleNumber: number ].join('\n\n'); } -export interface FilterOptions { - readonly versionNumber?: string; - readonly temporarilySuppressed?: boolean; - readonly permanentlySuppressed?: boolean; -} - -export function filterNotices(notices: Notice[], options: FilterOptions = {}): Notice[] { +export function filterNotices(notices: Notice[], options: FilterOptions): Notice[] { const filter = new NoticeFilter({ - cliVersion: options.versionNumber ?? versionNumber(), + cliVersion: options.cliVersion ?? versionNumber(), temporarilySuppressed: options.temporarilySuppressed ?? false, // coming from a command line option permanentlySuppressed: options.permanentlySuppressed ?? false, // coming from the cdk.json file + acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? new Set(), }); return notices.filter(notice => filter.apply(notice)); } @@ -127,7 +141,7 @@ export class CachedDataSource implements NoticeDataSource { try { return await fs.readJSON(this.fileName) as CachedNotices; } catch (e) { - debug(`Failed to load notices from cached: ${e}`); + debug(`Failed to load notices from cache: ${e}`); return { expiration: 0, notices: [], @@ -148,20 +162,20 @@ export interface NoticeFilterProps { permanentlySuppressed: boolean, temporarilySuppressed: boolean, cliVersion: string, - acknowledgedIssueNumbers?: number[], + acknowledgedIssueNumbers: Set, } export class NoticeFilter { - private readonly acknowledgedIssueNumbers: number[]; + private readonly acknowledgedIssueNumbers: Set; constructor(private readonly props: NoticeFilterProps) { - this.acknowledgedIssueNumbers = props.acknowledgedIssueNumbers ?? []; + this.acknowledgedIssueNumbers = props.acknowledgedIssueNumbers ?? new Set(); } apply(notice: Notice): boolean { if (this.props.permanentlySuppressed || this.props.temporarilySuppressed - || this.acknowledgedIssueNumbers.includes(notice.issueNumber)) { + || this.acknowledgedIssueNumbers.has(notice.issueNumber)) { return false; } @@ -186,4 +200,44 @@ export class FooNoticeFormatter implements NoticeFormatter { `\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`, ].join('\n\n'); } +} + +interface CachedAcks { + issueNumbers: number[]; +} + +export class Acknowledgements { + private readonly fileName: string = ACKNOWLEDGEMENTS_FILE_PATH; + + async getIssueNumbers(): Promise> { + const cachedData = await this.load(); + const issueNumbers = new Set(); + cachedData.issueNumbers.forEach(i => issueNumbers.add(i)); + return issueNumbers; + } + + async addIssueNumber(issueNumber: number) { + const cachedData = await this.load(); + cachedData.issueNumbers.push(issueNumber); + await this.save(cachedData); + } + + private async load(): Promise { + try { + return await fs.readJSON(this.fileName) as CachedAcks; + } catch (e) { + debug(`Failed to load acknowledgements from cache: ${e}`); + return { + issueNumbers: [], + }; + } + } + + private async save(cached: CachedAcks): Promise { + try { + await fs.writeJSON(this.fileName, cached); + } catch (e) { + debug(`Failed to store acknowledgements in the cache: ${e}`); + } + } } \ No newline at end of file diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index ed98a68a27a3d..9d403254d7d63 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -51,19 +51,19 @@ describe('cli notices', () => { test('correctly filter notices on cli', () => { const notices = [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]; expect(filterNotices(notices, { - versionNumber: '1.0.0', + cliVersion: '1.0.0', })).toEqual([BASIC_NOTICE]); expect(filterNotices(notices, { - versionNumber: '1.129.0', + cliVersion: '1.129.0', })).toEqual([MULTIPLE_AFFECTED_VERSIONS_NOTICE]); expect(filterNotices(notices, { - versionNumber: '1.126.0', + cliVersion: '1.126.0', })).toEqual(notices); expect(filterNotices(notices, { - versionNumber: '1.130.0', + cliVersion: '1.130.0', })).toEqual([]); }); From 5175a1315ef57d201253f12c868a58507f4d4db9 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 16 Feb 2022 15:20:21 -0500 Subject: [PATCH 06/28] refactor acknowledgements --- packages/aws-cdk/bin/cdk.ts | 23 +++++--- packages/aws-cdk/lib/cdk-toolkit.ts | 6 +- packages/aws-cdk/lib/notices.ts | 88 ++++++++++------------------- 3 files changed, 51 insertions(+), 66 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 90b4044f3bd53..5b2388b4daf99 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -283,11 +283,20 @@ async function initCommandLine() { ? await (argv.commandHandler as (opts: typeof commandOptions) => any)(commandOptions) : await main(cmd, argv); if (typeof returnValue === 'object') { - return toJsonOrYaml(returnValue); + return { + value: toJsonOrYaml(returnValue), + configuration, + }; } else if (typeof returnValue === 'string') { - return returnValue; + return { + value: returnValue, + configuration, + }; } else { - return returnValue; + return { + value: returnValue, + configuration, + }; } } finally { await version.displayVersionMessage(); @@ -532,17 +541,17 @@ function yargsNegativeAlias { + .then(async ({ value, configuration }) => { if (value == null) { return; } if (typeof value === 'string') { data(value); } else if (typeof value === 'number') { process.exitCode = value; } - }) - .then(async () => { await displayNotices({ - acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? new Set() : undefined, + configuration, + skipAcknowledgedIssueNumbers: SHOW_ALL_NOTICES, + }); }) .catch(err => { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 8b583c86dc577..c02446345ee23 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -16,7 +16,6 @@ import { CloudWatchLogEventMonitor } from './api/logs/logs-monitor'; import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; import { data, debug, error, highlight, print, success, warning } from './logging'; -import { suppressNotice } from './notices'; import { deserializeStructure } from './serialize'; import { Configuration, PROJECT_CONFIG } from './settings'; import { numberFromBool, partition } from './util'; @@ -81,7 +80,10 @@ export class CdkToolkit { } public async acknowledge(noticeId: string) { - return suppressNotice(Number(noticeId)); + const acks = this.props.configuration.context.get('acknowledged-ids') ?? []; + acks.push(Number(noticeId)); + this.props.configuration.context.set('acknowledged-ids', acks); + await this.props.configuration.saveContext(); } public async diff(options: DiffOptions): Promise { diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 9e7b2f68fcafd..27bd47640d352 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -3,34 +3,34 @@ import * as path from 'path'; import * as fs from 'fs-extra'; import * as semver from 'semver'; import { print, debug } from './logging'; +import { Configuration } from './settings'; import { cdkCacheDir } from './util/directories'; import { versionNumber } from './version'; const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json'); -const ACKNOWLEDGEMENTS_FILE_PATH = path.join(cdkCacheDir(), 'acks.json'); -export async function suppressNotice(issueNumber: number) { - await new Acknowledgements().addIssueNumber(issueNumber); -} - -export interface DisplayNoticesOptions extends FilterOptions { +export interface DisplayNoticesProps extends FilterOptions { + /** + * Application configuration (settings and context) + */ + readonly configuration: Configuration; } export interface FilterOptions { readonly temporarilySuppressed?: boolean; readonly permanentlySuppressed?: boolean; readonly cliVersion?: string; - readonly acknowledgedIssueNumbers?: Set; + readonly skipAcknowledgedIssueNumbers?: boolean; } -export async function displayNotices(options: DisplayNoticesOptions = {}) { +export async function displayNotices(props: DisplayNoticesProps) { const dataSource = dataSourceReference(); const notices = await dataSource.fetch(); const individualMessages = formatNotices(filterNotices(notices, { - temporarilySuppressed: options.temporarilySuppressed, - permanentlySuppressed: options.permanentlySuppressed, - acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? await new Acknowledgements().getIssueNumbers(), + temporarilySuppressed: props.temporarilySuppressed, + permanentlySuppressed: props.permanentlySuppressed, + acknowledgedIssueNumbers: props.skipAcknowledgedIssueNumbers ? new Set() : getAcknowledgedIssueNumbers(props.configuration), })); if (individualMessages.length > 0) { @@ -42,6 +42,13 @@ function dataSourceReference(): CachedDataSource { return new CachedDataSource(CACHE_FILE_PATH, new WebsiteNoticeDataSource()); } +function getAcknowledgedIssueNumbers(configuration: Configuration): Set { + const acks: number[] = configuration.context.get('acknowledged-ids'); + const issueNumbers = new Set(); + acks.forEach(a => issueNumbers.add(a)); + return issueNumbers; +} + function finalMessage(individualMessages: string[], exampleNumber: number): string { return [ 'NOTICES', @@ -50,7 +57,14 @@ function finalMessage(individualMessages: string[], exampleNumber: number): stri ].join('\n\n'); } -export function filterNotices(notices: Notice[], options: FilterOptions): Notice[] { +export interface FilterNoticeOptions { + permanentlySuppressed?: boolean, + temporarilySuppressed?: boolean, + cliVersion?: string, + acknowledgedIssueNumbers?: Set, +} + +export function filterNotices(notices: Notice[], options: FilterNoticeOptions): Notice[] { const filter = new NoticeFilter({ cliVersion: options.cliVersion ?? versionNumber(), temporarilySuppressed: options.temporarilySuppressed ?? false, // coming from a command line option @@ -168,20 +182,20 @@ export interface NoticeFilterProps { export class NoticeFilter { private readonly acknowledgedIssueNumbers: Set; - constructor(private readonly props: NoticeFilterProps) { - this.acknowledgedIssueNumbers = props.acknowledgedIssueNumbers ?? new Set(); + constructor(private readonly options: NoticeFilterProps) { + this.acknowledgedIssueNumbers = options.acknowledgedIssueNumbers; } apply(notice: Notice): boolean { - if (this.props.permanentlySuppressed - || this.props.temporarilySuppressed + if (this.options.permanentlySuppressed + || this.options.temporarilySuppressed || this.acknowledgedIssueNumbers.has(notice.issueNumber)) { return false; } const cliComponent = notice.components.find(component => component.name === 'cli'); const affectedCliVersion = cliComponent?.version; - return affectedCliVersion != null && semver.satisfies(this.props.cliVersion, affectedCliVersion); + return affectedCliVersion != null && semver.satisfies(this.options.cliVersion, affectedCliVersion); } } @@ -201,43 +215,3 @@ export class FooNoticeFormatter implements NoticeFormatter { ].join('\n\n'); } } - -interface CachedAcks { - issueNumbers: number[]; -} - -export class Acknowledgements { - private readonly fileName: string = ACKNOWLEDGEMENTS_FILE_PATH; - - async getIssueNumbers(): Promise> { - const cachedData = await this.load(); - const issueNumbers = new Set(); - cachedData.issueNumbers.forEach(i => issueNumbers.add(i)); - return issueNumbers; - } - - async addIssueNumber(issueNumber: number) { - const cachedData = await this.load(); - cachedData.issueNumbers.push(issueNumber); - await this.save(cachedData); - } - - private async load(): Promise { - try { - return await fs.readJSON(this.fileName) as CachedAcks; - } catch (e) { - debug(`Failed to load acknowledgements from cache: ${e}`); - return { - issueNumbers: [], - }; - } - } - - private async save(cached: CachedAcks): Promise { - try { - await fs.writeJSON(this.fileName, cached); - } catch (e) { - debug(`Failed to store acknowledgements in the cache: ${e}`); - } - } -} \ No newline at end of file From 4c32d0faf6275a95a37396c5a5acdea6baa0f7a7 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 16 Feb 2022 17:34:19 -0500 Subject: [PATCH 07/28] refactor acknowledgements and filter for them --- packages/aws-cdk/bin/cdk.ts | 6 +++--- packages/aws-cdk/lib/cdk-toolkit.ts | 4 ++-- packages/aws-cdk/lib/notices.ts | 22 +++++++--------------- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 5b2388b4daf99..fd774c47192b2 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -210,7 +210,7 @@ if (!process.stdout.isTTY) { process.env.FORCE_COLOR = '0'; } -async function initCommandLine() { +async function initCommandLine(): Promise<{ configuration: Configuration, value: any }> { const argv = await parseCommandLineArguments(); if (argv.verbose) { setLogLevel(argv.verbose); @@ -549,9 +549,9 @@ initCommandLine() process.exitCode = value; } await displayNotices({ - configuration, + acknowledgedIssueNumbers: configuration.context.get('acknowledged-issue-numbers') ?? [], skipAcknowledgedIssueNumbers: SHOW_ALL_NOTICES, - + permanentlySuppressed: configuration.context.get('no-notices'), }); }) .catch(err => { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index c02446345ee23..206284db0990a 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -80,9 +80,9 @@ export class CdkToolkit { } public async acknowledge(noticeId: string) { - const acks = this.props.configuration.context.get('acknowledged-ids') ?? []; + const acks = this.props.configuration.context.get('acknowledged-issue-numbers') ?? []; acks.push(Number(noticeId)); - this.props.configuration.context.set('acknowledged-ids', acks); + this.props.configuration.context.set('acknowledged-issue-numbers', acks); await this.props.configuration.saveContext(); } diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 27bd47640d352..9d42faa0f2f0a 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -3,20 +3,13 @@ import * as path from 'path'; import * as fs from 'fs-extra'; import * as semver from 'semver'; import { print, debug } from './logging'; -import { Configuration } from './settings'; import { cdkCacheDir } from './util/directories'; import { versionNumber } from './version'; const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json'); -export interface DisplayNoticesProps extends FilterOptions { - /** - * Application configuration (settings and context) - */ - readonly configuration: Configuration; -} - -export interface FilterOptions { +export interface DisplayNoticesProps { + readonly acknowledgedIssueNumbers: number[]; readonly temporarilySuppressed?: boolean; readonly permanentlySuppressed?: boolean; readonly cliVersion?: string; @@ -30,7 +23,7 @@ export async function displayNotices(props: DisplayNoticesProps) { const individualMessages = formatNotices(filterNotices(notices, { temporarilySuppressed: props.temporarilySuppressed, permanentlySuppressed: props.permanentlySuppressed, - acknowledgedIssueNumbers: props.skipAcknowledgedIssueNumbers ? new Set() : getAcknowledgedIssueNumbers(props.configuration), + acknowledgedIssueNumbers: props.skipAcknowledgedIssueNumbers ? new Set() : arrayToSet(props.acknowledgedIssueNumbers), })); if (individualMessages.length > 0) { @@ -42,11 +35,10 @@ function dataSourceReference(): CachedDataSource { return new CachedDataSource(CACHE_FILE_PATH, new WebsiteNoticeDataSource()); } -function getAcknowledgedIssueNumbers(configuration: Configuration): Set { - const acks: number[] = configuration.context.get('acknowledged-ids'); - const issueNumbers = new Set(); - acks.forEach(a => issueNumbers.add(a)); - return issueNumbers; +function arrayToSet(array: any[]): Set { + const set = new Set(); + array.forEach(a => set.add(a)); + return set; } function finalMessage(individualMessages: string[], exampleNumber: number): string { From e402447415be2ae7eb3b31c1c3ff8e20c6d1a3a0 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 16 Feb 2022 17:38:28 -0500 Subject: [PATCH 08/28] fix cdk notices --- packages/aws-cdk/bin/cdk.ts | 6 +++--- packages/aws-cdk/lib/notices.ts | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index fd774c47192b2..549f251ead0e8 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -447,7 +447,9 @@ async function initCommandLine(): Promise<{ configuration: Configuration, value: return cli.acknowledge(args.ID); case 'notices': - SHOW_ALL_NOTICES = true; + await displayNotices({ + acknowledgedIssueNumbers: [], + }); return; case 'init': @@ -539,7 +541,6 @@ function yargsNegativeAlias { if (value == null) { return; } @@ -550,7 +551,6 @@ initCommandLine() } await displayNotices({ acknowledgedIssueNumbers: configuration.context.get('acknowledged-issue-numbers') ?? [], - skipAcknowledgedIssueNumbers: SHOW_ALL_NOTICES, permanentlySuppressed: configuration.context.get('no-notices'), }); }) diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 9d42faa0f2f0a..4ab917a9117f1 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -13,7 +13,6 @@ export interface DisplayNoticesProps { readonly temporarilySuppressed?: boolean; readonly permanentlySuppressed?: boolean; readonly cliVersion?: string; - readonly skipAcknowledgedIssueNumbers?: boolean; } export async function displayNotices(props: DisplayNoticesProps) { @@ -23,7 +22,7 @@ export async function displayNotices(props: DisplayNoticesProps) { const individualMessages = formatNotices(filterNotices(notices, { temporarilySuppressed: props.temporarilySuppressed, permanentlySuppressed: props.permanentlySuppressed, - acknowledgedIssueNumbers: props.skipAcknowledgedIssueNumbers ? new Set() : arrayToSet(props.acknowledgedIssueNumbers), + acknowledgedIssueNumbers: arrayToSet(props.acknowledgedIssueNumbers), })); if (individualMessages.length > 0) { From 42742835c52e690164b39bb99180733448c4520b Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 16 Feb 2022 18:03:37 -0500 Subject: [PATCH 09/28] better cdk notices --- packages/aws-cdk/bin/cdk.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 549f251ead0e8..c9ae449fb86e6 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -447,9 +447,7 @@ async function initCommandLine(): Promise<{ configuration: Configuration, value: return cli.acknowledge(args.ID); case 'notices': - await displayNotices({ - acknowledgedIssueNumbers: [], - }); + SHOW_ALL_NOTICES = true; return; case 'init': @@ -541,17 +539,17 @@ function yargsNegativeAlias { - if (value == null) { return; } if (typeof value === 'string') { data(value); } else if (typeof value === 'number') { process.exitCode = value; } await displayNotices({ - acknowledgedIssueNumbers: configuration.context.get('acknowledged-issue-numbers') ?? [], - permanentlySuppressed: configuration.context.get('no-notices'), + acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? [] : configuration.context.get('acknowledged-issue-numbers') ?? [], + permanentlySuppressed: !configuration.context.get('notices'), }); }) .catch(err => { From 9b68e6fab7448afa8a29c302b5c567780098ca9c Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 16 Feb 2022 19:41:24 -0500 Subject: [PATCH 10/28] specify v2 framework --- packages/aws-cdk/bin/cdk.ts | 3 +- packages/aws-cdk/lib/notices.ts | 53 ++++++++++++++++++++++++++++----- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index c9ae449fb86e6..c4955ce00a7d7 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -548,8 +548,9 @@ initCommandLine() process.exitCode = value; } await displayNotices({ + outdir: configuration.settings.get(['output']) ?? 'cdk.out', acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? [] : configuration.context.get('acknowledged-issue-numbers') ?? [], - permanentlySuppressed: !configuration.context.get('notices'), + permanentlySuppressed: SHOW_ALL_NOTICES ? false : !(configuration.context.get('notices') ?? true), }); }) .catch(err => { diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 4ab917a9117f1..fefc93f76586f 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -9,6 +9,7 @@ import { versionNumber } from './version'; const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json'); export interface DisplayNoticesProps { + readonly outdir: string; readonly acknowledgedIssueNumbers: number[]; readonly temporarilySuppressed?: boolean; readonly permanentlySuppressed?: boolean; @@ -20,6 +21,7 @@ export async function displayNotices(props: DisplayNoticesProps) { const notices = await dataSource.fetch(); const individualMessages = formatNotices(filterNotices(notices, { + outdir: props.outdir, temporarilySuppressed: props.temporarilySuppressed, permanentlySuppressed: props.permanentlySuppressed, acknowledgedIssueNumbers: arrayToSet(props.acknowledgedIssueNumbers), @@ -49,15 +51,18 @@ function finalMessage(individualMessages: string[], exampleNumber: number): stri } export interface FilterNoticeOptions { + outdir?: string, permanentlySuppressed?: boolean, temporarilySuppressed?: boolean, cliVersion?: string, + frameworkVersion?: string, acknowledgedIssueNumbers?: Set, } export function filterNotices(notices: Notice[], options: FilterNoticeOptions): Notice[] { const filter = new NoticeFilter({ cliVersion: options.cliVersion ?? versionNumber(), + frameworkVersion: options.frameworkVersion ?? frameworkVersion(options.outdir ?? 'cdk.out'), temporarilySuppressed: options.temporarilySuppressed ?? false, // coming from a command line option permanentlySuppressed: options.permanentlySuppressed ?? false, // coming from the cdk.json file acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? new Set(), @@ -167,26 +172,39 @@ export interface NoticeFilterProps { permanentlySuppressed: boolean, temporarilySuppressed: boolean, cliVersion: string, + frameworkVersion: string | undefined, acknowledgedIssueNumbers: Set, } export class NoticeFilter { private readonly acknowledgedIssueNumbers: Set; - constructor(private readonly options: NoticeFilterProps) { - this.acknowledgedIssueNumbers = options.acknowledgedIssueNumbers; + constructor(private readonly props: NoticeFilterProps) { + this.acknowledgedIssueNumbers = props.acknowledgedIssueNumbers; } + /** + * Returns true if we should show this notice. + */ apply(notice: Notice): boolean { - if (this.options.permanentlySuppressed - || this.options.temporarilySuppressed + if (this.props.permanentlySuppressed + || this.props.temporarilySuppressed || this.acknowledgedIssueNumbers.has(notice.issueNumber)) { return false; } + return this.applyVersion(notice, 'cli', this.props.cliVersion) || + this.applyVersion(notice, 'framework', this.props.frameworkVersion); + } + + /** + * Returns true if we should show the notice. + */ + private applyVersion(notice: Notice, name: string, compareToVersion: string | undefined) { + if (compareToVersion === undefined) { return true; } - const cliComponent = notice.components.find(component => component.name === 'cli'); - const affectedCliVersion = cliComponent?.version; - return affectedCliVersion != null && semver.satisfies(this.options.cliVersion, affectedCliVersion); + const affectedComponent = notice.components.find(component => component.name === name); + const affectedVersion = affectedComponent?.version; + return affectedVersion != null && semver.satisfies(compareToVersion, affectedVersion); } } @@ -206,3 +224,24 @@ export class FooNoticeFormatter implements NoticeFormatter { ].join('\n\n'); } } + +function frameworkVersion(outdir: string): string | undefined { + const tree = loadTree().tree; + // v2 + if (tree?.constructInfo?.fqn.startsWith('aws-cdk-lib')) { + return tree.constructInfo.version; + } + // v1 + // TODO + return undefined; + + function loadTree() { + try { + return fs.readJSONSync(path.join(outdir, 'tree.json')); + } catch (e) { + print('debug'); + debug(`Failed to get tree.json file: ${e}`); + return {}; + } + } +} \ No newline at end of file From 8a245d2138b82c17ee288c4a360cd3e74559ac95 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Thu, 17 Feb 2022 10:43:55 +0000 Subject: [PATCH 11/28] Added tests for WebsiteNoticeDataSource --- packages/aws-cdk/lib/notices.ts | 25 +++++++------ packages/aws-cdk/test/notices.test.ts | 53 ++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index fefc93f76586f..ba5b4644793d4 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -24,7 +24,7 @@ export async function displayNotices(props: DisplayNoticesProps) { outdir: props.outdir, temporarilySuppressed: props.temporarilySuppressed, permanentlySuppressed: props.permanentlySuppressed, - acknowledgedIssueNumbers: arrayToSet(props.acknowledgedIssueNumbers), + acknowledgedIssueNumbers: new Set(props.acknowledgedIssueNumbers), })); if (individualMessages.length > 0) { @@ -36,12 +36,6 @@ function dataSourceReference(): CachedDataSource { return new CachedDataSource(CACHE_FILE_PATH, new WebsiteNoticeDataSource()); } -function arrayToSet(array: any[]): Set { - const set = new Set(); - array.forEach(a => set.add(a)); - return set; -} - function finalMessage(individualMessages: string[], exampleNumber: number): string { return [ 'NOTICES', @@ -94,7 +88,7 @@ export interface NoticeDataSource { export class WebsiteNoticeDataSource implements NoticeDataSource { fetch(): Promise { - return new Promise((resolve, reject) => { + return new Promise((resolve) => { https.get('https://cli.cdk.dev-tools.aws.dev/notices.json', res => { if (res.statusCode === 200) { res.setEncoding('utf8'); @@ -105,13 +99,20 @@ export class WebsiteNoticeDataSource implements NoticeDataSource { res.on('end', () => { try { const notices = JSON.parse(rawData).notices as Notice[]; - resolve(notices); + resolve(notices ?? []); } catch (e) { - reject(e); + debug(`Failed to parse notices: ${e}`); + resolve([]); } }); + res.on('error', e => { + debug(`Failed to fetch notices: ${e}`); + resolve([]); + }); + } else { + debug(`Failed to fetch notices. Status code: ${res.statusCode}`); + resolve([]); } - // Otherwise, we just ignore the result and move on }); }); } @@ -200,7 +201,7 @@ export class NoticeFilter { * Returns true if we should show the notice. */ private applyVersion(notice: Notice, name: string, compareToVersion: string | undefined) { - if (compareToVersion === undefined) { return true; } + if (compareToVersion === undefined) { return false; } const affectedComponent = notice.components.find(component => component.name === name); const affectedVersion = affectedComponent?.version; diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index 9d403254d7d63..6b727ef4f9f38 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -1,4 +1,5 @@ -import { formatNotices, filterNotices } from '../lib/notices'; +import * as nock from 'nock'; +import { formatNotices, filterNotices, WebsiteNoticeDataSource } from '../lib/notices'; const BASIC_NOTICE = { title: 'Toggling off auto_delete_objects for Bucket empties the bucket', @@ -71,4 +72,54 @@ describe('cli notices', () => { // TODO }); }); + + describe(WebsiteNoticeDataSource, () => { + const dataSource = new WebsiteNoticeDataSource(); + + test('returns data when download succeeds', async () => { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .reply(200, { + notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], + }); + + const result = await dataSource.fetch(); + + expect(result).toEqual([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + }); + + test('returns empty array when the server returns an unexpected status code', async () => { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .reply(500, { + notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], + }); + + const result = await dataSource.fetch(); + + expect(result).toEqual([]); + }); + + test('returns empty array when the server returns an unexpected structure', async () => { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .reply(200, { + foo: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], + }); + + const result = await dataSource.fetch(); + + expect(result).toEqual([]); + }); + + test('returns empty array when the server returns invalid json', async () => { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .reply(200, '-09aiskjkj838'); + + const result = await dataSource.fetch(); + + expect(result).toEqual([]); + }); + }); }); From 8546c001eb577a4cf55db2c8e8b354744e42b039 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Thu, 17 Feb 2022 10:52:52 +0000 Subject: [PATCH 12/28] Refactored tests --- packages/aws-cdk/test/notices.test.ts | 46 +++++++++++---------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index 6b727ef4f9f38..a90064a2d4123 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -1,5 +1,5 @@ import * as nock from 'nock'; -import { formatNotices, filterNotices, WebsiteNoticeDataSource } from '../lib/notices'; +import { filterNotices, formatNotices, Notice, WebsiteNoticeDataSource } from '../lib/notices'; const BASIC_NOTICE = { title: 'Toggling off auto_delete_objects for Bucket empties the bucket', @@ -77,49 +77,41 @@ describe('cli notices', () => { const dataSource = new WebsiteNoticeDataSource(); test('returns data when download succeeds', async () => { - nock('https://cli.cdk.dev-tools.aws.dev') - .get('/notices.json') - .reply(200, { - notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], - }); - - const result = await dataSource.fetch(); + const result = await mockCall(200, { + notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], + }); expect(result).toEqual([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); }); test('returns empty array when the server returns an unexpected status code', async () => { - nock('https://cli.cdk.dev-tools.aws.dev') - .get('/notices.json') - .reply(500, { - notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], - }); - - const result = await dataSource.fetch(); + const result = await mockCall(500, { + notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], + }); expect(result).toEqual([]); }); test('returns empty array when the server returns an unexpected structure', async () => { - nock('https://cli.cdk.dev-tools.aws.dev') - .get('/notices.json') - .reply(200, { - foo: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], - }); - - const result = await dataSource.fetch(); + const result = await mockCall(200, { + foo: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], + }); expect(result).toEqual([]); }); test('returns empty array when the server returns invalid json', async () => { - nock('https://cli.cdk.dev-tools.aws.dev') - .get('/notices.json') - .reply(200, '-09aiskjkj838'); - - const result = await dataSource.fetch(); + const result = await mockCall(200, '-09aiskjkj838'); expect(result).toEqual([]); }); + + function mockCall(statusCode: number, body: any): Promise { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .reply(statusCode, body); + + return dataSource.fetch(); + } }); }); From 2642b3a3ede6a18ba0bb15d4beafd996f518ec5e Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Thu, 17 Feb 2022 11:34:46 +0000 Subject: [PATCH 13/28] Getting the framework version for v1 --- packages/aws-cdk/lib/notices.ts | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index ba5b4644793d4..a5d32e3f9f8a3 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -65,8 +65,7 @@ export function filterNotices(notices: Notice[], options: FilterNoticeOptions): } export function formatNotices(notices: Notice[]): string[] { - const formatter = new FooNoticeFormatter(); - return notices.map(notice => formatter.apply(notice)); + return notices.map(formatNotice); } export interface Component { @@ -209,31 +208,22 @@ export class NoticeFilter { } } -export interface NoticeFormatter { - apply(notice: Notice): string, -} - -// TODO rename this -export class FooNoticeFormatter implements NoticeFormatter { - apply(notice: Notice): string { - const componentsValue = notice.components.map(c => `${c.name}: ${c.version}`).join(', '); - return [ - `${notice.issueNumber}\t${notice.title}`, - `\tOverview: ${notice.overview}`, - `\tAffected versions: ${componentsValue}`, - `\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`, - ].join('\n\n'); - } +function formatNotice(notice: Notice): string { + const componentsValue = notice.components.map(c => `${c.name}: ${c.version}`).join(', '); + return [ + `${notice.issueNumber}\t${notice.title}`, + `\tOverview: ${notice.overview}`, + `\tAffected versions: ${componentsValue}`, + `\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`, + ].join('\n\n'); } function frameworkVersion(outdir: string): string | undefined { const tree = loadTree().tree; - // v2 - if (tree?.constructInfo?.fqn.startsWith('aws-cdk-lib')) { + if (tree?.constructInfo?.fqn.startsWith('aws-cdk-lib') + || tree?.constructInfo?.fqn.startsWith('@aws-cdk/core')) { return tree.constructInfo.version; } - // v1 - // TODO return undefined; function loadTree() { From bcf9c13e5ea770ff07345d07498c03439dcd054a Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Thu, 17 Feb 2022 13:35:42 +0000 Subject: [PATCH 14/28] Added tests for CachedDataSource --- packages/aws-cdk/test/notices.test.ts | 65 ++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index a90064a2d4123..cd066f199d9fe 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -1,5 +1,8 @@ +import * as os from 'os'; +import * as path from 'path'; +import * as fs from 'fs-extra'; import * as nock from 'nock'; -import { filterNotices, formatNotices, Notice, WebsiteNoticeDataSource } from '../lib/notices'; +import { CachedDataSource, filterNotices, formatNotices, Notice, WebsiteNoticeDataSource } from '../lib/notices'; const BASIC_NOTICE = { title: 'Toggling off auto_delete_objects for Bucket empties the bucket', @@ -114,4 +117,64 @@ describe('cli notices', () => { return dataSource.fetch(); } }); + + describe(CachedDataSource, () => { + const fileName = path.join(os.tmpdir(), 'cache.json'); + const cachedData = [BASIC_NOTICE]; + const freshData = [MULTIPLE_AFFECTED_VERSIONS_NOTICE]; + + beforeEach(() => { + fs.writeFileSync(fileName, ''); + }); + + test('retrieves data from the delegate cache when the file is empty', async () => { + const dataSource = dataSourceWithDelegateReturning(freshData); + + const notices = await dataSource.fetch(); + + expect(notices).toEqual(freshData); + }); + + test('retrieves data from the file when the data is still valid', async () => { + fs.writeJsonSync(fileName, { + notices: cachedData, + expiration: Date.now() + 10000, + }); + const dataSource = dataSourceWithDelegateReturning(freshData); + + const notices = await dataSource.fetch(); + + expect(notices).toEqual(cachedData); + }); + + test('retrieves data from the delegate when the data is expired', async () => { + fs.writeJsonSync(fileName, { + notices: cachedData, + expiration: 0, + }); + const dataSource = dataSourceWithDelegateReturning(freshData); + + const notices = await dataSource.fetch(); + + expect(notices).toEqual(freshData); + }); + + test('retrieves data from the delegate when the file cannot be read', async () => { + const nonExistingFile = path.join(os.tmpdir(), 'cache.json'); + const dataSource = dataSourceWithDelegateReturning(freshData, nonExistingFile); + + const notices = await dataSource.fetch(); + + expect(notices).toEqual(freshData); + }); + + function dataSourceWithDelegateReturning(notices: Notice[], file: string = fileName) { + const delegate = { + fetch: jest.fn(), + }; + + delegate.fetch.mockResolvedValue(notices); + return new CachedDataSource(file, delegate); + } + }); }); From 527eb2b9e07e543225a80d2c176718e93809bfdb Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Thu, 17 Feb 2022 13:42:47 +0000 Subject: [PATCH 15/28] extract generateMessage function --- packages/aws-cdk/lib/notices.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index a5d32e3f9f8a3..31273b7489d3b 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -18,7 +18,10 @@ export interface DisplayNoticesProps { export async function displayNotices(props: DisplayNoticesProps) { const dataSource = dataSourceReference(); + await generateMessage(dataSource, props, print); +} +export async function generateMessage(dataSource: NoticeDataSource, props: DisplayNoticesProps, cb: (msg: string) => void) { const notices = await dataSource.fetch(); const individualMessages = formatNotices(filterNotices(notices, { outdir: props.outdir, @@ -28,7 +31,7 @@ export async function displayNotices(props: DisplayNoticesProps) { })); if (individualMessages.length > 0) { - print(finalMessage(individualMessages, notices[0].issueNumber)); + cb(finalMessage(individualMessages, notices[0].issueNumber)); } } From 0657b9fee3a343f87a6932588126c3b8ab630131 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Thu, 17 Feb 2022 12:22:05 -0500 Subject: [PATCH 16/28] add no-notices option --- packages/aws-cdk/bin/cdk.ts | 2 ++ packages/aws-cdk/lib/notices.ts | 30 +++++++++++++++++++++++++----- packages/aws-cdk/lib/settings.ts | 1 + 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index c4955ce00a7d7..990f84d22824d 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -67,6 +67,7 @@ async function parseCommandLineArguments() { .option('staging', { type: 'boolean', desc: 'Copy assets to the output directory (use --no-staging to disable, needed for local debugging the source files with SAM CLI)', default: true }) .option('output', { type: 'string', alias: 'o', desc: 'Emits the synthesized cloud assembly into a directory (default: cdk.out)', requiresArg: true }) .option('no-color', { type: 'boolean', desc: 'Removes colors and other style from console output', default: false }) + .option('notices', { type: 'boolean', desc: 'Show relevant notices', default: true }) .command(['list [STACKS..]', 'ls [STACKS..]'], 'Lists all stacks in the app', yargs => yargs .option('long', { type: 'boolean', default: false, alias: 'l', desc: 'Display environment information for each stack' }), ) @@ -551,6 +552,7 @@ initCommandLine() outdir: configuration.settings.get(['output']) ?? 'cdk.out', acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? [] : configuration.context.get('acknowledged-issue-numbers') ?? [], permanentlySuppressed: SHOW_ALL_NOTICES ? false : !(configuration.context.get('notices') ?? true), + temporarilySuppressed: !configuration.settings.get(['notices']), }); }) .catch(err => { diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 31273b7489d3b..8a27c63f577a2 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -9,11 +9,31 @@ import { versionNumber } from './version'; const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json'); export interface DisplayNoticesProps { + /** + * The cloud assembly directory. Usually 'cdk.out'. + */ readonly outdir: string; + + /** + * Issue numbers of notices that have been acknowledged by a user + * of the current CDK repository. These notices will be skipped. + */ readonly acknowledgedIssueNumbers: number[]; + + /** + * Setting that comes from the command-line option. + * For example, `cdk synth --no-notices`. + */ readonly temporarilySuppressed?: boolean; + + /** + * Setting that comes from cdk.json. For example, + * + * "context": { + * "notices": false + * } + */ readonly permanentlySuppressed?: boolean; - readonly cliVersion?: string; } export async function displayNotices(props: DisplayNoticesProps) { @@ -60,8 +80,8 @@ export function filterNotices(notices: Notice[], options: FilterNoticeOptions): const filter = new NoticeFilter({ cliVersion: options.cliVersion ?? versionNumber(), frameworkVersion: options.frameworkVersion ?? frameworkVersion(options.outdir ?? 'cdk.out'), - temporarilySuppressed: options.temporarilySuppressed ?? false, // coming from a command line option - permanentlySuppressed: options.permanentlySuppressed ?? false, // coming from the cdk.json file + temporarilySuppressed: options.temporarilySuppressed ?? false, + permanentlySuppressed: options.permanentlySuppressed ?? false, acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? new Set(), }); return notices.filter(notice => filter.apply(notice)); @@ -206,8 +226,8 @@ export class NoticeFilter { if (compareToVersion === undefined) { return false; } const affectedComponent = notice.components.find(component => component.name === name); - const affectedVersion = affectedComponent?.version; - return affectedVersion != null && semver.satisfies(compareToVersion, affectedVersion); + const affectedRange = affectedComponent?.version; + return affectedRange != null && semver.satisfies(compareToVersion, affectedRange); } } diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index 38723bffc0bc3..ddb28be756292 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -287,6 +287,7 @@ export class Settings { bundlingStacks, lookups: argv.lookups, rollback: argv.rollback, + notices: argv.notices, }); } From 977c0614540e78b0d8d758a9ab6d38b0f0b653c4 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Thu, 17 Feb 2022 12:53:32 -0500 Subject: [PATCH 17/28] finish merge --- packages/aws-cdk/lib/cli.ts | 39 ++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index b173ea12db43e..79a1a245ece86 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -19,6 +19,7 @@ import { realHandler as doctor } from '../lib/commands/doctor'; import { RequireApproval } from '../lib/diff'; import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib/init'; import { data, debug, error, print, setLogLevel } from '../lib/logging'; +import { displayNotices } from '../lib/notices'; import { PluginHost } from '../lib/plugin'; import { serializeStructure } from '../lib/serialize'; import { Command, Configuration, Settings } from '../lib/settings'; @@ -71,6 +72,7 @@ async function parseCommandLineArguments() { .option('role-arn', { type: 'string', alias: 'r', desc: 'ARN of Role to use when invoking CloudFormation', default: undefined, requiresArg: true }) .option('staging', { type: 'boolean', desc: 'Copy assets to the output directory (use --no-staging to disable, needed for local debugging the source files with SAM CLI)', default: true }) .option('output', { type: 'string', alias: 'o', desc: 'Emits the synthesized cloud assembly into a directory (default: cdk.out)', requiresArg: true }) + .option('notices', { type: 'boolean', desc: 'Show relevant notices', default: true }) .option('no-color', { type: 'boolean', desc: 'Removes colors and other style from console output', default: false }) .command(['list [STACKS..]', 'ls [STACKS..]'], 'Lists all stacks in the app', yargs => yargs .option('long', { type: 'boolean', default: false, alias: 'l', desc: 'Display environment information for each stack' }), @@ -193,6 +195,8 @@ async function parseCommandLineArguments() { .option('security-only', { type: 'boolean', desc: 'Only diff for broadened security changes', default: false }) .option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff', default: false })) .command('metadata [STACK]', 'Returns all metadata associated with this stack') + .command('acknowledge [ID]', 'Acknowledge a notice so that it does not show up anymore') + .command('notices', 'Returns a list of relevant notices') .command('init [TEMPLATE]', 'Create a new, empty CDK project from a template.', yargs => yargs .option('language', { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: initTemplateLanguages }) .option('list', { type: 'boolean', desc: 'List the available templates' }) @@ -226,7 +230,7 @@ if (!process.stdout.isTTY) { process.env.FORCE_COLOR = '0'; } -async function initCommandLine() { +async function initCommandLine(): Promise<{ configuration: Configuration, value: any }> { const argv = await parseCommandLineArguments(); if (argv.verbose) { setLogLevel(argv.verbose); @@ -315,11 +319,20 @@ async function initCommandLine() { } if (typeof returnValue === 'object') { - return toJsonOrYaml(returnValue); + return { + value: toJsonOrYaml(returnValue), + configuration, + }; } else if (typeof returnValue === 'string') { - return returnValue; + return { + value: returnValue, + configuration, + }; } else { - return returnValue; + return { + value: returnValue, + configuration, + }; } } finally { await version.displayVersionMessage(); @@ -463,10 +476,16 @@ async function initCommandLine() { return cli.synth(args.STACKS, true, args.quiet, args.validation); } - case 'metadata': return cli.metadata(args.STACK); + case 'acknowledge': + return cli.acknowledge(args.ID); + + case 'notices': + SHOW_ALL_NOTICES = true; + return; + case 'init': const language = configuration.settings.get(['language']); if (args.list) { @@ -556,15 +575,21 @@ function yargsNegativeAlias { - if (value == null) { return; } + .then(async ({ value, configuration }) => { if (typeof value === 'string') { data(value); } else if (typeof value === 'number') { process.exitCode = value; } + await displayNotices({ + outdir: configuration.settings.get(['output']) ?? 'cdk.out', + acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? [] : configuration.context.get('acknowledged-issue-numbers') ?? [], + permanentlySuppressed: SHOW_ALL_NOTICES ? false : !(configuration.context.get('notices') ?? true), + temporarilySuppressed: !configuration.settings.get(['notices']), + }); }) .catch(err => { error(err.message); From 303b9532c4710d3b7e5b75609591bcd82cdbcc97 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Thu, 17 Feb 2022 13:42:40 -0500 Subject: [PATCH 18/28] add readme section --- packages/aws-cdk/README.md | 120 ++++++++++++++++++++++++++++---- packages/aws-cdk/lib/notices.ts | 2 +- 2 files changed, 109 insertions(+), 13 deletions(-) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 027dc574572f5..686bcf559a26b 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -11,18 +11,20 @@ The AWS CDK Toolkit provides the `cdk` command-line interface that can be used to work with AWS CDK applications. -Command | Description -----------------------------------|------------------------------------------------------------------------------------- -[`cdk docs`](#cdk-docs) | Access the online documentation -[`cdk init`](#cdk-init) | Start a new CDK project (app or library) -[`cdk list`](#cdk-list) | List stacks in an application -[`cdk synth`](#cdk-synthesize) | Synthesize a CDK app to CloudFormation template(s) -[`cdk diff`](#cdk-diff) | Diff stacks against current state -[`cdk deploy`](#cdk-deploy) | Deploy a stack into an AWS account -[`cdk watch`](#cdk-watch) | Watches a CDK app for deployable and hotswappable changes -[`cdk destroy`](#cdk-destroy) | Deletes a stack from an AWS account -[`cdk bootstrap`](#cdk-bootstrap) | Deploy a toolkit stack to support deploying large stacks & artifacts -[`cdk doctor`](#cdk-doctor) | Inspect the environment and produce information useful for troubleshooting +Command | Description +--------------------------------------|--------------------------------------------------------------------------------- +[`cdk docs`](#cdk-docs) | Access the online documentation +[`cdk init`](#cdk-init) | Start a new CDK project (app or library) +[`cdk list`](#cdk-list) | List stacks in an application +[`cdk synth`](#cdk-synthesize) | Synthesize a CDK app to CloudFormation template(s) +[`cdk diff`](#cdk-diff) | Diff stacks against current state +[`cdk deploy`](#cdk-deploy) | Deploy a stack into an AWS account +[`cdk watch`](#cdk-watch) | Watches a CDK app for deployable and hotswappable changes +[`cdk destroy`](#cdk-destroy) | Deletes a stack from an AWS account +[`cdk bootstrap`](#cdk-bootstrap) | Deploy a toolkit stack to support deploying large stacks & artifacts +[`cdk doctor`](#cdk-doctor) | Inspect the environment and produce information useful for troubleshooting +[`cdk acknowledge`](#cdk-acknowledge) | Acknowledge (and hide) a notice by issue number +[`cdk notices`](#cdk-notices) | List all relevant notices for the application This module is part of the [AWS Cloud Development Kit](https://github.com/aws/aws-cdk) project. @@ -503,6 +505,100 @@ $ cdk doctor - AWS_SDK_LOAD_CONFIG = 1 ``` +## Notices + +> This feature exists on CDK CLI version 2.14.0 and up. + +CDK Notices are important messages regarding security vulnerabilities, regressions, and usage of unsupported +versions. Relevant notices appear on every command by default. For example, + +```console +$ cdk deploy + +... # Normal output of the command + +NOTICES + +16603 Toggling off auto_delete_objects for Bucket empties the bucket + + Overview: If a stack is deployed with an S3 bucket with + auto_delete_objects=True, and then re-deployed with + auto_delete_objects=False, all the objects in the bucket + will be deleted. + + Affected versions: <1.126.0. + + More information at: https://github.com/aws/aws-cdk/issues/16603 + +17061 Error when building EKS cluster with monocdk import + + Overview: When using monocdk/aws-eks to build a stack containing + an EKS cluster, error is thrown about missing + lambda-layer-node-proxy-agent/layer/package.json. + + Affected versions: >=1.126.0 <=1.130.0. + + More information at: https://github.com/aws/aws-cdk/issues/17061 + +If you don’t want to see an notice anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 16603". +``` + +You can suppress warnings in a variety of ways: + +- per individual execution: + + `cdk deploy --no-notices` + +- disable all notices indefinitely through context in `cdk.json`: + + ```json + { + "context": { + "notices": false + } + } + ``` + +- acknowleding individual notices via `cdk acknowledge` (see below). + +### `cdk acknowledge` + +To hide a particular notice that has been addressed or does not apply, call `cdk acknowledge` with the ID of +the notice: + +```console +$cdk acknowledge 16603 +``` + +> Please note that the acknowledgements are made project by project. If you acknowledge an notice in one CDK +> project, it will still appear on other projects when you run any CDK commands, unless you have suppressed +> or disabled notices. + + +### `cdk notices` + +List the notices that are relevant to the current CDK repository, regardless of context flags or notices that +have been acknowledged: + +```console +$ cdk notices + +NOTICES + +16603 Toggling off auto_delete_objects for Bucket empties the bucket + + Overview: if a stack is deployed with an S3 bucket with auto_delete_objects=True, and then re-deployed with auto_delete_objects=False, all the objects in the bucket will be deleted. + + Affected versions: framework: <=2.15.0 >=2.10.0 + + More information at: https://github.com/aws/aws-cdk/issues/16603 + +If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge 16603". +``` + +This command returns zero if there is no notice and non-zero otherwise. Users can then plug this into a +pipeline approval workflow and expect manual review if there are any notices. + ### Bundling By default asset bundling is skipped for `cdk list` and `cdk destroy`. For `cdk deploy`, `cdk diff` diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 8a27c63f577a2..dcbd1953490ed 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -61,7 +61,7 @@ function dataSourceReference(): CachedDataSource { function finalMessage(individualMessages: string[], exampleNumber: number): string { return [ - 'NOTICES', + '\nNOTICES', ...individualMessages, `If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge ${exampleNumber}".`, ].join('\n\n'); From f307cc00a079cf5b6ce272a369e44c462551b2fd Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Fri, 18 Feb 2022 11:26:59 +0000 Subject: [PATCH 19/28] Added more tests --- packages/aws-cdk/lib/notices.ts | 6 +- .../built-with-1_144_0/tree.json | 21 ++++ .../built-with-2_12_0/tree.json | 21 ++++ packages/aws-cdk/test/notices.test.ts | 103 +++++++++++++++++- 4 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 packages/aws-cdk/test/cloud-assembly-trees/built-with-1_144_0/tree.json create mode 100644 packages/aws-cdk/test/cloud-assembly-trees/built-with-2_12_0/tree.json diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index dcbd1953490ed..52af3df03c28c 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -207,7 +207,7 @@ export class NoticeFilter { } /** - * Returns true if we should show this notice. + * Returns true iff we should show this notice. */ apply(notice: Notice): boolean { if (this.props.permanentlySuppressed @@ -220,7 +220,7 @@ export class NoticeFilter { } /** - * Returns true if we should show the notice. + * Returns true iff we should show the notice. */ private applyVersion(notice: Notice, name: string, compareToVersion: string | undefined) { if (compareToVersion === undefined) { return false; } @@ -243,6 +243,7 @@ function formatNotice(notice: Notice): string { function frameworkVersion(outdir: string): string | undefined { const tree = loadTree().tree; + if (tree?.constructInfo?.fqn.startsWith('aws-cdk-lib') || tree?.constructInfo?.fqn.startsWith('@aws-cdk/core')) { return tree.constructInfo.version; @@ -253,7 +254,6 @@ function frameworkVersion(outdir: string): string | undefined { try { return fs.readJSONSync(path.join(outdir, 'tree.json')); } catch (e) { - print('debug'); debug(`Failed to get tree.json file: ${e}`); return {}; } diff --git a/packages/aws-cdk/test/cloud-assembly-trees/built-with-1_144_0/tree.json b/packages/aws-cdk/test/cloud-assembly-trees/built-with-1_144_0/tree.json new file mode 100644 index 0000000000000..a7b0c9e29c2f3 --- /dev/null +++ b/packages/aws-cdk/test/cloud-assembly-trees/built-with-1_144_0/tree.json @@ -0,0 +1,21 @@ +{ + "version": "tree-0.1", + "tree": { + "id": "App", + "path": "", + "children": { + "Tree": { + "id": "Tree", + "path": "Tree", + "constructInfo": { + "fqn": "@aws-cdk/core.Construct", + "version": "1.144.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/core.App", + "version": "1.144.0" + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk/test/cloud-assembly-trees/built-with-2_12_0/tree.json b/packages/aws-cdk/test/cloud-assembly-trees/built-with-2_12_0/tree.json new file mode 100644 index 0000000000000..41c11ceaf7868 --- /dev/null +++ b/packages/aws-cdk/test/cloud-assembly-trees/built-with-2_12_0/tree.json @@ -0,0 +1,21 @@ +{ + "version": "tree-0.1", + "tree": { + "id": "App", + "path": "", + "children": { + "Tree": { + "id": "Tree", + "path": "Tree", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.0.64" + } + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.App", + "version": "2.12.0" + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index cd066f199d9fe..e3a0de37af0c9 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -2,7 +2,14 @@ import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs-extra'; import * as nock from 'nock'; -import { CachedDataSource, filterNotices, formatNotices, Notice, WebsiteNoticeDataSource } from '../lib/notices'; +import { + CachedDataSource, + filterNotices, + formatNotices, + generateMessage, + Notice, + WebsiteNoticeDataSource, +} from '../lib/notices'; const BASIC_NOTICE = { title: 'Toggling off auto_delete_objects for Bucket empties the bucket', @@ -26,6 +33,17 @@ const MULTIPLE_AFFECTED_VERSIONS_NOTICE = { schemaVersion: '1', }; +const FRAMEWORK_2_1_0_AFFECTED_NOTICE = { + title: 'Regression on module foobar', + issueNumber: 1234, + overview: 'Some bug description', + components: [{ + name: 'framework', + version: '<= 2.1.0', + }], + schemaVersion: '1', +}; + describe('cli notices', () => { describe(formatNotices, () => { test('correct format', () => { @@ -72,7 +90,23 @@ describe('cli notices', () => { }); test('correctly filter notices on framework', () => { - // TODO + const notices = [FRAMEWORK_2_1_0_AFFECTED_NOTICE]; + + expect(filterNotices(notices, { + frameworkVersion: '2.0.0', + })).toEqual([FRAMEWORK_2_1_0_AFFECTED_NOTICE]); + + expect(filterNotices(notices, { + frameworkVersion: '2.2.0', + })).toEqual([]); + + expect(filterNotices(notices, { + outdir: path.join(__dirname, 'cloud-assembly-trees/built-with-2_12_0'), + })).toEqual([]); + + expect(filterNotices(notices, { + outdir: path.join(__dirname, 'cloud-assembly-trees/built-with-1_144_0'), + })).toEqual([FRAMEWORK_2_1_0_AFFECTED_NOTICE]); }); }); @@ -177,4 +211,69 @@ describe('cli notices', () => { return new CachedDataSource(file, delegate); } }); + + describe(generateMessage, () => { + test('does not show anything when there are no notices', async () => { + const dataSource = createDataSource(); + const print = jest.fn(); + dataSource.fetch.mockResolvedValue([]); + + await generateMessage(dataSource, { + temporarilySuppressed: false, + permanentlySuppressed: false, + acknowledgedIssueNumbers: [], + outdir: '/tmp', + }, print); + + expect(print).not.toHaveBeenCalled(); + }); + + test('does not show anything when no notices pass the filter', async () => { + const dataSource = createDataSource(); + const print = jest.fn(); + dataSource.fetch.mockResolvedValue([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + + await generateMessage(dataSource, { + temporarilySuppressed: true, + permanentlySuppressed: false, + acknowledgedIssueNumbers: [], + outdir: '/tmp', + }, print); + + expect(print).not.toHaveBeenCalled(); + }); + + test('shows notices that pass the filter', async () => { + const dataSource = createDataSource(); + const print = jest.fn(); + dataSource.fetch.mockResolvedValue([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + + await generateMessage(dataSource, { + temporarilySuppressed: false, + permanentlySuppressed: false, + acknowledgedIssueNumbers: [17061], + outdir: '/tmp', + }, print); + + expect(print).toHaveBeenCalledTimes(1); + expect(print).toHaveBeenCalledWith(` +NOTICES + +16603\tToggling off auto_delete_objects for Bucket empties the bucket + +\tOverview: If a stack is deployed with an S3 bucket with auto_delete_objects=True, and then re-deployed with auto_delete_objects=False, all the objects in the bucket will be deleted. + +\tAffected versions: cli: <=1.126.0 + +\tMore information at: https://github.com/aws/aws-cdk/issues/16603 + +If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge 16603".`); + }); + + function createDataSource() { + return { + fetch: jest.fn(), + }; + } + }); }); From d9ba71f1c61c59ce2433823dccd6271ed7485236 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 21 Feb 2022 08:47:43 +0000 Subject: [PATCH 20/28] Ignoring the cache when showing all notices --- packages/aws-cdk/lib/cli.ts | 1 + packages/aws-cdk/lib/notices.ts | 19 ++++++++++++++----- packages/aws-cdk/test/notices.test.ts | 16 ++++++++++++++-- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 79a1a245ece86..4c16d9366ee30 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -589,6 +589,7 @@ export function cli() { acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? [] : configuration.context.get('acknowledged-issue-numbers') ?? [], permanentlySuppressed: SHOW_ALL_NOTICES ? false : !(configuration.context.get('notices') ?? true), temporarilySuppressed: !configuration.settings.get(['notices']), + ignoreCache: SHOW_ALL_NOTICES, }); }) .catch(err => { diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 52af3df03c28c..36ad7e853fad7 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -34,10 +34,18 @@ export interface DisplayNoticesProps { * } */ readonly permanentlySuppressed?: boolean; + + /** + * Whether cached notices should be ignored. Setting this property + * to true will force the CLI to download fresh data + * + * @default false + */ + readonly ignoreCache?: boolean; } export async function displayNotices(props: DisplayNoticesProps) { - const dataSource = dataSourceReference(); + const dataSource = dataSourceReference(props.ignoreCache ?? false); await generateMessage(dataSource, props, print); } @@ -55,8 +63,8 @@ export async function generateMessage(dataSource: NoticeDataSource, props: Displ } } -function dataSourceReference(): CachedDataSource { - return new CachedDataSource(CACHE_FILE_PATH, new WebsiteNoticeDataSource()); +function dataSourceReference(ignoreCache: boolean): NoticeDataSource { + return new CachedDataSource(CACHE_FILE_PATH, new WebsiteNoticeDataSource(), ignoreCache); } function finalMessage(individualMessages: string[], exampleNumber: number): string { @@ -150,7 +158,8 @@ const TIME_TO_LIVE = 60 * 60 * 1000; // 1 hour export class CachedDataSource implements NoticeDataSource { constructor( private readonly fileName: string, - private readonly dataSource: NoticeDataSource) { + private readonly dataSource: NoticeDataSource, + private readonly skipCache?: boolean) { } async fetch(): Promise { @@ -158,7 +167,7 @@ export class CachedDataSource implements NoticeDataSource { const notices = cachedData.notices; const expiration = cachedData.expiration ?? 0; - if (Date.now() > expiration) { + if (Date.now() > expiration || this.skipCache) { const freshData = { expiration: Date.now() + TIME_TO_LIVE, notices: await this.dataSource.fetch(), diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index e3a0de37af0c9..5f06c13e1cae4 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -202,13 +202,25 @@ describe('cli notices', () => { expect(notices).toEqual(freshData); }); - function dataSourceWithDelegateReturning(notices: Notice[], file: string = fileName) { + test('retrieved data from the delegate when it is configured to ignore the cache', async () => { + fs.writeJsonSync(fileName, { + notices: cachedData, + expiration: Date.now() + 10000, + }); + const dataSource = dataSourceWithDelegateReturning(freshData, fileName, true); + + const notices = await dataSource.fetch(); + + expect(notices).toEqual(freshData); + }); + + function dataSourceWithDelegateReturning(notices: Notice[], file: string = fileName, ignoreCache: boolean = false) { const delegate = { fetch: jest.fn(), }; delegate.fetch.mockResolvedValue(notices); - return new CachedDataSource(file, delegate); + return new CachedDataSource(file, delegate, ignoreCache); } }); From 17cbce3d743dfba45f51e086426aa51b1ff62e28 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 21 Feb 2022 08:52:48 +0000 Subject: [PATCH 21/28] Removed paragraph about zero and non-zero return codes. --- packages/aws-cdk/README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 686bcf559a26b..3c05f296e2dc9 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -596,9 +596,6 @@ NOTICES If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge 16603". ``` -This command returns zero if there is no notice and non-zero otherwise. Users can then plug this into a -pipeline approval workflow and expect manual review if there are any notices. - ### Bundling By default asset bundling is skipped for `cdk list` and `cdk destroy`. For `cdk deploy`, `cdk diff` From a74eadcfe0f013da7fabb3a2a2098d8328729e6b Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 21 Feb 2022 18:23:42 +0000 Subject: [PATCH 22/28] Simplified handling of notices/no-notices flag and some other minor improvements --- packages/aws-cdk/lib/cdk-toolkit.ts | 12 +-- packages/aws-cdk/lib/cli.ts | 100 +++++++++------------- packages/aws-cdk/lib/notices.ts | 59 ++++--------- packages/aws-cdk/test/cdk-toolkit.test.ts | 17 +++- packages/aws-cdk/test/notices.test.ts | 34 ++------ 5 files changed, 85 insertions(+), 137 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 206284db0990a..d8cdd7c1dd1ee 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -16,7 +16,7 @@ import { CloudWatchLogEventMonitor } from './api/logs/logs-monitor'; import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; import { data, debug, error, highlight, print, success, warning } from './logging'; -import { deserializeStructure } from './serialize'; +import { deserializeStructure, serializeStructure } from './serialize'; import { Configuration, PROJECT_CONFIG } from './settings'; import { numberFromBool, partition } from './util'; @@ -74,9 +74,9 @@ export class CdkToolkit { constructor(private readonly props: CdkToolkitProps) { } - public async metadata(stackName: string) { + public async metadata(stackName: string, json: boolean) { const stacks = await this.selectSingleStackByName(stackName); - return stacks.firstStack.manifest.metadata ?? {}; + data(serializeStructure(stacks.firstStack.manifest.metadata ?? {}, json)); } public async acknowledge(noticeId: string) { @@ -424,13 +424,13 @@ export class CdkToolkit { * OUTPUT: If more than one stack ends up being selected, an output directory * should be supplied, where the templates will be written. */ - public async synth(stackNames: string[], exclusively: boolean, quiet: boolean, autoValidate?: boolean): Promise { + public async synth(stackNames: string[], exclusively: boolean, quiet: boolean, autoValidate?: boolean, json?: boolean): Promise { const stacks = await this.selectStacksForDiff(stackNames, exclusively, autoValidate); // if we have a single stack, print it to STDOUT if (stacks.stackCount === 1) { if (!quiet) { - return stacks.firstStack.template; + data(serializeStructure(stacks.firstStack.template, json ?? false)); } return undefined; } @@ -444,7 +444,7 @@ export class CdkToolkit { // behind an environment variable. const isIntegMode = process.env.CDK_INTEG_MODE === '1'; if (isIntegMode) { - return stacks.stackArtifacts.map(s => s.template); + data(serializeStructure(stacks.stackArtifacts.map(s => s.template), json ?? false)); } // not outputting template to stdout, let's explain things to the user a little bit... diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 4c16d9366ee30..a975a512f7d4a 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -21,7 +21,6 @@ import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib import { data, debug, error, print, setLogLevel } from '../lib/logging'; import { displayNotices } from '../lib/notices'; import { PluginHost } from '../lib/plugin'; -import { serializeStructure } from '../lib/serialize'; import { Command, Configuration, Settings } from '../lib/settings'; import * as version from '../lib/version'; @@ -72,7 +71,7 @@ async function parseCommandLineArguments() { .option('role-arn', { type: 'string', alias: 'r', desc: 'ARN of Role to use when invoking CloudFormation', default: undefined, requiresArg: true }) .option('staging', { type: 'boolean', desc: 'Copy assets to the output directory (use --no-staging to disable, needed for local debugging the source files with SAM CLI)', default: true }) .option('output', { type: 'string', alias: 'o', desc: 'Emits the synthesized cloud assembly into a directory (default: cdk.out)', requiresArg: true }) - .option('notices', { type: 'boolean', desc: 'Show relevant notices', default: true }) + .option('notices', { type: 'boolean', desc: 'Show relevant notices' }) .option('no-color', { type: 'boolean', desc: 'Removes colors and other style from console output', default: false }) .command(['list [STACKS..]', 'ls [STACKS..]'], 'Lists all stacks in the app', yargs => yargs .option('long', { type: 'boolean', default: false, alias: 'l', desc: 'Display environment information for each stack' }), @@ -195,7 +194,7 @@ async function parseCommandLineArguments() { .option('security-only', { type: 'boolean', desc: 'Only diff for broadened security changes', default: false }) .option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff', default: false })) .command('metadata [STACK]', 'Returns all metadata associated with this stack') - .command('acknowledge [ID]', 'Acknowledge a notice so that it does not show up anymore') + .command(['acknowledge [ID]', 'ack [ID]'], 'Acknowledge a notice so that it does not show up anymore') .command('notices', 'Returns a list of relevant notices') .command('init [TEMPLATE]', 'Create a new, empty CDK project from a template.', yargs => yargs .option('language', { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: initTemplateLanguages }) @@ -230,7 +229,7 @@ if (!process.stdout.isTTY) { process.env.FORCE_COLOR = '0'; } -async function initCommandLine(): Promise<{ configuration: Configuration, value: any }> { +async function initCommandLine() { const argv = await parseCommandLineArguments(); if (argv.verbose) { setLogLevel(argv.verbose); @@ -299,43 +298,29 @@ async function initCommandLine(): Promise<{ configuration: Configuration, value: const commandOptions = { args: argv, configuration, aws: sdkProvider }; try { + return await main(cmd, argv); + } finally { + await version.displayVersionMessage(); - let returnValue = undefined; - - switch (cmd) { - case 'context': - returnValue = await context(commandOptions); - break; - case 'docs': - returnValue = await docs(commandOptions); - break; - case 'doctor': - returnValue = await doctor(commandOptions); - break; - } - - if (returnValue === undefined) { - returnValue = await main(cmd, argv); + if (shouldDisplayNotices()) { + if (cmd === 'notices') { + await displayNotices({ + outdir: configuration.settings.get(['output']) ?? 'cdk.out', + acknowledgedIssueNumbers: [], + ignoreCache: true, + }); + } else { + await displayNotices({ + outdir: configuration.settings.get(['output']) ?? 'cdk.out', + acknowledgedIssueNumbers: configuration.context.get('acknowledged-issue-numbers') ?? [], + ignoreCache: false, + }); + } } - if (typeof returnValue === 'object') { - return { - value: toJsonOrYaml(returnValue), - configuration, - }; - } else if (typeof returnValue === 'string') { - return { - value: returnValue, - configuration, - }; - } else { - return { - value: returnValue, - configuration, - }; + function shouldDisplayNotices(): boolean { + return configuration.settings.get(['notices']) ?? true; } - } finally { - await version.displayVersionMessage(); } async function main(command: string, args: any): Promise { @@ -365,6 +350,15 @@ async function initCommandLine(): Promise<{ configuration: Configuration, value: }); switch (command) { + case 'context': + return context(commandOptions); + + case 'docs': + return docs(commandOptions); + + case 'doctor': + return doctor(commandOptions); + case 'ls': case 'list': return cli.list(args.STACKS, { long: args.long }); @@ -471,21 +465,22 @@ async function initCommandLine(): Promise<{ configuration: Configuration, value: case 'synthesize': case 'synth': if (args.exclusively) { - return cli.synth(args.STACKS, args.exclusively, args.quiet, args.validation); + return cli.synth(args.STACKS, args.exclusively, args.quiet, args.validation, argv.json); } else { - return cli.synth(args.STACKS, true, args.quiet, args.validation); + return cli.synth(args.STACKS, true, args.quiet, args.validation, argv.json); } + case 'notices': + // This is a valid command, but we're postponing its execution + return; + case 'metadata': - return cli.metadata(args.STACK); + return cli.metadata(args.STACK, argv.json); case 'acknowledge': + case 'ack': return cli.acknowledge(args.ID); - case 'notices': - SHOW_ALL_NOTICES = true; - return; - case 'init': const language = configuration.settings.get(['language']); if (args.list) { @@ -501,9 +496,6 @@ async function initCommandLine(): Promise<{ configuration: Configuration, value: } } - function toJsonOrYaml(object: any): string { - return serializeStructure(object, argv.json); - } } /** @@ -575,22 +567,12 @@ function yargsNegativeAlias { - if (typeof value === 'string') { - data(value); - } else if (typeof value === 'number') { + .then(async (value) => { + if (typeof value === 'number') { process.exitCode = value; } - await displayNotices({ - outdir: configuration.settings.get(['output']) ?? 'cdk.out', - acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? [] : configuration.context.get('acknowledged-issue-numbers') ?? [], - permanentlySuppressed: SHOW_ALL_NOTICES ? false : !(configuration.context.get('notices') ?? true), - temporarilySuppressed: !configuration.settings.get(['notices']), - ignoreCache: SHOW_ALL_NOTICES, - }); }) .catch(err => { error(err.message); diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 36ad7e853fad7..11cf2cb98d38a 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -2,7 +2,7 @@ import * as https from 'https'; import * as path from 'path'; import * as fs from 'fs-extra'; import * as semver from 'semver'; -import { print, debug } from './logging'; +import { debug, print } from './logging'; import { cdkCacheDir } from './util/directories'; import { versionNumber } from './version'; @@ -20,21 +20,6 @@ export interface DisplayNoticesProps { */ readonly acknowledgedIssueNumbers: number[]; - /** - * Setting that comes from the command-line option. - * For example, `cdk synth --no-notices`. - */ - readonly temporarilySuppressed?: boolean; - - /** - * Setting that comes from cdk.json. For example, - * - * "context": { - * "notices": false - * } - */ - readonly permanentlySuppressed?: boolean; - /** * Whether cached notices should be ignored. Setting this property * to true will force the CLI to download fresh data @@ -46,21 +31,21 @@ export interface DisplayNoticesProps { export async function displayNotices(props: DisplayNoticesProps) { const dataSource = dataSourceReference(props.ignoreCache ?? false); - await generateMessage(dataSource, props, print); + print(await generateMessage(dataSource, props)); + return 0; } -export async function generateMessage(dataSource: NoticeDataSource, props: DisplayNoticesProps, cb: (msg: string) => void) { - const notices = await dataSource.fetch(); - const individualMessages = formatNotices(filterNotices(notices, { +export async function generateMessage(dataSource: NoticeDataSource, props: DisplayNoticesProps) { + const data = await dataSource.fetch(); + const individualMessages = formatNotices(filterNotices(data, { outdir: props.outdir, - temporarilySuppressed: props.temporarilySuppressed, - permanentlySuppressed: props.permanentlySuppressed, acknowledgedIssueNumbers: new Set(props.acknowledgedIssueNumbers), })); if (individualMessages.length > 0) { - cb(finalMessage(individualMessages, notices[0].issueNumber)); + return finalMessage(individualMessages, data[0].issueNumber); } + return ''; } function dataSourceReference(ignoreCache: boolean): NoticeDataSource { @@ -77,26 +62,22 @@ function finalMessage(individualMessages: string[], exampleNumber: number): stri export interface FilterNoticeOptions { outdir?: string, - permanentlySuppressed?: boolean, - temporarilySuppressed?: boolean, cliVersion?: string, frameworkVersion?: string, acknowledgedIssueNumbers?: Set, } -export function filterNotices(notices: Notice[], options: FilterNoticeOptions): Notice[] { +export function filterNotices(data: Notice[], options: FilterNoticeOptions): Notice[] { const filter = new NoticeFilter({ cliVersion: options.cliVersion ?? versionNumber(), frameworkVersion: options.frameworkVersion ?? frameworkVersion(options.outdir ?? 'cdk.out'), - temporarilySuppressed: options.temporarilySuppressed ?? false, - permanentlySuppressed: options.permanentlySuppressed ?? false, acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? new Set(), }); - return notices.filter(notice => filter.apply(notice)); + return data.filter(notice => filter.apply(notice)); } -export function formatNotices(notices: Notice[]): string[] { - return notices.map(formatNotice); +export function formatNotices(data: Notice[]): string[] { + return data.map(formatNotice); } export interface Component { @@ -119,7 +100,7 @@ export interface NoticeDataSource { export class WebsiteNoticeDataSource implements NoticeDataSource { fetch(): Promise { return new Promise((resolve) => { - https.get('https://cli.cdk.dev-tools.aws.dev/notices.json', res => { + https.get('https://dev-otaviom.cdk.dev-tools.aws.dev/notices.json', res => { if (res.statusCode === 200) { res.setEncoding('utf8'); let rawData = ''; @@ -128,8 +109,8 @@ export class WebsiteNoticeDataSource implements NoticeDataSource { }); res.on('end', () => { try { - const notices = JSON.parse(rawData).notices as Notice[]; - resolve(notices ?? []); + const data = JSON.parse(rawData).notices as Notice[]; + resolve(data ?? []); } catch (e) { debug(`Failed to parse notices: ${e}`); resolve([]); @@ -164,7 +145,7 @@ export class CachedDataSource implements NoticeDataSource { async fetch(): Promise { const cachedData = await this.load(); - const notices = cachedData.notices; + const data = cachedData.notices; const expiration = cachedData.expiration ?? 0; if (Date.now() > expiration || this.skipCache) { @@ -175,7 +156,7 @@ export class CachedDataSource implements NoticeDataSource { await this.save(freshData); return freshData.notices; } else { - return notices; + return data; } } @@ -201,8 +182,6 @@ export class CachedDataSource implements NoticeDataSource { } export interface NoticeFilterProps { - permanentlySuppressed: boolean, - temporarilySuppressed: boolean, cliVersion: string, frameworkVersion: string | undefined, acknowledgedIssueNumbers: Set, @@ -219,9 +198,7 @@ export class NoticeFilter { * Returns true iff we should show this notice. */ apply(notice: Notice): boolean { - if (this.props.permanentlySuppressed - || this.props.temporarilySuppressed - || this.acknowledgedIssueNumbers.has(notice.issueNumber)) { + if (this.acknowledgedIssueNumbers.has(notice.issueNumber)) { return false; } return this.applyVersion(notice, 'cli', this.props.cliVersion) || diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index c7ee3b87130fc..f87ab3e76499c 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -47,6 +47,12 @@ const fakeChokidarWatch = { }, }; +const mockData = jest.fn(); +jest.mock('../lib/logging', () => ({ + ...jest.requireActual('../lib/logging'), + data: mockData, +})); + import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import { Bootstrapper } from '../lib/api/bootstrap'; @@ -676,7 +682,8 @@ describe('synth', () => { const toolkit = defaultToolkitSetup(); // THEN - await expect(toolkit.synth(['Test-Stack-A'], false, true)).resolves.toBeUndefined(); + await toolkit.synth(['Test-Stack-A'], false, true); + expect(mockData.mock.calls.length).toEqual(0); }); afterEach(() => { @@ -707,7 +714,8 @@ describe('synth', () => { test('causes synth to succeed if autoValidate=false', async() => { const toolkit = defaultToolkitSetup(); const autoValidate = false; - await expect(toolkit.synth([], false, true, autoValidate)).resolves.toBeUndefined(); + await toolkit.synth([], false, true, autoValidate); + expect(mockData.mock.calls.length).toEqual(0); }); }); @@ -757,7 +765,10 @@ describe('synth', () => { const toolkit = defaultToolkitSetup(); - await expect(toolkit.synth([MockStack.MOCK_STACK_D.stackName], true, false)).resolves.toBeDefined(); + await toolkit.synth([MockStack.MOCK_STACK_D.stackName], true, false); + + expect(mockData.mock.calls.length).toEqual(1); + expect(mockData.mock.calls[0][0]).toBeDefined(); }); }); diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index 5f06c13e1cae4..b4b20a21a2791 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -227,48 +227,26 @@ describe('cli notices', () => { describe(generateMessage, () => { test('does not show anything when there are no notices', async () => { const dataSource = createDataSource(); - const print = jest.fn(); dataSource.fetch.mockResolvedValue([]); - await generateMessage(dataSource, { - temporarilySuppressed: false, - permanentlySuppressed: false, + const result = await generateMessage(dataSource, { acknowledgedIssueNumbers: [], outdir: '/tmp', - }, print); - - expect(print).not.toHaveBeenCalled(); - }); - - test('does not show anything when no notices pass the filter', async () => { - const dataSource = createDataSource(); - const print = jest.fn(); - dataSource.fetch.mockResolvedValue([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); - - await generateMessage(dataSource, { - temporarilySuppressed: true, - permanentlySuppressed: false, - acknowledgedIssueNumbers: [], - outdir: '/tmp', - }, print); + }); - expect(print).not.toHaveBeenCalled(); + expect(result).toEqual(''); }); test('shows notices that pass the filter', async () => { const dataSource = createDataSource(); - const print = jest.fn(); dataSource.fetch.mockResolvedValue([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); - await generateMessage(dataSource, { - temporarilySuppressed: false, - permanentlySuppressed: false, + const result = await generateMessage(dataSource, { acknowledgedIssueNumbers: [17061], outdir: '/tmp', - }, print); + }); - expect(print).toHaveBeenCalledTimes(1); - expect(print).toHaveBeenCalledWith(` + expect(result).toEqual(` NOTICES 16603\tToggling off auto_delete_objects for Bucket empties the bucket From 10979a9f86c5805732084b2b62e2eecda76bd746 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 21 Feb 2022 20:54:12 +0000 Subject: [PATCH 23/28] Refresh notices and better formatting --- packages/aws-cdk/lib/cli.ts | 4 ++- packages/aws-cdk/lib/notices.ts | 23 ++++++++++-- packages/aws-cdk/test/notices.test.ts | 51 ++++++++++++++++----------- 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index a975a512f7d4a..0ae119c028d00 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -19,7 +19,7 @@ import { realHandler as doctor } from '../lib/commands/doctor'; import { RequireApproval } from '../lib/diff'; import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib/init'; import { data, debug, error, print, setLogLevel } from '../lib/logging'; -import { displayNotices } from '../lib/notices'; +import { displayNotices, refreshNotices } from '../lib/notices'; import { PluginHost } from '../lib/plugin'; import { Command, Configuration, Settings } from '../lib/settings'; import * as version from '../lib/version'; @@ -230,6 +230,8 @@ if (!process.stdout.isTTY) { } async function initCommandLine() { + void refreshNotices().then(_ => debug('Notices refreshed')); + const argv = await parseCommandLineArguments(); if (argv.verbose) { setLogLevel(argv.verbose); diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 11cf2cb98d38a..80c5e10f69230 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -29,6 +29,11 @@ export interface DisplayNoticesProps { readonly ignoreCache?: boolean; } +export async function refreshNotices() { + const dataSource = dataSourceReference(true); + return dataSource.fetch(); +} + export async function displayNotices(props: DisplayNoticesProps) { const dataSource = dataSourceReference(props.ignoreCache ?? false); print(await generateMessage(dataSource, props)); @@ -100,7 +105,7 @@ export interface NoticeDataSource { export class WebsiteNoticeDataSource implements NoticeDataSource { fetch(): Promise { return new Promise((resolve) => { - https.get('https://dev-otaviom.cdk.dev-tools.aws.dev/notices.json', res => { + https.get('https://cli.cdk.dev-tools.aws.dev/notices.json', res => { if (res.statusCode === 200) { res.setEncoding('utf8'); let rawData = ''; @@ -221,10 +226,22 @@ function formatNotice(notice: Notice): string { const componentsValue = notice.components.map(c => `${c.name}: ${c.version}`).join(', '); return [ `${notice.issueNumber}\t${notice.title}`, - `\tOverview: ${notice.overview}`, + formatOverview(notice.overview), `\tAffected versions: ${componentsValue}`, `\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`, - ].join('\n\n'); + ].join('\n\n') + '\n'; +} + +function formatOverview(text: string) { + const wrap = (s: string) => s.replace(/(?![^\n]{1,60}$)([^\n]{1,60})\s/g, '$1\n'); + + const heading = 'Overview: '; + const separator = `\n\t${' '.repeat(heading.length)}`; + const content = wrap(text) + .split('\n') + .join(separator); + + return '\t' + heading + content; } function frameworkVersion(outdir: string): string | undefined { diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index b4b20a21a2791..dff5ef8c8004a 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -47,25 +47,32 @@ const FRAMEWORK_2_1_0_AFFECTED_NOTICE = { describe('cli notices', () => { describe(formatNotices, () => { test('correct format', () => { - expect(formatNotices([BASIC_NOTICE])).toEqual([ - [ - `${BASIC_NOTICE.issueNumber}\t${BASIC_NOTICE.title}`, - `\tOverview: ${BASIC_NOTICE.overview}`, - '\tAffected versions: cli: <=1.126.0', - `\tMore information at: https://github.com/aws/aws-cdk/issues/${BASIC_NOTICE.issueNumber}`, - ].join('\n\n'), - ]); + const result = formatNotices([BASIC_NOTICE])[0]; + expect(result).toEqual(`16603 Toggling off auto_delete_objects for Bucket empties the bucket + + Overview: If a stack is deployed with an S3 bucket with + auto_delete_objects=True, and then re-deployed with + auto_delete_objects=False, all the objects in the bucket + will be deleted. + + Affected versions: cli: <=1.126.0 + + More information at: https://github.com/aws/aws-cdk/issues/16603 +`); }); test('multiple affect versions', () => { - expect(formatNotices([MULTIPLE_AFFECTED_VERSIONS_NOTICE])).toEqual([ - [ - `${MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber}\t${MULTIPLE_AFFECTED_VERSIONS_NOTICE.title}`, - `\tOverview: ${MULTIPLE_AFFECTED_VERSIONS_NOTICE.overview}`, - '\tAffected versions: cli: <1.130.0 >=1.126.0', - `\tMore information at: https://github.com/aws/aws-cdk/issues/${MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber}`, - ].join('\n\n'), - ]); + const result = formatNotices([MULTIPLE_AFFECTED_VERSIONS_NOTICE])[0]; + expect(result).toEqual(`17061 Error when building EKS cluster with monocdk import + + Overview: When using monocdk/aws-eks to build a stack containing an + EKS cluster, error is thrown about missing + lambda-layer-node-proxy-agent/layer/package.json. + + Affected versions: cli: <1.130.0 >=1.126.0 + + More information at: https://github.com/aws/aws-cdk/issues/17061 +`); }); }); @@ -249,13 +256,17 @@ describe('cli notices', () => { expect(result).toEqual(` NOTICES -16603\tToggling off auto_delete_objects for Bucket empties the bucket +16603 Toggling off auto_delete_objects for Bucket empties the bucket + + Overview: If a stack is deployed with an S3 bucket with + auto_delete_objects=True, and then re-deployed with + auto_delete_objects=False, all the objects in the bucket + will be deleted. -\tOverview: If a stack is deployed with an S3 bucket with auto_delete_objects=True, and then re-deployed with auto_delete_objects=False, all the objects in the bucket will be deleted. + Affected versions: cli: <=1.126.0 -\tAffected versions: cli: <=1.126.0 + More information at: https://github.com/aws/aws-cdk/issues/16603 -\tMore information at: https://github.com/aws/aws-cdk/issues/16603 If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge 16603".`); }); From fc4a56eff289de0dcae931ca138398160a844114 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 22 Feb 2022 10:03:58 +0000 Subject: [PATCH 24/28] feat(cli): support for matching notices with arbitrary module names --- packages/aws-cdk/lib/notices.ts | 75 ++++++++++-- .../experimental-module/tree.json | 110 ++++++++++++++++++ packages/aws-cdk/test/notices.test.ts | 24 ++++ 3 files changed, 200 insertions(+), 9 deletions(-) create mode 100644 packages/aws-cdk/test/cloud-assembly-trees/experimental-module/tree.json diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 80c5e10f69230..8a0e0e0496efb 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -1,6 +1,7 @@ import * as https from 'https'; import * as path from 'path'; import * as fs from 'fs-extra'; +import * as minimatch from 'minimatch'; import * as semver from 'semver'; import { debug, print } from './logging'; import { cdkCacheDir } from './util/directories'; @@ -77,6 +78,7 @@ export function filterNotices(data: Notice[], options: FilterNoticeOptions): Not cliVersion: options.cliVersion ?? versionNumber(), frameworkVersion: options.frameworkVersion ?? frameworkVersion(options.outdir ?? 'cdk.out'), acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? new Set(), + tree: loadTree(options.outdir ?? 'cdk.out').tree, }); return data.filter(notice => filter.apply(notice)); } @@ -190,6 +192,7 @@ export interface NoticeFilterProps { cliVersion: string, frameworkVersion: string | undefined, acknowledgedIssueNumbers: Set, + tree: Node, } export class NoticeFilter { @@ -206,8 +209,10 @@ export class NoticeFilter { if (this.acknowledgedIssueNumbers.has(notice.issueNumber)) { return false; } + // TODO Unify these three calls in a single call to match return this.applyVersion(notice, 'cli', this.props.cliVersion) || - this.applyVersion(notice, 'framework', this.props.frameworkVersion); + this.applyVersion(notice, 'framework', this.props.frameworkVersion) || + match(notice.components, this.props.tree); } /** @@ -244,21 +249,73 @@ function formatOverview(text: string) { return '\t' + heading + content; } +/** + * Whether any component in the tree matches any component in the query. + */ +function match(query: Component[], tree: Node): boolean { + return some(tree, node => { + return query.some(component => + node.constructInfo?.fqn != null && + minimatch(node.constructInfo.fqn, component.name) && + semver.satisfies(node.constructInfo?.version ?? '', component.version)); + }); +} + function frameworkVersion(outdir: string): string | undefined { - const tree = loadTree().tree; + const tree = loadTree(outdir).tree; if (tree?.constructInfo?.fqn.startsWith('aws-cdk-lib') || tree?.constructInfo?.fqn.startsWith('@aws-cdk/core')) { return tree.constructInfo.version; } return undefined; +} - function loadTree() { - try { - return fs.readJSONSync(path.join(outdir, 'tree.json')); - } catch (e) { - debug(`Failed to get tree.json file: ${e}`); - return {}; +function loadTree(outdir: string) { + try { + return fs.readJSONSync(path.join(outdir, 'tree.json')); + } catch (e) { + debug(`Failed to get tree.json file: ${e}`); + return {}; + } +} + +// TODO The classes below are a duplication of the classes found in core. Should we merge them? +/** + * Source information on a construct (class fqn and version) + */ +interface ConstructInfo { + readonly fqn: string; + readonly version: string; +} + +/** + * A node in the construct tree. + * @internal + */ +interface Node { + readonly id: string; + readonly path: string; + readonly children?: { [key: string]: Node }; + readonly attributes?: { [key: string]: any }; + + /** + * Information on the construct class that led to this node, if available + */ + readonly constructInfo?: ConstructInfo; +} + +function some(node: Node, predicate: (n: Node) => boolean): boolean { + return node != null && (predicate(node) || findInChildren()); + + function findInChildren(): boolean { + if (node.children == null) { return false; } + + for (const name in node.children) { + if (some(node.children[name], predicate)) { + return true; + } } + return false; } -} \ No newline at end of file +} diff --git a/packages/aws-cdk/test/cloud-assembly-trees/experimental-module/tree.json b/packages/aws-cdk/test/cloud-assembly-trees/experimental-module/tree.json new file mode 100644 index 0000000000000..bb2fa029b3aed --- /dev/null +++ b/packages/aws-cdk/test/cloud-assembly-trees/experimental-module/tree.json @@ -0,0 +1,110 @@ +{ + "version": "tree-0.1", + "tree": { + "id": "App", + "path": "", + "children": { + "Tree": { + "id": "Tree", + "path": "Tree", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.0.66" + } + }, + "SimulationStack": { + "id": "SimulationStack", + "path": "SimulationStack", + "children": { + "HttpApi": { + "id": "HttpApi", + "path": "SimulationStack/HttpApi", + "children": { + "Resource": { + "id": "Resource", + "path": "SimulationStack/HttpApi/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::ApiGatewayV2::Api", + "aws:cdk:cloudformation:props": { + "name": "HttpApi", + "protocolType": "HTTP" + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.aws_apigatewayv2.CfnApi", + "version": "2.8.0" + } + }, + "DefaultStage": { + "id": "DefaultStage", + "path": "SimulationStack/HttpApi/DefaultStage", + "children": { + "Resource": { + "id": "Resource", + "path": "SimulationStack/HttpApi/DefaultStage/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::ApiGatewayV2::Stage", + "aws:cdk:cloudformation:props": { + "apiId": { + "Ref": "HttpApiF5A9A8A7" + }, + "stageName": "$default", + "autoDeploy": true + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.aws_apigatewayv2.CfnStage", + "version": "2.8.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-apigatewayv2-alpha.HttpStage", + "version": "2.13.0-alpha.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-apigatewayv2-alpha.HttpApi", + "version": "2.13.0-alpha.0" + } + }, + "CDKMetadata": { + "id": "CDKMetadata", + "path": "SimulationStack/CDKMetadata", + "children": { + "Default": { + "id": "Default", + "path": "SimulationStack/CDKMetadata/Default", + "constructInfo": { + "fqn": "aws-cdk-lib.CfnResource", + "version": "2.8.0" + } + }, + "Condition": { + "id": "Condition", + "path": "SimulationStack/CDKMetadata/Condition", + "constructInfo": { + "fqn": "aws-cdk-lib.CfnCondition", + "version": "2.8.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.0.66" + } + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.Stack", + "version": "2.8.0" + } + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.App", + "version": "2.8.0" + } + } + } \ No newline at end of file diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index dff5ef8c8004a..a50842e03f189 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -44,6 +44,17 @@ const FRAMEWORK_2_1_0_AFFECTED_NOTICE = { schemaVersion: '1', }; +const EXPERIMENTAL_MODULE_AFFECTED_NOTICE = { + title: 'Regression on module foobar', + issueNumber: 1234, + overview: 'Some bug description', + components: [{ + name: '@aws-cdk/aws-apigatewayv2-alpha.*', + version: '<= 2.13.0-alpha.0', + }], + schemaVersion: '1', +}; + describe('cli notices', () => { describe(formatNotices, () => { test('correct format', () => { @@ -115,6 +126,19 @@ describe('cli notices', () => { outdir: path.join(__dirname, 'cloud-assembly-trees/built-with-1_144_0'), })).toEqual([FRAMEWORK_2_1_0_AFFECTED_NOTICE]); }); + + test('correctly filter notices on arbitrary modules', () => { + const notices = [EXPERIMENTAL_MODULE_AFFECTED_NOTICE]; + + expect(filterNotices(notices, { + outdir: path.join(__dirname, 'cloud-assembly-trees/experimental-module'), + })).toEqual([EXPERIMENTAL_MODULE_AFFECTED_NOTICE]); + + expect(filterNotices(notices, { + outdir: path.join(__dirname, 'cloud-assembly-trees/built-with-2_12_0'), + })).toEqual([]); + }); + }); describe(WebsiteNoticeDataSource, () => { From 1020b99ffecacbafbf79d8b9e062456246b2fdb7 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 22 Feb 2022 11:06:12 +0000 Subject: [PATCH 25/28] Simple prefix matching and unified framework version with the arbitrary matching --- packages/aws-cdk/README.md | 13 +++++++--- packages/aws-cdk/lib/notices.ts | 37 +++++++++++++-------------- packages/aws-cdk/test/notices.test.ts | 10 +------- 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 3c05f296e2dc9..372be74d765b3 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -507,8 +507,6 @@ $ cdk doctor ## Notices -> This feature exists on CDK CLI version 2.14.0 and up. - CDK Notices are important messages regarding security vulnerabilities, regressions, and usage of unsupported versions. Relevant notices appear on every command by default. For example, @@ -530,6 +528,7 @@ NOTICES More information at: https://github.com/aws/aws-cdk/issues/16603 + 17061 Error when building EKS cluster with monocdk import Overview: When using monocdk/aws-eks to build a stack containing @@ -540,6 +539,7 @@ NOTICES More information at: https://github.com/aws/aws-cdk/issues/17061 + If you don’t want to see an notice anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 16603". ``` @@ -553,8 +553,9 @@ You can suppress warnings in a variety of ways: ```json { + "notices": false, "context": { - "notices": false + ... } } ``` @@ -587,12 +588,16 @@ NOTICES 16603 Toggling off auto_delete_objects for Bucket empties the bucket - Overview: if a stack is deployed with an S3 bucket with auto_delete_objects=True, and then re-deployed with auto_delete_objects=False, all the objects in the bucket will be deleted. + Overview: if a stack is deployed with an S3 bucket with + auto_delete_objects=True, and then re-deployed with + auto_delete_objects=False, all the objects in the bucket + will be deleted. Affected versions: framework: <=2.15.0 >=2.10.0 More information at: https://github.com/aws/aws-cdk/issues/16603 + If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge 16603". ``` diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 8a0e0e0496efb..47debc224fe06 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -1,9 +1,9 @@ import * as https from 'https'; import * as path from 'path'; import * as fs from 'fs-extra'; -import * as minimatch from 'minimatch'; import * as semver from 'semver'; import { debug, print } from './logging'; +import { flatMap } from './util'; import { cdkCacheDir } from './util/directories'; import { versionNumber } from './version'; @@ -76,7 +76,6 @@ export interface FilterNoticeOptions { export function filterNotices(data: Notice[], options: FilterNoticeOptions): Notice[] { const filter = new NoticeFilter({ cliVersion: options.cliVersion ?? versionNumber(), - frameworkVersion: options.frameworkVersion ?? frameworkVersion(options.outdir ?? 'cdk.out'), acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? new Set(), tree: loadTree(options.outdir ?? 'cdk.out').tree, }); @@ -190,7 +189,6 @@ export class CachedDataSource implements NoticeDataSource { export interface NoticeFilterProps { cliVersion: string, - frameworkVersion: string | undefined, acknowledgedIssueNumbers: Set, tree: Node, } @@ -209,10 +207,23 @@ export class NoticeFilter { if (this.acknowledgedIssueNumbers.has(notice.issueNumber)) { return false; } - // TODO Unify these three calls in a single call to match + + const components = flatMap(notice.components, component => { + if (component.name === 'framework') { + return [{ + name: '@aws-cdk/core', + version: component.version, + }, { + name: 'aws-cdk-lib', + version: component.version, + }]; + } else { + return [component]; + } + }); + return this.applyVersion(notice, 'cli', this.props.cliVersion) || - this.applyVersion(notice, 'framework', this.props.frameworkVersion) || - match(notice.components, this.props.tree); + match(components, this.props.tree); } /** @@ -255,22 +266,11 @@ function formatOverview(text: string) { function match(query: Component[], tree: Node): boolean { return some(tree, node => { return query.some(component => - node.constructInfo?.fqn != null && - minimatch(node.constructInfo.fqn, component.name) && + node.constructInfo?.fqn.startsWith(component.name) && semver.satisfies(node.constructInfo?.version ?? '', component.version)); }); } -function frameworkVersion(outdir: string): string | undefined { - const tree = loadTree(outdir).tree; - - if (tree?.constructInfo?.fqn.startsWith('aws-cdk-lib') - || tree?.constructInfo?.fqn.startsWith('@aws-cdk/core')) { - return tree.constructInfo.version; - } - return undefined; -} - function loadTree(outdir: string) { try { return fs.readJSONSync(path.join(outdir, 'tree.json')); @@ -280,7 +280,6 @@ function loadTree(outdir: string) { } } -// TODO The classes below are a duplication of the classes found in core. Should we merge them? /** * Source information on a construct (class fqn and version) */ diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index a50842e03f189..e23d874ded027 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -49,7 +49,7 @@ const EXPERIMENTAL_MODULE_AFFECTED_NOTICE = { issueNumber: 1234, overview: 'Some bug description', components: [{ - name: '@aws-cdk/aws-apigatewayv2-alpha.*', + name: '@aws-cdk/aws-apigatewayv2-alpha', version: '<= 2.13.0-alpha.0', }], schemaVersion: '1', @@ -110,14 +110,6 @@ describe('cli notices', () => { test('correctly filter notices on framework', () => { const notices = [FRAMEWORK_2_1_0_AFFECTED_NOTICE]; - expect(filterNotices(notices, { - frameworkVersion: '2.0.0', - })).toEqual([FRAMEWORK_2_1_0_AFFECTED_NOTICE]); - - expect(filterNotices(notices, { - frameworkVersion: '2.2.0', - })).toEqual([]); - expect(filterNotices(notices, { outdir: path.join(__dirname, 'cloud-assembly-trees/built-with-2_12_0'), })).toEqual([]); From e7afe1d59ad74d4358ac98ed68e0943dae211285 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 22 Feb 2022 11:36:50 +0000 Subject: [PATCH 26/28] Added catch to refreshNotices call; Type of main updated; Refresh doesn't force a cache update --- packages/aws-cdk/lib/cdk-toolkit.ts | 5 +++-- packages/aws-cdk/lib/cli.ts | 8 +++++--- packages/aws-cdk/lib/notices.ts | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index d8cdd7c1dd1ee..6d95bb76ac81b 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -391,7 +391,7 @@ export class CdkToolkit { } } - public async list(selectors: string[], options: { long?: boolean } = { }) { + public async list(selectors: string[], options: { long?: boolean, json?: boolean } = { }): Promise { const stacks = await this.selectStacksForList(selectors); // if we are in "long" mode, emit the array as-is (JSON/YAML) @@ -404,7 +404,8 @@ export class CdkToolkit { environment: stack.environment, }); } - return long; // will be YAML formatted output + data(serializeStructure(long, options.json ?? false)); + return 0; } // just print stack IDs diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 0ae119c028d00..b8a0cd40c463b 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -230,7 +230,9 @@ if (!process.stdout.isTTY) { } async function initCommandLine() { - void refreshNotices().then(_ => debug('Notices refreshed')); + void refreshNotices() + .then(_ => debug('Notices refreshed')) + .catch(e => debug(`Notices refresh failed: ${e}`)); const argv = await parseCommandLineArguments(); if (argv.verbose) { @@ -325,7 +327,7 @@ async function initCommandLine() { } } - async function main(command: string, args: any): Promise { + async function main(command: string, args: any): Promise { const toolkitStackName: string = ToolkitInfo.determineName(configuration.settings.get(['toolkitStackName'])); debug(`Toolkit stack: ${chalk.bold(toolkitStackName)}`); @@ -363,7 +365,7 @@ async function initCommandLine() { case 'ls': case 'list': - return cli.list(args.STACKS, { long: args.long }); + return cli.list(args.STACKS, { long: args.long, json: argv.json }); case 'diff': const enableDiffNoFail = isFeatureEnabled(configuration, cxapi.ENABLE_DIFF_NO_FAIL); diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 47debc224fe06..850ed7a35035e 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -31,7 +31,7 @@ export interface DisplayNoticesProps { } export async function refreshNotices() { - const dataSource = dataSourceReference(true); + const dataSource = dataSourceReference(false); return dataSource.fetch(); } From f76d5c87244c297b1ad1a712413ddbd77437847e Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 22 Feb 2022 15:04:47 +0000 Subject: [PATCH 27/28] Improved name matching and some refactoring --- packages/aws-cdk/lib/notices.ts | 65 ++++++++++++++++++--------- packages/aws-cdk/test/notices.test.ts | 42 +++++++++++++++-- 2 files changed, 81 insertions(+), 26 deletions(-) diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 850ed7a35035e..8adae837bad34 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -190,7 +190,7 @@ export class CachedDataSource implements NoticeDataSource { export interface NoticeFilterProps { cliVersion: string, acknowledgedIssueNumbers: Set, - tree: Node, + tree: ConstructTreeNode, } export class NoticeFilter { @@ -208,22 +208,8 @@ export class NoticeFilter { return false; } - const components = flatMap(notice.components, component => { - if (component.name === 'framework') { - return [{ - name: '@aws-cdk/core', - version: component.version, - }, { - name: 'aws-cdk-lib', - version: component.version, - }]; - } else { - return [component]; - } - }); - return this.applyVersion(notice, 'cli', this.props.cliVersion) || - match(components, this.props.tree); + match(resolveAliases(notice.components), this.props.tree); } /** @@ -238,6 +224,32 @@ export class NoticeFilter { } } +/** + * Some component names are aliases to actual component names. For example "framework" + * is an alias for either the core library (v1) or the whole CDK library (v2). + * + * This function converts all aliases to their actual counterpart names, to be used to + * match against the construct tree. + * + * @param components a list of components. Components whose name is an alias will be + * transformed and all others will be left intact. + */ +function resolveAliases(components: Component[]): Component[] { + return flatMap(components, component => { + if (component.name === 'framework') { + return [{ + name: '@aws-cdk/core.', + version: component.version, + }, { + name: 'aws-cdk-lib.', + version: component.version, + }]; + } else { + return [component]; + } + }); +} + function formatNotice(notice: Notice): string { const componentsValue = notice.components.map(c => `${c.name}: ${c.version}`).join(', '); return [ @@ -263,12 +275,21 @@ function formatOverview(text: string) { /** * Whether any component in the tree matches any component in the query. */ -function match(query: Component[], tree: Node): boolean { +function match(query: Component[], tree: ConstructTreeNode): boolean { return some(tree, node => { return query.some(component => - node.constructInfo?.fqn.startsWith(component.name) && - semver.satisfies(node.constructInfo?.version ?? '', component.version)); + compareNames(component.name, node.constructInfo?.fqn) && + compareVersions(component.version, node.constructInfo?.version)); }); + + function compareNames(pattern: string, target: string | undefined): boolean { + if (target == null) { return false; } + return pattern.endsWith('.') ? target.startsWith(pattern) : pattern === target; + } + + function compareVersions(pattern: string, target: string | undefined): boolean { + return semver.satisfies(target ?? '', pattern); + } } function loadTree(outdir: string) { @@ -292,10 +313,10 @@ interface ConstructInfo { * A node in the construct tree. * @internal */ -interface Node { +interface ConstructTreeNode { readonly id: string; readonly path: string; - readonly children?: { [key: string]: Node }; + readonly children?: { [key: string]: ConstructTreeNode }; readonly attributes?: { [key: string]: any }; /** @@ -304,7 +325,7 @@ interface Node { readonly constructInfo?: ConstructInfo; } -function some(node: Node, predicate: (n: Node) => boolean): boolean { +function some(node: ConstructTreeNode, predicate: (n: ConstructTreeNode) => boolean): boolean { return node != null && (predicate(node) || findInChildren()); function findInChildren(): boolean { diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index e23d874ded027..15cdb3b2e9bae 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -44,12 +44,34 @@ const FRAMEWORK_2_1_0_AFFECTED_NOTICE = { schemaVersion: '1', }; -const EXPERIMENTAL_MODULE_AFFECTED_NOTICE = { +const NOTICE_FOR_APIGATEWAYV2 = { title: 'Regression on module foobar', issueNumber: 1234, overview: 'Some bug description', components: [{ - name: '@aws-cdk/aws-apigatewayv2-alpha', + name: '@aws-cdk/aws-apigatewayv2-alpha.', + version: '<= 2.13.0-alpha.0', + }], + schemaVersion: '1', +}; + +const NOTICE_FOR_APIGATEWAY = { + title: 'Regression on module foobar', + issueNumber: 1234, + overview: 'Some bug description', + components: [{ + name: '@aws-cdk/aws-apigateway', + version: '<= 2.13.0-alpha.0', + }], + schemaVersion: '1', +}; + +const NOTICE_FOR_APIGATEWAYV2_CFN_STAGE = { + title: 'Regression on module foobar', + issueNumber: 1234, + overview: 'Some bug description', + components: [{ + name: 'aws-cdk-lib.aws_apigatewayv2.CfnStage', version: '<= 2.13.0-alpha.0', }], schemaVersion: '1', @@ -120,15 +142,27 @@ describe('cli notices', () => { }); test('correctly filter notices on arbitrary modules', () => { - const notices = [EXPERIMENTAL_MODULE_AFFECTED_NOTICE]; + const notices = [NOTICE_FOR_APIGATEWAYV2]; + // module-level match expect(filterNotices(notices, { outdir: path.join(__dirname, 'cloud-assembly-trees/experimental-module'), - })).toEqual([EXPERIMENTAL_MODULE_AFFECTED_NOTICE]); + })).toEqual([NOTICE_FOR_APIGATEWAYV2]); + // no apigatewayv2 in the tree expect(filterNotices(notices, { outdir: path.join(__dirname, 'cloud-assembly-trees/built-with-2_12_0'), })).toEqual([]); + + // module name mismatch: apigateway != apigatewayv2 + expect(filterNotices([NOTICE_FOR_APIGATEWAY], { + outdir: path.join(__dirname, 'cloud-assembly-trees/experimental-module'), + })).toEqual([]); + + // construct-level match + expect(filterNotices([NOTICE_FOR_APIGATEWAYV2_CFN_STAGE], { + outdir: path.join(__dirname, 'cloud-assembly-trees/experimental-module'), + })).toEqual([NOTICE_FOR_APIGATEWAYV2_CFN_STAGE]); }); }); From 4b6ded6276905611525d381c22dd498573fcf30c Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 22 Feb 2022 15:12:41 +0000 Subject: [PATCH 28/28] Improved documentation of the match() function. --- packages/aws-cdk/lib/notices.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 8adae837bad34..ba815414f02b2 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -274,6 +274,13 @@ function formatOverview(text: string) { /** * Whether any component in the tree matches any component in the query. + * A match happens when: + * + * 1. The version of the node matches the version in the query, interpreted + * as a semver range. + * + * 2. The name in the query is a prefix of the node name when the query ends in '.', + * or the two names are exactly the same, otherwise. */ function match(query: Component[], tree: ConstructTreeNode): boolean { return some(tree, node => {