Skip to content

Commit

Permalink
feat(cli): notices on bootstrap version (#31555)
Browse files Browse the repository at this point in the history
### 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: cdklabs/aws-cdk-notices#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*
  • Loading branch information
iliapolo authored Sep 30, 2024
1 parent 5e08c98 commit b0e4a54
Show file tree
Hide file tree
Showing 7 changed files with 1,047 additions and 516 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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://');

}));
36 changes: 21 additions & 15 deletions packages/aws-cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
17 changes: 12 additions & 5 deletions packages/aws-cdk/lib/api/environment-resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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; }
Expand All @@ -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;
}

Expand All @@ -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'.`);
}
Expand Down
43 changes: 12 additions & 31 deletions packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -367,10 +367,10 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
});
await configuration.load();

if (shouldDisplayNotices()) {
void refreshNotices()
.catch(e => 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']),
Expand Down Expand Up @@ -422,8 +422,6 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n

loadPlugins(configuration.settings);

const cmd = argv._[0];

if (typeof(cmd) !== 'string') {
throw new Error(`First argument should be a string. Got: ${cmd} (${typeof(cmd)})`);
}
Expand All @@ -440,32 +438,15 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
// Do PSAs here
await version.displayVersionMessage();

if (shouldDisplayNotices()) {
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: [],
ignoreCache: true,
});
} else if (cmd !== 'version') {
await displayNotices({
outdir: configuration.settings.get(['output']) ?? 'cdk.out',
acknowledgedIssueNumbers: configuration.context.get('acknowledged-issue-numbers') ?? [],
ignoreCache: false,
});
}
if (cmd === 'notices') {
await notices.refresh({ force: true });
notices.display({ showTotal: argv.unacknowledged });

} else if (cmd !== 'version') {
await notices.refresh();
notices.display();
}
}

function shouldDisplayNotices(): boolean {
return configuration.settings.get(['notices']) ?? true;
}

async function main(command: string, args: any): Promise<number | void> {
Expand Down
Loading

0 comments on commit b0e4a54

Please sign in to comment.