Skip to content

Commit

Permalink
Remove redundant Fn::Length logic (#2985)
Browse files Browse the repository at this point in the history
  • Loading branch information
kddejong authored Dec 21, 2023
1 parent df45aea commit 04fd070
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 194 deletions.
13 changes: 8 additions & 5 deletions src/cfnlint/context/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,24 +182,26 @@ class Parameter(_Ref):
parameter: InitVar[Any]

def __post_init__(self, parameter) -> None:
self.default = None
self.allowed_values = []
self.min_value = None
self.max_value = None

t = parameter.get("Type")
if not isinstance(t, str):
raise ValueError("Type must be a string")
self.type = t

self.description = parameter.get("Description")

self.default = None
self.allowed_values = []
self.min_value = None
self.max_value = None
# SSM Parameter defaults and allowed values point to
# SSM paths not to the actual values
if self.type.startswith("AWS::SSM::Parameter::"):
return

if self.type == "CommaDelimitedList" or self.type.startswith("List<"):
self.default = parameter.get("Default", "").split(",")
if "Default" in parameter:
self.default = parameter.get("Default").split(",")
for allowed_value in parameter.get("AllowedValues", []):
self.allowed_values.append(allowed_value.split(","))
else:
Expand Down Expand Up @@ -307,6 +309,7 @@ def __post_init__(self, cfn: Any) -> None:
self.resources = {}
self.conditions = {}
self.mappings = {}

try:
self._init_parameters(cfn.template.get("Parameters", {}))
except (ValueError, AttributeError):
Expand Down
31 changes: 31 additions & 0 deletions src/cfnlint/jsonschema/_validators_cfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
REGEX_DYN_REF,
REGEX_SUB_PARAMETERS,
REGIONS,
VALID_PARAMETER_TYPES,
VALID_PARAMETER_TYPES_LIST,
ToPy,
)
from cfnlint.jsonschema import ValidationError, Validator
Expand Down Expand Up @@ -343,6 +345,33 @@ def _validator(self, validator: Validator) -> Validator:
),
)

def validate(
self, validator: Validator, s: Any, instance: Any, schema: Any
) -> ValidationResult:
yield from super().validate(validator, s, instance, schema)

_, value = self._key_value(instance)
if not validator.is_type(value, "string"):
return

if value not in validator.context.parameters:
return

parameter_type = validator.context.parameters[value].type
schema_types = ensure_list(s.get("type", ["string"]))
reprs = ", ".join(repr(type) for type in schema_types)

if all(
st not in ["string", "boolean", "integer", "number"] for st in schema_types
):
if parameter_type not in VALID_PARAMETER_TYPES_LIST:
yield ValidationError(f"{instance!r} is not of type {reprs}")
elif all(st not in ["array"] for st in schema_types):
if parameter_type not in [
x for x in VALID_PARAMETER_TYPES if x not in VALID_PARAMETER_TYPES_LIST
]:
yield ValidationError(f"{instance!r} is not of type {reprs}")


class FnGetAZs(_Fn):
def __init__(self) -> None:
Expand Down Expand Up @@ -803,8 +832,10 @@ def schema(self, validator, instance) -> Dict[str, Any]:
"Fn::FindInMap",
"Fn::Join",
"Fn::Select",
"Fn::Split",
"Fn::Sub",
"Fn::ToJsonString",
"Fn::GetAZs",
"Ref",
],
"schema": {
Expand Down
72 changes: 1 addition & 71 deletions src/cfnlint/rules/functions/Length.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
from cfnlint.languageExtensions import LanguageExtensions
from cfnlint.rules import CloudFormationLintRule, RuleMatch
from cfnlint.rules import CloudFormationLintRule


class Length(CloudFormationLintRule):
Expand All @@ -14,72 +13,3 @@ class Length(CloudFormationLintRule):
description = "Making sure Fn::Length is configured correctly"
source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-length.html"
tags = ["functions", "length"]

supported_functions = ["Fn::Split", "Fn::FindInMap"]

def match(self, cfn):
has_language_extensions_transform = cfn.has_language_extensions_transform()
intrinsic_function = "Fn::Length"
matches = []

fn_length_objects = cfn.search_deep_keys(intrinsic_function)
for fn_length_object in fn_length_objects:
tree = fn_length_object[:-1]
LanguageExtensions.validate_transform_is_declared(
self,
has_language_extensions_transform,
matches,
tree,
intrinsic_function,
)
self.validate_type(fn_length_object, matches, tree)
self.validate_ref(fn_length_object, matches, tree, cfn)
return matches

def validate_ref(self, fn_length_object, matches, tree, cfn):
fn_length_value = fn_length_object[-1]
if isinstance(fn_length_value, dict):
if len(fn_length_value.keys()) != 1 or (
list(fn_length_value.keys())[0]
not in ["Ref"] + self.supported_functions
):
self.addMatch(
matches,
tree,
(
"Fn::Length expects either an array, a Ref to an array or"
f" {', '.join(self.supported_functions)}, but found unexpected"
" object under Fn::Length at {0}"
),
)
return

if "Ref" in fn_length_value:
if fn_length_value["Ref"] not in cfn.get_parameter_names():
self.addMatch(
matches,
tree,
"Fn::Length can only reference list parameters at {0}",
)
else:
referenced_parameter = cfn.get_parameters().get(
fn_length_value["Ref"]
)
parameter_type = referenced_parameter.get("Type")
if "List" not in parameter_type:
self.addMatch(
matches,
tree,
"Fn::Length can only reference list parameters at {0}",
)

def addMatch(self, matches, tree, message):
matches.append(RuleMatch(tree[:], message.format("/".join(map(str, tree)))))

def validate_type(self, fn_length_object, matches, tree):
fn_length_value = fn_length_object[-1]
if not isinstance(fn_length_value, dict) and not isinstance(
fn_length_value, list
):
message = "Fn::Length needs a list or a reference to a list at {0}"
matches.append(RuleMatch(tree[:], message.format("/".join(map(str, tree)))))

This file was deleted.

44 changes: 0 additions & 44 deletions test/fixtures/templates/bad/functions/lengthWrongTypes.yaml

This file was deleted.

32 changes: 0 additions & 32 deletions test/fixtures/templates/good/functions/length.yaml

This file was deleted.

33 changes: 28 additions & 5 deletions test/unit/module/jsonschema/test_validators_cfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ def message_errors(name, instance, schema, errors, **kwargs):
schema=schema,
context=Context(
functions=FUNCTIONS,
parameters={
"MyParameter": Parameter({"Type": "String"}),
"MyArrayParameter": Parameter({"Type": "CommaDelimitedList"}),
},
resources={
"MyResource": Resource(
{
Expand Down Expand Up @@ -200,13 +204,13 @@ def message_transform_errors(name, instance, schema, errors, **kwargs):
parameters={
"MyParameter": Parameter(
{
"Type": "string",
"Type": "String",
"Default": "bar",
}
),
"MyResourceParameter": Parameter(
{
"Type": "string",
"Type": "String",
"Default": "MyResource",
}
),
Expand All @@ -227,14 +231,31 @@ def message_transform_errors(name, instance, schema, errors, **kwargs):
[
"['foo'] is not of type 'string'",
(
"['foo'] is not one of ['MyResource', 'AWS::NoValue', "
"['foo'] is not one of ['MyParameter', 'MyArrayParameter', "
"'MyResource', 'AWS::NoValue', "
"'AWS::AccountId', 'AWS::Partition', 'AWS::Region', "
"'AWS::StackId', 'AWS::StackName', 'AWS::URLSuffix', "
"'AWS::NotificationARNs']"
),
],
[],
),
(
"Invalid Ref with a type that doesn't match array schema",
{"Ref": "MyParameter"},
{"type": "array"},
["{'Ref': 'MyParameter'} is not of type 'array'"],
[],
),
(
"Invalid Ref with a type that doesn't match singular schema",
{"Ref": "MyArrayParameter"},
{"type": "string"},
[
"{'Ref': 'MyArrayParameter'} is not of type 'string'",
],
[],
),
]

# Fn::GetAtt
Expand Down Expand Up @@ -651,7 +672,8 @@ def message_transform_errors(name, instance, schema, errors, **kwargs):
{"type": "string"},
[
(
"'MyResource' is not one of ['AWS::NoValue', "
"'MyResource' is not one of ['MyParameter', "
"'MyArrayParameter', 'AWS::NoValue', "
"'AWS::AccountId', 'AWS::Partition', "
"'AWS::Region', 'AWS::StackId', 'AWS::StackName', "
"'AWS::URLSuffix', 'AWS::NotificationARNs']"
Expand Down Expand Up @@ -727,7 +749,8 @@ def message_transform_errors(name, instance, schema, errors, **kwargs):
{"type": "string"},
[
(
"'foo' is not one of ['MyResource', 'AWS::NoValue', 'AWS::AccountId', "
"'foo' is not one of ['MyParameter', 'MyArrayParameter', "
"'MyResource', 'AWS::NoValue', 'AWS::AccountId', "
"'AWS::Partition', 'AWS::Region', 'AWS::StackId', "
"'AWS::StackName', 'AWS::URLSuffix', "
"'AWS::NotificationARNs']"
Expand Down
29 changes: 0 additions & 29 deletions test/unit/rules/functions/test_length.py

This file was deleted.

0 comments on commit 04fd070

Please sign in to comment.