Skip to content

Commit

Permalink
V1 add ability for child rule to claim who their parent is (#3231)
Browse files Browse the repository at this point in the history
* Add parent relationships to rules
* Switch all CfnLint keyword rules to use parent mechanism
  • Loading branch information
kddejong authored May 10, 2024
1 parent 6f2fd82 commit bb13337
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 52 deletions.
55 changes: 26 additions & 29 deletions src/cfnlint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ def __init__(self) -> None:
self.config: Dict[str, Any] = {} # `-X E3012:strict=false`... Show more
self.config_definition: Dict[str, Any] = {}
self._child_rules: Dict[str, "CloudFormationLintRule"] = {}
# Parent IDs to do the opposite of child rules
self._parent_rules: List[str] = []
super().__init__()

def __repr__(self):
Expand All @@ -210,6 +212,14 @@ def child_rules(self):
def child_rules(self, rules: Dict[str, "CloudFormationLintRule"]):
self._child_rules = rules

@property
def parent_rules(self):
return self._parent_rules

@parent_rules.setter
def parent_rules(self, rules: List[str]):
self._parent_rules = rules

@property
def severity(self):
"""Severity level"""
Expand Down Expand Up @@ -297,13 +307,13 @@ def matchall_resource_properties(

class Rules(TypedRules):
def __init__(
self, dict: Dict[str, CloudFormationLintRule] | None = None, /, **kwargs
self, rules: dict[str, CloudFormationLintRule] | None = None, /, **kwargs
):
super().__init__()
self.data: Dict[str, CloudFormationLintRule] = {}
self._used_rules: Dict[str, CloudFormationLintRule] = {}
if dict is not None:
self.update(dict)
if rules is not None:
self.update(rules)
if kwargs:
self.update(kwargs)

Expand Down Expand Up @@ -346,23 +356,6 @@ def is_rule_enabled(
return True
return False

def runable_rules(self, config: ConfigMixIn) -> Iterator[CloudFormationLintRule]:
for rule in self.data.values():
if rule.is_enabled(
config.include_experimental,
ignore_rules=config.ignore_checks,
include_rules=config.include_checks,
mandatory_rules=config.mandatory_checks,
):
self._used_rules[rule.id] = rule
# rules that have children need to be run
# we will remove the results later
if self.is_rule_enabled(rule.id, config):
yield rule
continue
if rule.child_rules:
yield rule

def extend(self, rules: List[CloudFormationLintRule]):
for rule in rules:
self.register(rule)
Expand All @@ -374,6 +367,8 @@ def used_rules(self) -> Dict[str, CloudFormationLintRule]:
# pylint: disable=inconsistent-return-statements
def run_check(self, check, filename, rule_id, config, *args) -> Iterator[Match]:
"""Run a check"""
if self.is_rule_enabled(rule_id, config):
self._used_rules[rule_id] = self.data[rule_id]
try:
yield from iter(check(*args))
except Exception as err: # pylint: disable=W0703
Expand Down Expand Up @@ -401,37 +396,39 @@ def run(
self, filename: Optional[str], cfn: Template, config: ConfigMixIn
) -> Iterator[Match]:
"""Run rules"""
runable_rules = list(self.runable_rules(config))
for rule in runable_rules:
for rule_id, rule in self.data.items():
rule.configure(
config.configure_rules.get(rule.id, None), config.include_experimental
config.configure_rules.get(rule_id, None), config.include_experimental
)
rule.initialize(cfn)

for rule in runable_rules:
for rule_id, rule in self.data.items():
for key in rule.child_rules.keys():
if not any(key == r.id for r in runable_rules):
if not any(key == r for r in self.data.keys()):
continue
rule.child_rules[key] = self.data.get(key)
for parent_rule in rule.parent_rules:
if parent_rule in self.data:
self.data[parent_rule].child_rules[rule_id] = rule

for rule in runable_rules:
for rule_id, rule in self.data.items():
yield from self._filter_matches(
config,
self.run_check(rule.matchall, filename, rule.id, config, filename, cfn),
self.run_check(rule.matchall, filename, rule_id, config, filename, cfn),
)

for resource_name, resource_attributes in cfn.get_resources().items():
resource_type = resource_attributes.get("Type")
resource_properties = resource_attributes.get("Properties")
if isinstance(resource_type, str) and isinstance(resource_properties, dict):
path = ["Resources", resource_name, "Properties"]
for rule in self.runable_rules(config):
for rule_id, rule in self.data.items():
yield from self._filter_matches(
config,
self.run_check(
rule.matchall_resource_properties,
filename,
rule.id,
rule_id,
config,
filename,
cfn,
Expand Down
22 changes: 0 additions & 22 deletions src/cfnlint/rules/jsonschema/CfnLint.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
SPDX-License-Identifier: MIT-0
"""

import pathlib

from cfnlint.helpers import load_plugins
from cfnlint.rules import CloudFormationLintRule


Expand All @@ -15,25 +12,6 @@ class CfnLint(CloudFormationLintRule):
description = "Use supplemental logic to validate properties against"
tags = []

def __init__(self) -> None:
super().__init__()
# relative path to the parent of cfnlint.rules
root_dir = pathlib.Path(__file__).parent.parent
rules = load_plugins(
str(root_dir),
"CfnLintKeyword",
"cfnlint.rules.jsonschema.CfnLintKeyword",
)
rules.extend(
load_plugins(
str(root_dir),
"CfnLintJsonSchema",
"cfnlint.rules.jsonschema.CfnLintJsonSchema",
)
)
for rule in rules:
self.child_rules[rule.id] = None

# pylint: disable=unused-argument
def cfnLint(self, validator, keywords, instance, schema):
validator = validator.evolve(
Expand Down
1 change: 1 addition & 0 deletions src/cfnlint/rules/jsonschema/CfnLintJsonSchema.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def __init__(
) -> None:
super().__init__()
self.keywords = keywords or []
self.parent_rules = ["E1101"]
self.all_matches = all_matches
self._use_schema_arg = True
self._schema: Any = {}
Expand Down
1 change: 1 addition & 0 deletions src/cfnlint/rules/jsonschema/CfnLintKeyword.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class CfnLintKeyword(CloudFormationLintRule):
def __init__(self, keywords: Sequence[str] | None = None) -> None:
super().__init__()
self.keywords = keywords or []
self.parent_rules = ["E1101"]

def message(self, instance: Any, err: ValidationError) -> str:
return self.shortdesc
5 changes: 5 additions & 0 deletions src/cfnlint/rules/resources/properties/Tagging.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ def tagging(self, validator: Validator, t: Any, instance: Any, schema: Any):
if not t.get("taggable"):
return

validator = validator.evolve(
function_filter=validator.function_filter.evolve(
add_cfn_lint_keyword=False,
)
)
for err in validator.descend(
instance,
self._schema,
Expand Down
2 changes: 1 addition & 1 deletion src/cfnlint/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def _validate_filenames(self, filenames: Sequence[str | None]) -> Iterator[Match
matches = [match for match in matches if match.rule.id != "E0000"]
if matches:
yield from iter(matches)
return
return
else:
yield from iter(matches)
return
Expand Down

0 comments on commit bb13337

Please sign in to comment.