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

❗ NOTICE (cloudformation-include): RuntimeError: Invalid logical ID override: 'my_condition'. It must only contain alphanumeric characters. overrideLogicalId validation causing CfnInclude to error on Condition logical Id #30669

Closed
mjgoble opened this issue Jun 26, 2024 · 5 comments · Fixed by #30695
Labels
@aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package bug This issue is a bug. management/tracking Issues that track a subject or multiple issues p0

Comments

@mjgoble
Copy link

mjgoble commented Jun 26, 2024

Please add your +1 👍 to let us know you have encountered this

Status: IN-PROGRESS

Overview:

New synth time logical Id checks for CloudFormation Resources are also being applied to CloudFormation Conditions. Resource ID validation is more strict that Condition ID validation, causing valid Conditions to be caught by the new validation, and errors to be thrown.

Specifically, this bug affects cloudformation-include. When trying to cfnInclude a CloudFormation template that has defined Conditions using non-alphanumeric characters, e.g. snake case my_condition, will now throw an error.

Complete Error Message:

RuntimeError: Invalid logical ID override: 'my_condition'. It must only contain alphanumeric characters.

Workaround:

Pin the version of aws-cdk-lib to 2.146.0.

Solution:

Revert breaking PR.

Related Issues:

Original report

Describe the bug

The following PR #29708 introduced validation for overrideLogicalId

This has caused an issue with cloudformation-include when trying to cfnInclude a cloudformation template that has defined Conditions using non-alphanumeric characters, e.g. snake case my_condition

Cloudformation include by default preserves the logical Id that is defined in the cloudformation template. This is done in part by using the overrideLogicalId function to ensure the logical id of the resource is not changed during the cdk synth

private overrideLogicalIdIfNeeded(element: core.CfnElement, id: string): void {
if (this.preserveLogicalIds) {
element.overrideLogicalId(id);
}
}

Now due to the new validation on overrideLogicalId - Conditions cannot be defined as non-alphanumeric despite them never actually being deployed as resources in Cloudformation

Expected Behavior

Stack synth completes successfully, logical Ids defined in the included cloudformation template is preserved, including the snake case Condition logical Ids

Cloudformation deploy succeeds even despite having non-alphanumeric logical Ids defined for Conditions

Current Behavior

Stack synth fails with a Runtime Error
RuntimeError: Invalid logical ID override: 'my_condition'. It must only contain alphanumeric characters.

Reproduction Steps

cfn_include a cloudformation template that defines conditions using snake case logical id, e.g. my_condition

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.147.1

Framework Version

No response

Node.js Version

20.5.0

OS

Linux

Language

Python

Language Version

Python (3.12.3)

Other information

No response

@mjgoble mjgoble added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 26, 2024
@github-actions github-actions bot added the @aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package label Jun 26, 2024
@GavinZZ
Copy link
Contributor

GavinZZ commented Jun 27, 2024

Thanks @mjgoble for creating this issue. You're right that the logical id validation code is applied not only on the Resources properties only but also Conditions. This caused unexpected behaviours. As a temporary workaround, please downgrade to a previous version.

@GavinZZ GavinZZ removed the needs-triage This issue or PR still needs to be triaged. label Jun 27, 2024
@TheRealAmazonKendra
Copy link
Contributor

This is a regression so I'm labeling it a p0 and reverting the original PR.

@TheRealAmazonKendra
Copy link
Contributor

Revert: #30695

@mergify mergify bot closed this as completed in #30695 Jun 27, 2024
@mergify mergify bot closed this as completed in 0aa2be7 Jun 27, 2024
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

1 similar comment
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@scanlonp scanlonp changed the title cloudformation-include: overrideLogicalId validation causing CfnInclude to error on Condition logical Id cloudformation-include: RuntimeError: Invalid logical ID override: 'my_condition'. It must only contain alphanumeric characters. overrideLogicalId validation causing CfnInclude to error on Condition logical Id Jun 27, 2024
@scanlonp scanlonp added the management/tracking Issues that track a subject or multiple issues label Jun 27, 2024
@scanlonp scanlonp pinned this issue Jun 27, 2024
@scanlonp scanlonp changed the title cloudformation-include: RuntimeError: Invalid logical ID override: 'my_condition'. It must only contain alphanumeric characters. overrideLogicalId validation causing CfnInclude to error on Condition logical Id ❗ NOTICE cloudformation-include: RuntimeError: Invalid logical ID override: 'my_condition'. It must only contain alphanumeric characters. overrideLogicalId validation causing CfnInclude to error on Condition logical Id Jun 27, 2024
@scanlonp scanlonp changed the title ❗ NOTICE cloudformation-include: RuntimeError: Invalid logical ID override: 'my_condition'. It must only contain alphanumeric characters. overrideLogicalId validation causing CfnInclude to error on Condition logical Id ❗ NOTICE (cloudformation-include): RuntimeError: Invalid logical ID override: 'my_condition'. It must only contain alphanumeric characters. overrideLogicalId validation causing CfnInclude to error on Condition logical Id Jun 27, 2024
scanlonp added a commit to cdklabs/aws-cdk-notices that referenced this issue Jun 27, 2024
mergify bot pushed a commit to cdklabs/aws-cdk-notices that referenced this issue Jun 27, 2024
scanlonp pushed a commit that referenced this issue Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package bug This issue is a bug. management/tracking Issues that track a subject or multiple issues p0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants