CFn: correct disassociation of change sets from stacks #13128
Conversation
# Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # # On branch cfn/v2/delete-change-set-describe # Changes to be committed: # modified: localstack-core/localstack/services/cloudformation/v2/entities.py # modified: localstack-core/localstack/services/cloudformation/v2/provider.py # modified: tests/aws/services/cloudformation/api/test_changesets.py # modified: tests/aws/services/cloudformation/api/test_changesets.snapshot.json # modified: tests/aws/services/cloudformation/api/test_changesets.validation.json #
Test Results - Preflight, Unit22 124 tests 20 386 ✅ 6m 21s ⏱️ Results for commit 158f892. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Acceptance7 tests 5 ✅ 3m 7s ⏱️ Results for commit 158f892. ♻️ This comment has been updated with latest results. |
LocalStack Community integration with Pro 2 files 2 suites 34m 46s ⏱️ Results for commit 158f892. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 48m 11s ⏱️ Results for commit 158f892. ♻️ This comment has been updated with latest results. |
Test Results - Alternative Providers558 tests 328 ✅ 23m 53s ⏱️ Results for commit 158f892. ♻️ This comment has been updated with latest results. |
pinzon
left a comment
There was a problem hiding this comment.
Thanks for fixing this. I can see why it was hard to find the issue.
Answering your question: Though I believe the best solution would be to add a method that validates that the change set id is not duplicated I'm fine with using a set since at least it won't cause issues like an array.
I think my concern is how to surface the error to the user, or even more ideally in a way that we can find out about it. A runtime error is not great since it's not the user's fault and they are blocked. I think a There is a big part of me that wants to revert the |
|
Because this is affecting customers, I am going to merge this but I'd like to revisit this conversation soon |
Motivation
While executing the following set of operations:
the change set lookup failed.
We incorrectly associate the change set twice, once during the create and once during execute so after the execute stage the change set id was in the stack's list twice. Deleting the change set only disassociated one of the entries, which in effect deleted the stack from the store but left it associated with the stack.
Then calling describe or delete would iterate over the stack's change sets and reach a stale change set id, which is not present in the store.
Changes
Questions
I'm not sure we should make the association more robust with a set. If this situation occurs, it is a programming error on our part and we should fix it.