Skip to content

Commit

Permalink
Fix PreviousGenerationInstanceType family string matching (#2558)
Browse files Browse the repository at this point in the history
* fix(rule) - better approach for regex validation

PreviousGenerationInstanceType rule is currently using a validation
regex that is incorrectly targetting instance types like 'mac1.metal'
due to 'c1' being found in 'mac1.' string

Split instance type to find out which section is the family and then
perform a full 'match' instead of a 'search' on it

Family section may vary in position depending on the property attribute

* test - update test case with new instance type

* fix(rule): improve matching rule

Switch back to a full regex search approach that accomodates properties
like AWS::Redshift::Cluster NodeType without having to add exceptions
to the rule if we ever want to validate those

* fix(rule): missing capture group in regex

Co-authored-by: Pat Myron <PatMyron@users.noreply.github.com>

Co-authored-by: Pat Myron <PatMyron@users.noreply.github.com>
Co-authored-by: Kevin DeJong <kddejong@amazon.com>
  • Loading branch information
3 people authored Jan 25, 2023
1 parent 31b622d commit edbceeb
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/cfnlint/rules/resources/PreviousGenerationInstanceType.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ def match(self, cfn):
if isinstance(
resource.get("Properties", {}).get(property_type, ""), str
):
if re.search(
r"([cmr][1-3]|cc2|cg1|cr1|g2|hi1|hs1|i2|t1)\.",
if self.__is_previous_generation_instance_type(
resource.get("Properties", {}).get(property_type, ""),
):
matches.append(
Expand Down Expand Up @@ -69,8 +68,7 @@ def match(self, cfn):
.get(property_type, ""),
str,
):
if re.search(
r"([cmr][1-3]|cc2|cg1|cr1|g2|hi1|hs1|i2|t1)\.",
if self.__is_previous_generation_instance_type(
resource.get("Properties", {})
.get(top_level_property_type, {})
.get(property_type, ""),
Expand All @@ -91,3 +89,11 @@ def match(self, cfn):
)
)
return matches

def __is_previous_generation_instance_type(self, instance_type):
return (
re.search(
r"(^|\.)([cmr][1-3]|cc2|cg1|cr1|g2|hi1|hs1|i2|t1)($|\.)", instance_type
)
is not None
)
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,7 @@ Resources:
Properties:
ElasticsearchClusterConfig:
InstanceType: m3.medium.elasticsearch
Host:
Type: AWS::EC2::Host
Properties:
InstanceType: mac1.metal

0 comments on commit edbceeb

Please sign in to comment.