From 0acac1e13afe5be9c6019d6022ba8b5b287cb4c4 Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Thu, 14 Nov 2024 09:50:46 -0800 Subject: [PATCH] update error messagings from json schema (#3798) --- src/cfnlint/jsonschema/_keywords.py | 24 ++++++++++++------- src/cfnlint/rules/functions/_BaseFn.py | 2 +- .../jsonschema/test_validator_cfn.py | 18 +++----------- test/unit/module/jsonschema/test_validator.py | 21 +++++++--------- .../rules/functions/test_dynamic_reference.py | 7 ++---- test/unit/rules/functions/test_find_in_map.py | 3 ++- test/unit/rules/functions/test_if.py | 2 +- test/unit/rules/functions/test_sub.py | 2 +- .../unit/rules/functions/test_tojsonstring.py | 4 ++-- .../unit/rules/mappings/test_configuration.py | 2 +- test/unit/rules/outputs/test_configuration.py | 2 +- .../rules/parameters/test_configuration.py | 2 +- ...rvice_health_check_grace_period_seconds.py | 4 ++-- .../test_loadbalancer_application_subnets.py | 4 ++-- .../resources/properties/test_tagging.py | 10 +++----- .../resources/route53/test_recordsets.py | 2 +- .../templates/test_limitsize_description.py | 2 +- 17 files changed, 48 insertions(+), 63 deletions(-) diff --git a/src/cfnlint/jsonschema/_keywords.py b/src/cfnlint/jsonschema/_keywords.py index f88514c6bb..4932b111a4 100644 --- a/src/cfnlint/jsonschema/_keywords.py +++ b/src/cfnlint/jsonschema/_keywords.py @@ -323,7 +323,9 @@ def maxItems( validator: Validator, mI: Any, instance: Any, schema: dict[str, Any] ) -> ValidationResult: # pylint: disable=arguments-renamed if validator.is_type(instance, "array") and len(instance) > mI: - yield ValidationError(f"{instance!r} is too long ({mI})") + yield ValidationError( + f"expected maximum item count: {mI}, found: {len(instance)}" + ) def maxLength( @@ -333,7 +335,7 @@ def maxLength( return if len(instance) > mL: - yield ValidationError(f"{instance!r} is longer than {mL}") + yield ValidationError(f"expected maximum length: {mL}, found: {len(instance)}") def maxProperties( @@ -342,7 +344,9 @@ def maxProperties( if not validator.is_type(instance, "object"): return if validator.is_type(instance, "object") and len(instance) > mP: - yield ValidationError(f"{instance!r} has too many properties") + yield ValidationError( + f"expected maximum property count: {mP}, found: {len(instance)}" + ) def maximum( @@ -366,7 +370,9 @@ def minItems( validator: Validator, mI: Any, instance: Any, schema: dict[str, Any] ) -> ValidationResult: if validator.is_type(instance, "array") and len(instance) < mI: - yield ValidationError(f"{instance!r} is too short ({mI})") + yield ValidationError( + f"expected minimum item count: {mI}, found: {len(instance)}" + ) def minLength( @@ -376,14 +382,16 @@ def minLength( return if len(instance) < mL: - yield ValidationError(f"{instance!r} is shorter than {mL}") + yield ValidationError(f"expected minimum length: {mL}, found: {len(instance)}") def minProperties( validator: Validator, mP: Any, instance: Any, schema: dict[str, Any] ) -> ValidationResult: if validator.is_type(instance, "object") and len(instance) < mP: - yield ValidationError(f"{instance!r} does not have enough properties") + yield ValidationError( + f"expected minimum property count: {mP}, found: {len(instance)}" + ) def minimum( @@ -590,14 +598,14 @@ def uniqueItems( validator: Validator, uI: Any, instance: Any, schema: dict[str, Any] ) -> ValidationResult: if uI and validator.is_type(instance, "array") and not uniq(instance): - yield ValidationError(f"{instance!r} has non-unique elements") + yield ValidationError("array items are not unique") def uniqueKeys( validator: Validator, uKs: Any, instance: Any, schema: dict[str, Any] ) -> ValidationResult: if uKs and validator.is_type(instance, "array") and not uniq_keys(instance, uKs): - yield ValidationError(f"{instance!r} has non-unique elements for keys {uKs!r}") + yield ValidationError(f"array items are not unique for keys {uKs!r}") def type( diff --git a/src/cfnlint/rules/functions/_BaseFn.py b/src/cfnlint/rules/functions/_BaseFn.py index d1777e25eb..a229121d1e 100644 --- a/src/cfnlint/rules/functions/_BaseFn.py +++ b/src/cfnlint/rules/functions/_BaseFn.py @@ -87,7 +87,7 @@ def resolve( all_errs = [] for value, v, resolve_err in validator.resolve_value(instance): if resolve_err: - yield resolve_err + yield from self.fix_errors(iter([resolve_err])) continue errs = list( self.fix_errors( diff --git a/test/integration/jsonschema/test_validator_cfn.py b/test/integration/jsonschema/test_validator_cfn.py index a89b43b868..c577529a8d 100644 --- a/test/integration/jsonschema/test_validator_cfn.py +++ b/test/integration/jsonschema/test_validator_cfn.py @@ -189,11 +189,7 @@ def test_unique_items(self): schema_patches=schema_patch, expected_errs=[ ValidationError( - message=( - "[{'Key': 'Name', 'Value': 'Value'}, " - "{'Key': 'Name', 'Value': 'Value'}] has " - "non-unique elements" - ), + message=("array items are not unique"), path=deque(["Tags"]), validator="uniqueItems", validator_value=True, @@ -220,11 +216,7 @@ def test_unique_items(self): schema_patches=schema_patch, expected_errs=[ ValidationError( - message=( - "[{'Key': 'Name', 'Value': 'Value'}, " - "{'Key': 'Name', 'Value': 'Value'}] has " - "non-unique elements" - ), + message=("array items are not unique"), path=deque(["Tags"]), validator="uniqueItems", validator_value=True, @@ -289,11 +281,7 @@ def test_unique_items(self): schema_patches=schema_patch, expected_errs=[ ValidationError( - message=( - "[{'Key': 'Name', 'Value': 'Value'}, " - "{'Key': 'Name', 'Value': 'Value'}] has " - "non-unique elements" - ), + message=("array items are not unique"), path=deque(["Tags"]), validator="uniqueItems", validator_value=True, diff --git a/test/unit/module/jsonschema/test_validator.py b/test/unit/module/jsonschema/test_validator.py index c291a18074..ed3d0cb588 100644 --- a/test/unit/module/jsonschema/test_validator.py +++ b/test/unit/module/jsonschema/test_validator.py @@ -532,7 +532,7 @@ def test_validator(name, schema, instance, expected, validator): [], [ ValidationError( - "[] is too short (2)", + "expected minimum item count: 2, found: 0", ) ], ), @@ -552,11 +552,7 @@ def test_validator(name, schema, instance, expected, validator): "maxItems", {"maxItems": 0}, ["foo"], - [ - ValidationError( - "['foo'] is too long (0)", - ) - ], + [ValidationError("expected maximum item count: 0, found: 1")], ), ( "valid minLength", @@ -576,7 +572,7 @@ def test_validator(name, schema, instance, expected, validator): "", [ ValidationError( - "'' is shorter than 2", + "expected minimum length: 2, found: 0", ) ], ), @@ -598,7 +594,7 @@ def test_validator(name, schema, instance, expected, validator): "foo", [ ValidationError( - "'foo' is longer than 0", + "expected maximum length: 0, found: 3", ) ], ), @@ -768,7 +764,7 @@ def test_validator(name, schema, instance, expected, validator): {}, [ ValidationError( - "{} does not have enough properties", + "expected minimum property count: 1, found: 0", ) ], ), @@ -790,7 +786,7 @@ def test_validator(name, schema, instance, expected, validator): {"foo": {}, "bar": {}}, [ ValidationError( - "{'foo': {}, 'bar': {}} has too many properties", + "expected maximum property count: 1, found: 2", ) ], ), @@ -943,7 +939,7 @@ def test_validator(name, schema, instance, expected, validator): [1, 2, "1"], [ ValidationError( - "[1, 2, '1'] has non-unique elements", + "array items are not unique", ) ], ), @@ -1035,8 +1031,7 @@ def test_validator(name, schema, instance, expected, validator): ], [ ValidationError( - "[{'Name': 'foo'}, {'Name': 'foo'}] has non-unique " - "elements for keys ['Name']", + "array items are not unique " "for keys ['Name']", ) ], ), diff --git a/test/unit/rules/functions/test_dynamic_reference.py b/test/unit/rules/functions/test_dynamic_reference.py index d8713ecdb6..7350716154 100644 --- a/test/unit/rules/functions/test_dynamic_reference.py +++ b/test/unit/rules/functions/test_dynamic_reference.py @@ -110,7 +110,7 @@ def context(cfn): }, [ ValidationError( - "['resolve', 'ssm'] is too short (3)", + "expected minimum item count: 3, found: 2", validator="minItems", rule=DynamicReference(), ) @@ -182,10 +182,7 @@ def context(cfn): }, [ ValidationError( - "['resolve', 'secretsmanager', 'arn', 'aws', " - "'secretsmanager', 'us-east-1', '012345678901', " - "'secret', 'my-secret', 'SecretString', " - "'', '', '', ''] is too long (13)", + "expected maximum item count: 13, found: 14", validator="maxItems", rule=DynamicReference(), ) diff --git a/test/unit/rules/functions/test_find_in_map.py b/test/unit/rules/functions/test_find_in_map.py index 77fb167d6c..4f33a3033b 100644 --- a/test/unit/rules/functions/test_find_in_map.py +++ b/test/unit/rules/functions/test_find_in_map.py @@ -63,7 +63,7 @@ def context(cfn): None, [ ValidationError( - "['foo', 'bar', 'key', 'key2'] is too long (3)", + "expected maximum item count: 3, found: 4", path=deque(["Fn::FindInMap"]), schema_path=deque(["maxItems"]), validator="fn_findinmap", @@ -164,6 +164,7 @@ def context(cfn): "'C' is not one of ['B'] for mapping 'A'", path=deque(["Fn::FindInMap", 1]), schema_path=deque([]), + validator="fn_findinmap", ), ], ), diff --git a/test/unit/rules/functions/test_if.py b/test/unit/rules/functions/test_if.py index 7ad7c840a8..a72854013b 100644 --- a/test/unit/rules/functions/test_if.py +++ b/test/unit/rules/functions/test_if.py @@ -63,7 +63,7 @@ def validator(cfn, context): {"type": "string"}, [ ValidationError( - "['IsUsEast1', 'foo', 'bar', 'key'] is too long (3)", + "expected maximum item count: 3, found: 4", path=deque(["Fn::If"]), schema_path=deque(["maxItems"]), validator="fn_if", diff --git a/test/unit/rules/functions/test_sub.py b/test/unit/rules/functions/test_sub.py index 7cb09eca60..0e6c55c8e4 100644 --- a/test/unit/rules/functions/test_sub.py +++ b/test/unit/rules/functions/test_sub.py @@ -168,7 +168,7 @@ def context(cfn): {"type": "string"}, [ ValidationError( - "['${foo}', {'foo': 'bar'}, {}] is too long (2)", + "expected maximum item count: 2, found: 3", path=deque(["Fn::Sub"]), schema_path=deque(["maxItems"]), validator="fn_sub", diff --git a/test/unit/rules/functions/test_tojsonstring.py b/test/unit/rules/functions/test_tojsonstring.py index 2e6ffca930..b696bade6b 100644 --- a/test/unit/rules/functions/test_tojsonstring.py +++ b/test/unit/rules/functions/test_tojsonstring.py @@ -78,7 +78,7 @@ def context(cfn): {"transforms": Transforms(["AWS::LanguageExtensions"])}, [ ValidationError( - "{} does not have enough properties", + "expected minimum property count: 1, found: 0", path=deque(["Fn::ToJsonString"]), schema_path=deque(["minProperties"]), validator="fn_tojsonstring", @@ -93,7 +93,7 @@ def context(cfn): {"transforms": Transforms(["AWS::LanguageExtensions"])}, [ ValidationError( - "[] is too short (1)", + "expected minimum item count: 1, found: 0", path=deque(["Fn::ToJsonString"]), schema_path=deque(["minItems"]), validator="fn_tojsonstring", diff --git a/test/unit/rules/mappings/test_configuration.py b/test/unit/rules/mappings/test_configuration.py index 0accf53637..536dabc7ac 100644 --- a/test/unit/rules/mappings/test_configuration.py +++ b/test/unit/rules/mappings/test_configuration.py @@ -125,7 +125,7 @@ def rule(): {"a" * 256: {"Key": {"Value": "Foo"}}}, [ ValidationError( - f"{'a'*256!r} is longer than 255", + "expected maximum length: 255, found: 256", validator="maxLength", schema_path=deque(["propertyNames", "maxLength"]), rule=None, # empty because we didn't load child rules diff --git a/test/unit/rules/outputs/test_configuration.py b/test/unit/rules/outputs/test_configuration.py index 552cf3d264..2f5fc1988c 100644 --- a/test/unit/rules/outputs/test_configuration.py +++ b/test/unit/rules/outputs/test_configuration.py @@ -119,7 +119,7 @@ def context(cfn): }, [ ValidationError( - f"{'a'*256!r} is longer than 255", + "expected maximum length: 255, found: 256", validator="maxLength", schema_path=deque(["propertyNames", "maxLength"]), rule=None, # none becuase we don't load child rule diff --git a/test/unit/rules/parameters/test_configuration.py b/test/unit/rules/parameters/test_configuration.py index 046504b570..c2aefe5bdb 100644 --- a/test/unit/rules/parameters/test_configuration.py +++ b/test/unit/rules/parameters/test_configuration.py @@ -172,7 +172,7 @@ def context(cfn): }, [ ValidationError( - f"{'a'*256!r} is longer than 255", + "expected maximum length: 255, found: 256", validator="maxLength", schema_path=deque(["propertyNames", "maxLength"]), rule=None, # none becuase we don't load child rule diff --git a/test/unit/rules/resources/ecs/test_service_health_check_grace_period_seconds.py b/test/unit/rules/resources/ecs/test_service_health_check_grace_period_seconds.py index dcabd42182..91a81c943a 100644 --- a/test/unit/rules/resources/ecs/test_service_health_check_grace_period_seconds.py +++ b/test/unit/rules/resources/ecs/test_service_health_check_grace_period_seconds.py @@ -47,7 +47,7 @@ def template(): {"HealthCheckGracePeriodSeconds": "Foo", "LoadBalancers": []}, [ ValidationError( - "[] is too short (1)", + "expected minimum item count: 1, found: 0", rule=ServiceHealthCheckGracePeriodSeconds(), path=deque(["LoadBalancers"]), validator="minItems", @@ -80,7 +80,7 @@ def template(): }, [ ValidationError( - "[] is too short (1)", + "expected minimum item count: 1, found: 0", rule=ServiceHealthCheckGracePeriodSeconds(), path=deque(["LoadBalancers"]), validator="minItems", diff --git a/test/unit/rules/resources/elbv2/test_loadbalancer_application_subnets.py b/test/unit/rules/resources/elbv2/test_loadbalancer_application_subnets.py index 3ff797df51..8fd7cfe3d1 100644 --- a/test/unit/rules/resources/elbv2/test_loadbalancer_application_subnets.py +++ b/test/unit/rules/resources/elbv2/test_loadbalancer_application_subnets.py @@ -40,7 +40,7 @@ def rule(): {"Subnets": ["SubnetA"]}, [ ValidationError( - ("['SubnetA'] is too short (2)"), + "expected minimum item count: 2, found: 1", rule=LoadBalancerApplicationSubnets(), path=deque(["Subnets"]), validator="minItems", @@ -55,7 +55,7 @@ def rule(): }, [ ValidationError( - ("['SubnetA'] is too short (2)"), + "expected minimum item count: 2, found: 1", rule=LoadBalancerApplicationSubnets(), path=deque(["Subnets"]), validator="minItems", diff --git a/test/unit/rules/resources/properties/test_tagging.py b/test/unit/rules/resources/properties/test_tagging.py index 73acaad248..8cdfd29927 100644 --- a/test/unit/rules/resources/properties/test_tagging.py +++ b/test/unit/rules/resources/properties/test_tagging.py @@ -38,11 +38,7 @@ def rule(): {"taggable": True}, [ ValidationError( - ( - "[{'Key': 'Foo', 'Value': 'Bar'}, " - "{'Key': 'Foo', 'Value': 'Bar'}] " - "has non-unique elements for keys ['Key']" - ), + ("array items are not unique for keys ['Key']"), path=deque(["Tags"]), schema_path=deque(["properties", "Tags", "uniqueKeys"]), validator="tagging", @@ -59,7 +55,7 @@ def rule(): {"taggable": True}, [ ValidationError( - f"{'a'*257!r} is longer than 256", + "expected maximum length: 256, found: 257", path=deque(["Tags", 0, "Value"]), schema_path=deque( [ @@ -83,7 +79,7 @@ def rule(): {"taggable": True}, [ ValidationError( - f"{'a'*257!r} is longer than 256", + "expected maximum length: 256, found: 257", path=deque(["Tags", "Foo"]), schema_path=deque( [ diff --git a/test/unit/rules/resources/route53/test_recordsets.py b/test/unit/rules/resources/route53/test_recordsets.py index af86f05e29..63bfd54c3f 100644 --- a/test/unit/rules/resources/route53/test_recordsets.py +++ b/test/unit/rules/resources/route53/test_recordsets.py @@ -383,7 +383,7 @@ def rule(): ), ), ValidationError( - "['cname1.example.com', 'foo√bar'] is too long (1)", + "expected maximum item count: 1, found: 2", rule=RecordSet(), path=deque(["ResourceRecords"]), validator="maxItems", diff --git a/test/unit/rules/templates/test_limitsize_description.py b/test/unit/rules/templates/test_limitsize_description.py index 30ad900f7e..d7f2331310 100644 --- a/test/unit/rules/templates/test_limitsize_description.py +++ b/test/unit/rules/templates/test_limitsize_description.py @@ -41,7 +41,7 @@ def rule(): None, [ ValidationError( - ("'FooBar' is longer than 3"), + ("expected maximum length: 3, found: 6"), ) ], ),