Skip to content

Commit

Permalink
add rule E1041 to validate Ref format (#3914)
Browse files Browse the repository at this point in the history
* add rule E1041 to validate Ref format
* Add function to get cfn path in Template
  • Loading branch information
kddejong authored Jan 24, 2025
1 parent c2ccd76 commit c0e2823
Show file tree
Hide file tree
Showing 431 changed files with 3,305 additions and 510 deletions.
12 changes: 10 additions & 2 deletions docs/format_keyword.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,22 @@ In `cfn-lint`, we have extended the `format` keyword to support custom formats t

This format ensures that the value is a valid VPC ID, which is a string of the pattern `vpc-[0-9a-f]{8}` or `vpc-[0-9a-f]{17}`.

### AWS::EC2::SecurityGroup.GroupId
### AWS::EC2::SecurityGroup.Id

This format validates that the value is a valid Security Group ID, which is a string of the pattern `sg-[0-9a-f]{8}` or `sg-[0-9a-f]{17}`.

### AWS::EC2::SecurityGroup.GroupName
### AWS::EC2::SecurityGroup.Ids

This format validates that the value is a valid list of Security Group IDs, which is a string of the pattern `sg-[0-9a-f]{8}` or `sg-[0-9a-f]{17}`.

### AWS::EC2::SecurityGroup.Name

This format validates that the value is a valid Security Group Name, which must be a string of 1 to 255 characters, starting with a letter, and containing only letters, numbers, and certain special characters.

### AWS::EC2::Image.Id

This format validates that the value is a valid Amazon Machine Image (AMI), which is a string of the pattern `ami-[0-9a-f]{8}` or `ami-[0-9a-f]{17}`. More info in [docs](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/resource-ids.html)

### AWS::Logs::LogGroup.Name

This format validates that the value is a valid log group name, which is a string of the pattern `^[\.\-_\/#A-Za-z0-9]{1,512}\Z`. More info in [docs](https://docs.aws.amazon.com/AmazonCloudWatchLogs/latest/APIReference/API_LogGroup.html)
61 changes: 51 additions & 10 deletions scripts/update_schemas_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ def _create_security_group_ids_patch(type_name: str, ref: str, resolver: RefReso
ref=resolved["$ref"],
resolver=resolver,
)
if resolved.get("type") != "array":
return []
items = resolved.get("items")
if items:
if "$ref" in items:
Expand All @@ -89,7 +91,7 @@ def _create_security_group_ids_patch(type_name: str, ref: str, resolver: RefReso
path=ref[1:],
),
_create_patch(
{"format": "AWS::EC2::SecurityGroup.GroupId"},
{"format": "AWS::EC2::SecurityGroup.Id"},
items_path,
resolver=resolver,
),
Expand All @@ -110,7 +112,7 @@ def _create_security_group_id(type_name: str, ref: str, resolver: RefResolver):

return [
_create_patch(
{"format": "AWS::EC2::SecurityGroup.GroupId"},
{"format": "AWS::EC2::SecurityGroup.Id"},
ref,
resolver=resolver,
)
Expand All @@ -129,7 +131,7 @@ def _create_security_group_name(type_name: str, ref: str, resolver: RefResolver)

return [
_create_patch(
{"format": "AWS::EC2::SecurityGroup.GroupName"},
{"format": "AWS::EC2::SecurityGroup.Name"},
ref,
resolver=resolver,
)
Expand All @@ -150,23 +152,32 @@ def _create_patch(value: dict[str, str], ref: Sequence[str], resolver: RefResolv
_manual_patches = {
"AWS::EC2::SecurityGroup": [
Patch(
values={"format": "AWS::EC2::SecurityGroup.GroupId"},
values={"format": "AWS::EC2::SecurityGroup.Id"},
path="/properties/GroupId",
),
Patch(
values={"format": "AWS::EC2::SecurityGroup.GroupName"},
values={"format": "AWS::EC2::SecurityGroup.Name"},
path="/properties/GroupName",
),
Patch(
values={
"anyOf": [
{"format": "AWS::EC2::SecurityGroup.Id"},
{"format": "AWS::EC2::SecurityGroup.Name"},
]
},
path="/properties/Id",
),
],
"AWS::EC2::SecurityGroupIngress": [
Patch(
values={"format": "AWS::EC2::SecurityGroup.GroupId"},
values={"format": "AWS::EC2::SecurityGroup.Id"},
path="/properties/GroupId",
),
],
"AWS::EC2::SecurityGroupEgress": [
Patch(
values={"format": "AWS::EC2::SecurityGroup.GroupId"},
values={"format": "AWS::EC2::SecurityGroup.Id"},
path="/properties/GroupId",
),
],
Expand Down Expand Up @@ -231,7 +242,30 @@ def main():
)
)

for path in _descend(obj, ["SecurityGroupIds", "SecurityGroups"]):
for path in _descend(obj, ["LogGroupName"]):
if path[-2] == "properties":
resource_patches.append(
_create_patch(
value={"format": "AWS::Logs::LogGroup.Name"},
ref="#/" + "/".join(path),
resolver=resolver,
)
)

for path in _descend(
obj,
[
"CustomSecurityGroupIds",
"DBSecurityGroups",
"Ec2SecurityGroupIds",
"GroupSet",
"InputSecurityGroups",
"SecurityGroupIdList",
"SecurityGroupIds",
"SecurityGroups",
"VpcSecurityGroupIds",
],
):
if path[-2] == "properties":
resource_patches.extend(
_create_security_group_ids_patch(
Expand All @@ -242,11 +276,14 @@ def main():
for path in _descend(
obj,
[
"ClusterSecurityGroupId",
"DefaultSecurityGroup",
"SourceSecurityGroupId",
"DestinationSecurityGroupId",
"EC2SecurityGroupId",
"SecurityGroup",
"SecurityGroupId",
"SecurityGroupIngress" "SourceSecurityGroupId",
"VpcSecurityGroupId",
],
):
if path[-2] == "properties":
Expand All @@ -259,7 +296,11 @@ def main():
for path in _descend(
obj,
[
"SourceSecurityGroupName",
"CacheSecurityGroupName",
"ClusterSecurityGroupName",
"DBSecurityGroupName",
"EC2SecurityGroupName",
"SourceSecurityGroupName" "SourceSecurityGroupName",
],
):
if path[-2] == "properties":
Expand Down
2 changes: 2 additions & 0 deletions scripts/update_snapshot_results.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ cfn-lint test/fixtures/templates/integration/resources-cloudformation-init.yaml
cfn-lint test/fixtures/templates/integration/ref-no-value.yaml -e -c I --format json > test/fixtures/results/integration/ref-no-value.json
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
Expand Down
20 changes: 15 additions & 5 deletions src/cfnlint/context/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def ref_value(self, instance: str) -> Iterator[Tuple[str | list[str], "Context"]
yield pseudo_value, self.evolve(ref_values={instance: pseudo_value})
return
if instance in self.parameters:
for v, path in self.parameters[instance].ref(self):
for v, path in self.parameters[instance].ref_value(self):

# validate that ref is possible with path
# need to evaluate if Fn::If would be not true if value is
Expand Down Expand Up @@ -278,7 +278,11 @@ class _Ref(ABC):
"""

@abstractmethod
def ref(self, context: Context) -> Iterator[Any]:
def ref(self, region: str) -> dict[str, Any]:
pass

@abstractmethod
def ref_value(self, context: Context) -> Iterator[Tuple[Any, deque]]:
pass


Expand Down Expand Up @@ -351,7 +355,10 @@ def __post_init__(self, parameter) -> None:
if parameter.get("NoEcho") in list(BOOLEAN_STRINGS_TRUE) + [True]:
self.no_echo = True

def ref(self, context: Context) -> Iterator[Tuple[Any, deque]]:
def ref(self, region: str = REGION_PRIMARY) -> dict[str, Any]:
return {}

def ref_value(self, context: Context) -> Iterator[Tuple[Any, deque]]:
if self.allowed_values:
for i, allowed_value in enumerate(self.allowed_values):
if isinstance(allowed_value, list):
Expand Down Expand Up @@ -402,10 +409,13 @@ def __post_init__(self, resource) -> None:
raise ValueError("Condition must be a string")
self.condition = c

def get_atts(self, region: str = "us-east-1") -> AttributeDict:
def get_atts(self, region: str = REGION_PRIMARY) -> AttributeDict:
return PROVIDER_SCHEMA_MANAGER.get_type_getatts(self.type, region)

def ref(self, context: Context) -> Iterator[Any]:
def ref(self, region: str = REGION_PRIMARY) -> dict[str, Any]:
return PROVIDER_SCHEMA_MANAGER.get_type_ref(self.type, region)

def ref_value(self, context: Context) -> Iterator[Tuple[Any, deque]]:
return
yield

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
"then": {
"properties": {
"Options": {
"properties": {
"awslogs-group": {
"format": "AWS::Logs::LogGroup.Name",
"type": "string"
}
},
"propertyNames": {
"enum": [
"awslogs-create-group",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
{
"op": "add",
"path": "/properties/SecurityGroups/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
{
"op": "add",
"path": "/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"op": "add",
"path": "/definitions/Log/properties/LogGroupName/format",
"value": "AWS::Logs::LogGroup.Name"
},
{
"op": "add",
"path": "/definitions/WindowsEvent/properties/LogGroupName/format",
"value": "AWS::Logs::LogGroup.Name"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
{
"op": "add",
"path": "/properties/SecurityGroups/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
{
"op": "add",
"path": "/definitions/VpcConfig/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
{
"op": "add",
"path": "/definitions/VpcConfig/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
{
"op": "add",
"path": "/definitions/VpcConfig/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
{
"op": "add",
"path": "/definitions/SecurityGroupId/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
{
"op": "add",
"path": "/properties/SecurityGroups/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"op": "add",
"path": "/properties/LogGroupName/format",
"value": "AWS::Logs::LogGroup.Name"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@
{
"op": "add",
"path": "/definitions/ComputeResources/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"op": "add",
"path": "/definitions/LoggingConfig/properties/LogGroupName/format",
"value": "AWS::Logs::LogGroup.Name"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"op": "add",
"path": "/properties/LogGroupName/format",
"value": "AWS::Logs::LogGroup.Name"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"op": "add",
"path": "/definitions/LoggingConfig/properties/LogGroupName/format",
"value": "AWS::Logs::LogGroup.Name"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"op": "add",
"path": "/definitions/LoggingConfig/properties/LogGroupName/format",
"value": "AWS::Logs::LogGroup.Name"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@
{
"op": "add",
"path": "/definitions/VpcConfig/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@
{
"op": "add",
"path": "/definitions/VpcConfig/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
{
"op": "add",
"path": "/definitions/VpcConfig/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
{
"op": "add",
"path": "/definitions/VpcConfig/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
{
"op": "add",
"path": "/properties/SecurityGroupIds/items/format",
"value": "AWS::EC2::SecurityGroup.GroupId"
"value": "AWS::EC2::SecurityGroup.Id"
}
]
Loading

0 comments on commit c0e2823

Please sign in to comment.