Skip to content

Commit f208933

Browse files
authored
Fixes for bad findinmaps (#2827)
1 parent a1212b1 commit f208933

File tree

2 files changed

+36
-3
lines changed

2 files changed

+36
-3
lines changed

src/cfnlint/template/transforms/_language_extensions.py

+13-3
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ def transform(self, cfn: Any) -> TransformResult:
104104
"""Transform the template"""
105105
return [], self._walk(cfn.template, {}, cfn)
106106

107+
# pylint: disable=too-many-return-statements
107108
def _walk(self, item: Any, params: MutableMapping[str, Any], cfn: Any):
108109
obj = deepcopy(item)
109110
if isinstance(obj, dict):
@@ -134,6 +135,9 @@ def _walk(self, item: Any, params: MutableMapping[str, Any], cfn: Any):
134135
f_k,
135136
)
136137
del obj[k]
138+
elif k == "Fn::ToJsonString":
139+
# extra special handing for this as {} could be a valid value
140+
return obj
137141
elif k == "Fn::Sub":
138142
if isinstance(v, str):
139143
only_string, obj[k] = self._replace_string_params(v, params)
@@ -150,12 +154,15 @@ def _walk(self, item: Any, params: MutableMapping[str, Any], cfn: Any):
150154
try:
151155
mapping = _ForEachValueFnFindInMap(get_hash(v), v)
152156
map_value = mapping.value(cfn, params, True)
157+
# if we get None this means its all strings but couldn't be resolved
158+
# we will pass this forward
153159
if map_value is None:
154-
del obj[k]
155160
continue
156-
if isinstance(map_value, str):
161+
# if we can resolve it we will return it
162+
if isinstance(map_value, _SCALAR_TYPES):
157163
return map_value
158164
except Exception as e: # pylint: disable=broad-exception-caught
165+
# We couldn't resolve the FindInMap so we are going to leave it as it is
159166
LOGGER.debug("Transform and Fn::FindInMap error: %s", {str(e)})
160167
elif k == "Ref":
161168
if isinstance(v, str):
@@ -169,7 +176,10 @@ def _walk(self, item: Any, params: MutableMapping[str, Any], cfn: Any):
169176
obj[k] = r
170177
else:
171178
sub_value = self._walk(v, params, cfn)
172-
if sub_value is None:
179+
# a sub object may be none or we have returned
180+
# an empty object. We don't want to remove empty
181+
# strings "" or 0 (zeros)
182+
if sub_value is None or sub_value == {}:
173183
del obj[k]
174184
else:
175185
obj[k] = self._walk(v, params, cfn)

test/unit/module/template/transforms/test_language_extensions.py

+23
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,29 @@ def test_transform_error(self):
396396
self.assertIsNone(template)
397397
self.assertEqual(len(matches), 1)
398398

399+
def test_bad_mapping(self):
400+
template_obj = deepcopy(self.template_obj)
401+
nested_set(
402+
template_obj,
403+
[
404+
"Resources",
405+
"Fn::ForEach::Buckets",
406+
2,
407+
"S3Bucket${Identifier}",
408+
"Properties",
409+
"Tags",
410+
],
411+
[{"Key": "Foo", "Value": {"Fn::FindInMap": ["Bucket", "Tags", "Key"]}}],
412+
)
413+
cfn = Template(filename="", template=template_obj, regions=["us-east-1"])
414+
415+
matches, template = language_extension(cfn)
416+
self.assertListEqual(matches, [])
417+
self.assertListEqual(
418+
template["Resources"]["S3BucketA"]["Properties"]["Tags"],
419+
[{"Key": "Foo", "Value": {"Fn::FindInMap": ["Bucket", "Tags", "Key"]}}],
420+
)
421+
399422

400423
def nested_set(dic, keys, value):
401424
for key in keys[:-1]:

0 commit comments

Comments
 (0)