diff --git a/scripts/update_schemas_format.py b/scripts/update_schemas_format.py index 4a574d9dd3..c8120bdb8d 100755 --- 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 index fbc1866105..a9b506f677 100755 --- 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 index ebd6746224..3cb22c0fdb 100644 --- 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 index f3c5c0f525..4e083e39f9 100644 --- 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 index bd156a5a86..8652b2d11d 100644 --- 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 index 45e5286ce7..61382d1f9e 100644 --- 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 index 538a3f94bd..cfe886b8fb 100644 --- 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 index 93817173f9..1368b27ab3 100644 --- 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 index fa44f9dda7..d07fdfc357 100644 --- 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 index a05149077c..aee970b9ff 100644 --- 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 index 9db46e5389..f121430f7e 100644 --- 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 index f4e5311cf2..552c088dd8 100644 --- 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 index 1c6657b480..100b5c9cae 100644 --- 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 new file mode 100644 index 0000000000..bd700aae0b --- /dev/null +++ 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 index f291af9372..c7b5a4e5d7 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": "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", diff --git a/test/fixtures/results/quickstart/nist_vpc_management.json b/test/fixtures/results/quickstart/nist_vpc_management.json index f3166597b5..5a27a800ec 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": "f0f419da-862a-4317-0b95-8af2df6e8ad2", + "Id": "1c7a978c-0331-1e35-7ac6-6edc8d947e1c", "Level": "Warning", "Location": { "End": { @@ -435,7 +435,7 @@ "LineNumber": 513 } }, - "Message": "{'Ref': 'pBastionAmi'} is not a 'AWS::EC2::Image.Id' when 'Ref' is resolved", + "Message": "{'Ref': 'pBastionAmi'} is not a 'AWS::EC2::Image.Id' with pattern {'Ref': 'pBastionAmi'} 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 d1e295eacd..27448d3b25 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": "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", @@ -615,7 +615,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": { @@ -634,7 +634,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", @@ -877,7 +877,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": { @@ -896,7 +896,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", diff --git a/test/fixtures/templates/integration/formats.yaml b/test/fixtures/templates/integration/formats.yaml new file mode 100644 index 0000000000..2853b4ddf6 --- /dev/null +++ b/test/fixtures/templates/integration/formats.yaml @@ -0,0 +1,44 @@ +Parameters: + SecurityGroupsIds: + Type: List + SecurityGroupId: + Type: AWS::EC2::SecurityGroup::Id + +Resources: + Vpc: + Type: AWS::EC2::VPC + Properties: + CidrBlock: 10.0.0.0/16 + + Subnet: + Type: AWS::EC2::Subnet + Properties: + VpcId: !Ref Vpc + CidrBlock: 10.0.0.0/24 + + SecurityGroup: + Type: AWS::EC2::SecurityGroup + Properties: + VpcId: !Ref Vpc + GroupDescription: Some security group + + Instance1: + Type: AWS::EC2::Instance + Properties: + ImageId: ami-abcdef12 + InstanceType: t2.micro + NetworkInterfaces: + - DeviceIndex: 0 + SubnetId: !Ref Subnet + GroupSet: !Ref SecurityGroupsIds + - DeviceIndex: 1 + SubnetId: !Ref Subnet + GroupSet: + - !Ref SecurityGroupId + - !Ref SecurityGroup + - !GetAtt Vpc.DefaultSecurityGroup + - sg-dne + - DeviceIndex: 2 + SubnetId: !Ref Subnet + GroupSet: + - !GetAtt SecurityGroup.GroupName diff --git a/test/integration/test_integration_templates.py b/test/integration/test_integration_templates.py index b0a40d60b1..8d81cd0037 100644 --- a/test/integration/test_integration_templates.py +++ b/test/integration/test_integration_templates.py @@ -26,7 +26,7 @@ class TestQuickStartTemplates(BaseCliTestCase): { "filename": "test/fixtures/templates/integration/dynamic-references.yaml", "results_filename": ( - "test/fixtures/results/integration" "/dynamic-references.json" + "test/fixtures/results/integration/dynamic-references.json" ), "exit_code": 2, }, @@ -57,6 +57,11 @@ class TestQuickStartTemplates(BaseCliTestCase): "results_filename": ("test/fixtures/results/integration/ref-types.json"), "exit_code": 2, }, + { + "filename": ("test/fixtures/templates/integration/formats.yaml"), + "results_filename": ("test/fixtures/results/integration/formats.json"), + "exit_code": 2, + }, { "filename": ( "test/fixtures/templates/integration/aws-ec2-networkinterface.yaml" diff --git a/test/unit/module/cfn_json/test_cfn_json.py b/test/unit/module/cfn_json/test_cfn_json.py index 1443f54d1e..f387d4bf3f 100644 --- a/test/unit/module/cfn_json/test_cfn_json.py +++ b/test/unit/module/cfn_json/test_cfn_json.py @@ -35,7 +35,7 @@ def setUp(self): }, "nat_instance": { "filename": "test/fixtures/templates/quickstart/nat-instance.json", - "failures": 5, + "failures": 7, }, "vpc_management": { "filename": "test/fixtures/templates/quickstart/vpc-management.json",