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

aws-cdk-lib/aws-ecs-patterns: setting maxHealthyPercent incorrectly causes stack to fail to deploy #26158

Closed
cloventt opened this issue Jun 29, 2023 · 2 comments · Fixed by #26193
Closed

aws-cdk-lib/aws-ecs-patterns: setting maxHealthyPercent incorrectly causes stack to fail to deploy #26158

cloventt opened this issue Jun 29, 2023 · 2 comments · Fixed by #26193
Labels
@aws-cdk/triggers Related to the triggers package bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@cloventt
Copy link

Describe the bug

Setting the maxHealthyPercent on an ApplicationLoadBalancedFargateService to an invalid value does not raise a synth-time error, but it generates invalid CFN templates.

Expected Behavior

Setting the maxHealthyPercent on an ApplicationLoadBalancedFargateService to a value such as 0.5 should trigger a compile-time error.

Current Behavior

The invalid config is synthed without raising any warnings and results in the stack failing to apply.

Reproduction Steps

Setting the maxHealthyPercent on an ApplicationLoadBalancedFargateService to a value such as 0.5.

Possible Solution

Throw an exception at synth-time if maxHealthyPercent is not a whole integer between 0 and 100 inclusive.

Additional Information/Context

No response

CDK CLI Version

2.79.1

Framework Version

No response

Node.js Version

19.1.0

OS

Linux

Language

Typescript

Language Version

No response

Other information

No response
@cloventt cloventt added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 29, 2023
@github-actions github-actions bot added the @aws-cdk/triggers Related to the triggers package label Jun 29, 2023
@peterwoodworth
Copy link
Contributor

Throw an exception at synth-time if maxHealthyPercent is not a whole integer between 0 and 100 inclusive.

I can't find anywhere in the documentation what the acceptable range of values is - but it can certainly be above 100 according to the docs. It does appear to only accept whole numbers though, so we could check for that, and clarify in our docs that a whole number is required

@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 29, 2023
@lpizzinidev lpizzinidev mentioned this issue Jul 1, 2023
Merged
lpizzinidev added a commit to lpizzinidev/aws-cdk that referenced this issue Jul 1, 2023
@lpizzinidev
Merge branch 'main' into awsgh-26158
0f801e8
lpizzinidev added a commit to lpizzinidev/aws-cdk that referenced this issue Jul 23, 2023
@lpizzinidev
Merge branch 'main' into awsgh-26158
cdc40d1
lpizzinidev added a commit to lpizzinidev/aws-cdk that referenced this issue Jul 25, 2023
@lpizzinidev
Merge branch 'main' into awsgh-26158
bf947cb
@mergify mergify bot closed this as completed in #26193 Jul 25, 2023
mergify bot pushed a commit that referenced this issue Jul 25, 2023
@lpizzinidev
fix(ecs-patterns): minHealthyPercent and maxHealthyPercent props …
bdfdc91 
…validation (#26193)

Setting `maxHealthyPercent` to a non-integer value was not raising synth-time errors, but was generating invalid CFN templates.
This fix adds validation for both `maxHealthyPercent` and `minHealthyPercent`.

Closes #26158.

----

*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

⚠️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 Jul 26, 2023
Closed
Closed
bmoffatt pushed a commit to bmoffatt/aws-cdk that referenced this issue Jul 29, 2023
@lpizzinidev @bmoffatt
fix(ecs-patterns): minHealthyPercent and maxHealthyPercent props …
978cbbb 
…validation (aws#26193)

Setting `maxHealthyPercent` to a non-integer value was not raising synth-time errors, but was generating invalid CFN templates.
This fix adds validation for both `maxHealthyPercent` and `minHealthyPercent`.

Closes aws#26158.

----

*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/triggers Related to the triggers package bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ecs-patterns): minHealthyPercent and maxHealthyPercent props validation lpizzinidev/aws-cdk
2 participants
@cloventt @peterwoodworth