From f208933099a93fb1b2a1a97b58c4346d2a88248b Mon Sep 17 00:00:00 2001
From: Kevin DeJong <kddejong@amazon.com>
Date: Fri, 28 Jul 2023 08:45:24 -0700
Subject: [PATCH] Fixes for bad findinmaps (#2827)

---
 .../transforms/_language_extensions.py        | 16 ++++++++++---
 .../transforms/test_language_extensions.py    | 23 +++++++++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/src/cfnlint/template/transforms/_language_extensions.py b/src/cfnlint/template/transforms/_language_extensions.py
index 569f647eca..2d7577b4fb 100644
--- a/src/cfnlint/template/transforms/_language_extensions.py
+++ b/src/cfnlint/template/transforms/_language_extensions.py
@@ -104,6 +104,7 @@ def transform(self, cfn: Any) -> TransformResult:
         """Transform the template"""
         return [], self._walk(cfn.template, {}, cfn)
 
+    # pylint: disable=too-many-return-statements
     def _walk(self, item: Any, params: MutableMapping[str, Any], cfn: Any):
         obj = deepcopy(item)
         if isinstance(obj, dict):
@@ -134,6 +135,9 @@ def _walk(self, item: Any, params: MutableMapping[str, Any], cfn: Any):
                                     f_k,
                                 )
                     del obj[k]
+                elif k == "Fn::ToJsonString":
+                    # extra special handing for this as {} could be a valid value
+                    return obj
                 elif k == "Fn::Sub":
                     if isinstance(v, str):
                         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):
                     try:
                         mapping = _ForEachValueFnFindInMap(get_hash(v), v)
                         map_value = mapping.value(cfn, params, True)
+                        # if we get None this means its all strings but couldn't be resolved
+                        # we will pass this forward
                         if map_value is None:
-                            del obj[k]
                             continue
-                        if isinstance(map_value, str):
+                        # if we can resolve it we will return it
+                        if isinstance(map_value, _SCALAR_TYPES):
                             return map_value
                     except Exception as e:  # pylint: disable=broad-exception-caught
+                        # We couldn't resolve the FindInMap so we are going to leave it as it is
                         LOGGER.debug("Transform and Fn::FindInMap error: %s", {str(e)})
                 elif k == "Ref":
                     if isinstance(v, str):
@@ -169,7 +176,10 @@ def _walk(self, item: Any, params: MutableMapping[str, Any], cfn: Any):
                         obj[k] = r
                 else:
                     sub_value = self._walk(v, params, cfn)
-                    if sub_value is None:
+                    # a sub object may be none or we have returned
+                    # an empty object.  We don't want to remove empty
+                    # strings "" or 0 (zeros)
+                    if sub_value is None or sub_value == {}:
                         del obj[k]
                     else:
                         obj[k] = self._walk(v, params, cfn)
diff --git a/test/unit/module/template/transforms/test_language_extensions.py b/test/unit/module/template/transforms/test_language_extensions.py
index f0d3b6c009..ea7ffb4d16 100644
--- a/test/unit/module/template/transforms/test_language_extensions.py
+++ b/test/unit/module/template/transforms/test_language_extensions.py
@@ -396,6 +396,29 @@ def test_transform_error(self):
         self.assertIsNone(template)
         self.assertEqual(len(matches), 1)
 
+    def test_bad_mapping(self):
+        template_obj = deepcopy(self.template_obj)
+        nested_set(
+            template_obj,
+            [
+                "Resources",
+                "Fn::ForEach::Buckets",
+                2,
+                "S3Bucket${Identifier}",
+                "Properties",
+                "Tags",
+            ],
+            [{"Key": "Foo", "Value": {"Fn::FindInMap": ["Bucket", "Tags", "Key"]}}],
+        )
+        cfn = Template(filename="", template=template_obj, regions=["us-east-1"])
+
+        matches, template = language_extension(cfn)
+        self.assertListEqual(matches, [])
+        self.assertListEqual(
+            template["Resources"]["S3BucketA"]["Properties"]["Tags"],
+            [{"Key": "Foo", "Value": {"Fn::FindInMap": ["Bucket", "Tags", "Key"]}}],
+        )
+
 
 def nested_set(dic, keys, value):
     for key in keys[:-1]: