Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): support for notices #18936

Merged
merged 26 commits into from Feb 22, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 30 additions & 7 deletions packages/aws-cdk/bin/cdk.ts
Expand Up @@ -17,6 +17,7 @@ import { CdkToolkit } from '../lib/cdk-toolkit';
import { RequireApproval } from '../lib/diff';
import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib/init';
import { data, debug, error, print, setLogLevel } from '../lib/logging';
import { displayNotices } from '../lib/notices';
import { PluginHost } from '../lib/plugin';
import { serializeStructure } from '../lib/serialize';
import { Command, Configuration, Settings } from '../lib/settings';
Expand Down Expand Up @@ -184,6 +185,8 @@ async function parseCommandLineArguments() {
.option('security-only', { type: 'boolean', desc: 'Only diff for broadened security changes', default: false })
.option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff', default: false }))
.command('metadata [STACK]', 'Returns all metadata associated with this stack')
.command('acknowledge [ID]', 'Acknowledge a notice so that it does not show up anymore')
.command('notices', 'Returns a list of relevant notices')
.command('init [TEMPLATE]', 'Create a new, empty CDK project from a template.', yargs => yargs
.option('language', { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: initTemplateLanguages })
.option('list', { type: 'boolean', desc: 'List the available templates' })
Expand All @@ -207,7 +210,7 @@ if (!process.stdout.isTTY) {
process.env.FORCE_COLOR = '0';
}

async function initCommandLine() {
async function initCommandLine(): Promise<{ configuration: Configuration, value: any }> {
const argv = await parseCommandLineArguments();
if (argv.verbose) {
setLogLevel(argv.verbose);
Expand Down Expand Up @@ -280,11 +283,20 @@ async function initCommandLine() {
? await (argv.commandHandler as (opts: typeof commandOptions) => any)(commandOptions)
: await main(cmd, argv);
if (typeof returnValue === 'object') {
return toJsonOrYaml(returnValue);
return {
value: toJsonOrYaml(returnValue),
configuration,
};
} else if (typeof returnValue === 'string') {
return returnValue;
return {
value: returnValue,
configuration,
};
} else {
return returnValue;
return {
value: returnValue,
configuration,
};
}
} finally {
await version.displayVersionMessage();
Expand Down Expand Up @@ -428,10 +440,16 @@ async function initCommandLine() {
return cli.synth(args.STACKS, true, args.quiet, args.validation);
}


case 'metadata':
return cli.metadata(args.STACK);

case 'acknowledge':
return cli.acknowledge(args.ID);

case 'notices':
SHOW_ALL_NOTICES = true;
return;

case 'init':
const language = configuration.settings.get(['language']);
if (args.list) {
Expand Down Expand Up @@ -521,14 +539,19 @@ function yargsNegativeAlias<T extends { [x in S | L ]: boolean | undefined }, S
};
}

let SHOW_ALL_NOTICES = false;
initCommandLine()
.then(value => {
if (value == null) { return; }
.then(async ({ value, configuration }) => {
otaviomacedo marked this conversation as resolved.
Show resolved Hide resolved
if (typeof value === 'string') {
data(value);
} else if (typeof value === 'number') {
process.exitCode = value;
}
await displayNotices({
outdir: configuration.settings.get(['output']) ?? 'cdk.out',
acknowledgedIssueNumbers: SHOW_ALL_NOTICES ? [] : configuration.context.get('acknowledged-issue-numbers') ?? [],
permanentlySuppressed: SHOW_ALL_NOTICES ? false : !(configuration.context.get('notices') ?? true),
});
})
.catch(err => {
error(err.message);
Expand Down
7 changes: 7 additions & 0 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Expand Up @@ -79,6 +79,13 @@ export class CdkToolkit {
return stacks.firstStack.manifest.metadata ?? {};
}

public async acknowledge(noticeId: string) {
otaviomacedo marked this conversation as resolved.
Show resolved Hide resolved
const acks = this.props.configuration.context.get('acknowledged-issue-numbers') ?? [];
acks.push(Number(noticeId));
this.props.configuration.context.set('acknowledged-issue-numbers', acks);
await this.props.configuration.saveContext();
}

public async diff(options: DiffOptions): Promise<number> {
const stacks = await this.selectStacksForDiff(options.stackNames, options.exclusively);

Expand Down
247 changes: 247 additions & 0 deletions packages/aws-cdk/lib/notices.ts
@@ -0,0 +1,247 @@
import * as https from 'https';
import * as path from 'path';
import * as fs from 'fs-extra';
import * as semver from 'semver';
import { print, debug } from './logging';
import { cdkCacheDir } from './util/directories';
import { versionNumber } from './version';

const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json');

export interface DisplayNoticesProps {
readonly outdir: string;
readonly acknowledgedIssueNumbers: number[];
readonly temporarilySuppressed?: boolean;
otaviomacedo marked this conversation as resolved.
Show resolved Hide resolved
readonly permanentlySuppressed?: boolean;
otaviomacedo marked this conversation as resolved.
Show resolved Hide resolved
readonly cliVersion?: string;
}

export async function displayNotices(props: DisplayNoticesProps) {
const dataSource = dataSourceReference();

const notices = await dataSource.fetch();
const individualMessages = formatNotices(filterNotices(notices, {
outdir: props.outdir,
temporarilySuppressed: props.temporarilySuppressed,
permanentlySuppressed: props.permanentlySuppressed,
acknowledgedIssueNumbers: arrayToSet(props.acknowledgedIssueNumbers),
}));

if (individualMessages.length > 0) {
print(finalMessage(individualMessages, notices[0].issueNumber));
}
}

function dataSourceReference(): CachedDataSource {
return new CachedDataSource(CACHE_FILE_PATH, new WebsiteNoticeDataSource());
}

function arrayToSet(array: any[]): Set<number> {
const set = new Set<number>();
array.forEach(a => set.add(a));
return set;
}

function finalMessage(individualMessages: string[], exampleNumber: number): string {
return [
'NOTICES',
...individualMessages,
`If you don’t want to see a notice anymore, use "cdk acknowledge <id>". For example, "cdk acknowledge ${exampleNumber}".`,
].join('\n\n');
}

export interface FilterNoticeOptions {
outdir?: string,
permanentlySuppressed?: boolean,
temporarilySuppressed?: boolean,
cliVersion?: string,
frameworkVersion?: string,
acknowledgedIssueNumbers?: Set<number>,
}

export function filterNotices(notices: Notice[], options: FilterNoticeOptions): Notice[] {
const filter = new NoticeFilter({
cliVersion: options.cliVersion ?? versionNumber(),
frameworkVersion: options.frameworkVersion ?? frameworkVersion(options.outdir ?? 'cdk.out'),
temporarilySuppressed: options.temporarilySuppressed ?? false, // coming from a command line option
permanentlySuppressed: options.permanentlySuppressed ?? false, // coming from the cdk.json file
acknowledgedIssueNumbers: options.acknowledgedIssueNumbers ?? new Set(),
});
return notices.filter(notice => filter.apply(notice));
}

export function formatNotices(notices: Notice[]): string[] {
const formatter = new FooNoticeFormatter();
return notices.map(notice => formatter.apply(notice));
}

export interface Component {
name: string;
version: string;
}

export interface Notice {
title: string;
issueNumber: number;
overview: string;
components: Component[];
schemaVersion: string;
}

export interface NoticeDataSource {
fetch(): Promise<Notice[]>,
}

export class WebsiteNoticeDataSource implements NoticeDataSource {
fetch(): Promise<Notice[]> {
return new Promise((resolve, reject) => {
https.get('https://cli.cdk.dev-tools.aws.dev/notices.json', res => {
if (res.statusCode === 200) {
res.setEncoding('utf8');
let rawData = '';
res.on('data', (chunk) => {
rawData += chunk;
});
res.on('end', () => {
try {
const notices = JSON.parse(rawData).notices as Notice[];
resolve(notices);
} catch (e) {
reject(e);
}
});
}
// Otherwise, we just ignore the result and move on
});
});
}
}

interface CachedNotices {
expiration: number,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still find this a little weird. Why wouldn't we use the mtime of the cached notices file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're probably right. But I think I've got a bit of a tunnel vision here: the code for retrieving the contents (including some form of timestamp) and the code for comparison/retrieval should be kept separate. So we need some data structure to send data from one to the other, which means this interface would still exist. The only difference is that the expiration time would be a little more hidden.

Now tell me how I am wrong :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, you're not wrong. I guess it doesn't matter. I'm having a hard time thinking up cases in which they produce different results.

notices: Notice[],
}

const TIME_TO_LIVE = 60 * 60 * 1000; // 1 hour

export class CachedDataSource implements NoticeDataSource {
constructor(
private readonly fileName: string,
private readonly dataSource: NoticeDataSource) {
}

async fetch(): Promise<Notice[]> {
const cachedData = await this.load();
const notices = cachedData.notices;
const expiration = cachedData.expiration ?? 0;

if (Date.now() > expiration) {
const freshData = {
expiration: Date.now() + TIME_TO_LIVE,
notices: await this.dataSource.fetch(),
};
await this.save(freshData);
return freshData.notices;
} else {
return notices;
}
}

private async load(): Promise<CachedNotices> {
try {
return await fs.readJSON(this.fileName) as CachedNotices;
} catch (e) {
debug(`Failed to load notices from cache: ${e}`);
return {
expiration: 0,
notices: [],
};
}
}

private async save(cached: CachedNotices): Promise<void> {
try {
await fs.writeJSON(this.fileName, cached);
} catch (e) {
debug(`Failed to store notices in the cache: ${e}`);
}
}
}

export interface NoticeFilterProps {
permanentlySuppressed: boolean,
temporarilySuppressed: boolean,
cliVersion: string,
frameworkVersion: string | undefined,
acknowledgedIssueNumbers: Set<number>,
}

export class NoticeFilter {
private readonly acknowledgedIssueNumbers: Set<number>;

constructor(private readonly props: NoticeFilterProps) {
this.acknowledgedIssueNumbers = props.acknowledgedIssueNumbers;
}

/**
* Returns true if we should show this notice.
*/
apply(notice: Notice): boolean {
if (this.props.permanentlySuppressed
|| this.props.temporarilySuppressed
|| this.acknowledgedIssueNumbers.has(notice.issueNumber)) {
return false;
}
return this.applyVersion(notice, 'cli', this.props.cliVersion) ||
this.applyVersion(notice, 'framework', this.props.frameworkVersion);
}

/**
* Returns true if we should show the notice.
*/
private applyVersion(notice: Notice, name: string, compareToVersion: string | undefined) {
if (compareToVersion === undefined) { return true; }

const affectedComponent = notice.components.find(component => component.name === name);
const affectedVersion = affectedComponent?.version;
return affectedVersion != null && semver.satisfies(compareToVersion, affectedVersion);
}
}

export interface NoticeFormatter {
apply(notice: Notice): string,
}

// TODO rename this
export class FooNoticeFormatter implements NoticeFormatter {
apply(notice: Notice): string {
const componentsValue = notice.components.map(c => `${c.name}: ${c.version}`).join(', ');
return [
`${notice.issueNumber}\t${notice.title}`,
`\tOverview: ${notice.overview}`,
`\tAffected versions: ${componentsValue}`,
`\tMore information at: https://github.com/aws/aws-cdk/issues/${notice.issueNumber}`,
].join('\n\n');
}
}

function frameworkVersion(outdir: string): string | undefined {
const tree = loadTree().tree;
// v2
if (tree?.constructInfo?.fqn.startsWith('aws-cdk-lib')) {
return tree.constructInfo.version;
}
// v1
// TODO
otaviomacedo marked this conversation as resolved.
Show resolved Hide resolved
return undefined;

function loadTree() {
try {
return fs.readJSONSync(path.join(outdir, 'tree.json'));
} catch (e) {
print('debug');
debug(`Failed to get tree.json file: ${e}`);
return {};
}
}
}