Skip to content

Commit

Permalink
feat(CLI): synth displays "AssertDescription: CDK bootstrap stack ver…
Browse files Browse the repository at this point in the history
…sion 6 required" (#31092)

### Issue # (if applicable)

Closes #17942. 

### Reason for this change

The CDK CLI shows the stack template, which includes the CFN Rule `CheckBootstrapVersion`. This rule will fail a deployment if the bootstrap is not right. Customers think this rule is an error message. 

### Description of changes

Obscure this `CheckBootstrapVersion` Rule from the template when we print it, if it exists. If it is the only Rule, remove the `Rules` section entirely. 

### Description of how you validated changes

Manual testing, unit tests, and CLI integration 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
comcalvi authored Aug 17, 2024
1 parent f2babd9 commit 751a922
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ integTest('cdk synth', withDefaultFixture(async (fixture) => {
},
}));

expect(await fixture.cdkSynth({
options: [fixture.fullStackName('test-1')],
})).not.toEqual(expect.stringContaining(`
Rules:
CheckBootstrapVersion:`));

await fixture.cdk(['synth', fixture.fullStackName('test-2')], { verbose: false });
expect(fixture.template('test-2')).toEqual(expect.objectContaining({
Resources: {
Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ $ # Diff against the currently deployed stack with quiet parameter enabled
$ cdk diff --quiet --app='node bin/main.js' MyStackName
```

Note that the CDK::Metadata resource and the `CheckBootstrapVersion` Rule are excluded from `cdk diff` by default. You can force `cdk diff` to display them by passing the `--strict` flag.

The `change-set` flag will make `diff` create a change set and extract resource replacement data from it. This is a bit slower, but will provide no false positives for resource replacement.
The `--no-change-set` mode will consider any change to a property that requires replacement to be a resource replacement,
even if the change is purely cosmetic (like replacing a resource reference with a hardcoded arn).
Expand Down
37 changes: 31 additions & 6 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class CdkToolkit {

public async metadata(stackName: string, json: boolean) {
const stacks = await this.selectSingleStackByName(stackName);
data(serializeStructure(stacks.firstStack.manifest.metadata ?? {}, json));
printSerializedObject(stacks.firstStack.manifest.metadata ?? {}, json);
}

public async acknowledge(noticeId: string) {
Expand Down Expand Up @@ -632,7 +632,7 @@ export class CdkToolkit {
});

if (options.long && options.showDeps) {
data(serializeStructure(stacks, options.json ?? false));
printSerializedObject(stacks, options.json ?? false);
return 0;
}

Expand All @@ -646,7 +646,7 @@ export class CdkToolkit {
});
}

data(serializeStructure(stackDeps, options.json ?? false));
printSerializedObject(stackDeps, options.json ?? false);
return 0;
}

Expand All @@ -660,7 +660,7 @@ export class CdkToolkit {
environment: stack.environment,
});
}
data(serializeStructure(long, options.json ?? false));
printSerializedObject(long, options.json ?? false);
return 0;
}

Expand All @@ -687,7 +687,7 @@ export class CdkToolkit {
// if we have a single stack, print it to STDOUT
if (stacks.stackCount === 1) {
if (!quiet) {
data(serializeStructure(stacks.firstStack.template, json ?? false));
printSerializedObject(obscureTemplate(stacks.firstStack.template), json ?? false);
}
return undefined;
}
Expand All @@ -701,7 +701,7 @@ export class CdkToolkit {
// behind an environment variable.
const isIntegMode = process.env.CDK_INTEG_MODE === '1';
if (isIntegMode) {
data(serializeStructure(stacks.stackArtifacts.map(s => s.template), json ?? false));
printSerializedObject(stacks.stackArtifacts.map(s => obscureTemplate(s.template)), json ?? false);
}

// not outputting template to stdout, let's explain things to the user a little bit...
Expand Down Expand Up @@ -1045,6 +1045,13 @@ export class CdkToolkit {
}
}

/**
* Print a serialized object (YAML or JSON) to stdout.
*/
function printSerializedObject(obj: any, json: boolean) {
data(serializeStructure(obj, json));
}

export interface DiffOptions {
/**
* Stack names to diff
Expand Down Expand Up @@ -1526,3 +1533,21 @@ function buildParameterMap(parameters: {

return parameterMap;
}

/**
* Remove any template elements that we don't want to show users.
*/
function obscureTemplate(template: any = {}) {
if (template.Rules) {
// see https://github.com/aws/aws-cdk/issues/17942
if (template.Rules.CheckBootstrapVersion) {
if (Object.keys(template.Rules).length > 1) {
delete template.Rules.CheckBootstrapVersion;
} else {
delete template.Rules;
}
}
}

return template;
}
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ async function parseCommandLineArguments(args: string[]) {
.option('exclusively', { type: 'boolean', alias: 'e', desc: 'Only diff requested stacks, don\'t include dependencies' })
.option('context-lines', { type: 'number', desc: 'Number of context lines to include in arbitrary JSON diff rendering', default: 3, requiresArg: true })
.option('template', { type: 'string', desc: 'The path to the CloudFormation template to compare with', requiresArg: true })
.option('strict', { type: 'boolean', desc: 'Do not filter out AWS::CDK::Metadata resources or mangled non-ASCII characters', default: false })
.option('strict', { type: 'boolean', desc: 'Do not filter out AWS::CDK::Metadata resources, mangled non-ASCII characters, or the CheckBootstrapVersionRule', default: false })
.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' })
.option('processed', { type: 'boolean', desc: 'Whether to compare against the template with Transforms already processed', default: false })
Expand Down
39 changes: 31 additions & 8 deletions packages/aws-cdk/lib/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { print, warning } from './logging';
*
* @param oldTemplate the old/current state of the stack.
* @param newTemplate the new/target state of the stack.
* @param strict do not filter out AWS::CDK::Metadata
* @param strict do not filter out AWS::CDK::Metadata or Rules
* @param context lines of context to use in arbitrary JSON diff
* @param quiet silences \'There were no differences\' messages
*
Expand Down Expand Up @@ -50,13 +50,9 @@ export function printStackDiff(
}

// filter out 'AWS::CDK::Metadata' resources from the template
if (diff.resources && !strict) {
diff.resources = diff.resources.filter(change => {
if (!change) { return true; }
if (change.newResourceType === 'AWS::CDK::Metadata') { return false; }
if (change.oldResourceType === 'AWS::CDK::Metadata') { return false; }
return true;
});
// filter out 'CheckBootstrapVersion' rules from the template
if (!strict) {
obscureDiff(diff);
}

let stackDiffCount = 0;
Expand Down Expand Up @@ -165,3 +161,30 @@ function logicalIdMapFromTemplate(template: any) {
}
return ret;
}

/**
* Remove any template elements that we don't want to show users.
* This is currently:
* - AWS::CDK::Metadata resource
* - CheckBootstrapVersion Rule
*/
function obscureDiff(diff: TemplateDiff) {
if (diff.unknown) {
// see https://github.com/aws/aws-cdk/issues/17942
diff.unknown = diff.unknown.filter(change => {
if (!change) { return true; }
if (change.newValue?.CheckBootstrapVersion) { return false; }
if (change.oldValue?.CheckBootstrapVersion) { return false; }
return true;
});
}

if (diff.resources) {
diff.resources = diff.resources.filter(change => {
if (!change) { return true; }
if (change.newResourceType === 'AWS::CDK::Metadata') { return false; }
if (change.oldResourceType === 'AWS::CDK::Metadata') { return false; }
return true;
});
}
}
89 changes: 89 additions & 0 deletions packages/aws-cdk/test/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,95 @@ Resources
});
});

describe('--strict', () => {
const templatePath = 'oldTemplate.json';
beforeEach(() => {
const oldTemplate = {};

cloudExecutable = new MockCloudExecutable({
stacks: [{
stackName: 'A',
template: {
Resources: {
MetadataResource: {
Type: 'AWS::CDK::Metadata',
Properties: {
newMeta: 'newData',
},
},
SomeOtherResource: {
Type: 'AWS::Something::Amazing',
},
},
Rules: {
CheckBootstrapVersion: {
newCheck: 'newBootstrapVersion',
},
},
},
}],
});

toolkit = new CdkToolkit({
cloudExecutable,
deployments: cloudFormation,
configuration: cloudExecutable.configuration,
sdkProvider: cloudExecutable.sdkProvider,
});

fs.writeFileSync(templatePath, JSON.stringify(oldTemplate));
});

afterEach(() => fs.rmSync(templatePath));

test('--strict does not obscure CDK::Metadata or CheckBootstrapVersion', async () => {
// GIVEN
const buffer = new StringWritable();

// WHEN
const exitCode = await toolkit.diff({
stackNames: ['A'],
stream: buffer,
strict: true,
});

// THEN
const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
expect(plainTextOutput.trim()).toEqual(`Stack A
Resources
[+] AWS::CDK::Metadata MetadataResource
[+] AWS::Something::Amazing SomeOtherResource
Other Changes
[+] Unknown Rules: {\"CheckBootstrapVersion\":{\"newCheck\":\"newBootstrapVersion\"}}
✨ Number of stacks with differences: 1`);
expect(exitCode).toBe(0);
});

test('--no-strict obscures CDK::Metadata and CheckBootstrapVersion', async () => {
// GIVEN
const buffer = new StringWritable();

// WHEN
const exitCode = await toolkit.diff({
stackNames: ['A'],
stream: buffer,
});

// THEN
const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
expect(plainTextOutput.trim()).toEqual(`Stack A
Resources
[+] AWS::Something::Amazing SomeOtherResource
✨ Number of stacks with differences: 1`);
expect(exitCode).toBe(0);
});
});

class StringWritable extends Writable {
public data: string;
private readonly _decoder: StringDecoder;
Expand Down

0 comments on commit 751a922

Please sign in to comment.