Fix: CFn: mappings with references #2
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR addresses an issue with the FindInMap function in CloudFormation templates when using Ref for map and top-level keys, improving error handling and implementing a fix for resolving references from template parameters.
- Updated
resolve_conditionfunction intemplate_utils.pyto handle Ref cases within Fn::FindInMap - Added new test case
test_mapping_ref_map_keyintest_mappings.pyto validate the fix - Introduced
TemplateErrorclass inerrors.pyfor more specific error handling in CloudFormation templates - Created
mapping-ref-map-key.yamltemplate to demonstrate mappings with references in conditions - Added tests for deletion of failed nested stacks in
test_nested_stacks.py
12 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings
| if isinstance(map_name, dict) and "Ref" in map_name: | ||
| ref_name = map_name["Ref"] | ||
| param = parameters.get(ref_name) | ||
| if not param: | ||
| raise TemplateError( | ||
| f"Invalid reference: '{ref_name}' does not exist in parameters: '{parameters}'" | ||
| ) | ||
| map_name = param.get("ResolvedValue") or param.get("ParameterValue") |
There was a problem hiding this comment.
style: Consider extracting this Ref resolution logic into a separate function to reduce code duplication
| value = top_level_map.get(second_level_key) | ||
| if not value: | ||
| raise TemplateError( | ||
| f"Invalid reference: '{second_level_key}' could not be found in the '{top_level_key}' mapping: '{top_level_map}'" | ||
| ) |
There was a problem hiding this comment.
style: Consider using .get() with a default value instead of checking for None
| context = {**payload["callbackContext"], **event.custom_context} | ||
| payload["callbackContext"] = context | ||
| payload["requestData"]["resourceProperties"] = event.resource_model | ||
| resource["Properties"] = event.resource_model |
There was a problem hiding this comment.
style: Consider using a more descriptive variable name instead of 'context'
| LambdaExecutionRole: | ||
| Type: AWS::IAM::Role | ||
| Properties: | ||
| RoleName: LambdaExecutionRole |
There was a problem hiding this comment.
style: Using a hardcoded RoleName may cause issues with multiple deployments or updates. Consider using a dynamic naming strategy
| BucketStackId: | ||
| Value: !Ref ChildStack |
There was a problem hiding this comment.
style: Consider adding a description for this output to improve template readability.
Motivation
A customer presented an error which involved a
FindInMapin aCondition. TheFindInMapusesRefsfor its map and top level keys, which raised an error:Changes