Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(core): max resources quota enforced on nested stacks #28207

Closed
wants to merge 2 commits into from

Conversation

lpizzinidev
Copy link
Contributor

@lpizzinidev lpizzinidev commented Nov 30, 2023

The max resource quota of 500 should not be applied on nested stacks.
AWS CloudFormation quotas.

Closes #28186.


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

@github-actions github-actions bot added the distinguished-contributor [Pilot] contributed 50+ PRs to the CDK label Nov 30, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team November 30, 2023 16:42
@github-actions github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels Nov 30, 2023
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@lpizzinidev
Copy link
Contributor Author

Exemption Request.
Unit tests should provide enough coverage here.

@aws-cdk-automation aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/needs-maintainer-review This PR needs a review from a Core Team Member labels Nov 30, 2023
@scanlonp scanlonp self-assigned this Nov 30, 2023
@scanlonp
Copy link
Contributor

@lpizzinidev I am a little concerned that the interpretation of the docs are wrong in the original issue. In my mind, there is a difference between a nested stack and a child stack. The nested stack is the entire structure, which is made up of parent and child stacks, similar to a tree vs nodes. It does not seem plausible that just because a single stack is a child stack, that its resource limit jumps from 500 to 2500. I believe the 2500 limit is of the entire nested structure.

I think an integ test, for verification at least, is necessary. If you would like to set this up, feel free. I have been told CloudFormation::WaitConditionHandle is a good lightweight resource to make many of. If you do not feel comfortable provisioning this many resources, one of the team members can try to do so.

@lpizzinidev
Copy link
Contributor Author

lpizzinidev commented Dec 1, 2023

@scanlonp
Makes sense.
Now that I have reviewed it, the MAX_RESOURCES = 500 limit seems correct.
We should probably enforce the 2500 limit only if the stack is nested.

@scanlonp
Copy link
Contributor

scanlonp commented Dec 1, 2023

@lpizzinidev, sounds good. This could just be something we let cloudformation enforce. I doubt too many users will be running into the 2500 resource limit.

@lpizzinidev
Copy link
Contributor Author

@scanlonp
Yeah, probably the best approach here is to not validate the number of resources if the stack is nested.

@aws-cdk-automation aws-cdk-automation dismissed their stale review December 2, 2023 16:31

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@lpizzinidev lpizzinidev changed the title fix(core): wrong quota for nested stacks fix(core): max resources quota enforced on nested stacks Dec 2, 2023
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: b431586
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@scanlonp
Copy link
Contributor

scanlonp commented Dec 4, 2023

I still think this is a no-op. Commented here #28186 (comment).

@lpizzinidev
Copy link
Contributor Author

Closing due to #28186 (comment)

@lpizzinidev lpizzinidev closed this Dec 5, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug This issue is a bug. distinguished-contributor [Pilot] contributed 50+ PRs to the CDK effort/small Small work item – less than a day of effort p1 pr/needs-maintainer-review This PR needs a review from a Core Team Member pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(aws-cdk): (Unable to create nested stack with > 500 resources)
3 participants