CloudFormation v2 Engine: V1 Test Porting and Annotations and Batch of Parity Improvements #12660
Conversation
Test Results - Alternative Providers914 tests 526 ✅ 21m 23s ⏱️ Results for commit bd336d9. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 20m 26s ⏱️ Results for commit bd336d9. ♻️ This comment has been updated with latest results. |
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 1h 42m 50s ⏱️ + 1m 12s Results for commit bd336d9. ± Comparison against base commit e42e683. This pull request skips 1 test.♻️ This comment has been updated with latest results. |
| @@ -29,6 +29,12 @@ def capture(stack_name: str) -> PerResourceStackEvents: | |||
| return capture | |||
|
|
|||
|
|
|||
| def _normalise_describe_change_set_output(value: DescribeChangeSetOutput) -> None: | |||
There was a problem hiding this comment.
I feel the order of changes in the output gives us clues into the implementation on the AWS side. If we have a difference then perhaps our visiting order needs adjusting instead?
There was a problem hiding this comment.
From what I’ve observed, AWS returns changes in the same order as they are declared in the template.
Our implementation currently mirrors this behavior (it is also true that the json library preserving key order after 3.7, but this is not the only reason for our compliance here), except in cases where the user doesn’t explicitly sort dependencies in the template. In those situations, our system places the dependee’s resource change first (if a change exists there), though it otherwise respects the original order of the template for all other changes.
I’ve decided to omit these specifics for now, but it’s unfortunate that the snapshot is being influenced by the normalization process.
There was a problem hiding this comment.
Can we either add a skipped test that exercises this behaviour or at least a TODO comment in the describe_change_set provider method to come back to in the future? I agree it's not critical for parity at this stage, but it is still a discrepancy.
There was a problem hiding this comment.
Yes absolutely, I'm adding this TODO in the PR for Fn::Split, after I merge this train of PRs today
| @@ -0,0 +1,45 @@ | |||
| { | |||
There was a problem hiding this comment.
Why are there so many of these files?
f90a2db to
66937e0
Compare
* Extends #12650
Motivation
The introduction of the CloudFormation v2 engine laid the foundation for a redesigned engine capable of accurately determining update requirements between CloudFormation deployments, while also enabling parallel execution during updates. However, the current implementation still has significant parity gaps with the CloudFormation v1 provider. These changes: port the v1 test suite to v2, enabling it for CI runs on the new provider; bring parity issue annotations to guide development; resolve a number of parity issues in the v2 provider.
Changes
resources,api, andengineCFNV2:<issue-identifier>format to guide development towards parity with the v1 engineFn::GetAttto use the dot path syntax