From c87981c8d156e3690b8f5e39002df0025e3ee7f3 Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Thu, 2 Nov 2023 21:44:40 -0700 Subject: [PATCH] Code cleanup and improve testing (#2937) --- src/cfnlint/context/context.py | 1 - src/cfnlint/jsonschema/_validators_cfn.py | 29 +++++++------- src/cfnlint/rules/conditions/JsonSchema.py | 1 + src/cfnlint/rules/functions/RefInCondition.py | 39 ------------------- .../resources/properties/AvailabilityZone.py | 16 ++------ 5 files changed, 21 insertions(+), 65 deletions(-) delete mode 100644 src/cfnlint/rules/functions/RefInCondition.py diff --git a/src/cfnlint/context/context.py b/src/cfnlint/context/context.py index 7c9669fd74..06e445e499 100644 --- a/src/cfnlint/context/context.py +++ b/src/cfnlint/context/context.py @@ -77,7 +77,6 @@ class Context: ref_values: Dict[str, Any] = field(init=True, default_factory=dict) # Resolved value - resolved_value: bool = field(init=True, default=False) resolved_conditions: Mapping[str, bool] = field(init=True, default_factory=dict) transforms: Transforms = field(init=True, default_factory=lambda: Transforms([])) diff --git a/src/cfnlint/jsonschema/_validators_cfn.py b/src/cfnlint/jsonschema/_validators_cfn.py index b1669d98be..f4671f54c8 100644 --- a/src/cfnlint/jsonschema/_validators_cfn.py +++ b/src/cfnlint/jsonschema/_validators_cfn.py @@ -152,6 +152,21 @@ def iter_errors( validator.descend(value, self.schema(validator, instance), key) ) + def iter_errors_resolved( + self, + validator: Validator, + s: Any, + instance: Any, + schema: Any, + ) -> ValidationResult: + key = list(instance.keys())[0] + + for value in validator.resolve_value(instance): + for err in self._resolve_errors(validator.descend(value, s, key)): + err.message = err.message.replace(f"{value!r}", f"{instance!r}") + err.message = f"{err.message} when {self.fn.name!r} is resolved" + yield err + class FnArray: def __init__(self, name: str, min_length: int = 0, max_length: int = 0) -> None: @@ -233,7 +248,6 @@ def iter_errors( for err in self._resolve_errors( self.items[i].iter_errors(validator, s, value[i], schema) ): - err.validator = self.fn.py err.path.extendleft([i, key]) yield err except (IndexError, StopIteration): @@ -274,9 +288,6 @@ def ref( yield err return - validator = validator.evolve( - context=validator.context.evolve(resolved_value=True) - ) for value in validator.resolve_value(instance): if value is None: continue @@ -415,15 +426,7 @@ def get_azs( if errs: return - validator = validator.evolve( - context=validator.context.evolve(resolved_value=True) - ) - for value in validator.resolve_value(instance): - for err in validator.descend(value, s): - err.path.appendleft(self.fn.name) - err.message = err.message.replace(f"{value!r}", f"{instance!r}") - err.message = f"{err.message} when {self.fn.name!r} is resolved" - yield err + yield from self.iter_errors_resolved(validator, s, instance, schema) class FnImportValue(Fn): diff --git a/src/cfnlint/rules/conditions/JsonSchema.py b/src/cfnlint/rules/conditions/JsonSchema.py index 958b1f8b99..b6790a41d1 100644 --- a/src/cfnlint/rules/conditions/JsonSchema.py +++ b/src/cfnlint/rules/conditions/JsonSchema.py @@ -34,6 +34,7 @@ def __init__(self): "fn_and": "E8004", "fn_or": "E8006", "condition": "E8007", + "ref": "E1020", } self.child_rules = dict.fromkeys(list(self.rule_set.values())) self.schema = load_resource(schema_conditions, "conditions.json") diff --git a/src/cfnlint/rules/functions/RefInCondition.py b/src/cfnlint/rules/functions/RefInCondition.py deleted file mode 100644 index 582a7b4228..0000000000 --- a/src/cfnlint/rules/functions/RefInCondition.py +++ /dev/null @@ -1,39 +0,0 @@ -""" -Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -SPDX-License-Identifier: MIT-0 -""" -from cfnlint.rules import CloudFormationLintRule, RuleMatch - - -class RefInCondition(CloudFormationLintRule): - """Check if Ref value is a string""" - - id = "E1026" - shortdesc = "Cannot reference resources in the Conditions block of the template" - description = "Check that any Refs in the Conditions block uses no resources" - source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-conditions.html#w2ab2c21c28c21c45" - tags = ["conditions", "functions", "ref"] - - def match(self, cfn): - matches = [] - - ref_objs = cfn.search_deep_keys("Ref") - resource_names = cfn.get_resource_names() - - for ref_obj in ref_objs: - if ref_obj[0] == "Conditions": - value = ref_obj[-1] - if isinstance(value, (str, int)): - if value in resource_names: - message = ( - "Cannot reference resource {0} in the Conditions block of" - " the template at {1}" - ) - matches.append( - RuleMatch( - ref_obj[:-1], - message.format(value, "/".join(map(str, ref_obj[:-1]))), - ) - ) - - return matches diff --git a/src/cfnlint/rules/resources/properties/AvailabilityZone.py b/src/cfnlint/rules/resources/properties/AvailabilityZone.py index 3f4e153455..44d3e8c7b5 100644 --- a/src/cfnlint/rules/resources/properties/AvailabilityZone.py +++ b/src/cfnlint/rules/resources/properties/AvailabilityZone.py @@ -23,18 +23,14 @@ def __init__(self): # pylint: disable=unused-argument def availabilityzone(self, validator, aZ, zone, schema): - if validator.context.resolved_value: - return - if not validator.is_type(zone, "string"): return if zone in self.exceptions: return - if len(validator.context.path) > 0: - if validator.context.path[-1] in FUNCTIONS: - return + if any(fn in validator.context.path for fn in FUNCTIONS): + return yield ValidationError( f"Avoid hardcoding availability zones {zone!r}", @@ -43,15 +39,11 @@ def availabilityzone(self, validator, aZ, zone, schema): # pylint: disable=unused-argument def availabilityzones(self, validator, aZ, zones, schema): - if validator.context.resolved_value: - return - if not validator.is_type(zones, "array"): return - if len(validator.context.path) > 0: - if validator.context.path[-1] in FUNCTIONS: - return + if any(fn in validator.context.path for fn in FUNCTIONS): + return for zone in zones: yield from self.availabilityzone(validator, aZ, zone, schema)