-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
CFN: Validate resource last status in describe_stack_resource #13269
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
CFN: Validate resource last status in describe_stack_resource #13269
Conversation
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 37m 30s ⏱️ - 1h 21m 28s Results for commit a8e5001. ± Comparison against base commit 48d30e0. This pull request removes 4232 tests.♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files ± 0 5 suites ±0 49m 32s ⏱️ - 1h 50m 9s Results for commit a8e5001. ± Comparison against base commit 48d30e0. This pull request removes 4582 tests.♻️ This comment has been updated with latest results. |
Test Results - Alternative Providers584 tests - 784 330 ✅ - 382 26m 30s ⏱️ - 13m 47s Results for commit a8e5001. ± Comparison against base commit 48d30e0. This pull request removes 784 tests.♻️ This comment has been updated with latest results. |
simonrw
left a comment
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.
Some minor suggestions and questions but nothing blocking. Thanks for tackling this!
localstack-core/localstack/services/cloudformation/v2/provider.py
Outdated
Show resolved
Hide resolved
|
|
||
| try: | ||
| resource = stack.resolved_resources[logical_resource_id] | ||
| if resource.get("ResourceStatus") not in ["CREATE_COMPLETE", "UPDATE_COMPLETE"]: |
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.
question: do we need to account for ROLLBACK_COMPLETE?
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.
Probably yes. I'm adding it even if we can't test it for now.
Motivation
This PR addresses an issue where users encountered errors in the describe_stack_resource operation when the resource description was incomplete or inconsistent. The root cause was that the ChangeSetExecutor retained metadata for resources even after deletion or in the case of failed creation attempts.
While this retained state is necessary for handling nested stacks correctly, it can lead to failed responses if not properly validated before returning attempting to generate a response.
The fix adds explicit validation of the resource status before returning a response. Only resources with active states are included in the output.
Changes
Testing