From bf9cc1c676d3a650054a36cace8cedae48bb66d8 Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Fri, 31 Jan 2025 10:38:31 -0800 Subject: [PATCH] Fix format error messaging when no pattern (#3943) * Fix format error messaging when no pattern --- src/cfnlint/rules/formats/Format.py | 2 +- src/cfnlint/rules/formats/ImageId.py | 6 ++-- .../results/quickstart/nist_application.json | 12 ++++---- .../quickstart/nist_vpc_management.json | 4 +-- .../non_strict/nist_application.json | 12 ++++---- test/unit/rules/formats/test_format.py | 28 +++++++++++++++++-- 6 files changed, 45 insertions(+), 19 deletions(-) diff --git a/src/cfnlint/rules/formats/Format.py b/src/cfnlint/rules/formats/Format.py index cfe886b8fb..db43abc54a 100644 --- a/src/cfnlint/rules/formats/Format.py +++ b/src/cfnlint/rules/formats/Format.py @@ -34,7 +34,7 @@ def format( if format == rule.format_keyword: # type: ignore if not rule.format(validator, instance): # type: ignore - if hasattr(rule, "pattern"): + if hasattr(rule, "pattern") and rule.pattern: yield ValidationError( ( f"{instance!r} is not a {format!r} with " diff --git a/src/cfnlint/rules/formats/ImageId.py b/src/cfnlint/rules/formats/ImageId.py index d552d329eb..b26f5eb840 100644 --- a/src/cfnlint/rules/formats/ImageId.py +++ b/src/cfnlint/rules/formats/ImageId.py @@ -21,7 +21,9 @@ class ImageId(FormatKeyword): source_url = "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/format_keyword.md#AWS::EC2::Image.Id" def __init__(self): - super().__init__(format="AWS::EC2::Image.Id") + super().__init__( + format="AWS::EC2::Image.Id", pattern=r"^ami-([0-9a-z]{8}|[0-9a-z]{17})$" + ) def format(self, validator: Validator, instance: Any) -> bool: if not isinstance(instance, str): @@ -34,7 +36,7 @@ def format(self, validator: Validator, instance: Any) -> bool: if instance.startswith("resolve:ssm"): return True - if re.match(r"^ami-([0-9a-z]{8}|[0-9a-z]{17})$", instance): + if re.match(self.pattern, instance): return True return False diff --git a/test/fixtures/results/quickstart/nist_application.json b/test/fixtures/results/quickstart/nist_application.json index 6df6e81e37..2e6833233e 100644 --- a/test/fixtures/results/quickstart/nist_application.json +++ b/test/fixtures/results/quickstart/nist_application.json @@ -419,7 +419,7 @@ }, { "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", - "Id": "1613319d-ac84-a032-c510-b54d3061c0b1", + "Id": "e2b23fec-440d-6576-01e9-f0a09f5874d4", "Level": "Warning", "Location": { "End": { @@ -438,7 +438,7 @@ "LineNumber": 379 } }, - "Message": "{'Ref': 'pAppAmi'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved", + "Message": "{'Ref': 'pAppAmi'} is not a 'AWS::EC2::Image.Id' with pattern '^ami-([0-9a-z]{8}|[0-9a-z]{17})$' when 'Ref' is resolved", "ParentId": null, "Rule": { "Description": "Resolve the Ref and then validate the values against the schema", @@ -674,7 +674,7 @@ }, { "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", - "Id": "a5d3cdc6-0e20-ebf4-e842-6341804c7ec3", + "Id": "499f19a1-f0ba-d118-c02a-b8519ce4dcd7", "Level": "Warning", "Location": { "End": { @@ -693,7 +693,7 @@ "LineNumber": 511 } }, - "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved", + "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '^ami-([0-9a-z]{8}|[0-9a-z]{17})$' when 'Ref' is resolved", "ParentId": null, "Rule": { "Description": "Resolve the Ref and then validate the values against the schema", @@ -1732,7 +1732,7 @@ }, { "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", - "Id": "ba50a58d-877c-2610-cc0f-4418059c3d40", + "Id": "a793117d-34b7-967d-1a68-ef79479b9f52", "Level": "Warning", "Location": { "End": { @@ -1751,7 +1751,7 @@ "LineNumber": 801 } }, - "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved", + "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '^ami-([0-9a-z]{8}|[0-9a-z]{17})$' when 'Ref' is resolved", "ParentId": null, "Rule": { "Description": "Resolve the Ref and then validate the values against the schema", diff --git a/test/fixtures/results/quickstart/nist_vpc_management.json b/test/fixtures/results/quickstart/nist_vpc_management.json index 5a27a800ec..2762cc9580 100644 --- a/test/fixtures/results/quickstart/nist_vpc_management.json +++ b/test/fixtures/results/quickstart/nist_vpc_management.json @@ -416,7 +416,7 @@ }, { "Filename": "test/fixtures/templates/quickstart/nist_vpc_management.yaml", - "Id": "1c7a978c-0331-1e35-7ac6-6edc8d947e1c", + "Id": "9584e85a-7daa-e6fa-e582-de25884d042c", "Level": "Warning", "Location": { "End": { @@ -435,7 +435,7 @@ "LineNumber": 513 } }, - "Message": "{'Ref': 'pBastionAmi'} is not a 'AWS::EC2::Image.Id' with pattern {'Ref': 'pBastionAmi'} when 'Ref' is resolved", + "Message": "{'Ref': 'pBastionAmi'} is not a 'AWS::EC2::Image.Id' with pattern '^ami-([0-9a-z]{8}|[0-9a-z]{17})$' when 'Ref' is resolved", "ParentId": null, "Rule": { "Description": "Resolve the Ref and then validate the values against the schema", diff --git a/test/fixtures/results/quickstart/non_strict/nist_application.json b/test/fixtures/results/quickstart/non_strict/nist_application.json index c9eb80db81..ecb72d6d2f 100644 --- a/test/fixtures/results/quickstart/non_strict/nist_application.json +++ b/test/fixtures/results/quickstart/non_strict/nist_application.json @@ -419,7 +419,7 @@ }, { "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", - "Id": "1613319d-ac84-a032-c510-b54d3061c0b1", + "Id": "e2b23fec-440d-6576-01e9-f0a09f5874d4", "Level": "Warning", "Location": { "End": { @@ -438,7 +438,7 @@ "LineNumber": 379 } }, - "Message": "{'Ref': 'pAppAmi'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved", + "Message": "{'Ref': 'pAppAmi'} is not a 'AWS::EC2::Image.Id' with pattern '^ami-([0-9a-z]{8}|[0-9a-z]{17})$' when 'Ref' is resolved", "ParentId": null, "Rule": { "Description": "Resolve the Ref and then validate the values against the schema", @@ -645,7 +645,7 @@ }, { "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", - "Id": "a5d3cdc6-0e20-ebf4-e842-6341804c7ec3", + "Id": "499f19a1-f0ba-d118-c02a-b8519ce4dcd7", "Level": "Warning", "Location": { "End": { @@ -664,7 +664,7 @@ "LineNumber": 511 } }, - "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved", + "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '^ami-([0-9a-z]{8}|[0-9a-z]{17})$' when 'Ref' is resolved", "ParentId": null, "Rule": { "Description": "Resolve the Ref and then validate the values against the schema", @@ -937,7 +937,7 @@ }, { "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", - "Id": "ba50a58d-877c-2610-cc0f-4418059c3d40", + "Id": "a793117d-34b7-967d-1a68-ef79479b9f52", "Level": "Warning", "Location": { "End": { @@ -956,7 +956,7 @@ "LineNumber": 801 } }, - "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved", + "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '^ami-([0-9a-z]{8}|[0-9a-z]{17})$' when 'Ref' is resolved", "ParentId": null, "Rule": { "Description": "Resolve the Ref and then validate the values against the schema", diff --git a/test/unit/rules/formats/test_format.py b/test/unit/rules/formats/test_format.py index 9bf34ab968..14e3272227 100644 --- a/test/unit/rules/formats/test_format.py +++ b/test/unit/rules/formats/test_format.py @@ -36,6 +36,16 @@ def format(self, validator, instance): return False +class _FailWithPattern(FormatKeyword): + id = "BBBBB" + + def __init__(self): + super().__init__(format="test", pattern=r"^.*$") + + def format(self, validator, instance): + return False + + class _NoRuleId(FormatKeyword): def __init__(self): super().__init__(format="test") @@ -101,7 +111,7 @@ def format(self, validator, instance): }, [ ValidationError( - "'10.10.10.10' is not a 'test' with pattern ''", + "'10.10.10.10' is not a valid 'test'", rule=_Fail(), ) ], @@ -116,11 +126,25 @@ def format(self, validator, instance): }, [ ValidationError( - "'10.10.10.10' is not a 'test' with pattern ''", + "'10.10.10.10' is not a valid 'test'", rule=_Fail(), ) ], ), + ( + "Fail with pattern", + "test", + "10.10.10.10", + { + "AAAAA": _FailWithPattern(), + }, + [ + ValidationError( + "'10.10.10.10' is not a 'test' with pattern '^.*$'", + rule=_FailWithPattern(), + ) + ], + ), ], ) def test_validate(name, format, instance, child_rules, expected, rule, validator):