Return all errors from resolution (#3489)

aws-cloudformation · Jul 11, 2024 · 1e7161b · 1e7161b
1 parent 46efe54
commit 1e7161b
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 48 deletions.
diff --git a/src/cfnlint/context/context.py b/src/cfnlint/context/context.py
@@ -153,6 +153,9 @@ class Context:
 
     transforms: Transforms = field(init=True, default_factory=lambda: Transforms([]))
 
+    # is the value a resolved value
+    is_resolved_value: bool = field(init=True, default=False)
+
     def evolve(self, **kwargs) -> "Context":
         """
         Create a new context without merging together attributes

diff --git a/src/cfnlint/jsonschema/validators.py b/src/cfnlint/jsonschema/validators.py
@@ -176,6 +176,11 @@ def resolve_value(self, instance: Any) -> ResolutionResult:
                                 r_validator.context.conditions.status,
                                 r_validator.context.ref_values,
                             ):
+                                r_validator = r_validator.evolve(
+                                    context=r_validator.context.evolve(
+                                        is_resolved_value=True,
+                                    )
+                                )
                                 yield r_value, r_validator, r_errs
                         except UnknownSatisfisfaction as err:
                             LOGGER.debug(err)

diff --git a/src/cfnlint/rules/functions/_BaseFn.py b/src/cfnlint/rules/functions/_BaseFn.py
@@ -62,12 +62,12 @@ def resolve(
     ) -> ValidationResult:
         key, _ = self.key_value(instance)
 
-        return_err: ValidationError | None = None
         validator = validator.evolve(
             context=validator.context.evolve(
                 strict_types=False,
             ),
         )
+        all_errs = []
         for value, v, resolve_err in validator.resolve_value(instance):
             if resolve_err:
                 yield resolve_err
@@ -85,16 +85,12 @@ def resolve(
             if not errs:
                 return
 
-            return_err = errs[0]
+            for err in errs:
+                err.message = err.message.replace(f"{value!r}", f"{instance!r}")
+                err.message = f"{err.message} when {self.fn.name!r} is resolved"
+                all_errs.append(err)
 
-        if return_err:
-            return_err.message = return_err.message.replace(
-                f"{value!r}", f"{instance!r}"
-            )
-            return_err.message = (
-                f"{return_err.message} when {self.fn.name!r} is resolved"
-            )
-            yield return_err
+        yield from iter(all_errs)
 
     def _resolve_ref(self, validator, schema) -> Any:
 

diff --git a/src/cfnlint/rules/resources/properties/Type.py b/src/cfnlint/rules/resources/properties/Type.py
@@ -46,7 +46,9 @@ def type(self, validator, types, instance, schema):
                 ):
                     return
 
-        if self.config.get("strict") or validator.context.strict_types:
+        if (
+            self.config.get("strict") or validator.context.strict_types
+        ) and not validator.context.is_resolved_value:
             validator = validator.evolve(
                 context=validator.context.evolve(strict_types=True)
             )

diff --git a/test/fixtures/results/public/watchmaker.json b/test/fixtures/results/public/watchmaker.json
@@ -929,41 +929,6 @@
             "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-sub.html"
         }
     },
-    {
-        "Filename": "test/fixtures/templates/public/watchmaker.json",
-        "Id": "c686663b-d84c-9054-d89b-e04373ffff7a",
-        "Level": "Error",
-        "Location": {
-            "End": {
-                "ColumnNumber": 46,
-                "LineNumber": 1374
-            },
-            "Path": [
-                "Resources",
-                "WatchmakerInstance",
-                "Properties",
-                "BlockDeviceMappings",
-                1,
-                "Fn::If",
-                1,
-                "Ebs",
-                "VolumeSize",
-                "Ref"
-            ],
-            "Start": {
-                "ColumnNumber": 41,
-                "LineNumber": 1374
-            }
-        },
-        "Message": "{'Ref': 'AppVolumeSize'} is not of type 'integer' when 'Ref' is resolved",
-        "ParentId": null,
-        "Rule": {
-            "Description": "Making sure the Ref has a String value (no other functions are supported)",
-            "Id": "E1020",
-            "ShortDescription": "Ref validation of value",
-            "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-ref.html"
-        }
-    },
     {
         "Filename": "test/fixtures/templates/public/watchmaker.json",
         "Id": "913a07a4-0962-ced6-de7c-a1e4b678ecc9",

diff --git a/test/unit/module/cfn_yaml/test_yaml.py b/test/unit/module/cfn_yaml/test_yaml.py
@@ -27,7 +27,7 @@ def setUp(self):
             },
             "generic_bad": {
                 "filename": "test/fixtures/templates/bad/generic.yaml",
-                "failures": 27,
+                "failures": 32,
             },
         }
 

diff --git a/test/unit/module/test_rules_collections.py b/test/unit/module/test_rules_collections.py
@@ -69,7 +69,7 @@ def test_fail_run(self):
         filename = "test/fixtures/templates/bad/generic.yaml"
         template = cfnlint.decode.cfn_yaml.load(filename)
         cfn = Template(filename, template, ["us-east-1"])
-        expected_err_count = 30
+        expected_err_count = 35
         matches = []
         matches.extend(self.rules.run(filename, cfn))
         assert (

diff --git a/test/unit/rules/functions/test_sub.py b/test/unit/rules/functions/test_sub.py
@@ -290,6 +290,12 @@ def context(cfn):
                     schema_path=deque(["const"]),
                     validator="fn_sub",
                 ),
+                ValidationError(
+                    ("'three' was expected when 'Fn::Sub' is resolved"),
+                    path=deque(["Fn::Sub"]),
+                    schema_path=deque(["const"]),
+                    validator="fn_sub",
+                ),
             ],
         ),
     ],