diff --git a/src/cfnlint/jsonschema/_filter.py b/src/cfnlint/jsonschema/_filter.py index 8204046465..2c24b99674 100644 --- a/src/cfnlint/jsonschema/_filter.py +++ b/src/cfnlint/jsonschema/_filter.py @@ -36,18 +36,18 @@ class FunctionFilter: init=False, default_factory=lambda: [ "dependencies", - "if", - "then", - "else", - "required", - "requiredXor", - "requiredAtLeastOne", "dependentExcluded", "dependentRequired", - "minItems", - "minProperties", + "else", + "if", "maxItems", "maxProperties", + "minItems", + "minProperties", + "required", + "requiredAtLeastOne", + "requiredXor", + "then", "uniqueItems", ], ) @@ -116,6 +116,12 @@ def filter(self, validator: Any, instance: Any, schema: Any): return return + # if there are no functions then we don't need to worry + # about ref AWS::NoValue or If conditions + if not validator.context.functions: + yield instance, schema + return + # dependencies, required, minProperties, maxProperties # need to have filtered properties to validate # because of Ref: AWS::NoValue diff --git a/src/cfnlint/rules/functions/FindInMap.py b/src/cfnlint/rules/functions/FindInMap.py index 192ebccd9f..78734f240e 100644 --- a/src/cfnlint/rules/functions/FindInMap.py +++ b/src/cfnlint/rules/functions/FindInMap.py @@ -62,24 +62,62 @@ def schema(self, validator: Validator, instance: Any) -> dict[str, Any]: scalar_schema, scalar_schema, { + "functions": [], "schema": { + "findinmap_parameters": True, "type": ["object"], "properties": { "DefaultValue": { - "type": ("array",) + singular_types, + "default_value": True, } }, "additionalProperties": False, "required": ["DefaultValue"], - } + }, }, ] return schema + def _default_value( + self, validator: Validator, s: Any, instance: Any, schema: Any + ) -> ValidationResult: + validator = validator.evolve( + context=validator.context.evolve( + functions=["Ref"], + ), + ) + yield from validator.descend( + instance, + { + "type": ("array",) + singular_types, + }, + ) + def fn_findinmap( self, validator: Validator, s: Any, instance: Any, schema: Any ) -> ValidationResult: + + if validator.context.transforms.has_language_extensions_transform(): + # we have to use a special validator for this + # as we don't want DefaultValue: !Ref AWS::NoValue + # is valid + mapping_validator = validator.extend( + validators={ + "default_value": self._default_value, + }, + )(validator.schema) + validator = mapping_validator.evolve( + context=validator.context.evolve( + resources={}, + ), + cfn=validator.cfn, + function_filter=validator.function_filter, + resolver=validator.resolver, + ) + yield from super().validate(validator, s, instance, schema) + return + validator = validator.evolve( context=validator.context.evolve( resources={}, diff --git a/test/unit/module/jsonschema/test_filter.py b/test/unit/module/jsonschema/test_filter.py index 0a012e2d0e..9d399e845b 100644 --- a/test/unit/module/jsonschema/test_filter.py +++ b/test/unit/module/jsonschema/test_filter.py @@ -20,7 +20,7 @@ def filter(): @pytest.mark.parametrize( - "name,instance,schema,path,expected", + "name,instance,schema,path,functions,expected", [ ( "Don't validate dynamic references inside of function", @@ -28,21 +28,60 @@ def filter(): {"enum": "Foo"}, deque(["Foo", "Test", "Fn::Sub"]), [], + [], ), ( "Validate dynamic references", "{{resolve:ssm:secret}}", {"enum": "Foo"}, deque(["Foo", "Test"]), + [], [ ("{{resolve:ssm:secret}}", {"dynamicReference": {"enum": "Foo"}}), ], ), + ( + "Lack of functions returns the schema and instance", + { + "Foo": {"Ref": "AWS::NoValue"}, + }, + {"required": ["Foo"]}, + deque([]), + [], + [ + ( + { + "Foo": {"Ref": "AWS::NoValue"}, + }, + {"required": ["Foo"]}, + ), + ], + ), + ( + "Filtered schemas", + { + "Foo": {"Ref": "AWS::NoValue"}, + }, + {"required": ["Foo"]}, + deque([]), + ["Ref", "Fn::If"], + [ + ( + {}, + {"required": ["Foo"]}, + ), + ({"Foo": {"Ref": "AWS::NoValue"}}, {"cfnLint": [""]}), + ], + ), ], ) -def test_filter(name, instance, schema, path, expected, filter): +def test_filter(name, instance, schema, path, functions, expected, filter): validator = CfnTemplateValidator( - context=Context(regions=["us-east-1"], path=Path(path)), + context=Context( + regions=["us-east-1"], + path=Path(path), + functions=functions, + ), schema=schema, ) results = list(filter.filter(validator, instance, schema)) diff --git a/test/unit/rules/functions/test_find_in_map.py b/test/unit/rules/functions/test_find_in_map.py index 9f6e51c737..fb23ba0957 100644 --- a/test/unit/rules/functions/test_find_in_map.py +++ b/test/unit/rules/functions/test_find_in_map.py @@ -46,7 +46,7 @@ def context(cfn): {"Fn::FindInMap": ["A", "B", "C"]}, {"type": "string"}, {}, - [], + None, [], ), ( @@ -54,7 +54,7 @@ def context(cfn): {"Fn::FindInMap": ["foo", "bar", "key", "key2"]}, {"type": "string"}, {}, - [], + None, [ ValidationError( "['foo', 'bar', 'key', 'key2'] is too long (3)", @@ -69,7 +69,7 @@ def context(cfn): {"Fn::FindInMap": {"foo": "bar"}}, {"type": "string"}, {}, - [], + None, [ ValidationError( "{'foo': 'bar'} is not of type 'array'", @@ -84,7 +84,7 @@ def context(cfn): {"Fn::FindInMap": [{"Fn::GetAtt": "MyResource.Arn"}, "foo", "bar"]}, {"type": "string"}, {}, - [], + None, [ ValidationError( "{'Fn::GetAtt': 'MyResource.Arn'} is not of type 'string'", @@ -99,7 +99,7 @@ def context(cfn): {"Fn::FindInMap": ["A", "B", "C", {"DefaultValue": "D"}]}, {"type": "string"}, {"transforms": Transforms(["AWS::LanguageExtensions"])}, - [], + None, [], ), ( @@ -107,7 +107,7 @@ def context(cfn): {"Fn::FindInMap": ["A", "B", "C", []]}, {"type": "string"}, {"transforms": Transforms(["AWS::LanguageExtensions"])}, - [], + None, [ ValidationError( "[] is not of type 'object'", @@ -122,7 +122,7 @@ def context(cfn): {"Fn::FindInMap": ["A", "B", "C", {}]}, {"type": "string"}, {"transforms": Transforms(["AWS::LanguageExtensions"])}, - [], + None, [ ValidationError( "'DefaultValue' is a required property", @@ -147,6 +147,21 @@ def context(cfn): ), ], ), + ( + "Valid Fn::FindInMap with a Ref to AWS::NoValue", + { + "Fn::FindInMap": [ + "A", + "B", + "C", + {"DefaultValue": {"Ref": "AWS::NoValue"}}, + ] + }, + {"type": "string"}, + {"transforms": Transforms(["AWS::LanguageExtensions"])}, + [], + [], + ), ], ) def test_validate( @@ -162,10 +177,14 @@ def test_validate( ): context = context.evolve(**context_evolve) ref_mock = MagicMock() - ref_mock.return_value = iter(ref_mock_values) + ref_mock.return_value = iter(ref_mock_values or []) validator = CfnTemplateValidator({}).extend(validators={"ref": ref_mock})( context=context, cfn=cfn ) errs = list(rule.fn_findinmap(validator, schema, instance, {})) - assert ref_mock.call_count == len(ref_mock_values) + + if ref_mock_values is None: + ref_mock.assert_not_called() + else: + assert ref_mock.call_count == len(ref_mock_values) or 1 assert errs == expected, f"Test {name!r} got {errs!r}" diff --git a/test/unit/rules/resources/ecs/test_task_definition_essentials.py b/test/unit/rules/resources/ecs/test_task_definition_essentials.py index cb6b556d1c..ade6c27d63 100644 --- a/test/unit/rules/resources/ecs/test_task_definition_essentials.py +++ b/test/unit/rules/resources/ecs/test_task_definition_essentials.py @@ -57,8 +57,8 @@ def rule(): "At least one essential container is required", rule=TaskDefinitionEssentialContainer(), path=deque([]), - validator="minItems", - schema_path=deque(["minItems"]), + validator="contains", + schema_path=deque(["contains"]), ) ], ), diff --git a/test/unit/rules/resources/route53/test_recordsets.py b/test/unit/rules/resources/route53/test_recordsets.py index 70637e5101..af86f05e29 100644 --- a/test/unit/rules/resources/route53/test_recordsets.py +++ b/test/unit/rules/resources/route53/test_recordsets.py @@ -341,22 +341,6 @@ def rule(): ], }, [ - ValidationError( - "['cname1.example.com', 'foo√bar'] is too long (1)", - rule=RecordSet(), - path=deque(["ResourceRecords"]), - validator="maxItems", - schema_path=deque( - [ - "allOf", - 3, - "then", - "properties", - "ResourceRecords", - "maxItems", - ] - ), - ), ValidationError( "'foo√bar' is not valid under any of the given schemas", rule=RecordSet(), @@ -398,6 +382,22 @@ def rule(): ] ), ), + ValidationError( + "['cname1.example.com', 'foo√bar'] is too long (1)", + rule=RecordSet(), + path=deque(["ResourceRecords"]), + validator="maxItems", + schema_path=deque( + [ + "allOf", + 3, + "then", + "properties", + "ResourceRecords", + "maxItems", + ] + ), + ), ], ), (