From 5a0e007bbd37d08c28fb9eb0a0a4c748f4b162fe Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Fri, 9 Aug 2024 10:46:44 -0700 Subject: [PATCH] Add better testing --- src/cfnlint/context/_mappings.py | 8 ++--- src/cfnlint/rules/functions/_BaseFn.py | 1 - test/unit/module/context/test_mappings.py | 13 ++++++++ .../module/jsonschema/test_resolvers_cfn.py | 9 +++++- test/unit/rules/functions/test_basefn.py | 30 +++++++++++++++++++ 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/cfnlint/context/_mappings.py b/src/cfnlint/context/_mappings.py index b01fca50bb..7599aea010 100644 --- a/src/cfnlint/context/_mappings.py +++ b/src/cfnlint/context/_mappings.py @@ -32,7 +32,7 @@ def create_from_dict(cls, instance: Any) -> Mappings: for k, v in instance.items(): if k == "Fn::Transform": is_transform = True - else: + elif isinstance(k, str): result[k] = Map.create_from_dict(v) return cls(result, is_transform) except (ValueError, AttributeError) as e: @@ -65,10 +65,8 @@ def create_from_dict(cls, instance: Any) -> _MappingSecondaryKey: for k, v in instance.items(): if k == "Fn::Transform": is_transform = True - elif isinstance(v, (str, list, int, float)): + elif isinstance(k, str) and isinstance(v, (str, list, int, float)): keys[k] = v - else: - continue return cls(keys, is_transform) @@ -95,6 +93,6 @@ def create_from_dict(cls, instance: Any) -> Map: for k, v in instance.items(): if k == "Fn::Transform": is_transform = True - else: + elif isinstance(k, str): keys[k] = _MappingSecondaryKey.create_from_dict(v) return cls(keys, is_transform) diff --git a/src/cfnlint/rules/functions/_BaseFn.py b/src/cfnlint/rules/functions/_BaseFn.py index b01ff2d18e..d1777e25eb 100644 --- a/src/cfnlint/rules/functions/_BaseFn.py +++ b/src/cfnlint/rules/functions/_BaseFn.py @@ -65,7 +65,6 @@ def _clean_resolve_errors( err.rule = self.child_rules[self.resolved_rule] for i, err_ctx in enumerate(err.context): err.context[i] = self._clean_resolve_errors(err_ctx, value, instance) - return err return err def resolve( diff --git a/test/unit/module/context/test_mappings.py b/test/unit/module/context/test_mappings.py index 6769871ba9..510eaada9a 100644 --- a/test/unit/module/context/test_mappings.py +++ b/test/unit/module/context/test_mappings.py @@ -82,6 +82,19 @@ def test_transforms(): }, Mappings({}, True), ), + ( + "Invalid mappings with wrong types", + { + "A": {True: {"C": "foo"}}, + "1": {"2": {False: "foo"}}, + }, + Mappings( + { + "A": Map({}, False), + "1": Map({"2": _MappingSecondaryKey({}, False)}), + } + ), + ), ], ) def test_mapping_creation(name, mappings, expected): diff --git a/test/unit/module/jsonschema/test_resolvers_cfn.py b/test/unit/module/jsonschema/test_resolvers_cfn.py index 278d551dd9..ec98e2bcfe 100644 --- a/test/unit/module/jsonschema/test_resolvers_cfn.py +++ b/test/unit/module/jsonschema/test_resolvers_cfn.py @@ -246,7 +246,8 @@ def test_invalid_functions(name, instance, response): ValidationError( ( "'bar' is not one of ['foo', " - "'transformFirstKey', 'transformSecondKey']" + "'transformFirstKey', 'transformSecondKey', " + "'integers']" ), path=deque(["Fn::FindInMap", 0]), ), @@ -306,6 +307,11 @@ def test_invalid_functions(name, instance, response): ) ], ), + ( + "Valid FindInMap with integer types", + {"Fn::FindInMap": ["integers", 1, 2]}, + [("Value", deque(["Mappings", "integers", "1", "2"]), None)], + ), ( "Valid FindInMap with a bad second key and default", {"Fn::FindInMap": ["foo", "first", "third", {"DefaultValue": "default"}]}, @@ -340,6 +346,7 @@ def test_valid_functions(name, instance, response): "foo": {"first": {"second": "bar"}}, "transformFirstKey": {"Fn::Transform": {"second": "bar"}}, "transformSecondKey": {"first": {"Fn::Transform": "bar"}}, + "integers": {"1": {"2": "Value"}}, } ) ) diff --git a/test/unit/rules/functions/test_basefn.py b/test/unit/rules/functions/test_basefn.py index 255b46652a..6363f47f06 100644 --- a/test/unit/rules/functions/test_basefn.py +++ b/test/unit/rules/functions/test_basefn.py @@ -56,6 +56,36 @@ def rule(): ) ], ), + ( + "Errors with context error", + {"Fn::Sub": "Bar"}, + {"anyOf": [{"enum": ["Foo"]}]}, + [ + ValidationError( + message=( + "{'Fn::Sub': 'Bar'} is not valid " + "under any of the given schemas " + "when '' is resolved" + ), + path=deque(["Fn::Sub"]), + validator="", + schema_path=deque(["anyOf"]), + rule=_ChildRule(), + context=[ + ValidationError( + message=( + "{'Fn::Sub': 'Bar'} is not one of " + "['Foo'] when '' is resolved" + ), + path=deque([]), + validator="enum", + schema_path=deque([0, "enum"]), + rule=_ChildRule(), + ) + ], + ) + ], + ), ], ) def test_resolve(name, instance, schema, expected, validator, rule):