Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add rule E1041 to validate Ref format #3914

Merged
merged 12 commits into from
Jan 24, 2025
  •  
  •  
  •  
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 @@
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 @@
"""

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

Check warning on line 282 in src/cfnlint/context/context.py

View check run for this annotation

Codecov / codecov/patch

src/cfnlint/context/context.py#L282

Added line #L282 was not covered by tests

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


Expand Down Expand Up @@ -351,7 +355,10 @@
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 @@
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
Loading