Fix format error messaging when no pattern (#3943)

* Fix format error messaging when no pattern
aws-cloudformation · Jan 31, 2025 · bf9cc1c · bf9cc1c
1 parent 02c6d24
commit bf9cc1c
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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):