From b0e4a544aecce86e8b41e7cd148a139c2e34bfbd Mon Sep 17 00:00:00 2001 From: Eli Polonsky Date: Mon, 30 Sep 2024 12:05:14 +0300 Subject: [PATCH] feat(cli): notices on bootstrap version (#31555) ### Reason for this change We would like to be able to send customers a notice when issues with bootstrap templates are discovered. ### Description of changes Currently, our notices mechanism can only match against CLI/Framework versions. In order to match against a bootstrap stack version, we need to hook into the deploy process, where we already perform bootstrap version checks. There were two options to implement the change: 1. Bubble up the bootstrap stack version all the up to the CLI entry-point, where notices are initialized. 2. Allow access to notices from anywhere in our CLI code base. I opted for number 2 because it is less disruptive (in terms of files changed) and more flexible for future code that might want to take advantage of the notices mechanism. The tricky thing is, notices are dependent on user configuration (i.e `Configuration`), which we don't have access to in this part of the code. To make it work, I created a new `Notices` singleton class. It is instantiated in the CLI entry-point (via `Notices.create` with user configuration), and can then be accessed from anywhere in the code (via `Notices.get()`). This change resulted in a pretty big refactor to the notices code, but keeps everything else untouched. ### Docs Documentation of enhanced notice authoring capabilities: https://github.com/cdklabs/aws-cdk-notices/pull/631 ### Description of how you validated changes Added unit tests. ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../tests/cli-integ-tests/cli.integtest.ts | 31 + packages/aws-cdk/README.md | 36 +- .../aws-cdk/lib/api/environment-resources.ts | 17 +- packages/aws-cdk/lib/cli.ts | 43 +- packages/aws-cdk/lib/notices.ts | 494 ++++++---- .../test/api/environment-resources.test.ts | 34 + packages/aws-cdk/test/notices.test.ts | 908 ++++++++++++------ 7 files changed, 1047 insertions(+), 516 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index cfb4680987d2c..f093ce86236d6 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -2259,3 +2259,34 @@ integTest( expect(noticesUnacknowledged).toEqual(noticesUnacknowledgedAlias); }), ); + +integTest('cdk notices for bootstrap', withDefaultFixture(async (fixture) => { + + const cache = { + expiration: 4125963264000, // year 2100 so we never overwrite the cache + notices: [{ + title: 'Bootstrap stack outdated', + issueNumber: 16600, + overview: 'Your environments "{resolve:ENVIRONMENTS}" are running an outdated bootstrap stack.', + components: [{ + name: 'bootstrap', + version: '<2000', + }], + schemaVersion: '1', + }], + }; + + const cdkCacheDir = path.join(fixture.integTestDir, 'cache'); + await fs.mkdir(cdkCacheDir); + await fs.writeFile(path.join(cdkCacheDir, 'notices.json'), JSON.stringify(cache)); + + const output = await fixture.cdkDeploy('test-2', { + verbose: false, + modEnv: { + CDK_HOME: fixture.integTestDir, + }, + }); + + expect(output).toContain('Your environments \"aws://'); + +})); diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 268f0989c6a5e..57081731a7c4c 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -879,33 +879,39 @@ $ cdk deploy NOTICES -16603 Toggling off auto_delete_objects for Bucket empties the bucket +22090 cli: cdk init produces EACCES: permission denied and does not fill the directory - 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: The CLI is unable to initialize new apps if CDK is + installed globally in a directory owned by root - Affected versions: <1.126.0. + Affected versions: cli: 2.42.0. - More information at: https://github.com/aws/aws-cdk/issues/16603 + More information at: https://github.com/aws/aws-cdk/issues/22090 -17061 Error when building EKS cluster with monocdk import +27547 Incorrect action in policy of Bucket `grantRead` method - 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. + Overview: Using the `grantRead` method on `aws-cdk-lib/aws-s3.Bucket` + results in an invalid action attached to the resource policy + which can cause unexpected failures when interacting + with the bucket. - Affected versions: >=1.126.0 <=1.130.0. + Affected versions: aws-cdk-lib.aws_s3.Bucket: 2.101.0. - More information at: https://github.com/aws/aws-cdk/issues/17061 + More information at: https://github.com/aws/aws-cdk/issues/27547 -If you don’t want to see an notice anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 16603". +If you don’t want to see a notice anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 16603". ``` -You can suppress warnings in a variety of ways: +There are several types of notices you may encounter, differentiated by the affected component: + +- **cli**: notifies you about issues related to your CLI version. +- **framework**: notifies you about issues related to your version of core constructs (e.g `Stack`). +- **aws-cdk-lib.{module}.{construct}**: notifies you about issue related to your version of a specific construct (e.g `aws-cdk-lib.aws_s3.Bucket`) +- **bootstrap**: notifies you about issues related to your version of the bootstrap stack. Note that these types of notices are only shown during the `cdk deploy` command. + +You can suppress notices in a variety of ways: - per individual execution: diff --git a/packages/aws-cdk/lib/api/environment-resources.ts b/packages/aws-cdk/lib/api/environment-resources.ts index 0b262a13ad919..937e813ad8f95 100644 --- a/packages/aws-cdk/lib/api/environment-resources.ts +++ b/packages/aws-cdk/lib/api/environment-resources.ts @@ -2,6 +2,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import { ISDK } from './aws-auth'; import { EcrRepositoryInfo, ToolkitInfo } from './toolkit-info'; import { debug, warning } from '../logging'; +import { Notices } from '../notices'; /** * Registry class for `EnvironmentResources`. @@ -77,7 +78,7 @@ export class EnvironmentResources { if (ssmParameterName !== undefined) { try { - doValidate(await this.versionFromSsmParameter(ssmParameterName)); + doValidate(await this.versionFromSsmParameter(ssmParameterName), this.environment); return; } catch (e: any) { if (e.code !== 'AccessDeniedException') { throw e; } @@ -92,7 +93,7 @@ export class EnvironmentResources { const bootstrapStack = await this.lookupToolkit(); if (bootstrapStack.found && bootstrapStack.version < BOOTSTRAP_TEMPLATE_VERSION_INTRODUCING_GETPARAMETER) { warning(`Could not read SSM parameter ${ssmParameterName}: ${e.message}, falling back to version from ${bootstrapStack}`); - doValidate(bootstrapStack.version); + doValidate(bootstrapStack.version, this.environment); return; } @@ -102,9 +103,15 @@ export class EnvironmentResources { // No SSM parameter const bootstrapStack = await this.lookupToolkit(); - doValidate(bootstrapStack.version); - - function doValidate(version: number) { + doValidate(bootstrapStack.version, this.environment); + + function doValidate(version: number, environment: cxapi.Environment) { + const notices = Notices.get(); + if (notices) { + // if `Notices` hasn't been initialized there is probably a good + // reason for it. handle gracefully. + notices.addBootstrappedEnvironment({ bootstrapStackVersion: version, environment }); + } if (defExpectedVersion > version) { throw new Error(`This CDK deployment requires bootstrap stack version '${expectedVersion}', found '${version}'. Please run 'cdk bootstrap'.`); } diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index f63c7cbf21eef..9d61a448c9a91 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -26,7 +26,7 @@ import { MIGRATE_SUPPORTED_LANGUAGES, getMigrateScanType } from '../lib/commands import { RequireApproval } from '../lib/diff'; import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib/init'; import { data, debug, error, print, setLogLevel, setCI } from '../lib/logging'; -import { displayNotices, refreshNotices } from '../lib/notices'; +import { Notices } from '../lib/notices'; import { Command, Configuration, Settings } from '../lib/settings'; import * as version from '../lib/version'; @@ -367,10 +367,10 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise debug(`Could not refresh notices: ${e}`)); - } + const cmd = argv._[0]; + + const notices = Notices.create({ configuration, includeAcknowlegded: cmd === 'notices' ? !argv.unacknowledged : false }); + await notices.refresh(); const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({ profile: configuration.settings.get(['profile']), @@ -422,8 +422,6 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise { diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts index 747078e0a11da..7976a9c4ed448 100644 --- a/packages/aws-cdk/lib/notices.ts +++ b/packages/aws-cdk/lib/notices.ts @@ -1,107 +1,319 @@ import { ClientRequest } from 'http'; import * as https from 'https'; import * as path from 'path'; +import type { Environment } from '@aws-cdk/cx-api'; import * as fs from 'fs-extra'; import * as semver from 'semver'; -import { debug, print } from './logging'; -import { ConstructTreeNode, loadTreeFromDir, some } from './tree'; +import { debug, print, warning, error } from './logging'; +import { Configuration } from './settings'; +import { loadTreeFromDir, some } from './tree'; import { flatMap } from './util'; import { cdkCacheDir } from './util/directories'; import { versionNumber } from './version'; const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json'); -export interface DisplayNoticesProps { +export interface NoticesProps { /** - * The cloud assembly directory. Usually 'cdk.out'. + * A `Configuration` instance holding CDK context and settings. */ - readonly outdir: string; + readonly configuration: Configuration; /** - * Issue numbers of notices that have been acknowledged by a user - * of the current CDK repository. These notices will be skipped. + * Include notices that have already been acknowledged. + * + * @default false */ - readonly acknowledgedIssueNumbers: number[]; + readonly includeAcknowlegded?: boolean; + +} + +export interface NoticesPrintOptions { /** - * Whether cached notices should be ignored. Setting this property - * to true will force the CLI to download fresh data + * Whether to append the total number of unacknowledged notices to the display. * * @default false */ - readonly ignoreCache?: boolean; + readonly showTotal?: boolean; +} + +export interface NoticesRefreshOptions { /** - * Whether to append the number of unacknowledged notices to the display. + * Whether to force a cache refresh regardless of expiration time. * * @default false */ - readonly unacknowledged?: boolean; -} + readonly force?: boolean; -export async function refreshNotices() { - const dataSource = dataSourceReference(false); - return dataSource.fetch(); + /** + * Data source for fetch notices from. + * + * @default - WebsiteNoticeDataSource + */ + readonly dataSource?: NoticeDataSource; } -export async function displayNotices(props: DisplayNoticesProps) { - const dataSource = dataSourceReference(props.ignoreCache ?? false); - print(await generateMessage(dataSource, props)); - return 0; +export interface NoticesFilterFilterOptions { + readonly data: Notice[]; + readonly cliVersion: string; + readonly outDir: string; + readonly bootstrappedEnvironments: BootstrappedEnvironment[]; } -export async function generateMessage(dataSource: NoticeDataSource, props: DisplayNoticesProps) { - const data = await dataSource.fetch(); - const filteredNotices = filterNotices(data, { - outdir: props.outdir, - acknowledgedIssueNumbers: new Set(props.acknowledgedIssueNumbers), - }); +export class NoticesFilter { - let messageString: string = ''; - if (filteredNotices.length > 0) { - messageString = getFilteredMessages(filteredNotices); + public static filter(options: NoticesFilterFilterOptions): FilteredNotice[] { + return [ + ...this.findForCliVersion(options.data, options.cliVersion), + ...this.findForFrameworkVersion(options.data, options.outDir), + ...this.findForBootstrapVersion(options.data, options.bootstrappedEnvironments), + ]; } - if (props.unacknowledged) { - messageString = [messageString, `There are ${filteredNotices.length} unacknowledged notice(s).`].join('\n\n'); + + private static findForCliVersion(data: Notice[], cliVersion: string): FilteredNotice[] { + return flatMap(data, notice => { + const affectedComponent = notice.components.find(component => component.name === 'cli'); + const affectedRange = affectedComponent?.version; + + if (affectedRange == null) { + return []; + } + + if (!semver.satisfies(cliVersion, affectedRange)) { + return []; + } + + return [new FilteredNotice(notice)]; + }); + } - return messageString; -} -function getFilteredMessages(filteredNotices: Notice[]): string { - const individualMessages = formatNotices(filteredNotices); - return finalMessage(individualMessages, filteredNotices[0].issueNumber); -} + private static findForFrameworkVersion(data: Notice[], outDir: string): FilteredNotice[] { + const tree = loadTreeFromDir(outDir); + return flatMap(data, notice => { + + // A match happens when: + // + // 1. The version of the node matches the version in the notice, interpreted + // as a semver range. + // + // AND + // + // 2. The name in the notice is a prefix of the node name when the query ends in '.', + // or the two names are exactly the same, otherwise. + + const matched = some(tree, node => { + return this.resolveAliases(notice.components).some(component => + compareNames(component.name, node.constructInfo?.fqn) && + compareVersions(component.version, node.constructInfo?.version)); + }); + + if (!matched) { + return []; + } -function dataSourceReference(ignoreCache: boolean): NoticeDataSource { - return new CachedDataSource(CACHE_FILE_PATH, new WebsiteNoticeDataSource(), ignoreCache); -} + return [new FilteredNotice(notice)]; -function finalMessage(individualMessages: string[], exampleNumber: number): string { - return [ - '\nNOTICES (What\'s this? https://github.com/aws/aws-cdk/wiki/CLI-Notices)', - ...individualMessages, - `If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge ${exampleNumber}".`, - ].join('\n\n'); -} + 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); + } + + }); + + } + + private static findForBootstrapVersion(data: Notice[], bootstrappedEnvironments: BootstrappedEnvironment[]): FilteredNotice[] { + return flatMap(data, notice => { + const affectedComponent = notice.components.find(component => component.name === 'bootstrap'); + const affectedRange = affectedComponent?.version; + + if (affectedRange == null) { + return []; + } + + const affected = bootstrappedEnvironments.filter(i => { + + const semverBootstrapVersion = semver.coerce(i.bootstrapStackVersion); + if (!semverBootstrapVersion) { + // we don't throw because notices should never crash the cli. + warning(`While filtering notices, could not coerce bootstrap version '${i.bootstrapStackVersion}' into semver`); + return false; + } + + return semver.satisfies(semverBootstrapVersion, affectedRange); + + }); + + if (affected.length === 0) { + return []; + } + + const filtered = new FilteredNotice(notice); + filtered.addDynamicValue('ENVIRONMENTS', affected.map(s => s.environment.name).join(',')); + + return [filtered]; + + }); + } + + private static 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]; + } + }); + } -export interface FilterNoticeOptions { - outdir?: string; - cliVersion?: string; - frameworkVersion?: string; - acknowledgedIssueNumbers?: Set; } -export function filterNotices(data: Notice[], options: FilterNoticeOptions): Notice[] { - const filter = new NoticeFilter({ - cliVersion: options.cliVersion ?? versionNumber(), - acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? new Set(), - tree: loadTreeFromDir(options.outdir ?? 'cdk.out'), - }); - return data.filter(notice => filter.apply(notice)); +/** + * Infomration about a bootstrapped environment. + */ +export interface BootstrappedEnvironment { + readonly bootstrapStackVersion: number; + readonly environment: Environment; } -export function formatNotices(data: Notice[]): string[] { - return data.map(formatNotice); +/** + * Provides access to notices the CLI can display. + */ +export class Notices { + + /** + * Create an instance. Note that this replaces the singleton. + */ + public static create(props: NoticesProps): Notices { + this._instance = new Notices(props); + return this._instance; + } + + /** + * Get the singleton instance. May return `undefined` if `create` has not been called. + */ + public static get(): Notices | undefined { + return this._instance; + } + + private static _instance: Notices | undefined; + + private readonly configuration: Configuration; + private readonly acknowledgedIssueNumbers: Set; + private readonly includeAcknowlegded: boolean; + + private data: Set = new Set(); + + // sets don't deduplicate interfaces, so we use a map. + private readonly bootstrappedEnvironments: Map = new Map(); + + private constructor(props: NoticesProps) { + this.configuration = props.configuration; + this.acknowledgedIssueNumbers = new Set(this.configuration.context.get('acknowledged-issue-numbers') ?? []); + this.includeAcknowlegded = props.includeAcknowlegded ?? false; + } + + /** + * Add a bootstrap information to filter on. Can have multiple values + * in case of multi-environment deployments. + */ + public addBootstrappedEnvironment(bootstrapped: BootstrappedEnvironment) { + const key = [ + bootstrapped.bootstrapStackVersion, + bootstrapped.environment.account, + bootstrapped.environment.region, + bootstrapped.environment.name, + ].join(':'); + this.bootstrappedEnvironments.set(key, bootstrapped); + } + + /** + * Refresh the list of notices this instance is aware of. + * To make sure this never crashes the CLI process, all failures are caught and + * slitently logged. + * + * If context is configured to not display notices, this will no-op. + */ + public async refresh(options: NoticesRefreshOptions = {}) { + + if (!this.shouldDisplay()) { + return; + } + + try { + const dataSource = new CachedDataSource(CACHE_FILE_PATH, options.dataSource ?? new WebsiteNoticeDataSource(), options.force ?? false); + const notices = await dataSource.fetch(); + this.data = new Set(this.includeAcknowlegded ? notices : notices.filter(n => !this.acknowledgedIssueNumbers.has(n.issueNumber))); + } catch (e: any) { + debug(`Could not refresh notices: ${e}`); + } + } + + /** + * Display the relevant notices (unless context dictates we shouldn't). + */ + public display(options: NoticesPrintOptions = {}) { + + if (!this.shouldDisplay()) { + return; + } + + const filteredNotices = NoticesFilter.filter({ + data: Array.from(this.data), + cliVersion: versionNumber(), + outDir: this.configuration.settings.get(['output']) ?? 'cdk.out', + bootstrappedEnvironments: Array.from(this.bootstrappedEnvironments.values()), + }); + + if (filteredNotices.length > 0) { + print(''); + print('NOTICES (What\'s this? https://github.com/aws/aws-cdk/wiki/CLI-Notices)'); + print(''); + for (const filtered of filteredNotices) { + const formatted = filtered.format(); + switch (filtered.notice.severity) { + case 'warning': + warning(formatted); + break; + case 'error': + error(formatted); + break; + default: + print(formatted); + } + print(''); + } + print(`If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge ${filteredNotices[0].notice.issueNumber}".`); + } + + if (options.showTotal ?? false) { + print(''); + print(`There are ${filteredNotices.length} unacknowledged notice(s).`); + } + + } + + /** + * Determine whether or not notices should be displayed based on the + * configuration provided at instantiation time. + */ + private shouldDisplay(): boolean { + return this.configuration.settings.get(['notices']) ?? true; + } + } export interface Component { @@ -115,6 +327,52 @@ export interface Notice { overview: string; components: Component[]; schemaVersion: string; + severity?: string; +} + +/** + * Notice after passing the filter. A filter can augment a notice with + * dynamic values as it has access to the dynamic matching data. + */ +export class FilteredNotice { + + private readonly dynamicValues: { [key: string]: string } = {}; + + public constructor(public readonly notice: Notice) {} + + public addDynamicValue(key: string, value: string) { + this.dynamicValues[`{resolve:${key}}`] = value; + } + + public format(): string { + + const componentsValue = this.notice.components.map(c => `${c.name}: ${c.version}`).join(', '); + return this.resolveDynamicValues([ + `${this.notice.issueNumber}\t${this.notice.title}`, + this.formatOverview(), + `\tAffected versions: ${componentsValue}`, + `\tMore information at: https://github.com/aws/aws-cdk/issues/${this.notice.issueNumber}`, + ].join('\n\n') + '\n'); + + } + + private formatOverview() { + 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(this.notice.overview) + .split('\n') + .join(separator); + + return '\t' + heading + content; + } + + private resolveDynamicValues(input: string): string { + const pattern = new RegExp(Object.keys(this.dynamicValues).join('|'), 'g'); + return input.replace(pattern, (matched) => this.dynamicValues[matched] ?? matched); + } + } export interface NoticeDataSource { @@ -240,115 +498,3 @@ export class CachedDataSource implements NoticeDataSource { } } } - -export interface NoticeFilterProps { - cliVersion: string; - acknowledgedIssueNumbers: Set; - tree: ConstructTreeNode; -} - -export class NoticeFilter { - private readonly acknowledgedIssueNumbers: Set; - - constructor(private readonly props: NoticeFilterProps) { - this.acknowledgedIssueNumbers = props.acknowledgedIssueNumbers; - } - - /** - * Returns true if we should show this notice. - */ - apply(notice: Notice): boolean { - if (this.acknowledgedIssueNumbers.has(notice.issueNumber)) { - return false; - } - - return this.applyVersion(notice, 'cli', this.props.cliVersion) || - match(resolveAliases(notice.components), this.props.tree); - } - - /** - * Returns true if we should show the notice. - */ - private applyVersion(notice: Notice, name: string, compareToVersion: string | undefined) { - if (compareToVersion === undefined) { return false; } - - const affectedComponent = notice.components.find(component => component.name === name); - const affectedRange = affectedComponent?.version; - return affectedRange != null && semver.satisfies(compareToVersion, affectedRange); - } -} - -/** - * 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 [ - `${notice.issueNumber}\t${notice.title}`, - formatOverview(notice.overview), - `\tAffected versions: ${componentsValue}`, - `\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`, - ].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; -} - -/** - * 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 => { - return query.some(component => - 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); - } -} diff --git a/packages/aws-cdk/test/api/environment-resources.test.ts b/packages/aws-cdk/test/api/environment-resources.test.ts index 1d4d1b67352bd..2e48b41cebe50 100644 --- a/packages/aws-cdk/test/api/environment-resources.test.ts +++ b/packages/aws-cdk/test/api/environment-resources.test.ts @@ -1,7 +1,10 @@ /* eslint-disable import/order */ import { ToolkitInfo } from '../../lib/api'; import { EnvironmentResourcesRegistry } from '../../lib/api/environment-resources'; +import { CachedDataSource, Notices, NoticesFilter } from '../../lib/notices'; +import { Configuration } from '../../lib/settings'; import { errorWithCode, mockBootstrapStack, MockSdk } from '../util/mock-sdk'; +import * as version from '../../lib/version'; import { MockToolkitInfo } from '../util/mock-toolkitinfo'; let mockSdk: MockSdk; @@ -66,6 +69,10 @@ describe('validateversion without bootstrap stack', () => { mockToolkitInfo(ToolkitInfo.bootstrapStackNotFoundInfo('TestBootstrapStack')); }); + afterEach(() => { + jest.clearAllMocks(); + }); + test('validating version with explicit SSM parameter succeeds', async () => { // GIVEN mockSdk.stubSsm({ @@ -74,8 +81,35 @@ describe('validateversion without bootstrap stack', () => { }, }); + // disable notices caching + jest.spyOn(CachedDataSource.prototype as any, 'save').mockImplementation((_: any) => Promise.resolve()); + jest.spyOn(CachedDataSource.prototype as any, 'load').mockImplementation(() => Promise.resolve({ expiration: 0, notices: [] })); + + // mock cli version number + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); + // THEN + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ dataSource: { fetch: async () => [] } }); await expect(envResources().validateVersion(8, '/abc')).resolves.toBeUndefined(); + + const filter = jest.spyOn(NoticesFilter, 'filter'); + notices.display(); + + expect(filter).toHaveBeenCalledTimes(1); + expect(filter).toHaveBeenCalledWith({ + bootstrappedEnvironments: [{ + bootstrapStackVersion: 10, + environment: { + account: '11111111', + region: 'us-nowhere', + name: 'aws://11111111/us-nowhere', + }, + }], + cliVersion: '1.0.0', + data: [], + outDir: 'cdk.out', + }); }); test('validating version without explicit SSM parameter fails', async () => { diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index f0cae20adf12d..b1a9cdf8bf31b 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -7,13 +7,59 @@ import * as nock from 'nock'; import * as logging from '../lib/logging'; import { CachedDataSource, - filterNotices, - formatNotices, - generateMessage, Notice, + Notices, + NoticesFilter, + FilteredNotice, WebsiteNoticeDataSource, + BootstrappedEnvironment, } from '../lib/notices'; import * as version from '../lib/version'; +import { Configuration } from '../lib/settings'; + +const BASIC_BOOTSTRAP_NOTICE = { + title: 'Exccessive permissions on file asset publishing role', + issueNumber: 16600, + overview: 'FilePublishingRoleDefaultPolicy has too many permissions', + components: [{ + name: 'bootstrap', + version: '<25', + }], + schemaVersion: '1', +}; + +const BOOTSTRAP_NOTICE_V10 = { + title: 'Bootstrap version 10 is no good', + issueNumber: 16600, + overview: 'overview', + components: [{ + name: 'bootstrap', + version: '=10', + }], + schemaVersion: '1', +}; + +const BOOTSTRAP_NOTICE_V11 = { + title: 'Bootstrap version 11 is no good', + issueNumber: 16600, + overview: 'overview', + components: [{ + name: 'bootstrap', + version: '=11', + }], + schemaVersion: '1', +}; + +const BASIC_DYNAMIC_NOTICE = { + title: 'Toggling off auto_delete_objects for Bucket empties the bucket', + issueNumber: 16603, + overview: '{resolve:DYNAMIC1} this is a notice with dynamic values {resolve:DYNAMIC2}', + components: [{ + name: 'cli', + version: '<=1.126.0', + }], + schemaVersion: '1', +}; const BASIC_NOTICE = { title: 'Toggling off auto_delete_objects for Bucket empties the bucket', @@ -26,40 +72,38 @@ const BASIC_NOTICE = { 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.', +const BASIC_WARNING_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.130.0 >=1.126.0', + version: '<=1.126.0', }], schemaVersion: '1', + severity: 'warning', }; -const CLI_2_132_AFFECTED_NOTICE_1 = { - title: '(cli): Some bug affecting cdk deploy.', - issueNumber: 29420, - overview: 'cdk deploy bug', - components: [ - { - name: 'cli', - version: '2.132.0', - }, - ], +const BASIC_ERROR_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', + severity: 'error', }; -const CLI_2_132_AFFECTED_NOTICE_2 = { - title: '(cli): Some bug affecting cdk diff.', - issueNumber: 29483, - overview: 'cdk diff bug', - components: [ - { - name: 'cli', - version: '>=2.132.0 <=2.132.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', }; @@ -107,21 +151,33 @@ const NOTICE_FOR_APIGATEWAYV2_CFN_STAGE = { schemaVersion: '1', }; -describe('cli notices', () => { - beforeAll(() => { - jest - .spyOn(version, 'versionNumber') - .mockImplementation(() => '1.0.0'); - }); +describe(FilteredNotice, () => { - afterAll(() => { - jest.restoreAllMocks(); - }); + describe('format', () => { + + test('resolves dynamic values', () => { - describe(formatNotices, () => { - test('correct format', () => { - const result = formatNotices([BASIC_NOTICE])[0]; - expect(result).toEqual(`16603 Toggling off auto_delete_objects for Bucket empties the bucket + const filteredNotice = new FilteredNotice(BASIC_DYNAMIC_NOTICE); + filteredNotice.addDynamicValue('DYNAMIC1', 'dynamic-value1'); + filteredNotice.addDynamicValue('DYNAMIC2', 'dynamic-value2'); + + expect(filteredNotice.format()).toMatchInlineSnapshot(` +"16603 Toggling off auto_delete_objects for Bucket empties the bucket + + Overview: dynamic-value1 this is a notice with dynamic values + dynamic-value2 + + Affected versions: cli: <=1.126.0 + + More information at: https://github.com/aws/aws-cdk/issues/16603 +" +`); + }); + + test('single version range', () => { + + expect(new FilteredNotice(BASIC_NOTICE).format()).toMatchInlineSnapshot(` +"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 @@ -131,12 +187,14 @@ describe('cli notices', () => { Affected versions: cli: <=1.126.0 More information at: https://github.com/aws/aws-cdk/issues/16603 +" `); }); - test('multiple affect versions', () => { - const result = formatNotices([MULTIPLE_AFFECTED_VERSIONS_NOTICE])[0]; - expect(result).toEqual(`17061 Error when building EKS cluster with monocdk import + test('multiple version ranges', () => { + + expect(new FilteredNotice(MULTIPLE_AFFECTED_VERSIONS_NOTICE).format()).toMatchInlineSnapshot(` +"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 @@ -145,396 +203,664 @@ describe('cli notices', () => { Affected versions: cli: <1.130.0 >=1.126.0 More information at: https://github.com/aws/aws-cdk/issues/17061 +" `); }); + }); - describe(filterNotices, () => { - test('correctly filter notices on cli', () => { +}); + +describe(NoticesFilter, () => { + + describe('filter', () => { + + test('cli', async () => { + const notices = [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]; - expect(filterNotices(notices, { - cliVersion: '1.0.0', - })).toEqual([BASIC_NOTICE]); - expect(filterNotices(notices, { - cliVersion: '1.129.0', - })).toEqual([MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + // doesn't matter for this test because our data only has CLI notices + const outDir = path.join(__dirname, 'cloud-assembly-trees', 'built-with-2_12_0'); - expect(filterNotices(notices, { - cliVersion: '1.126.0', - })).toEqual(notices); + expect(NoticesFilter.filter({ data: notices, bootstrappedEnvironments: [], outDir, cliVersion: '1.0.0' }).map(f => f.notice)).toEqual([BASIC_NOTICE]); + expect(NoticesFilter.filter({ data: notices, bootstrappedEnvironments: [], outDir, cliVersion: '1.129.0' }).map(f => f.notice)).toEqual([MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + expect(NoticesFilter.filter({ data: notices, bootstrappedEnvironments: [], outDir, cliVersion: '1.126.0' }).map(f => f.notice)).toEqual([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + expect(NoticesFilter.filter({ data: notices, bootstrappedEnvironments: [], outDir, cliVersion: '1.130.0' }).map(f => f.notice)).toEqual([]); - expect(filterNotices(notices, { - cliVersion: '1.130.0', - })).toEqual([]); }); - test('correctly filter notices on framework', () => { + test('framework', () => { + const notices = [FRAMEWORK_2_1_0_AFFECTED_NOTICE]; - expect(filterNotices(notices, { - outdir: path.join(__dirname, 'cloud-assembly-trees', 'built-with-2_12_0'), - })).toEqual([]); + // doesn't matter for this test because our data only has framework notices + const cliVersion = '1.0.0'; + + expect(NoticesFilter.filter({ data: notices, cliVersion, bootstrappedEnvironments: [], outDir: path.join(__dirname, 'cloud-assembly-trees', 'built-with-2_12_0') }).map(f => f.notice)).toEqual([]); + expect(NoticesFilter.filter({ data: notices, cliVersion, bootstrappedEnvironments: [], outDir: path.join(__dirname, 'cloud-assembly-trees', 'built-with-1_144_0') }).map(f => f.notice)).toEqual([FRAMEWORK_2_1_0_AFFECTED_NOTICE]); - expect(filterNotices(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 = [NOTICE_FOR_APIGATEWAYV2]; + test('module', () => { + + // doesn't matter for this test because our data only has framework notices + const cliVersion = '1.0.0'; // module-level match - expect(filterNotices(notices, { - outdir: path.join(__dirname, 'cloud-assembly-trees', 'experimental-module'), - })).toEqual([NOTICE_FOR_APIGATEWAYV2]); + expect(NoticesFilter.filter({ data: [NOTICE_FOR_APIGATEWAYV2], cliVersion, bootstrappedEnvironments: [], outDir: path.join(__dirname, 'cloud-assembly-trees', 'experimental-module') }).map(f => f.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([]); + expect(NoticesFilter.filter({ data: [NOTICE_FOR_APIGATEWAYV2], cliVersion, bootstrappedEnvironments: [], outDir: path.join(__dirname, 'cloud-assembly-trees', 'built-with-2_12_0') }).map(f => f.notice)).toEqual([]); // module name mismatch: apigateway != apigatewayv2 - expect(filterNotices([NOTICE_FOR_APIGATEWAY], { - outdir: path.join(__dirname, 'cloud-assembly-trees', 'experimental-module'), - })).toEqual([]); + expect(NoticesFilter.filter({ data: [NOTICE_FOR_APIGATEWAY], cliVersion, bootstrappedEnvironments: [], outDir: path.join(__dirname, 'cloud-assembly-trees', 'experimental-module') }).map(f => f.notice)).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]); + expect(NoticesFilter.filter({ data: [NOTICE_FOR_APIGATEWAYV2_CFN_STAGE], cliVersion, bootstrappedEnvironments: [], outDir: path.join(__dirname, 'cloud-assembly-trees', 'experimental-module') }).map(f => f.notice)).toEqual([NOTICE_FOR_APIGATEWAYV2_CFN_STAGE]); + }); - }); + test('bootstrap', () => { + + // doesn't matter for this test because our data only has bootstrap notices + const outDir = path.join(__dirname, 'cloud-assembly-trees', 'built-with-2_12_0'); + const cliVersion = '1.0.0'; + + const bootstrappedEnvironments: BootstrappedEnvironment[] = [ + { + // affected + bootstrapStackVersion: 22, + environment: { + account: 'account', + region: 'region1', + name: 'env1', + }, + }, + { + // affected + bootstrapStackVersion: 21, + environment: { + account: 'account', + region: 'region2', + name: 'env2', + }, + }, + { + // not affected + bootstrapStackVersion: 28, + environment: { + account: 'account', + region: 'region3', + name: 'env3', + }, + }, + ]; + + expect(NoticesFilter.filter({ + data: [BASIC_BOOTSTRAP_NOTICE], + cliVersion, + outDir, + bootstrappedEnvironments: bootstrappedEnvironments, + }).map(f => f.notice)).toEqual([BASIC_BOOTSTRAP_NOTICE]); - describe(WebsiteNoticeDataSource, () => { - const dataSource = new WebsiteNoticeDataSource(); + }); - test('returns data when download succeeds', async () => { - const result = await mockCall(200, { - notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], - }); + test('ignores invalid bootstrap versions', () => { + + // doesn't matter for this test because our data only has bootstrap notices + const outDir = path.join(__dirname, 'cloud-assembly-trees', 'built-with-2_12_0'); + const cliVersion = '1.0.0'; + + expect(NoticesFilter.filter({ + data: [BASIC_BOOTSTRAP_NOTICE], + cliVersion, + outDir, + bootstrappedEnvironments: [{ bootstrapStackVersion: NaN, environment: { account: 'account', region: 'region', name: 'env' } }], + }).map(f => f.notice)).toEqual([]); - expect(result).toEqual([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); }); - test('returns appropriate error when the server returns an unexpected status code', async () => { - const result = mockCall(500, { - notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], - }); + }); - await expect(result).rejects.toThrow(/500/); +}); + +describe(WebsiteNoticeDataSource, () => { + const dataSource = new WebsiteNoticeDataSource(); + + test('returns data when download succeeds', async () => { + const result = await mockCall(200, { + notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], }); - test('returns appropriate error when the server returns an unexpected structure', async () => { - const result = mockCall(200, { - foo: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], - }); + expect(result).toEqual([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + }); - await expect(result).rejects.toThrow(/key is missing/); + test('returns appropriate error when the server returns an unexpected status code', async () => { + const result = mockCall(500, { + notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], }); - test('returns appropriate error when the server returns invalid json', async () => { - const result = mockCall(200, '-09aiskjkj838'); + await expect(result).rejects.toThrow(/500/); + }); - await expect(result).rejects.toThrow(/Failed to parse/); + test('returns appropriate error when the server returns an unexpected structure', async () => { + const result = mockCall(200, { + foo: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], }); - test('returns appropriate error when HTTPS call throws', async () => { - const mockGet = jest.spyOn(https, 'get') - .mockImplementation(() => { throw new Error('No connection'); }); + await expect(result).rejects.toThrow(/key is missing/); + }); - const result = dataSource.fetch(); + test('returns appropriate error when the server returns invalid json', async () => { + const result = mockCall(200, '-09aiskjkj838'); - await expect(result).rejects.toThrow(/No connection/); + await expect(result).rejects.toThrow(/Failed to parse/); + }); - mockGet.mockRestore(); - }); + test('returns appropriate error when HTTPS call throws', async () => { + const mockGet = jest.spyOn(https, 'get') + .mockImplementation(() => { throw new Error('No connection'); }); - test('returns appropriate error when the request has an error', async () => { - nock('https://cli.cdk.dev-tools.aws.dev') - .get('/notices.json') - .replyWithError('DNS resolution failed'); + const result = dataSource.fetch(); - const result = dataSource.fetch(); + await expect(result).rejects.toThrow(/No connection/); - await expect(result).rejects.toThrow(/DNS resolution failed/); - }); + mockGet.mockRestore(); + }); + + test('returns appropriate error when the request has an error', async () => { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .replyWithError('DNS resolution failed'); + + const result = dataSource.fetch(); + + await expect(result).rejects.toThrow(/DNS resolution failed/); + }); + + test('returns appropriate error when the connection stays idle for too long', async () => { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .delayConnection(3500) + .reply(200, { + notices: [BASIC_NOTICE], + }); + + const result = dataSource.fetch(); - test('returns appropriate error when the connection stays idle for too long', async () => { - nock('https://cli.cdk.dev-tools.aws.dev') - .get('/notices.json') - .delayConnection(3500) - .reply(200, { - notices: [BASIC_NOTICE], - }); + await expect(result).rejects.toThrow(/timed out/); + }); + + test('returns empty array when the request takes too long to finish', async () => { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .delayBody(3500) + .reply(200, { + notices: [BASIC_NOTICE], + }); + + const result = dataSource.fetch(); + + await expect(result).rejects.toThrow(/timed out/); + }); + + function mockCall(statusCode: number, body: any): Promise { + nock('https://cli.cdk.dev-tools.aws.dev') + .get('/notices.json') + .reply(statusCode, body); + + return dataSource.fetch(); + } +}); - const result = dataSource.fetch(); +describe(CachedDataSource, () => { + const fileName = path.join(os.tmpdir(), 'cache.json'); + const cachedData = [BASIC_NOTICE]; + const freshData = [MULTIPLE_AFFECTED_VERSIONS_NOTICE]; - await expect(result).rejects.toThrow(/timed out/); + 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); - test('returns empty array when the request takes too long to finish', async () => { - nock('https://cli.cdk.dev-tools.aws.dev') - .get('/notices.json') - .delayBody(3500) - .reply(200, { - notices: [BASIC_NOTICE], - }); + const notices = await dataSource.fetch(); - const result = dataSource.fetch(); + expect(notices).toEqual(cachedData); + }); - await expect(result).rejects.toThrow(/timed out/); + test('retrieves data from the delegate when the data is expired', async () => { + fs.writeJsonSync(fileName, { + notices: cachedData, + expiration: 0, }); + const dataSource = dataSourceWithDelegateReturning(freshData); - function mockCall(statusCode: number, body: any): Promise { - nock('https://cli.cdk.dev-tools.aws.dev') - .get('/notices.json') - .reply(statusCode, body); + const notices = await dataSource.fetch(); - return dataSource.fetch(); - } + expect(notices).toEqual(freshData); }); - describe(CachedDataSource, () => { - const fileName = path.join(os.tmpdir(), 'cache.json'); - const cachedData = [BASIC_NOTICE]; - const freshData = [MULTIPLE_AFFECTED_VERSIONS_NOTICE]; + test('retrieves data from the delegate when the file cannot be read', async () => { + const debugSpy = jest.spyOn(logging, 'debug'); - beforeEach(() => { - fs.writeFileSync(fileName, ''); - }); + if (fs.existsSync('does-not-exist.json')) { + fs.unlinkSync('does-not-exist.json'); + } + + const dataSource = dataSourceWithDelegateReturning(freshData, 'does-not-exist.json'); - test('retrieves data from the delegate cache when the file is empty', async () => { - const dataSource = dataSourceWithDelegateReturning(freshData); + const notices = await dataSource.fetch(); - const notices = await dataSource.fetch(); + expect(notices).toEqual(freshData); + expect(debugSpy).not.toHaveBeenCalled(); - expect(notices).toEqual(freshData); + debugSpy.mockRestore(); + + if (fs.existsSync('does-not-exist.json')) { + fs.unlinkSync('does-not-exist.json'); + } + }); + + 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); + }); + + test('error in delegate gets turned into empty result by cached source', async () => { + // GIVEN + const delegate = { + fetch: jest.fn().mockRejectedValue(new Error('fetching failed')), + }; + const dataSource = new CachedDataSource(fileName, delegate, true); + + // WHEN + const notices = await dataSource.fetch(); + + // THEN + expect(notices).toEqual([]); + }); + + function dataSourceWithDelegateReturning(notices: Notice[], file: string = fileName, ignoreCache: boolean = false) { + const delegate = { + fetch: jest.fn(), + }; + + delegate.fetch.mockResolvedValue(notices); + return new CachedDataSource(file, delegate, ignoreCache); + } +}); + +describe(Notices, () => { + + beforeEach(() => { + // disable caching + jest.spyOn(CachedDataSource.prototype as any, 'save').mockImplementation((_: any) => Promise.resolve()); + jest.spyOn(CachedDataSource.prototype as any, 'load').mockImplementation(() => Promise.resolve({ expiration: 0, notices: [] })); - test('retrieves data from the file when the data is still valid', async () => { - fs.writeJsonSync(fileName, { - notices: cachedData, - expiration: Date.now() + 10000, + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('addBootstrapVersion', () => { + + test('can add multiple values', async () => { + const notices = Notices.create({ configuration: new Configuration() }); + notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } }); + notices.addBootstrappedEnvironment({ bootstrapStackVersion: 11, environment: { account: 'account', region: 'region', name: 'env' } }); + + await notices.refresh({ + dataSource: { fetch: async () => [BOOTSTRAP_NOTICE_V10, BOOTSTRAP_NOTICE_V11] }, }); - const dataSource = dataSourceWithDelegateReturning(freshData); - const notices = await dataSource.fetch(); + const print = jest.spyOn(logging, 'print'); + + notices.display(); + expect(print).toHaveBeenCalledWith(new FilteredNotice(BOOTSTRAP_NOTICE_V10).format()); + expect(print).toHaveBeenCalledWith(new FilteredNotice(BOOTSTRAP_NOTICE_V11).format()); - expect(notices).toEqual(cachedData); }); - test('retrieves data from the delegate when the data is expired', async () => { - fs.writeJsonSync(fileName, { - notices: cachedData, - expiration: 0, + test('deduplicates', async () => { + const notices = Notices.create({ configuration: new Configuration() }); + notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } }); + notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } }); + + // mock cli version number + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); + + notices.display(); + + const filter = jest.spyOn(NoticesFilter, 'filter'); + notices.display(); + + expect(filter).toHaveBeenCalledTimes(1); + expect(filter).toHaveBeenCalledWith({ + bootstrappedEnvironments: [{ + bootstrapStackVersion: 10, + environment: { + account: 'account', + region: 'region', + name: 'env', + }, + }], + cliVersion: '1.0.0', + data: [], + outDir: 'cdk.out', }); - 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 debugSpy = jest.spyOn(logging, 'debug'); + }); + + describe('refresh', () => { - if (fs.existsSync('does-not-exist.json')) { - fs.unlinkSync('does-not-exist.json'); - } + test('deduplicates notices', async () => { - const dataSource = dataSourceWithDelegateReturning(freshData, 'does-not-exist.json'); + // within the affected version range of the notice + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); - const notices = await dataSource.fetch(); + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] }, + }); - expect(notices).toEqual(freshData); - expect(debugSpy).not.toHaveBeenCalled(); + const print = jest.spyOn(logging, 'print'); - debugSpy.mockRestore(); + notices.display(); + expect(print).toHaveBeenCalledWith(new FilteredNotice(BASIC_NOTICE).format()); - if (fs.existsSync('does-not-exist.json')) { - fs.unlinkSync('does-not-exist.json'); - } }); - test('retrieved data from the delegate when it is configured to ignore the cache', async () => { - fs.writeJsonSync(fileName, { - notices: cachedData, - expiration: Date.now() + 10000, + test('clears notices if empty', async () => { + + // within the affected version range of the notice + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); + + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ + dataSource: { fetch: async () => [] }, }); - const dataSource = dataSourceWithDelegateReturning(freshData, fileName, true); - const notices = await dataSource.fetch(); + const print = jest.spyOn(logging, 'print'); + + notices.display({ showTotal: true }); + expect(print).toHaveBeenNthCalledWith(1, ''); + expect(print).toHaveBeenNthCalledWith(2, 'There are 0 unacknowledged notice(s).'); + expect(print).toHaveBeenCalledTimes(2); - expect(notices).toEqual(freshData); }); - test('error in delegate gets turned into empty result by cached source', async () => { - // GIVEN - const delegate = { - fetch: jest.fn().mockRejectedValue(new Error('fetching failed')), - }; - const dataSource = new CachedDataSource(fileName, delegate, true); + test('doesnt throw', async () => { - // WHEN - const notices = await dataSource.fetch(); + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ + dataSource: { + fetch: async () => { + throw new Error('Should not fail refresh'); + }, + }, + }); - // THEN - expect(notices).toEqual([]); }); - function dataSourceWithDelegateReturning(notices: Notice[], file: string = fileName, ignoreCache: boolean = false) { - const delegate = { - fetch: jest.fn(), - }; + test('does nothing when we shouldnt display', async () => { - delegate.fetch.mockResolvedValue(notices); - return new CachedDataSource(file, delegate, ignoreCache); - } - }); + const settings: any = { notices: false }; + const configuration = new Configuration(); + (configuration.settings as any) = { get: (s_path: string[]) => settings[s_path[0]] }; - describe(generateMessage, () => { - test('does not show anything when there are no notices', async () => { - const dataSource = createDataSource(); - dataSource.fetch.mockResolvedValue([]); + let refreshCalled = false; - const result = await generateMessage(dataSource, { - acknowledgedIssueNumbers: [], - outdir: '/tmp', + const notices = Notices.create({ configuration }); + await notices.refresh({ + dataSource: { + fetch: async () => { + refreshCalled = true; + return Promise.resolve([]); + }, + }, }); - expect(result).toEqual(''); + expect(refreshCalled).toBeFalsy(); + }); - test('Shows no notices when there are no notices with --unacknowledged', async () => { - const dataSource = createDataSource(); - dataSource.fetch.mockResolvedValue([]); + test('filters out acknowledged notices by default', async () => { + + // within the affected version range of both notices + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0'); - const result = await generateMessage(dataSource, { - acknowledgedIssueNumbers: [], - outdir: '/tmp', - unacknowledged: true, + const context: any = { 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }; + const configuration = new Configuration(); + (configuration.context as any) = { get: (key: string) => context[key] }; + + const notices = Notices.create({ configuration }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, }); - expect(result).toEqual('\n\nThere are 0 unacknowledged notice(s).'); + const print = jest.spyOn(logging, 'print'); + + notices.display(); + expect(print).toHaveBeenNthCalledWith(4, new FilteredNotice(BASIC_NOTICE).format()); + expect(print).toHaveBeenNthCalledWith(6, 'If you don’t want to see a notice anymore, use \"cdk acknowledge \". For example, \"cdk acknowledge 16603\".'); + }); - test('shows notices that pass the filter', async () => { - const dataSource = createDataSource(); - dataSource.fetch.mockResolvedValue([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]); + test('preserves acknowledged notices if requested', async () => { - const result = await generateMessage(dataSource, { - acknowledgedIssueNumbers: [17061], - outdir: '/tmp', + // within the affected version range of both notices + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0'); + + const context: any = { 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }; + const configuration = new Configuration(); + (configuration.context as any) = { get: (key: string) => context[key] }; + + const notices = Notices.create({ configuration, includeAcknowlegded: true }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, }); - expect(result).toEqual(` -NOTICES (What's this? https://github.com/aws/aws-cdk/wiki/CLI-Notices) + const print = jest.spyOn(logging, 'print'); -16603 Toggling off auto_delete_objects for Bucket empties the bucket + notices.display(); + expect(print).toHaveBeenCalledWith(new FilteredNotice(BASIC_NOTICE).format()); + expect(print).toHaveBeenCalledWith(new FilteredNotice(MULTIPLE_AFFECTED_VERSIONS_NOTICE).format()); - 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 + describe('display', () => { + test('notices envelop', async () => { + + // within the affected version range of the notice + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); + + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] }, + }); + + const print = jest.spyOn(logging, 'print'); + + notices.display(); + expect(print).toHaveBeenNthCalledWith(2, 'NOTICES (What\'s this? https://github.com/aws/aws-cdk/wiki/CLI-Notices)'); + expect(print).toHaveBeenNthCalledWith(6, 'If you don’t want to see a notice anymore, use \"cdk acknowledge \". For example, \"cdk acknowledge 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(), - }; - } - }); -}); + test('deduplicates notices', async () => { -describe('mock cdk version 2.132.0', () => { - beforeAll(() => { - jest - .spyOn(version, 'versionNumber') - .mockImplementation(() => '2.132.0'); - }); + // within the affected version range of the notice + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); - afterAll(() => { - jest.restoreAllMocks(); - }); + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] }, + }); - test('Shows notices that pass the filter with --unacknowledged', async () => { - const dataSource = createDataSource(); - dataSource.fetch.mockResolvedValue([CLI_2_132_AFFECTED_NOTICE_1, CLI_2_132_AFFECTED_NOTICE_2]); + const print = jest.spyOn(logging, 'print'); + + notices.display(); + expect(print).toHaveBeenNthCalledWith(4, new FilteredNotice(BASIC_NOTICE).format()); + expect(print).toHaveBeenNthCalledWith(6, 'If you don’t want to see a notice anymore, use \"cdk acknowledge \". For example, \"cdk acknowledge 16603\".'); - const allNotices = await generateMessage(dataSource, { - acknowledgedIssueNumbers: [], - outdir: '/tmp', - unacknowledged: true, }); - expect(allNotices).toEqual(` -NOTICES (What's this? https://github.com/aws/aws-cdk/wiki/CLI-Notices) + test('does nothing when we shouldnt display', async () => { -29420 (cli): Some bug affecting cdk deploy. + const settings: any = { notices: false }; + const configuration = new Configuration(); + (configuration.settings as any) = { get: (s_path: string[]) => settings[s_path[0]] }; - Overview: cdk deploy bug + const notices = Notices.create({ configuration }); + await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE] } }); - Affected versions: cli: 2.132.0 + const print = jest.spyOn(logging, 'print'); - More information at: https://github.com/aws/aws-cdk/issues/29420 + notices.display(); + expect(print).toHaveBeenCalledTimes(0); + }); -29483 (cli): Some bug affecting cdk diff. + test('nothing when there are no notices', async () => { - Overview: cdk diff bug + const print = jest.spyOn(logging, 'print'); - Affected versions: cli: >=2.132.0 <=2.132.1 + Notices.create({ configuration: new Configuration() }).display(); + expect(print).toHaveBeenCalledTimes(0); - More information at: https://github.com/aws/aws-cdk/issues/29483 + }); + test('total count when show total is true', async () => { -If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge 29420". + const print = jest.spyOn(logging, 'print'); -There are 2 unacknowledged notice(s).`); + Notices.create({ configuration: new Configuration() }).display({ showTotal: true }); + expect(print).toHaveBeenNthCalledWith(2, 'There are 0 unacknowledged notice(s).'); - const acknowledgeNotice29420 = await generateMessage(dataSource, { - acknowledgedIssueNumbers: [29420], - outdir: '/tmp', - unacknowledged: true, }); - expect(acknowledgeNotice29420).toEqual(` -NOTICES (What's this? https://github.com/aws/aws-cdk/wiki/CLI-Notices) + test('warning', async () => { -29483 (cli): Some bug affecting cdk diff. + // within the affected version range of the notice + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); - Overview: cdk diff bug + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_WARNING_NOTICE] }, + }); + + const warning = jest.spyOn(logging, 'warning'); + + notices.display(); + expect(warning).toHaveBeenNthCalledWith(1, new FilteredNotice(BASIC_NOTICE).format()); + expect(warning).toHaveBeenCalledTimes(1); - Affected versions: cli: >=2.132.0 <=2.132.1 + }); + + test('error', async () => { - More information at: https://github.com/aws/aws-cdk/issues/29483 + // within the affected version range of the notice + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_ERROR_NOTICE] }, + }); -If you don’t want to see a notice anymore, use "cdk acknowledge ". For example, "cdk acknowledge 29483". + const error = jest.spyOn(logging, 'error'); -There are 1 unacknowledged notice(s).`); + notices.display(); + expect(error).toHaveBeenNthCalledWith(1, new FilteredNotice(BASIC_NOTICE).format()); + expect(error).toHaveBeenCalledTimes(1); - const allAcknowledgedNotices = await generateMessage(dataSource, { - acknowledgedIssueNumbers: [29420, 29483], - outdir: '/tmp', - unacknowledged: true, }); - expect(allAcknowledgedNotices).toEqual('\n\nThere are 0 unacknowledged notice(s).'); + test('only relevant notices', async () => { + + // within the affected version range of the notice + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); + + const notices = Notices.create({ configuration: new Configuration() }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_NOTICE] }, + }); + + const print = jest.spyOn(logging, 'print'); + + notices.display(); + expect(print).toHaveBeenNthCalledWith(4, new FilteredNotice(BASIC_NOTICE).format()); + + }); + + test('only unacknowledged notices', async () => { + + // within the affected version range of both notices + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0'); + + const context: any = { 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }; + const configuration = new Configuration(); + (configuration.context as any) = { get: (key: string) => context[key] }; + + const notices = Notices.create({ configuration }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, + }); + + const print = jest.spyOn(logging, 'print'); + + notices.display(); + expect(print).toHaveBeenNthCalledWith(4, new FilteredNotice(BASIC_NOTICE).format()); + + }); + + test('can include acknowledged notices if requested', async () => { + + // within the affected version range of both notices + jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0'); + + const context: any = { 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }; + const configuration = new Configuration(); + (configuration.context as any) = { get: (key: string) => context[key] }; + + const notices = Notices.create({ configuration, includeAcknowlegded: true }); + await notices.refresh({ + dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, + }); + + const print = jest.spyOn(logging, 'print'); + + notices.display(); + expect(print).toHaveBeenNthCalledWith(4, new FilteredNotice(BASIC_NOTICE).format()); + expect(print).toHaveBeenNthCalledWith(6, new FilteredNotice(MULTIPLE_AFFECTED_VERSIONS_NOTICE).format()); + + }); - function createDataSource() { - return { - fetch: jest.fn(), - }; - } }); });