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): synth displays "AssertDescription: CDK bootstrap stack version 6 required" #31092

Merged
merged 8 commits into from
Aug 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought you had mentioned that the rules would only be deleted if the check bootstrap version was the only rules. Did I misinterpret that, or did we switch to always removing the bootstrap version check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention is to always remove the Rule, but to remove the entire Rules section if that is the only rule.

} 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
Loading