fix(core): detect and resolve stringified number tokens #19578

rix0rrr · 2022-03-26T17:16:13Z

Number tokens are encoded as a range of very large negative numbers (for
example: -1.888154589709072e+289). When these are naively stringified,
the resolve() method doesn't recognize and translate them anymore,
and these numbers end up in the target template in a confusing way.

However, recognizing them is actually not that hard and can be done
using a regex. We can then do the token resolution appropriately, making
it so that construct authors do not have to call
Tokenization.stringifyNumber() anymore in order to support
stringification of number values.

Fixes #19546, closes #19550.

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use cdk-integ to deploy the infrastructure and generate the snapshot (i.e. cdk-integ without --dry-run)?

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

Number tokens are encoded as a range of very large negative numbers (for example: -1.888154589709072e+289). When these are naively stringified, the `resolve()` method doesn't recognize and translate them anymore, and these numbers end up in the target template in a confusing way. However, recognizing them is actually not that hard and can be done using a regex. We can then do the token resolution appropriately, making it so that construct authors do not have to call `Tokenization.stringifyNumber()` anymore in order to support stringification of number values. Fixes #19546.

gitpod-io · 2022-03-26T17:16:19Z

peterwoodworth · 2022-03-28T16:24:07Z

This looks like a much more stable fix than mine, should we close #19550?

skinny85

"I understand some of these words" 😃

skinny85 · 2022-03-28T23:38:35Z

packages/@aws-cdk/core/lib/private/cloudformation-lang.ts

@@ -43,8 +43,8 @@ export class CloudFormationLang {

    // Some case analysis to produce minimal expressions
    if (parts.length === 1) { return parts[0]; }
-    if (parts.length === 2 && typeof parts[0] === 'string' && typeof parts[1] === 'string') {
-      return parts[0] + parts[1];
+    if (parts.length === 2 && ['string', 'number'].includes(typeof parts[0]) && ['string', 'number'].includes(typeof parts[1])) {


Can we use isConcatable() here? Or can these actually be Tokens themselves? If so, do we need a different helper?

mergify · 2022-03-31T14:11:54Z

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

mergify · 2022-04-01T09:35:08Z

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

mergify · 2022-04-01T12:05:15Z

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

aws-cdk-automation · 2022-04-01T12:05:21Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: d3000f7
Result: SUCCEEDED
Build Logs (available for 30 days)

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

See [CHANGELOG](https://github.com/aws/aws-cdk/blob/bump/1.152.0/CHANGELOG.md) For convenience, extracted the relevant CHANGELOG entry: ## [1.152.0](v1.151.0...v1.152.0) (2022-04-06) ### Features * **cfnspec:** cloudformation spec v63.0.0 ([#19679](#19679)) ([dba96a9](dba96a9)) * **cfnspec:** cloudformation spec v65.0.0 ([#19745](#19745)) ([796fc64](796fc64)) * **cli:** add --build option ([#19663](#19663)) ([eb9b8e2](eb9b8e2)), closes [#19667](#19667) * **cli:** preview of `cdk import` ([#17666](#17666)) ([4f12209](4f12209)) * **core:** throw error when stack name exceeds max length ([#19725](#19725)) ([1ffd45e](1ffd45e)) * **eks:** add k8s v1.22 ([#19756](#19756)) ([9a518c5](9a518c5)) * **opensearch:** Add latest Opensearch Version 1.2 ([#19749](#19749)) ([a2ac36e](a2ac36e)) * add new integration test runner ([#19754](#19754)) ([1b4d010](1b4d010)) * **eks:** alb-controller v2.4.1 ([#19653](#19653)) ([1ec08df](1ec08df)) * **lambda:** add support for ephemeral storage ([#19552](#19552)) ([f1d9b6a](f1d9b6a)), closes [#19605](#19605) * **s3:** EventBridge bucket notifications ([#18614](#18614)) ([d8e602b](d8e602b)), closes [#18076](#18076) * **synthetics:** new puppeteer 3.5 runtime ([#19673](#19673)) ([ce2b91b](ce2b91b)), closes [#19634](#19634) ### Bug Fixes * **aws_applicationautoscaling:** Add missing members to PredefinedMetric enum ([#18978](#18978)) ([75a6fa7](75a6fa7)), closes [#18969](#18969) * **cli:** apps with many resources scroll resource output offscreen ([#19742](#19742)) ([053d22c](053d22c)), closes [#19160](#19160) * **cli:** support attributes of DynamoDB Tables for hotswapping ([#19620](#19620)) ([2321ece](2321ece)), closes [#19421](#19421) * **cloudwatch:** automatic metric math label cannot be suppressed ([#17639](#17639)) ([7fa3bf2](7fa3bf2)) * **codedeploy:** add name validation for Application, Deployment Group and Deployment Configuration ([#19473](#19473)) ([9185042](9185042)) * **codedeploy:** the Service Principal is wrong in isolated regions ([#19729](#19729)) ([7e9a43d](7e9a43d)), closes [#19399](#19399) * **core:** `Fn.select` incorrectly short-circuits complex expressions ([#19680](#19680)) ([7f26fad](7f26fad)) * **core:** detect and resolve stringified number tokens ([#19578](#19578)) ([7d9ab2a](7d9ab2a)), closes [#19546](#19546) [#19550](#19550) * **core:** reduce CFN template indent size to save bytes ([#19656](#19656)) ([fd63ca3](fd63ca3)) * **ecs:** 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens ([#19453](#19453)) ([c852239](c852239)), closes [#16648](#16648) * **ecs:** remove unnecessary error when adding volume to external task definition ([#19774](#19774)) ([5446ded](5446ded)), closes [#19259](#19259) * **iam:** policies aren't minimized as far as possible ([#19764](#19764)) ([876ed8a](876ed8a)), closes [#19751](#19751) * **logs:** Faulty Resource Policy Generated ([#19640](#19640)) ([1fdf122](1fdf122)), closes [#17544](#17544)

See [CHANGELOG](https://github.com/aws/aws-cdk/blob/bump/2.20.0/CHANGELOG.md) For convenience, extracted the relevant CHANGELOG entry: ## [2.20.0](v2.19.0...v2.20.0) (2022-04-07) ### Features * **cfnspec:** cloudformation spec v63.0.0 ([#19679](#19679)) ([dba96a9](dba96a9)) * **cfnspec:** cloudformation spec v65.0.0 ([#19745](#19745)) ([796fc64](796fc64)) * **cli:** add --build option ([#19663](#19663)) ([eb9b8e2](eb9b8e2)), closes [#19667](#19667) * **cli:** preview of `cdk import` ([#17666](#17666)) ([4f12209](4f12209)) * **core:** throw error when stack name exceeds max length ([#19725](#19725)) ([1ffd45e](1ffd45e)) * **eks:** add k8s v1.22 ([#19756](#19756)) ([9a518c5](9a518c5)) * **opensearch:** Add latest Opensearch Version 1.2 ([#19749](#19749)) ([a2ac36e](a2ac36e)) * add new integration test runner ([#19754](#19754)) ([1b4d010](1b4d010)) * **eks:** alb-controller v2.4.1 ([#19653](#19653)) ([1ec08df](1ec08df)) * **lambda:** add support for ephemeral storage ([#19552](#19552)) ([f1d9b6a](f1d9b6a)), closes [#19605](#19605) * **s3:** EventBridge bucket notifications ([#18614](#18614)) ([d8e602b](d8e602b)), closes [#18076](#18076) ### Bug Fixes * **aws_applicationautoscaling:** Add missing members to PredefinedMetric enum ([#18978](#18978)) ([75a6fa7](75a6fa7)), closes [#18969](#18969) * **cli:** apps with many resources scroll resource output offscreen ([#19742](#19742)) ([053d22c](053d22c)), closes [#19160](#19160) * **cli:** support attributes of DynamoDB Tables for hotswapping ([#19620](#19620)) ([2321ece](2321ece)), closes [#19421](#19421) * **cloudwatch:** automatic metric math label cannot be suppressed ([#17639](#17639)) ([7fa3bf2](7fa3bf2)) * **codedeploy:** add name validation for Application, Deployment Group and Deployment Configuration ([#19473](#19473)) ([9185042](9185042)) * **codedeploy:** the Service Principal is wrong in isolated regions ([#19729](#19729)) ([7e9a43d](7e9a43d)), closes [#19399](#19399) * **core:** `Fn.select` incorrectly short-circuits complex expressions ([#19680](#19680)) ([7f26fad](7f26fad)) * **core:** detect and resolve stringified number tokens ([#19578](#19578)) ([7d9ab2a](7d9ab2a)), closes [#19546](#19546) [#19550](#19550) * **core:** reduce CFN template indent size to save bytes ([#19656](#19656)) ([fd63ca3](fd63ca3)) * **ecs:** 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens ([#19453](#19453)) ([c852239](c852239)), closes [#16648](#16648) * **ecs:** remove unnecessary error when adding volume to external task definition ([#19774](#19774)) ([5446ded](5446ded)), closes [#19259](#19259) * **iam:** policies aren't minimized as far as possible ([#19764](#19764)) ([876ed8a](876ed8a)), closes [#19751](#19751) * **logs:** Faulty Resource Policy Generated ([#19640](#19640)) ([1fdf122](1fdf122)), closes [#17544](#17544)

Number tokens are encoded as a range of very large negative numbers (for example: -1.888154589709072e+289). When these are naively stringified, the `resolve()` method doesn't recognize and translate them anymore, and these numbers end up in the target template in a confusing way. However, recognizing them is actually not that hard and can be done using a regex. We can then do the token resolution appropriately, making it so that construct authors do not have to call `Tokenization.stringifyNumber()` anymore in order to support stringification of number values. Fixes aws#19546, closes aws#19550. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

rix0rrr requested a review from a team March 26, 2022 17:16

rix0rrr self-assigned this Mar 26, 2022

github-actions bot added @aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels Mar 26, 2022

mergify bot added the contribution/core This is a PR that came from AWS. label Mar 26, 2022

skinny85 approved these changes Mar 28, 2022

View reviewed changes

skinny85 added the pr/do-not-merge This PR should not be merged at this time. label Mar 28, 2022

rix0rrr mentioned this pull request Mar 29, 2022

fix(stepfunctions-tasks): batch properties do not accept JsonPaths #19550

Closed

4 tasks

Reuse a function

184a748

rix0rrr removed the pr/do-not-merge This PR should not be merged at this time. label Mar 31, 2022

mergify bot and others added 2 commits March 31, 2022 17:54

Merge branch 'master' into huijbers/detect-number-token

4bd2716

Deal with rounding of different numbers

685e2f0

Merge branch 'master' into huijbers/detect-number-token

d3000f7

mergify bot merged commit 7d9ab2a into master Apr 1, 2022

mergify bot deleted the huijbers/detect-number-token branch April 1, 2022 12:05

This was referenced Apr 7, 2022

chore(release): 1.152.0 #19781

Merged

chore(release): 2.20.0 #19801

Merged

otaviomacedo mentioned this pull request Jul 1, 2022

(cfnSpec): Attributes returned as integers output dummy values instead of calling getAtt #18282

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): detect and resolve stringified number tokens #19578

fix(core): detect and resolve stringified number tokens #19578

rix0rrr commented Mar 26, 2022 •

edited

gitpod-io bot commented Mar 26, 2022

peterwoodworth commented Mar 28, 2022

skinny85 left a comment

skinny85 Mar 28, 2022

mergify bot commented Mar 31, 2022

mergify bot commented Apr 1, 2022

mergify bot commented Apr 1, 2022

aws-cdk-automation commented Apr 1, 2022

fix(core): detect and resolve stringified number tokens #19578

fix(core): detect and resolve stringified number tokens #19578

Conversation

rix0rrr commented Mar 26, 2022 • edited

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Mar 26, 2022

peterwoodworth commented Mar 28, 2022

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 Mar 28, 2022

Choose a reason for hiding this comment

mergify bot commented Mar 31, 2022

mergify bot commented Apr 1, 2022

mergify bot commented Apr 1, 2022

aws-cdk-automation commented Apr 1, 2022

AWS CodeBuild CI Report

rix0rrr commented Mar 26, 2022 •

edited