-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
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. |
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.
Looks good. I just had some smaller comments.
|
||
resource_type = resource.get("Type", None) | ||
if resource_type is None: |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.
🚀
* 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? |
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::ForEach
intrinsic 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 package
andaws cloudformation deploy
on CloudFormation templates that contained theFn::ForEach
intrinsic function.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.