feat(CLI): synth displays "AssertDescription: CDK bootstrap stack version 6 required"

[comcalvi]

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

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[mrgrain]

This seems like not a great idea to me to obscure things from the template the we later deploy regardless. What other options where considered as a solution?

Feel free to dismiss my review if the has been discussed sufficiently in your team.

[comcalvi]

@mrgrain the team has not discussed this yet. I think this is the best path forward, and it's a precedent we've set by obscuring AWS::CDK::Metadata from the diff.

I see the following options:

1. Update our docs to tell users it's expected when you run synth or diff.
2. Update the error message, telling people they can ignore it if they see it in CLI output.
3. Obscure it from the CLI output, a precedent we set with AWS::CDK::Metadata.

I don't think the first two address the issue.

For 1: People will likely run synth once before they read whatever doc tells them about it, and be confused.
For 2: The error message will become more confusing if they get it during deployment and it says "you can maybe ignore this."

3 addresses the issue by ensuring people only see the error message where they need to see it; if a deployment is failing because of it. If people want to see the full, un-altered diff, they can pass --strict, which will keep it there.

[mrgrain]

Hmm I've forget about metadata, my bad. That's a great argument for it. --strict is also nice, although it's not a great name for what it apparently does. Carry on. 😁

---

There were a few other ideas in the issue:

• slightly adjust the message
• move it to the top of the template
• print "success" to stderr

I also wonder if this check needs to be in the template or if it could be done by the cli.

[comcalvi]

@mrgrain that's a good point. The reason this is a Rule in the template is that deployments are sometimes initiated from pipelines, without the CDK CLI; this is documented here: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/core/lib/stack-synthesizers/stack-synthesizer.ts#L309

[mrgrain]

@mrgrain that's a good point. The reason this is a Rule in the template is that deployments are sometimes initiated from pipelines, without the CDK CLI; this is documented here: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/core/lib/stack-synthesizers/stack-synthesizer.ts#L309

Cool! This is probably true, but if you are curious let's dig deeper. How are pipelines deploying stacks? What do they need the Bootstrap stack for?

My guess here is that it's the cdk-asset command that really cares about this. But I'm not sure.

Either way, this is not a blocker.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[scanlonp]

Approving with 1 nit.

We should update the cli option description here:

aws-cdk/packages/aws-cdk/lib/cli.ts

Line 263 in 9acd528

.option('strict', { type: 'boolean', desc: 'Do not filter out AWS::CDK::Metadata resources or mangled non-ASCII characters', default: false })

.

[scanlonp]

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?

[comcalvi]

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

[aws-cdk-automation]

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

non-blocking

[comcalvi]

@mrgrain I am curious but I'm not able to dig deeper into this right now.

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: d9f84e5
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.