-
Notifications
You must be signed in to change notification settings - Fork 4.3k
fix(cloudformation-include): aws::novalue type validation error for non-string properties #35425
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
Conversation
...@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.novalue-nonstring.ts
Show resolved
Hide resolved
Comments on closed issues and PRs are hard for our team to see. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there :). I have two comments to add to the integ test, which you can add verbatim. The reason is because looking at the test by itself it does not make it clear what its testing and how.
...@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.novalue-nonstring.ts
Show resolved
Hide resolved
...@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.novalue-nonstring.ts
Show resolved
Hide resolved
…include/test/integ.novalue-nonstring.ts Co-authored-by: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
…include/test/integ.novalue-nonstring.ts Co-authored-by: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay!
Issue # (if applicable)
Closes #18420
Reason for this change
When we try to include a Cloudformation template that uses
{ Ref: 'AWS::NoValue' }
for a boolean-typed property, we get an error like:Error: Expected 'true' or 'false' for boolean value, got: '${Token[AWS.NoValue.7]}'
This problem occurs because the CFN parser uses the
Aws.NO_VALUE
constant (which is defined as a string). CDK sees a string value for a boolean property and attempts to convert the string to boolean values. This results in the error we see above.Description of changes
This change modifies the CFN parser in cloudformation-include to return a token value instead of a direct string constant for AWS::NoValue references. This makes it an IResolvable token that works with any property type. The parser then, in turn, ignores the default error being thrown whenever a non-string is passed.
Describe any new or updated permissions being added
No new IAM permissions required
Description of how you validated changes
A unit test is added to check if an IResolvable token is returned for AWS::NoValue. An integration test deploys a Cloudformation stack with
AWS::NoValue
in a boolean property. Both tests pass. An assertion isn't needed here since the deployment of the infrastructure indicates that the test succeeded.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license