Skip to content

Commit

Permalink
chore: add option unacknowledged to cdk notices (#31250)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)

Closes #23078

### Reason for this change

`cdk notices` will show all the notices the cdk version is affected by.
The additional command `cdk notices --unacknowledged` will show the notices the customer hasn't acknowledge with `cdk acknowledge <NUMBER>`.

The output will display the number of unacknowledged notices, with the following line as the last statement:
`There are <NUMBER> unacknowledged notice(s).`
Customer or automation tools can use this command to filter for the number of unacknowledged notices.


### 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*
  • Loading branch information
awslukeguan committed Sep 4, 2024
1 parent 130b62b commit cd58b50
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2190,3 +2190,13 @@ async function listChildren(parent: string, pred: (x: string) => Promise<boolean
async function listChildDirs(parent: string) {
return listChildren(parent, async (fullPath: string) => (await fs.stat(fullPath)).isDirectory());
}

integTest(
'cdk notices with --unacknowledged',
withDefaultFixture(async (fixture) => {
const noticesUnacknowledged = await fixture.cdk(['notices', '--unacknowledged'], { verbose: false });
const noticesUnacknowledgedAlias = await fixture.cdk(['notices', '-u'], { verbose: false });
expect(noticesUnacknowledged).toEqual(expect.stringMatching(/There are \d{1,} unacknowledged notice\(s\)./));
expect(noticesUnacknowledged).toEqual(noticesUnacknowledgedAlias);
}),
);
25 changes: 25 additions & 0 deletions packages/aws-cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,31 @@ NOTICES
If you don’t want to see a notice anymore, use "cdk acknowledge <id>". For example, "cdk acknowledge 16603".
```

List the unacknowledged notices:

```console
$ cdk notices --unacknowledged

NOTICES (What's this? https://github.com/aws/aws-cdk/wiki/CLI-Notices)

29483 (cli): Upgrading to v2.132.0 or v2.132.1 breaks cdk diff functionality

Overview: cdk diff functionality used to rely on assuming lookup-role.
With a recent change present in v2.132.0 and v2.132.1, it is
now trying to assume deploy-role with the lookup-role. This
leads to an authorization error if permissions were not
defined to assume deploy-role.

Affected versions: cli: >=2.132.0 <=2.132.1

More information at: https://github.com/aws/aws-cdk/issues/29483


If you don’t want to see a notice anymore, use "cdk acknowledge <id>". For example, "cdk acknowledge 29483".

There are 1 unacknowledged notice(s).
```

### Bundling

By default asset bundling is skipped for `cdk list` and `cdk destroy`. For `cdk deploy`, `cdk diff`
Expand Down
13 changes: 11 additions & 2 deletions packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ async function parseCommandLineArguments(args: string[]) {
.option('change-set', { type: 'boolean', alias: 'changeset', desc: 'Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role.', default: true }))
.command('metadata [STACK]', 'Returns all metadata associated with this stack')
.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('notices', 'Returns a list of relevant notices', (yargs: Argv) => yargs
.option('unacknowledged', { type: 'boolean', alias: 'u', default: false, desc: 'Returns a list of unacknowledged notices' }),
)
.command('init [TEMPLATE]', 'Create a new, empty CDK project from a template.', (yargs: Argv) => 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' })
Expand Down Expand Up @@ -439,7 +441,14 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
await version.displayVersionMessage();

if (shouldDisplayNotices()) {
if (cmd === 'notices') {
if (cmd === 'notices' && argv.unacknowledged === true) {
await displayNotices({
outdir: configuration.settings.get(['output']) ?? 'cdk.out',
acknowledgedIssueNumbers: configuration.context.get('acknowledged-issue-numbers') ?? [],
ignoreCache: true,
unacknowledged: true,
});
} else if (cmd === 'notices') {
await displayNotices({
outdir: configuration.settings.get(['output']) ?? 'cdk.out',
acknowledgedIssueNumbers: [],
Expand Down
23 changes: 19 additions & 4 deletions packages/aws-cdk/lib/notices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as path from 'path';
import * as fs from 'fs-extra';
import * as semver from 'semver';
import { debug, print } from './logging';
import { some, ConstructTreeNode, loadTreeFromDir } from './tree';
import { ConstructTreeNode, loadTreeFromDir, some } from './tree';
import { flatMap } from './util';
import { cdkCacheDir } from './util/directories';
import { versionNumber } from './version';
Expand All @@ -30,6 +30,13 @@ export interface DisplayNoticesProps {
* @default false
*/
readonly ignoreCache?: boolean;

/**
* Whether to append the number of unacknowledged notices to the display.
*
* @default false
*/
readonly unacknowledged?: boolean;
}

export async function refreshNotices() {
Expand All @@ -50,11 +57,19 @@ export async function generateMessage(dataSource: NoticeDataSource, props: Displ
acknowledgedIssueNumbers: new Set(props.acknowledgedIssueNumbers),
});

let messageString: string = '';
if (filteredNotices.length > 0) {
const individualMessages = formatNotices(filteredNotices);
return finalMessage(individualMessages, filteredNotices[0].issueNumber);
messageString = getFilteredMessages(filteredNotices);
}
if (props.unacknowledged) {
messageString = [messageString, `There are ${filteredNotices.length} unacknowledged notice(s).`].join('\n\n');
}
return '';
return messageString;
}

function getFilteredMessages(filteredNotices: Notice[]): string {
const individualMessages = formatNotices(filteredNotices);
return finalMessage(individualMessages, filteredNotices[0].issueNumber);
}

function dataSourceReference(ignoreCache: boolean): NoticeDataSource {
Expand Down
123 changes: 123 additions & 0 deletions packages/aws-cdk/test/notices.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,32 @@ const MULTIPLE_AFFECTED_VERSIONS_NOTICE = {
schemaVersion: '1',
};

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',
},
],
schemaVersion: '1',
};

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',
},
],
schemaVersion: '1',
};

const FRAMEWORK_2_1_0_AFFECTED_NOTICE = {
title: 'Regression on module foobar',
issueNumber: 1234,
Expand Down Expand Up @@ -381,6 +407,19 @@ describe('cli notices', () => {
expect(result).toEqual('');
});

test('Shows no notices when there are no notices with --unacknowledged', async () => {
const dataSource = createDataSource();
dataSource.fetch.mockResolvedValue([]);

const result = await generateMessage(dataSource, {
acknowledgedIssueNumbers: [],
outdir: '/tmp',
unacknowledged: true,
});

expect(result).toEqual('\n\nThere are 0 unacknowledged notice(s).');
});

test('shows notices that pass the filter', async () => {
const dataSource = createDataSource();
dataSource.fetch.mockResolvedValue([BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE]);
Expand Down Expand Up @@ -415,3 +454,87 @@ If you don’t want to see a notice anymore, use "cdk acknowledge <id>". For exa
}
});
});

describe('mock cdk version 2.132.0', () => {
beforeAll(() => {
jest
.spyOn(version, 'versionNumber')
.mockImplementation(() => '2.132.0');
});

afterAll(() => {
jest.restoreAllMocks();
});

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 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)
29420 (cli): Some bug affecting cdk deploy.
Overview: cdk deploy bug
Affected versions: cli: 2.132.0
More information at: https://github.com/aws/aws-cdk/issues/29420
29483 (cli): Some bug affecting cdk diff.
Overview: cdk diff bug
Affected versions: cli: >=2.132.0 <=2.132.1
More information at: https://github.com/aws/aws-cdk/issues/29483
If you don’t want to see a notice anymore, use "cdk acknowledge <id>". For example, "cdk acknowledge 29420".
There are 2 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)
29483 (cli): Some bug affecting cdk diff.
Overview: cdk diff bug
Affected versions: cli: >=2.132.0 <=2.132.1
More information at: https://github.com/aws/aws-cdk/issues/29483
If you don’t want to see a notice anymore, use "cdk acknowledge <id>". For example, "cdk acknowledge 29483".
There are 1 unacknowledged notice(s).`);

const allAcknowledgedNotices = await generateMessage(dataSource, {
acknowledgedIssueNumbers: [29420, 29483],
outdir: '/tmp',
unacknowledged: true,
});

expect(allAcknowledgedNotices).toEqual('\n\nThere are 0 unacknowledged notice(s).');

function createDataSource() {
return {
fetch: jest.fn(),
};
}
});
});

0 comments on commit cd58b50

Please sign in to comment.