Add in instance group set and integ tests

aws-cloudformation · Jan 23, 2025 · c36671e · c36671e
1 parent bf253f0
commit c36671e
Show file tree

Hide file tree

Showing 20 changed files with 202 additions and 93 deletions.
diff --git a/scripts/update_schemas_format.py b/scripts/update_schemas_format.py
@@ -262,6 +262,7 @@ def main():
                     "CustomSecurityGroupIds",
                     "InputSecurityGroups",
                     "SecurityGroupIdList",
+                    "GroupSet",
                 ],
             ):
                 if path[-2] == "properties":

diff --git a/scripts/update_snapshot_results.sh b/scripts/update_snapshot_results.sh
@@ -7,6 +7,7 @@ cfn-lint test/fixtures/templates/integration/ref-no-value.yaml -e -c I --format
 cfn-lint test/fixtures/templates/integration/availability-zones.yaml -e -c I --format json > test/fixtures/results/integration/availability-zones.json
 cfn-lint test/fixtures/templates/integration/getatt-types.yaml -e -c I --format json > test/fixtures/results/integration/getatt-types.json
 cfn-lint test/fixtures/templates/integration/ref-types.yaml -e -c I --format json > test/fixtures/results/integration/ref-types.json
+cfn-lint test/fixtures/templates/integration/formats.yaml -e -c I --format json > test/fixtures/results/integration/formats.json
 cfn-lint test/fixtures/templates/integration/aws-ec2-networkinterface.yaml -e -c I --format json > test/fixtures/results/integration/aws-ec2-networkinterface.json
 cfn-lint test/fixtures/templates/integration/aws-ec2-instance.yaml -e -c I --format json > test/fixtures/results/integration/aws-ec2-instance.json
 cfn-lint test/fixtures/templates/integration/aws-ec2-launchtemplate.yaml -e -c I --format json > test/fixtures/results/integration/aws-ec2-launchtemplate.json

diff --git a/src/cfnlint/data/schemas/patches/extensions/all/aws_ec2_instance/format.json b/src/cfnlint/data/schemas/patches/extensions/all/aws_ec2_instance/format.json
@@ -19,6 +19,16 @@
   "path": "/properties/SubnetId/format",
   "value": "AWS::EC2::Subnet.Id"
  },
+ {
+  "op": "add",
+  "path": "/definitions/NetworkInterface/properties/GroupSet/format",
+  "value": "AWS::EC2::SecurityGroup.Ids"
+ },
+ {
+  "op": "add",
+  "path": "/definitions/NetworkInterface/properties/GroupSet/items/format",
+  "value": "AWS::EC2::SecurityGroup.Id"
+ },
  {
   "op": "add",
   "path": "/properties/SecurityGroupIds/format",

diff --git a/src/cfnlint/data/schemas/patches/extensions/all/aws_ec2_networkinterface/format.json b/src/cfnlint/data/schemas/patches/extensions/all/aws_ec2_networkinterface/format.json
@@ -8,5 +8,15 @@
   "op": "add",
   "path": "/properties/SubnetId/format",
   "value": "AWS::EC2::Subnet.Id"
+ },
+ {
+  "op": "add",
+  "path": "/properties/GroupSet/format",
+  "value": "AWS::EC2::SecurityGroup.Ids"
+ },
+ {
+  "op": "add",
+  "path": "/properties/GroupSet/items/format",
+  "value": "AWS::EC2::SecurityGroup.Id"
  }
 ]
diff --git a/src/cfnlint/data/schemas/providers/us_east_1/aws-ec2-instance.json b/src/cfnlint/data/schemas/providers/us_east_1/aws-ec2-instance.json
@@ -233,8 +233,10 @@
      "type": "string"
     },
     "GroupSet": {
+     "format": "AWS::EC2::SecurityGroup.Ids",
      "insertionOrder": false,
      "items": {
+      "format": "AWS::EC2::SecurityGroup.Id",
       "type": "string"
      },
      "type": "array",

diff --git a/src/cfnlint/data/schemas/providers/us_east_1/aws-ec2-networkinterface.json b/src/cfnlint/data/schemas/providers/us_east_1/aws-ec2-networkinterface.json
@@ -118,8 +118,10 @@
    "type": "boolean"
   },
   "GroupSet": {
+   "format": "AWS::EC2::SecurityGroup.Ids",
    "insertionOrder": false,
    "items": {
+    "format": "AWS::EC2::SecurityGroup.Id",
     "type": "string"
    },
    "type": "array",

diff --git a/src/cfnlint/rules/formats/Format.py b/src/cfnlint/rules/formats/Format.py
@@ -34,7 +34,16 @@ def format(
 
             if format == rule.format_keyword:  # type: ignore
                 if not rule.format(validator, instance):  # type: ignore
+                    if hasattr(rule, "pattern"):
+                        yield ValidationError(
+                            (
+                                f"{instance!r} is not a {format!r} with "
+                                f"pattern {rule.pattern!r}"
+                            ),
+                            rule=rule,
+                        )
+                        continue
                     yield ValidationError(
-                        f"{instance!r} is not a {format!r}",
+                        f"{instance!r} is not a valid {format!r}",
                         rule=rule,
                     )
diff --git a/src/cfnlint/rules/formats/FormatKeyword.py b/src/cfnlint/rules/formats/FormatKeyword.py
@@ -5,12 +5,27 @@
 
 from __future__ import annotations
 
+from typing import Any
+
+import regex as re
+
+from cfnlint.jsonschema import Validator
 from cfnlint.rules import CloudFormationLintRule
 
 
 class FormatKeyword(CloudFormationLintRule):
 
-    def __init__(self, format: str | None = None) -> None:
+    def __init__(self, format: str | None = None, pattern: str | None = None) -> None:
         super().__init__()
         self.format_keyword = format
         self.parent_rules = ["E1103"]
+        self.pattern = pattern or r""
+
+    def format(self, validator: Validator, instance: Any) -> bool:
+        if not isinstance(instance, str):
+            return True
+
+        if re.match(self.pattern, instance):
+            return True
+
+        return False
diff --git a/src/cfnlint/rules/formats/LogGroupName.py b/src/cfnlint/rules/formats/LogGroupName.py
@@ -5,11 +5,6 @@
 
 from __future__ import annotations
 
-from typing import Any
-
-import regex as re
-
-from cfnlint.jsonschema import Validator
 from cfnlint.rules.formats.FormatKeyword import FormatKeyword
 
 
@@ -21,13 +16,6 @@ class LogGroupName(FormatKeyword):
     source_url = "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/format_keyword.md#AWS::Logs::LogGroup.Name"
 
     def __init__(self):
-        super().__init__(format="AWS::EC2::Subnet.Id")
-
-    def format(self, validator: Validator, instance: Any) -> bool:
-        if not isinstance(instance, str):
-            return True
-
-        if re.match(r"^[\.\-_\/#A-Za-z0-9]{1,512}\Z", instance):
-            return True
-
-        return False
+        super().__init__(
+            format="AWS::EC2::Subnet.Id", pattern=r"^[\.\-_\/#A-Za-z0-9]{1,512}\Z"
+        )
diff --git a/src/cfnlint/rules/formats/SecurityGroupId.py b/src/cfnlint/rules/formats/SecurityGroupId.py
@@ -5,11 +5,6 @@
 
 from __future__ import annotations
 
-from typing import Any
-
-import regex as re
-
-from cfnlint.jsonschema import Validator
 from cfnlint.rules.formats.FormatKeyword import FormatKeyword
 
 
@@ -24,13 +19,7 @@ class SecurityGroupId(FormatKeyword):
     source_url = "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/format_keyword.md#AWS::EC2::SecurityGroup.Id"
 
     def __init__(self):
-        super().__init__(format="AWS::EC2::SecurityGroup.Id")
-
-    def format(self, validator: Validator, instance: Any) -> bool:
-        if not isinstance(instance, str):
-            return True
-
-        if re.match(r"^sg-([a-fA-F0-9]{8}|[a-fA-F0-9]{17})$", instance):
-            return True
-
-        return False
+        super().__init__(
+            format="AWS::EC2::SecurityGroup.Id",
+            pattern=r"^sg-([a-fA-F0-9]{8}|[a-fA-F0-9]{17})$",
+        )
diff --git a/src/cfnlint/rules/formats/SecurityGroupName.py b/src/cfnlint/rules/formats/SecurityGroupName.py
@@ -5,11 +5,6 @@
 
 from __future__ import annotations
 
-from typing import Any
-
-import regex as re
-
-from cfnlint.jsonschema import Validator
 from cfnlint.rules.formats.FormatKeyword import FormatKeyword
 
 
@@ -21,13 +16,7 @@ class SecurityGroupName(FormatKeyword):
     source_url = "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/format_keyword.md#AWS::EC2::SecurityGroup.Name"
 
     def __init__(self):
-        super().__init__(format="AWS::EC2::SecurityGroup.Name")
-
-    def format(self, validator: Validator, instance: Any) -> bool:
-        if not isinstance(instance, str):
-            return True
-
-        if re.match(r"^[a-zA-Z0-9 \._\-:\/()#\,@\[\]+=&;\{\}!\$\*]+$", instance):
-            return True
-
-        return False
+        super().__init__(
+            format="AWS::EC2::SecurityGroup.Name",
+            pattern=r"^[a-zA-Z0-9 \._\-:\/()#\,@\[\]+=&;\{\}!\$\*]+$",
+        )
diff --git a/src/cfnlint/rules/formats/SubnetId.py b/src/cfnlint/rules/formats/SubnetId.py
@@ -5,11 +5,6 @@
 
 from __future__ import annotations
 
-from typing import Any
-
-import regex as re
-
-from cfnlint.jsonschema import Validator
 from cfnlint.rules.formats.FormatKeyword import FormatKeyword
 
 
@@ -21,13 +16,7 @@ class SubnetId(FormatKeyword):
     source_url = "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/format_keyword.md#AWS::EC2::Subnet.Id"
 
     def __init__(self):
-        super().__init__(format="AWS::EC2::Subnet.Id")
-
-    def format(self, validator: Validator, instance: Any) -> bool:
-        if not isinstance(instance, str):
-            return True
-
-        if re.match(r"^subnet-(([0-9A-Fa-f]{8})|([0-9A-Fa-f]{17}))$", instance):
-            return True
-
-        return False
+        super().__init__(
+            format="AWS::EC2::Subnet.Id",
+            pattern=r"^subnet-(([0-9A-Fa-f]{8})|([0-9A-Fa-f]{17}))$",
+        )
diff --git a/src/cfnlint/rules/formats/VpcId.py b/src/cfnlint/rules/formats/VpcId.py
@@ -5,11 +5,6 @@
 
 from __future__ import annotations
 
-from typing import Any
-
-import regex as re
-
-from cfnlint.jsonschema import Validator
 from cfnlint.rules.formats.FormatKeyword import FormatKeyword
 
 
@@ -21,13 +16,7 @@ class VpcId(FormatKeyword):
     source_url = "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/format_keyword.md#AWS::EC2::VPC.Id"
 
     def __init__(self):
-        super().__init__(format="AWS::EC2::VPC.Id")
-
-    def format(self, validator: Validator, instance: Any) -> bool:
-        if not isinstance(instance, str):
-            return True
-
-        if re.match(r"^vpc-(([0-9A-Fa-f]{8})|([0-9A-Fa-f]{17}))$", instance):
-            return True
-
-        return False
+        super().__init__(
+            format="AWS::EC2::VPC.Id",
+            pattern=r"^vpc-(([0-9A-Fa-f]{8})|([0-9A-Fa-f]{17}))$",
+        )
diff --git a/test/fixtures/results/integration/formats.json b/test/fixtures/results/integration/formats.json
@@ -0,0 +1,66 @@
+[
+    {
+        "Filename": "test/fixtures/templates/integration/formats.yaml",
+        "Id": "0030de3f-c28c-0148-a503-13e3a6130c1e",
+        "Level": "Error",
+        "Location": {
+            "End": {
+                "ColumnNumber": 19,
+                "LineNumber": 40
+            },
+            "Path": [
+                "Resources",
+                "Instance1",
+                "Properties",
+                "NetworkInterfaces",
+                1,
+                "GroupSet",
+                3
+            ],
+            "Start": {
+                "ColumnNumber": 13,
+                "LineNumber": 40
+            }
+        },
+        "Message": "'sg-dne' is not a 'AWS::EC2::SecurityGroup.Id' with pattern '^sg-([a-fA-F0-9]{8}|[a-fA-F0-9]{17})$'",
+        "ParentId": null,
+        "Rule": {
+            "Description": "Security groups have to ref/gettatt to a security group or has the valid pattern",
+            "Id": "E1150",
+            "ShortDescription": "Validate security group format",
+            "Source": "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/format_keyword.md#AWS::EC2::SecurityGroup.Id"
+        }
+    },
+    {
+        "Filename": "test/fixtures/templates/integration/formats.yaml",
+        "Id": "f76fa81f-837e-7502-0be7-3d8ff34024e1",
+        "Level": "Error",
+        "Location": {
+            "End": {
+                "ColumnNumber": 44,
+                "LineNumber": 44
+            },
+            "Path": [
+                "Resources",
+                "Instance1",
+                "Properties",
+                "NetworkInterfaces",
+                2,
+                "GroupSet",
+                0
+            ],
+            "Start": {
+                "ColumnNumber": 13,
+                "LineNumber": 44
+            }
+        },
+        "Message": "{'Fn::GetAtt': ['SecurityGroup', 'GroupName']} with format 'AWS::EC2::SecurityGroup.Name' does not match destination format of 'AWS::EC2::SecurityGroup.Id'",
+        "ParentId": null,
+        "Rule": {
+            "Description": "Validate that if source and destination format exists that they match",
+            "Id": "E1040",
+            "ShortDescription": "Check if GetAtt matches destination format",
+            "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/best-practices.html#parmtypes"
+        }
+    }
+]
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": "6ecda82c-9ccf-e772-b940-41eafbc02f6e",
+        "Id": "1613319d-ac84-a032-c510-b54d3061c0b1",
         "Level": "Warning",
         "Location": {
             "End": {
@@ -438,7 +438,7 @@
                 "LineNumber": 379
             }
         },
-        "Message": "{'Ref': 'pAppAmi'} is not a 'AWS::EC2::Image.Id' when 'Ref' is resolved",
+        "Message": "{'Ref': 'pAppAmi'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved",
         "ParentId": null,
         "Rule": {
             "Description": "Resolve the Ref and then validate the values against the schema",
@@ -644,7 +644,7 @@
     },
     {
         "Filename": "test/fixtures/templates/quickstart/nist_application.yaml",
-        "Id": "300dd743-adad-ad4c-c09d-63fd5e9efcf2",
+        "Id": "a5d3cdc6-0e20-ebf4-e842-6341804c7ec3",
         "Level": "Warning",
         "Location": {
             "End": {
@@ -663,7 +663,7 @@
                 "LineNumber": 511
             }
         },
-        "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' when 'Ref' is resolved",
+        "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved",
         "ParentId": null,
         "Rule": {
             "Description": "Resolve the Ref and then validate the values against the schema",
@@ -1672,7 +1672,7 @@
     },
     {
         "Filename": "test/fixtures/templates/quickstart/nist_application.yaml",
-        "Id": "ce028b65-d316-4519-8018-ac6ec0477964",
+        "Id": "ba50a58d-877c-2610-cc0f-4418059c3d40",
         "Level": "Warning",
         "Location": {
             "End": {
@@ -1691,7 +1691,7 @@
                 "LineNumber": 801
             }
         },
-        "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' when 'Ref' is resolved",
+        "Message": "{'Ref': 'pWebServerAMI'} is not a 'AWS::EC2::Image.Id' with pattern '' when 'Ref' is resolved",
         "ParentId": null,
         "Rule": {
             "Description": "Resolve the Ref and then validate the values against the schema",