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.

Sign up for GitHub

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

Jump to bottom

(ecs.FargateTaskDefinition): runtime check for EphemeralStorageGiB makes it incompatible with CfnParameter tokens #16648

Closed
nacitar opened this issue Sep 24, 2021 · 4 comments · Fixed by #19453
Closed

(ecs.FargateTaskDefinition): runtime check for EphemeralStorageGiB makes it incompatible with CfnParameter tokens #16648

nacitar opened this issue Sep 24, 2021 · 4 comments · Fixed by #19453
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@nacitar
Copy link

nacitar commented Sep 24, 2021 •
edited
Loading

aws-cdk/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-task-definition.ts

Line 134 in 5aa6ac0

throw new Error('Ephemeral storage size must be between 21GiB and 200GiB');

The check here validates the values, however when using a CfnParameter's ValueAsNumber, you get a value such as:
-1.8881545897087957E+289

This value is, of course, a token. However, because of this local parameter validation, and it not exempting these token values from the check, you simply cannot use a CfnParameter for this field. If the check was done on the AWS side it would be fine, as it would know the real value by that point.

Reproduction Steps

Make a fargate task definition and try to pass in a CfnParameter's ValueAsNumber as the EphemeralStorageGiB, it will throw no matter what default/value it has because it validates the token as if it's the real value.

What did you expect to happen?

No exception, but rather a failure at deploy time IFF the value is out of the valid range.

What actually happened?

An exception was thrown preventing me from even generating the template in the first place, despite no real value being present to even fail the validation.

Environment

  • CDK CLI Version : 1.116.0 (build d04661d)
  • Framework Version: .NET Core 3.1
  • Node.js Version: v14.17.6
  • OS : Windows 10
  • Language (Version): C#

This is 🐛 Bug Report
@nacitar nacitar added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 24, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Sep 24, 2021
@nacitar
Copy link
Author

nacitar commented Sep 29, 2021

Might be more worthy of a distinct ticket, but the same identical problem exists here:
https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts#L417

That check makes the DesiredCount impossible to express via a CfnParameter.

@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 15, 2021
@peterwoodworth
Copy link
Contributor

Hey @nacitar, thanks for your patience. I'll try to reproduce this. If I'm able to (which i suspect I will be) this should be a pretty simple fix

@peterwoodworth peterwoodworth removed the needs-reproduction This issue needs reproduction. label Oct 21, 2021
@peterwoodworth
Copy link
Contributor

I've been able to confirm the behavior. Will see about getting a PR up soon

@peterwoodworth peterwoodworth mentioned this issue Oct 22, 2021
Closed
@peterwoodworth peterwoodworth removed their assignment Oct 28, 2021
@peterwoodworth peterwoodworth mentioned this issue Mar 17, 2022
Merged
@mergify mergify bot closed this as completed in #19453 Apr 5, 2022
mergify bot pushed a commit that referenced this issue Apr 5, 2022
@peterwoodworth
fix(ecs): 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens (#…
c852239 
…19453)

fixes #16648


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Apr 5, 2022

⚠️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.

This was referenced Apr 7, 2022
Merged
Merged
mergify bot added a commit that referenced this issue Apr 7, 2022
@mergify
chore(release): 1.152.0 (#19781)
9487b39 
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)
mergify bot added a commit that referenced this issue Apr 7, 2022
@mergify
chore(release): 2.20.0 (#19801)
738ef49 
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)
StevePotter pushed a commit to StevePotter/aws-cdk that referenced this issue Apr 27, 2022
@peterwoodworth
fix(ecs): 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens (a…
48fbaeb 
…ws#19453)

fixes aws#16648


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Milestone
No milestone
3 participants
@nacitar @peterwoodworth @madeline-k