Skip to content

Commit

Permalink
Default fn validator context will be not strict type checking (#3386)
Browse files Browse the repository at this point in the history
* Default fn validator context will be not strict type checking

* Update getatt testing as well
  • Loading branch information
kddejong committed Jun 24, 2024
1 parent 3dfb0e4 commit 9bd2a20
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 11 deletions.
7 changes: 5 additions & 2 deletions src/cfnlint/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,9 @@ def _translate_types(types: Sequence[str]) -> list[str]:


def is_types_compatible(
source_types: str | Sequence[str], destination_types: str | Sequence[str]
source_types: str | Sequence[str],
destination_types: str | Sequence[str],
strict_types: bool = False,
) -> bool:
"""
Validate if desination types are compatible with source types.
Expand All @@ -576,7 +578,8 @@ def is_types_compatible(
Returns:
bool: If any type of source is compatible with any type in the destination
"""
source_types = _translate_types(ensure_list(source_types))
if not strict_types:
source_types = _translate_types(ensure_list(source_types))
destination_types = ensure_list(destination_types)

if any(schema_type in source_types for schema_type in destination_types):
Expand Down
8 changes: 6 additions & 2 deletions src/cfnlint/jsonschema/_keywords_cfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ def validate(
for (index, item), subschema in zip(enumerate(instance), s):
yield from validator.evolve(
context=validator.context.evolve(
functions=subschema.get("functions", [])
functions=subschema.get("functions", []),
strict_types=False,
),
).descend(
instance=item,
Expand All @@ -69,7 +70,10 @@ def validate(
else:
for index, item in enumerate(instance):
yield from validator.evolve(
context=validator.context.evolve(functions=s.get("functions", [])),
context=validator.context.evolve(
functions=s.get("functions", []),
strict_types=False,
),
).descend(
instance=item,
schema=s.get("schema", {}),
Expand Down
4 changes: 3 additions & 1 deletion src/cfnlint/rules/functions/GetAtt.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ def _resolve_getatt(

types = ensure_list(s.get("type"))

if is_types_compatible(types, schema_types):
if is_types_compatible(
types, schema_types, validator.context.strict_types
):
continue

reprs = ", ".join(repr(type) for type in types)
Expand Down
1 change: 1 addition & 0 deletions src/cfnlint/rules/functions/Sub.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def _validate_string(
path=validator.context.path.descend(
path=key,
),
strict_types=True,
),
function_filter=validator.function_filter.evolve(
add_cfn_lint_keyword=False,
Expand Down
17 changes: 16 additions & 1 deletion test/unit/module/jsonschema/test_keywords_cfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from typing import List

from cfnlint.context import Context
from cfnlint.jsonschema._keywords_cfn import cfn_type
from cfnlint.jsonschema._keywords_cfn import FnItems, cfn_type
from cfnlint.jsonschema.exceptions import UnknownType
from cfnlint.jsonschema.validators import CfnTemplateValidator

Expand Down Expand Up @@ -136,3 +136,18 @@ def test_cfn_type_failure(self):
self.assertIn(
"Unknown type 'bar' for validator with schema", str(err.exception)
)


class TestFnItems(Base):

def test_change_type(self):
schema = [
{
"schema": {"type": "string"},
},
]
validator = CfnTemplateValidator({})

items = list(FnItems().validate(validator, schema, [1], {}))

self.assertListEqual(items, [])
29 changes: 28 additions & 1 deletion test/unit/rules/functions/test_getatt.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,32 @@ def validate(self, validator, s, instance, schema):
"Valid GetAtt with integer to string",
{"Fn::GetAtt": "MyCodePipeline.Version"},
{"type": ["integer"]},
{},
{
"strict_types": False,
},
{},
[],
),
(
"Invalid GetAtt with integer to string",
{"Fn::GetAtt": "MyCodePipeline.Version"},
{"type": ["integer"]},
{
"strict_types": True,
},
{},
[
ValidationError(
(
"{'Fn::GetAtt': 'MyCodePipeline.Version'} "
"is not of type 'integer'"
),
path=deque(["Fn::GetAtt"]),
schema_path=deque(["type"]),
validator="fn_getatt",
)
],
),
(
"Valid GetAtt with one good response type",
{"Fn::GetAtt": "MyBucket.Arn"},
Expand Down Expand Up @@ -244,4 +266,9 @@ def test_validate(
rule.child_rules = child_rules
validator = CfnTemplateValidator({}, context=context, cfn=cfn)
errs = list(rule.fn_getatt(validator, schema, instance, {}))

for err in errs:
print(err.validator)
print(err.path)
print(err.schema_path)
assert errs == expected, f"Test {name!r} got {errs!r}"
23 changes: 19 additions & 4 deletions test/unit/rules/functions/test_sub.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def cfn():
"Resources": {
"MyResource": {"Type": "AWS::S3::Bucket"},
"MySimpleAd": {"Type": "AWS::DirectoryService::SimpleAD"},
"MyPolicy": {"Type": "AWS::IAM::ManagedPolicy"},
},
},
regions=["us-east-1"],
Expand Down Expand Up @@ -95,7 +96,7 @@ def context(cfn):
ValidationError(
(
"'bar' is not one of ["
"'MyResource', 'MySimpleAd', 'AWS::AccountId', "
"'MyResource', 'MySimpleAd', 'MyPolicy', 'AWS::AccountId', "
"'AWS::NoValue', 'AWS::NotificationARNs', "
"'AWS::Partition', 'AWS::Region', "
"'AWS::StackId', 'AWS::StackName', 'AWS::URLSuffix']"
Expand All @@ -114,7 +115,7 @@ def context(cfn):
ValidationError(
(
"'foo' is not one of ["
"'MyResource', 'MySimpleAd', 'AWS::AccountId', "
"'MyResource', 'MySimpleAd', 'MyPolicy', 'AWS::AccountId', "
"'AWS::NoValue', 'AWS::NotificationARNs', "
"'AWS::Partition', 'AWS::Region', "
"'AWS::StackId', 'AWS::StackName', 'AWS::URLSuffix']"
Expand Down Expand Up @@ -228,7 +229,7 @@ def context(cfn):
{"type": "string"},
[
ValidationError(
"'Foo' is not one of ['MyResource', 'MySimpleAd']",
"'Foo' is not one of ['MyResource', 'MySimpleAd', 'MyPolicy']",
path=deque(["Fn::Sub"]),
schema_path=deque([]),
validator="fn_sub",
Expand All @@ -241,7 +242,21 @@ def context(cfn):
{"type": "string"},
[
ValidationError(
("'MySimpleAd.DnsIpAddresses' is not " "of type 'string'"),
("'MySimpleAd.DnsIpAddresses' is not of type 'string'"),
instance="MySimpleAd.DnsIpAddresses",
path=deque(["Fn::Sub"]),
schema_path=deque([]),
validator="fn_sub",
),
],
),
(
"Invalid Fn::Sub with a GetAtt to an integer",
{"Fn::Sub": "${MyPolicy.AttachmentCount}"},
{"type": "string"},
[
ValidationError(
("'MyPolicy.AttachmentCount' is not of type 'string'"),
instance="MySimpleAd.DnsIpAddresses",
path=deque(["Fn::Sub"]),
schema_path=deque([]),
Expand Down

0 comments on commit 9bd2a20

Please sign in to comment.