diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index f40a71945485b..99c9fb080229c 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -84,7 +84,12 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise s.environment.name).join(',')); return [filtered]; - }); } @@ -178,7 +186,6 @@ export class NoticesFilter { } }); } - } /** @@ -193,7 +200,6 @@ export interface BootstrappedEnvironment { * Provides access to notices the CLI can display. */ export class Notices { - /** * Create an instance. Note that this replaces the singleton. */ @@ -211,7 +217,9 @@ export class Notices { private static _instance: Notices | undefined; - private readonly configuration: Configuration; + private readonly context: Context; + private readonly output: string; + private readonly shouldDisplay: boolean; private readonly acknowledgedIssueNumbers: Set; private readonly includeAcknowlegded: boolean; @@ -221,9 +229,11 @@ export class Notices { 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.context = props.context; + this.acknowledgedIssueNumbers = new Set(this.context.get('acknowledged-issue-numbers') ?? []); this.includeAcknowlegded = props.includeAcknowledged ?? false; + this.output = props.output ?? 'cdk.out'; + this.shouldDisplay = props.shouldDisplay ?? true; } /** @@ -248,8 +258,7 @@ export class Notices { * If context is configured to not display notices, this will no-op. */ public async refresh(options: NoticesRefreshOptions = {}) { - - if (!this.shouldDisplay()) { + if (!this.shouldDisplay) { return; } @@ -266,15 +275,14 @@ export class Notices { * Display the relevant notices (unless context dictates we shouldn't). */ public display(options: NoticesPrintOptions = {}) { - - if (!this.shouldDisplay()) { + if (!this.shouldDisplay) { return; } const filteredNotices = NoticesFilter.filter({ data: Array.from(this.data), cliVersion: versionNumber(), - outDir: this.configuration.settings.get(['output']) ?? 'cdk.out', + outDir: this.output, bootstrappedEnvironments: Array.from(this.bootstrappedEnvironments.values()), }); @@ -303,17 +311,7 @@ export class Notices { 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 { @@ -335,7 +333,6 @@ export interface Notice { * 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) {} @@ -353,7 +350,6 @@ export class FilteredNotice { `\tAffected versions: ${componentsValue}`, `\tMore information at: https://github.com/aws/aws-cdk/issues/${this.notice.issueNumber}`, ].join('\n\n') + '\n'); - } private formatOverview() { @@ -372,7 +368,6 @@ export class FilteredNotice { const pattern = new RegExp(Object.keys(this.dynamicValues).join('|'), 'g'); return input.replace(pattern, (matched) => this.dynamicValues[matched] ?? matched); } - } export interface NoticeDataSource { diff --git a/packages/aws-cdk/test/api/environment-resources.test.ts b/packages/aws-cdk/test/api/environment-resources.test.ts index d2ed83ba85658..aa29b750afb10 100644 --- a/packages/aws-cdk/test/api/environment-resources.test.ts +++ b/packages/aws-cdk/test/api/environment-resources.test.ts @@ -2,7 +2,7 @@ import { GetParameterCommand } from '@aws-sdk/client-ssm'; 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 { Context } from '../../lib/settings'; import * as version from '../../lib/version'; import { MockSdk, mockBootstrapStack, mockSSMClient } from '../util/mock-sdk'; import { MockToolkitInfo } from '../util/mock-toolkitinfo'; @@ -98,7 +98,7 @@ describe('validateversion without bootstrap stack', () => { jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); // THEN - const notices = Notices.create({ configuration: new Configuration() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => [] } }); await expect(envResources().validateVersion(8, '/abc')).resolves.toBeUndefined(); diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index a4813da812731..9c6f1d0786fb0 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -15,7 +15,7 @@ import { BootstrappedEnvironment, } from '../lib/notices'; import * as version from '../lib/version'; -import { Configuration } from '../lib/settings'; +import { Context, Settings } from '../lib/settings'; const BASIC_BOOTSTRAP_NOTICE = { title: 'Exccessive permissions on file asset publishing role', @@ -152,11 +152,8 @@ const NOTICE_FOR_APIGATEWAYV2_CFN_STAGE = { }; describe(FilteredNotice, () => { - describe('format', () => { - test('resolves dynamic values', () => { - const filteredNotice = new FilteredNotice(BASIC_DYNAMIC_NOTICE); filteredNotice.addDynamicValue('DYNAMIC1', 'dynamic-value1'); filteredNotice.addDynamicValue('DYNAMIC2', 'dynamic-value2'); @@ -175,7 +172,6 @@ describe(FilteredNotice, () => { }); test('single version range', () => { - expect(new FilteredNotice(BASIC_NOTICE).format()).toMatchInlineSnapshot(` "16603 Toggling off auto_delete_objects for Bucket empties the bucket @@ -192,7 +188,6 @@ describe(FilteredNotice, () => { }); test('multiple version ranges', () => { - expect(new FilteredNotice(MULTIPLE_AFFECTED_VERSIONS_NOTICE).format()).toMatchInlineSnapshot(` "17061 Error when building EKS cluster with monocdk import @@ -206,17 +201,12 @@ describe(FilteredNotice, () => { " `); }); - }); - }); describe(NoticesFilter, () => { - describe('filter', () => { - test('cli', async () => { - const notices = [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]; // doesn't matter for this test because our data only has CLI notices @@ -226,11 +216,9 @@ describe(NoticesFilter, () => { 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([]); - }); test('framework', () => { - const notices = [FRAMEWORK_2_1_0_AFFECTED_NOTICE]; // doesn't matter for this test because our data only has framework notices @@ -238,11 +226,9 @@ describe(NoticesFilter, () => { 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]); - }); test('module', () => { - // doesn't matter for this test because our data only has framework notices const cliVersion = '1.0.0'; @@ -257,11 +243,9 @@ describe(NoticesFilter, () => { // construct-level match 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'; @@ -302,11 +286,9 @@ describe(NoticesFilter, () => { outDir, bootstrappedEnvironments: bootstrappedEnvironments, }).map(f => f.notice)).toEqual([BASIC_BOOTSTRAP_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'; @@ -317,11 +299,8 @@ describe(NoticesFilter, () => { outDir, bootstrappedEnvironments: [{ bootstrapStackVersion: NaN, environment: { account: 'account', region: 'region', name: 'env' } }], }).map(f => f.notice)).toEqual([]); - }); - }); - }); describe(WebsiteNoticeDataSource, () => { @@ -509,7 +488,6 @@ describe(CachedDataSource, () => { }); describe(Notices, () => { - beforeEach(() => { // disable caching jest.spyOn(CachedDataSource.prototype as any, 'save').mockImplementation((_: any) => Promise.resolve()); @@ -521,9 +499,8 @@ describe(Notices, () => { }); describe('addBootstrapVersion', () => { - test('can add multiple values', async () => { - const notices = Notices.create({ configuration: new Configuration() }); + const notices = Notices.create({ context: new Context() }); notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } }); notices.addBootstrappedEnvironment({ bootstrapStackVersion: 11, environment: { account: 'account', region: 'region', name: 'env' } }); @@ -539,7 +516,7 @@ describe(Notices, () => { }); test('deduplicates', async () => { - const notices = Notices.create({ configuration: new Configuration() }); + const notices = Notices.create({ context: new Context() }); notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } }); notices.addBootstrappedEnvironment({ bootstrapStackVersion: 10, environment: { account: 'account', region: 'region', name: 'env' } }); @@ -565,19 +542,15 @@ describe(Notices, () => { data: [], outDir: 'cdk.out', }); - }); - }); describe('refresh', () => { - test('deduplicates 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() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] }, }); @@ -586,15 +559,13 @@ describe(Notices, () => { notices.display(); expect(print).toHaveBeenCalledWith(new FilteredNotice(BASIC_NOTICE).format()); - }); 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() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => [] }, }); @@ -605,12 +576,10 @@ describe(Notices, () => { expect(print).toHaveBeenNthCalledWith(1, ''); expect(print).toHaveBeenNthCalledWith(2, 'There are 0 unacknowledged notice(s).'); expect(print).toHaveBeenCalledTimes(2); - }); test('doesnt throw', async () => { - - const notices = Notices.create({ configuration: new Configuration() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => { @@ -618,18 +587,11 @@ describe(Notices, () => { }, }, }); - }); test('does nothing when we shouldnt display', async () => { - - const settings: any = { notices: false }; - const configuration = new Configuration(); - (configuration.settings as any) = { get: (s_path: string[]) => settings[s_path[0]] }; - let refreshCalled = false; - - const notices = Notices.create({ configuration }); + const notices = Notices.create({ context: new Context(), shouldDisplay: false }); await notices.refresh({ dataSource: { fetch: async () => { @@ -640,19 +602,15 @@ describe(Notices, () => { }); expect(refreshCalled).toBeFalsy(); - }); 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 context: any = { 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }; - const configuration = new Configuration(); - (configuration.context as any) = { get: (key: string) => context[key] }; + const context = new Context(new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] })); - const notices = Notices.create({ configuration }); + const notices = Notices.create({ context }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, }); @@ -662,19 +620,15 @@ describe(Notices, () => { 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('preserves 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 context = new Context(new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] })); - const notices = Notices.create({ configuration, includeAcknowledged: true }); + const notices = Notices.create({ context, includeAcknowledged: true }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, }); @@ -684,19 +638,15 @@ describe(Notices, () => { notices.display(); expect(print).toHaveBeenCalledWith(new FilteredNotice(BASIC_NOTICE).format()); expect(print).toHaveBeenCalledWith(new FilteredNotice(MULTIPLE_AFFECTED_VERSIONS_NOTICE).format()); - }); - }); 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() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] }, }); @@ -706,15 +656,13 @@ describe(Notices, () => { 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\".'); - }); test('deduplicates 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() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE, BASIC_NOTICE] }, }); @@ -724,49 +672,37 @@ describe(Notices, () => { 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('does nothing when we shouldnt display', async () => { - - const settings: any = { notices: false }; - const configuration = new Configuration(); - (configuration.settings as any) = { get: (s_path: string[]) => settings[s_path[0]] }; - - const notices = Notices.create({ configuration }); + const notices = Notices.create({ context: new Context(), shouldDisplay: false }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE] } }); const print = jest.spyOn(logging, 'print'); notices.display(); expect(print).toHaveBeenCalledTimes(0); - }); test('nothing when there are no notices', async () => { - const print = jest.spyOn(logging, 'print'); - Notices.create({ configuration: new Configuration() }).display(); + Notices.create({ context: new Context() }).display(); expect(print).toHaveBeenCalledTimes(0); - }); test('total count when show total is true', async () => { - const print = jest.spyOn(logging, 'print'); - Notices.create({ configuration: new Configuration() }).display({ showTotal: true }); + Notices.create({ context: new Context() }).display({ showTotal: true }); expect(print).toHaveBeenNthCalledWith(2, 'There are 0 unacknowledged notice(s).'); - }); test('warning', async () => { - // within the affected version range of the notice jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); - const notices = Notices.create({ configuration: new Configuration() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_WARNING_NOTICE] }, }); @@ -776,15 +712,13 @@ describe(Notices, () => { notices.display(); expect(warning).toHaveBeenNthCalledWith(1, new FilteredNotice(BASIC_NOTICE).format()); expect(warning).toHaveBeenCalledTimes(1); - }); test('error', async () => { - // within the affected version range of the notice jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0'); - const notices = Notices.create({ configuration: new Configuration() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_ERROR_NOTICE] }, }); @@ -794,15 +728,13 @@ describe(Notices, () => { notices.display(); expect(error).toHaveBeenNthCalledWith(1, new FilteredNotice(BASIC_NOTICE).format()); expect(error).toHaveBeenCalledTimes(1); - }); 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() }); + const notices = Notices.create({ context: new Context() }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE] }, }); @@ -811,19 +743,15 @@ describe(Notices, () => { 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 context = new Context(new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] })); - const notices = Notices.create({ configuration }); + const notices = Notices.create({ context }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, }); @@ -832,19 +760,14 @@ describe(Notices, () => { 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, includeAcknowledged: true }); + const context = new Context(new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] })); + const notices = Notices.create({ context, includeAcknowledged: true }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, }); @@ -854,8 +777,6 @@ describe(Notices, () => { notices.display(); expect(print).toHaveBeenNthCalledWith(4, new FilteredNotice(BASIC_NOTICE).format()); expect(print).toHaveBeenNthCalledWith(6, new FilteredNotice(MULTIPLE_AFFECTED_VERSIONS_NOTICE).format()); - }); - }); });