Allow "aws cloudformation package" to succeed when a Resource is not a key-value pair #8096
Conversation
…s that aren't defined as key-value pairs
|
Added a warning message that local artifacts aren't currently supported in Resources that aren't defined as key-value pairs |
…ic function by recursively calling the export function on the fragment property of Fn::ForEach
|
In the latest commit, modified artifact_explorer, where instead of skipping over resources that aren't dictionaries, modifying the export method to be able to handle the Fn::ForEach intrinsic function by recursively calling the export function on the fragment property of Fn::ForEach |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #8096 +/- ##
==========================================
- Coverage 0.08% 0.08% -0.01%
==========================================
Files 208 208
Lines 16673 16682 +9
==========================================
Hits 14 14
- Misses 16659 16668 +9 ☔ View full report in Codecov by Sentry. |
kyleknap
left a comment
kyleknap
left a comment
There was a problem hiding this comment.
Looks good. I just had some smaller comments.
| continue | ||
|
|
||
| resource_type = resource.get("Type", None) | ||
| if resource_type is None: |
There was a problem hiding this comment.
Is there a particular reason this if check was added for the Type? A sample CFN template snippet would help illustrate what it is addressing.
There was a problem hiding this comment.
I had added this (removed it from the latest commit) to prevent the need to go through the items in self.resources_to_export if the resource_type resolves to None (see loop below)
resource_type = resource.get("Type", None)
# ...
for exporter_class in self.resources_to_export:
if exporter_class.RESOURCE_TYPE != resource_type:
continue
# ...However, looking at the Resource class, if RESOURCE_TYPE is not defined for a subclass of a Resource, it would also resolve to None, thought I don't think we'd every have such a subclass, but it is possible, so opting to not have this change as part of this PR
tests/unit/customizations/cloudformation/test_artifact_exporter.py
Outdated
Show resolved
Hide resolved
…ensure there aren't unaccounted behaviors during export
* release-1.32.2: Bumping version to 1.32.2 Update changelog based on model updates Support Fn::ForEach intrinsic function (#8096)
Allow "aws cloudformation package" to succeed when a Resource is not a key-value pair.
|
A fix very similar to what this PR addressed is needed for the sam cli. Is there any chance that the authors of this PR would be willing to take that on? |
Fixes the 2.5-year-old bug where SAM CLI crashed with AttributeError when processing templates using CloudFormation's Fn::ForEach. Following AWS CLI's approach (aws/aws-cli#8096), we now detect and skip Fn::ForEach constructs during local parsing, letting CloudFormation expand them server-side. Changes: - Added Fn::ForEach to unresolvable intrinsics - Updated resource metadata normalizer to skip ForEach blocks - Added informative logging - Updated providers to handle ForEach gracefully - Added integration tests Testing: - All 5,870 unit tests pass - 94.12% code coverage - Verified with real templates Closes aws#5647
|
Hey everyone! 🎉 Just opened a PR that fixes this issue: PR ##5647 TL;DR: SAM CLI now supports Fn::ForEach! No more AttributeError crashes. We followed the same approach AWS CLI took in their PR #8096 - detect ForEach constructs, skip them during local parsing, and let CloudFormation handle the expansion server-side (where it belongs). Tested thoroughly:
Been waiting 2.5 years for this one - should be good to go! 🚀 |
Fixes the 2.5-year-old bug where SAM CLI crashed with AttributeError when processing templates using CloudFormation's Fn::ForEach. Following AWS CLI's approach (aws/aws-cli#8096), we now detect and skip Fn::ForEach constructs during local parsing, letting CloudFormation expand them server-side. Changes: - Added Fn::ForEach to unresolvable intrinsics - Updated resource metadata normalizer to skip ForEach blocks - Added informative logging - Updated providers to handle ForEach gracefully - Added integration tests Testing: - All 5,870 unit tests pass - 94.12% code coverage - Verified with real templates Closes aws#5647
6 participants
Issue #, if available: #8075
Description of changes:
Allow "aws cloudformation package" to succeed when a Resource is not a key-value pair.
This includes supporting the new
Fn::ForEachintrinsic function, which can be provided as a key-value pair within the Resources section, where the value is not a key-value pair, as expected, but a list.Note: was able to successfully run
aws cloudformation packageandaws cloudformation deployon CloudFormation templates that contained theFn::ForEachintrinsic function.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.