From 155e3f4243e7cea0ee10b8151bd3c90241ec33fa Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Mon, 13 Feb 2023 16:12:27 -0800 Subject: [PATCH] Increase coverage and update rules --- .../resource/configuration.json | 2 +- .../all/aws-autoscaling-autoscalinggroup.json | 10 + .../all/aws-dax-cluster.json | 12 ++ .../all/aws-dms-replicationinstance.json | 7 + .../all/aws-ec2-host.json | 7 + .../all/aws-ec2-instance.json | 5 + .../all/aws-ec2-launchtemplate.json | 5 + .../all/aws-ec2-spotfleet.json | 10 + .../all/aws-ec2-subnet.json | 7 + .../all/aws-ec2-volume.json | 7 + ...aws-elasticloadbalancing-loadbalancer.json | 12 ++ ...ws-elasticloadbalancingv2-targetgroup.json | 7 + .../all/aws-emr-cluster.json | 7 + .../all/aws-glue-connection.json | 7 + .../all/aws-opsworks-instance.json | 7 +- .../all/aws-rds-dbcluster.json | 10 + .../all/aws-rds-dbinstance.json | 5 + src/cfnlint/helpers.py | 59 ++---- src/cfnlint/jsonschema/_utils.py | 8 + src/cfnlint/jsonschema/_validators.py | 2 +- src/cfnlint/rules/__init__.py | 14 -- src/cfnlint/rules/functions/GetAtt.py | 13 +- src/cfnlint/rules/resources/Configuration.py | 4 +- .../resources/properties/AllowedValue.py | 16 +- .../resources/properties/AvailabilityZone.py | 102 ++-------- .../rules/resources/properties/AwsType.py | 41 +++++ .../resources/properties/ListDuplicates.py | 17 +- .../rules/resources/properties/Properties.py | 8 +- .../properties/ValuePrimitiveType.py | 28 ++- .../resources/properties/ValueRefGetAtt.py | 26 --- src/cfnlint/template/template.py | 2 +- .../bad/functions/dynamic_reference.yaml | 7 + .../fixtures/templates/bad/properties_az.yaml | 38 ---- .../good/resources/properties/az.yaml | 24 --- test/integration/test_patches.py | 27 +++ .../module/helpers/test_get_url_retrieve.py | 54 ++++++ test/unit/module/helpers/test_regex_dict.py | 9 + test/unit/module/rule/test_matching.py | 17 +- .../rules/functions/test_dynamic_reference.py | 2 +- .../rules/parameters/test_allowed_value.py | 6 + .../properties/test_allowed_pattern.py | 26 ++- .../properties/test_allowed_value.py | 38 +++- .../properties/test_availability_zone.py | 72 ++++++-- .../resources/properties/test_aws_type.py | 36 ++++ .../properties/test_list_duplicates.py | 3 + .../resources/properties/test_properties.py | 108 +++++++++++ .../resources/properties/test_string_size.py | 13 +- .../properties/test_value_primitive_type.py | 174 +++++++++++++++++- 48 files changed, 819 insertions(+), 302 deletions(-) create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-dax-cluster.json create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-dms-replicationinstance.json create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-host.json create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-subnet.json create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-volume.json create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-elasticloadbalancing-loadbalancer.json create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-elasticloadbalancingv2-targetgroup.json create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-emr-cluster.json create mode 100644 src/cfnlint/data/ExtendedProviderSchema/all/aws-glue-connection.json create mode 100644 src/cfnlint/rules/resources/properties/AwsType.py delete mode 100644 src/cfnlint/rules/resources/properties/ValueRefGetAtt.py delete mode 100644 test/fixtures/templates/bad/properties_az.yaml delete mode 100644 test/fixtures/templates/good/resources/properties/az.yaml create mode 100644 test/integration/test_patches.py create mode 100644 test/unit/module/helpers/test_get_url_retrieve.py create mode 100644 test/unit/rules/resources/properties/test_aws_type.py diff --git a/src/cfnlint/data/AdditionalSchemas/resource/configuration.json b/src/cfnlint/data/AdditionalSchemas/resource/configuration.json index 754e331634..2eca2e2d3e 100644 --- a/src/cfnlint/data/AdditionalSchemas/resource/configuration.json +++ b/src/cfnlint/data/AdditionalSchemas/resource/configuration.json @@ -28,7 +28,7 @@ }, "Type": { "type": "string", - "cfnType": true + "awsType": true }, "UpdateReplacePolicy": { "type": "string", diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-autoscaling-autoscalinggroup.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-autoscaling-autoscalinggroup.json index f04a36bf14..e48e96dc89 100644 --- a/src/cfnlint/data/ExtendedProviderSchema/all/aws-autoscaling-autoscalinggroup.json +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-autoscaling-autoscalinggroup.json @@ -6,6 +6,16 @@ "aws-autoscaling-autoscalinggroup/onlyone" ] }, + { + "op": "add", + "path": "/properties/AvailabilityZones/awsType", + "value": "AvailabilityZones" + }, + { + "op": "add", + "path": "/properties/AvailabilityZones/items/awsType", + "value": "AvailabilityZone" + }, { "op": "add", "path": "/definitions/LaunchTemplateSpecification/cfnSchema", diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-dax-cluster.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-dax-cluster.json new file mode 100644 index 0000000000..294137079d --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-dax-cluster.json @@ -0,0 +1,12 @@ +[ + { + "op": "add", + "path": "/properties/AvailabilityZones/awsType", + "value": "AvailabilityZones" + }, + { + "op": "add", + "path": "/properties/AvailabilityZones/items/awsType", + "value": "AvailabilityZone" + } +] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-dms-replicationinstance.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-dms-replicationinstance.json new file mode 100644 index 0000000000..1d8a066246 --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-dms-replicationinstance.json @@ -0,0 +1,7 @@ +[ + { + "op": "add", + "path": "/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" + } +] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-host.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-host.json new file mode 100644 index 0000000000..1d8a066246 --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-host.json @@ -0,0 +1,7 @@ +[ + { + "op": "add", + "path": "/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" + } +] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-instance.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-instance.json index f1861583f7..e40ad3a86f 100644 --- a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-instance.json +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-instance.json @@ -14,5 +14,10 @@ "aws-ec2-instance/blockdevicemapping-virtualname-exclusive", "aws-ec2-instance/blockdevicemapping-onlyone" ] + }, + { + "op": "add", + "path": "/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" } ] diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-launchtemplate.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-launchtemplate.json index ddb12564b5..6bf3d24db3 100644 --- a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-launchtemplate.json +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-launchtemplate.json @@ -3,5 +3,10 @@ "op": "add", "path": "/definitions/BlockDeviceMapping/cfnSchema", "value": ["aws-ec2-launchtemplate/blockdevicemapping-virtualname"] + }, + { + "op": "add", + "path": "/definitions/Placement/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" } ] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-spotfleet.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-spotfleet.json index ad40d30987..4dcfed63a5 100644 --- a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-spotfleet.json +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-spotfleet.json @@ -11,5 +11,15 @@ "op": "add", "path": "/definitions/SpotFleetRequestConfigData/cfnSchema", "value": ["aws-ec2-spotfleet/spotfleetrequestconfigdata-onlyone"] + }, + { + "op": "add", + "path": "/definitions/LaunchTemplateOverrides/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" + }, + { + "op": "add", + "path": "/definitions/SpotPlacement/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" } ] diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-subnet.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-subnet.json new file mode 100644 index 0000000000..1d8a066246 --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-subnet.json @@ -0,0 +1,7 @@ +[ + { + "op": "add", + "path": "/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" + } +] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-volume.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-volume.json new file mode 100644 index 0000000000..9a91fba021 --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-ec2-volume.json @@ -0,0 +1,7 @@ +[ + { + "op": "add", + "path": "/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" + } +] diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-elasticloadbalancing-loadbalancer.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-elasticloadbalancing-loadbalancer.json new file mode 100644 index 0000000000..294137079d --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-elasticloadbalancing-loadbalancer.json @@ -0,0 +1,12 @@ +[ + { + "op": "add", + "path": "/properties/AvailabilityZones/awsType", + "value": "AvailabilityZones" + }, + { + "op": "add", + "path": "/properties/AvailabilityZones/items/awsType", + "value": "AvailabilityZone" + } +] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-elasticloadbalancingv2-targetgroup.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-elasticloadbalancingv2-targetgroup.json new file mode 100644 index 0000000000..7aaa29e3b7 --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-elasticloadbalancingv2-targetgroup.json @@ -0,0 +1,7 @@ +[ + { + "op": "add", + "path": "/definitions/TargetDescription/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" + } +] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-emr-cluster.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-emr-cluster.json new file mode 100644 index 0000000000..eeaafc6287 --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-emr-cluster.json @@ -0,0 +1,7 @@ +[ + { + "op": "add", + "path": "/definitions/PlacementType/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" + } +] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-glue-connection.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-glue-connection.json new file mode 100644 index 0000000000..d9fc3f80dc --- /dev/null +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-glue-connection.json @@ -0,0 +1,7 @@ +[ + { + "op": "add", + "path": "/definitions/PhysicalConnectionRequirements/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" + } +] diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-opsworks-instance.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-opsworks-instance.json index e2d09b8764..9fe4828edf 100644 --- a/src/cfnlint/data/ExtendedProviderSchema/all/aws-opsworks-instance.json +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-opsworks-instance.json @@ -6,5 +6,10 @@ "aws-opsworks-instance/blockdevicemapping-virtualname", "aws-opsworks-instance/blockdevicemapping-onlyone" ] + }, + { + "op": "add", + "path": "/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" } -] \ No newline at end of file +] diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-rds-dbcluster.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-rds-dbcluster.json index b83b113549..73ab2aa0af 100644 --- a/src/cfnlint/data/ExtendedProviderSchema/all/aws-rds-dbcluster.json +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-rds-dbcluster.json @@ -7,5 +7,15 @@ "aws-rds-dbcluster/snapshotidentifier-exclusive", "aws-rds-dbcluster/serverless-exclusive" ] + }, + { + "op": "add", + "path": "/properties/AvailabilityZones/awsType", + "value": "AvailabilityZones" + }, + { + "op": "add", + "path": "/properties/AvailabilityZones/items/awsType", + "value": "AvailabilityZone" } ] diff --git a/src/cfnlint/data/ExtendedProviderSchema/all/aws-rds-dbinstance.json b/src/cfnlint/data/ExtendedProviderSchema/all/aws-rds-dbinstance.json index 055d09b1c4..abe21bcd23 100644 --- a/src/cfnlint/data/ExtendedProviderSchema/all/aws-rds-dbinstance.json +++ b/src/cfnlint/data/ExtendedProviderSchema/all/aws-rds-dbinstance.json @@ -6,5 +6,10 @@ "aws-rds-dbinstance/sourcedbinstanceidentifier-exclusive", "aws-rds-dbinstance/aurora-exclusive" ] + }, + { + "op": "add", + "path": "/properties/AvailabilityZone/awsType", + "value": "AvailabilityZone" } ] diff --git a/src/cfnlint/helpers.py b/src/cfnlint/helpers.py index 2675b6c817..988d346951 100644 --- a/src/cfnlint/helpers.py +++ b/src/cfnlint/helpers.py @@ -17,11 +17,9 @@ import re import sys from io import BytesIO -from typing import Dict, List +from typing import List from urllib.request import Request, urlopen, urlretrieve -import jsonpatch - LOGGER = logging.getLogger(__name__) SPEC_REGIONS = { @@ -110,10 +108,8 @@ FUNCTION_NOT = "Fn::Not" FUNCTION_EQUALS = "Fn::Equals" -PSEUDOPARAMS = [ +PSEUDOPARAMS_SINGLE = [ "AWS::AccountId", - "AWS::NotificationARNs", - "AWS::NoValue", "AWS::Partition", "AWS::Region", "AWS::StackId", @@ -121,6 +117,12 @@ "AWS::URLSuffix", ] +PSEUDOPARAMS_MULTIPLE = [ + "AWS::NotificationARNs", +] + +PSEUDOPARAMS = ["AWS::NoValue"] + PSEUDOPARAMS_SINGLE + PSEUDOPARAMS_MULTIPLE + LIMITS = { "Mappings": {"number": 200, "attributes": 200, "name": 255}, # in characters "Outputs": { @@ -220,15 +222,10 @@ def __getitem__(self, item): return possible_items[longest_match] def __contains__(self, item): - for k, v in self.items(): - if isinstance(v, dict): - if v.get("Type") == "MODULE": - if re.match(k, item): - return True - else: - if k == item: - return True - elif re.match(k, item): + if isinstance(item, (dict, list)): + return False + for k, _ in self.items(): + if re.fullmatch(k, item): return True return False @@ -328,7 +325,6 @@ def get_url_retrieve(url: str, caching: bool = False) -> str: """ if caching: - # Need to wrap this in a try, as URLLib2 in Python2 doesn't support HEAD requests req = Request(url, method="HEAD") with urlopen(req) as res: if res.info().get("ETag"): @@ -380,19 +376,6 @@ def load_resource(package, filename="us-east-1.json"): REGISTRY_SCHEMAS: List[dict] = [] -def merge_spec(source, destination): - """Recursive merge spec dict""" - - for key, value in source.items(): - if isinstance(value, dict): - node = destination.setdefault(key, {}) - merge_spec(value, node) - else: - destination[key] = value - - return destination - - def is_custom_resource(resource_type): """Return True if resource_type is a custom resource""" return resource_type and ( @@ -486,21 +469,3 @@ def onerror(os_error): result.extend(create_rules(mod)) return result - - -def apply_json_patch(data: Dict, patches: List[Dict], region: str) -> Dict: - # Process the generic patches 1 by 1 so we can "ignore" failed ones - for patch in patches: - try: - jsonpatch.JsonPatch([patch]).apply(data, in_place=True) - except jsonpatch.JsonPatchConflict: - LOGGER.debug("Patch (%s) not applied in region %s", patch, region) - except jsonpatch.JsonPointerException: - # Debug as the parent element isn't supported in the region - LOGGER.debug( - "Parent element not found for patch (%s) in region %s", - patches, - region, - ) - - return data diff --git a/src/cfnlint/jsonschema/_utils.py b/src/cfnlint/jsonschema/_utils.py index ce460f8e32..d670b538c8 100644 --- a/src/cfnlint/jsonschema/_utils.py +++ b/src/cfnlint/jsonschema/_utils.py @@ -14,3 +14,11 @@ class Unset: def __repr__(self): return "" + + +def unbool(element, true=object(), false=object()): + if element is True: + return true + if element is False: + return false + return element diff --git a/src/cfnlint/jsonschema/_validators.py b/src/cfnlint/jsonschema/_validators.py index 49605639cf..fca6383e8e 100644 --- a/src/cfnlint/jsonschema/_validators.py +++ b/src/cfnlint/jsonschema/_validators.py @@ -15,10 +15,10 @@ equal, extras_msg, find_additional_properties, - unbool, uniq, ) +from cfnlint.jsonschema._utils import unbool from cfnlint.jsonschema.exceptions import ValidationError diff --git a/src/cfnlint/rules/__init__.py b/src/cfnlint/rules/__init__.py index 6104c471b2..461720f141 100644 --- a/src/cfnlint/rules/__init__.py +++ b/src/cfnlint/rules/__init__.py @@ -54,9 +54,6 @@ def wrapper(self, filename: str, cfn: Template, *args, **kwargs): if match_type == "match_resource_properties": if args[1] not in self.resource_property_types: return [] - elif match_type == "match_resource_sub_properties": - if args[1] not in self.resource_sub_property_types: - return [] start = datetime.now() LOGGER.debug("Starting match function for rule %s at %s", self.id, start) @@ -220,7 +217,6 @@ def configure(self, configs=None, experimental=False): match: Callable[[Template], List[RuleMatch]] = None # type: ignore match_resource_properties: Callable[[Dict, str, List[str], Template], List[RuleMatch]] = None # type: ignore - match_resource_sub_properties: Callable[[Dict, str, List[str], Template], List[RuleMatch]] = None # type: ignore @matching("match") # pylint: disable=W0613 @@ -238,16 +234,6 @@ def matchall_resource_properties( resource_properties, property_type, path, cfn ) - @matching("match_resource_sub_properties") - # pylint: disable=W0613 - def matchall_resource_sub_properties( - self, filename, cfn, resource_properties, property_type, path - ): - """Check for resource properties type""" - return self.match_resource_sub_properties( # pylint: disable=E1102 - resource_properties, property_type, path, cfn - ) - # pylint: disable=too-many-instance-attributes class RulesCollection: diff --git a/src/cfnlint/rules/functions/GetAtt.py b/src/cfnlint/rules/functions/GetAtt.py index 8a5e02d839..3e6b41231a 100644 --- a/src/cfnlint/rules/functions/GetAtt.py +++ b/src/cfnlint/rules/functions/GetAtt.py @@ -10,7 +10,7 @@ from jsonschema.exceptions import best_match from jsonschema.validators import extend -from cfnlint.jsonschema import ValidationError +from cfnlint.jsonschema import ValidationError, _utils from cfnlint.rules import CloudFormationLintRule, RuleMatch from cfnlint.template import Template @@ -26,19 +26,12 @@ class GetAtt(CloudFormationLintRule): source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-getatt.html" tags = ["functions", "getatt"] - def _unbool(self, element, true=object(), false=object()): - if element is True: - return true - if element is False: - return false - return element - # pylint: disable=unused-argument def _enum(self, validator, enums, instance, schema): enums.sort() if instance in (0, 1): - unbooled = self._unbool(instance) - if all(unbooled != self._unbool(each) for each in enums): + unbooled = _utils.unbool(instance) + if all(unbooled != _utils.unbool(each) for each in enums): yield ValidationError(f"{instance!r} is not one of {enums!r}") elif instance not in enums: if validator.is_type(instance, "string"): diff --git a/src/cfnlint/rules/resources/Configuration.py b/src/cfnlint/rules/resources/Configuration.py index f31416d8f9..20a4624968 100644 --- a/src/cfnlint/rules/resources/Configuration.py +++ b/src/cfnlint/rules/resources/Configuration.py @@ -27,7 +27,7 @@ def __init__(self): self.regions = [] self.validator = create_validator( validators={ - "cfnType": self._cfnType, + "awsType": self._awsType, }, cfn=None, rules=None, @@ -38,7 +38,7 @@ def initialize(self, cfn): self.regions = cfn.regions # pylint: disable=unused-argument - def _cfnType(self, validator, iT, instance, schema): + def _awsType(self, validator, iT, instance, schema): if not validator.is_type(instance, "string"): return for region in self.regions: diff --git a/src/cfnlint/rules/resources/properties/AllowedValue.py b/src/cfnlint/rules/resources/properties/AllowedValue.py index c9b462af81..657c5828c3 100644 --- a/src/cfnlint/rules/resources/properties/AllowedValue.py +++ b/src/cfnlint/rules/resources/properties/AllowedValue.py @@ -2,7 +2,7 @@ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 """ -from cfnlint.jsonschema import ValidationError +from cfnlint.jsonschema import ValidationError, _utils from cfnlint.rules import CloudFormationLintRule @@ -18,24 +18,18 @@ class AllowedValue(CloudFormationLintRule): "W2030": None, } - def _unbool(self, element, true=object(), false=object()): - if element is True: - return true - if element is False: - return false - return element - # pylint: disable=unused-argument def enum(self, validator, enums, instance, schema): if isinstance(instance, dict): if len(instance) == 1: for k, v in instance.items(): if k == "Ref": - yield from self.child_rules["W2030"].validate(v, enums) + if self.child_rules.get("W2030"): + yield from self.child_rules["W2030"].validate(v, enums) return if instance in (0, 1): - unbooled = self._unbool(instance) - if all(unbooled != self._unbool(each) for each in enums): + unbooled = _utils.unbool(instance) + if all(unbooled != _utils.unbool(each) for each in enums): yield ValidationError(f"{instance!r} is not one of {enums!r}") elif instance not in enums: yield ValidationError(f"{instance!r} is not one of {enums!r}") diff --git a/src/cfnlint/rules/resources/properties/AvailabilityZone.py b/src/cfnlint/rules/resources/properties/AvailabilityZone.py index 9a02bc6890..3f6ed40640 100644 --- a/src/cfnlint/rules/resources/properties/AvailabilityZone.py +++ b/src/cfnlint/rules/resources/properties/AvailabilityZone.py @@ -2,14 +2,15 @@ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 """ -from cfnlint.rules import CloudFormationLintRule, RuleMatch +from cfnlint.jsonschema import ValidationError +from cfnlint.rules import CloudFormationLintRule class AvailabilityZone(CloudFormationLintRule): """Check Availibility Zone parameter checks""" id = "W3010" - shortdesc = "Availability Zone Parameters should not be hardcoded" + shortdesc = "Availability zone properties should not be hardcoded" description = "Check if an Availability Zone property is hardcoded." source_url = "https://github.com/aws-cloudformation/cfn-python-lint" tags = ["parameters", "availabilityzone"] @@ -17,90 +18,25 @@ class AvailabilityZone(CloudFormationLintRule): def __init__(self): """Init""" super().__init__() - resource_type_specs = [ - "AWS::AutoScaling::AutoScalingGroup", - "AWS::DAX::Cluster", - "AWS::DMS::ReplicationInstance", - "AWS::EC2::Host", - "AWS::EC2::Instance", - "AWS::EC2::Subnet", - "AWS::EC2::Volume", - "AWS::ElasticLoadBalancing::LoadBalancer", - "AWS::OpsWorks::Instance", - "AWS::RDS::DBCluster", - "AWS::RDS::DBInstance", - ] + self.exceptions = ["all"] - property_type_specs = [ - # Singular - "AWS::EC2::LaunchTemplate.Placement", - "AWS::EC2::SpotFleet.LaunchTemplateOverrides", - "AWS::EC2::SpotFleet.SpotPlacement", - "AWS::EMR::Cluster.PlacementType", - "AWS::ElasticLoadBalancingV2::TargetGroup.TargetDescription", - "AWS::Glue::Connection.PhysicalConnectionRequirements", - ] + # pylint: disable=unused-argument + def availabilityzone(self, validator, aZ, zone, schema): + if not validator.is_type(zone, "string"): + return - for resource_type_spec in resource_type_specs: - self.resource_property_types.append(resource_type_spec) - for property_type_spec in property_type_specs: - self.resource_sub_property_types.append(property_type_spec) + if zone in self.exceptions: + return - # pylint: disable=W0613 - def check_az_value(self, value, path): - matches = [] - - # value of `all` is a valide exception in AWS::ElasticLoadBalancingV2::TargetGroup - if value not in ["all"]: - if path[-1] != ["Fn::GetAZs"]: - message = "Don't hardcode {0} for AvailabilityZones" - matches.append(RuleMatch(path, message.format(value))) - - return matches - - def check(self, properties, resource_type, path, cfn): - """Check itself""" - matches = [] - - matches.extend( - cfn.check_value( - properties, - "AvailabilityZone", - path, - check_value=self.check_az_value, - check_ref=None, - check_find_in_map=None, - check_split=None, - check_join=None, - ) - ) - matches.extend( - cfn.check_value( - properties, - "AvailabilityZones", - path, - check_value=self.check_az_value, - check_ref=None, - check_find_in_map=None, - check_split=None, - check_join=None, - ) + yield ValidationError( + f"Avoid hardcoding availability zones '{zone}'", + rule=self, ) - return matches - - def match_resource_sub_properties(self, properties, property_type, path, cfn): - """Match for sub properties""" - matches = [] - - matches.extend(self.check(properties, property_type, path, cfn)) - - return matches - - def match_resource_properties(self, properties, resource_type, path, cfn): - """Check CloudFormation Properties""" - matches = [] - - matches.extend(self.check(properties, resource_type, path, cfn)) + # pylint: disable=unused-argument + def availabilityzones(self, validator, aZ, zones, schema): + if not validator.is_type(zones, "array"): + return - return matches + for zone in zones: + yield from self.availabilityzone(validator, aZ, zone, schema) diff --git a/src/cfnlint/rules/resources/properties/AwsType.py b/src/cfnlint/rules/resources/properties/AwsType.py new file mode 100644 index 0000000000..78b5aa1b87 --- /dev/null +++ b/src/cfnlint/rules/resources/properties/AwsType.py @@ -0,0 +1,41 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +from cfnlint.rules import CloudFormationLintRule + + +class AwsType(CloudFormationLintRule): + """Check if Resource Properties are correct""" + + id = "E3008" + shortdesc = "Parent rule to validate values against AWS types" + description = "Checks the types of resource property values" + tags = ["resources", "ref", "getatt"] + + def __init__(self): + super().__init__() + self.cfn = None + + self.child_rules = { + "W3010": None, + } + + self.types = { + "AvailabilityZone": "W3010", + "AvailabilityZones": "W3010", + } + + def initialize(self, cfn): + self.cfn = cfn + return super().initialize(cfn) + + # pylint: disable=unused-argument + def awsType(self, validator, uI, instance, schema): + rule = self.child_rules.get(self.types.get(uI, "")) + if not rule: + return + + if hasattr(rule, uI.lower()) and callable(getattr(rule, uI.lower())): + validate = getattr(rule, uI.lower()) + yield from validate(validator, uI, instance, schema) diff --git a/src/cfnlint/rules/resources/properties/ListDuplicates.py b/src/cfnlint/rules/resources/properties/ListDuplicates.py index 77946d0577..b8f9ef7b27 100644 --- a/src/cfnlint/rules/resources/properties/ListDuplicates.py +++ b/src/cfnlint/rules/resources/properties/ListDuplicates.py @@ -5,7 +5,7 @@ import itertools from typing import Mapping, Sequence -from cfnlint.jsonschema import ValidationError +from cfnlint.jsonschema import ValidationError, _utils from cfnlint.rules import CloudFormationLintRule @@ -24,13 +24,6 @@ class ListDuplicates(CloudFormationLintRule): "I3037": None, } - def _unbool(self, element, true=object(), false=object()): - if element is True: - return true - if element is False: - return false - return element - def _mapping_equal(self, one, two): if len(one) != len(two): return False @@ -50,11 +43,11 @@ def _equal(self, one, two): return self._sequence_equal(one, two) if isinstance(one, Mapping) and isinstance(two, Mapping): return self._mapping_equal(one, two) - return self._unbool(one) == self._unbool(two) + return _utils.unbool(one) == _utils.unbool(two) def _uniq(self, container): try: - sort = sorted(self._unbool(i) for i in container) + sort = sorted(_utils.unbool(i) for i in container) sliced = itertools.islice(sort, 1, None) for i, j in zip(sort, sliced): @@ -64,7 +57,7 @@ def _uniq(self, container): except (NotImplementedError, TypeError): seen = [] for e in container: - e = self._unbool(e) + e = _utils.unbool(e) for i in seen: if self._equal(i, e): return False @@ -77,7 +70,7 @@ def uniqueItems(self, validator, uI, instance, schema): if validator.is_type(instance, "array") and not self._uniq(instance): if uI: yield ValidationError(f"{instance!r} has non-unique elements") - else: + elif self.child_rules.get("I3037"): yield ValidationError( f"{instance!r} has non-unique elements", rule=self.child_rules["I3037"], diff --git a/src/cfnlint/rules/resources/properties/Properties.py b/src/cfnlint/rules/resources/properties/Properties.py index 3b83f16a55..5a3eeefea7 100644 --- a/src/cfnlint/rules/resources/properties/Properties.py +++ b/src/cfnlint/rules/resources/properties/Properties.py @@ -30,15 +30,15 @@ def properties(self, validator, properties, instance, schema): break if len(v) == 3: yield from validator.descend( - v[0], + v[1], schema, - path=k[0] if len(k) > 0 else "Fn::If", + path="Fn::If", schema_path=None, ) yield from validator.descend( - v[1], + v[2], schema, - path=k[0] if len(k) > 0 else "Fn::If", + path="Fn::If", schema_path=None, ) return diff --git a/src/cfnlint/rules/resources/properties/ValuePrimitiveType.py b/src/cfnlint/rules/resources/properties/ValuePrimitiveType.py index 675db942d1..f6d548d17d 100644 --- a/src/cfnlint/rules/resources/properties/ValuePrimitiveType.py +++ b/src/cfnlint/rules/resources/properties/ValuePrimitiveType.py @@ -4,7 +4,12 @@ """ from typing import Any, List -from cfnlint.helpers import FUNCTIONS, FUNCTIONS_MULTIPLE +from cfnlint.helpers import ( + FUNCTIONS, + FUNCTIONS_MULTIPLE, + PSEUDOPARAMS_MULTIPLE, + PSEUDOPARAMS_SINGLE, +) from cfnlint.jsonschema import ValidationError from cfnlint.rules import CloudFormationLintRule from cfnlint.template.template import Template @@ -129,12 +134,29 @@ def type(self, validator, types, instance, schema): if t == "array": if v in valid_refs: ref_type = valid_refs.get(v).get("Type") - if "List" in ref_type: + ref_from = valid_refs.get(v).get("From") + if ref_from == "Pseudo": + if v in PSEUDOPARAMS_MULTIPLE: + return + if ( + "List" in ref_type + and ref_from == "Parameters" + ): return elif t in ["string", "number", "integer", "boolean"]: if v in valid_refs: ref_type = valid_refs.get(v).get("Type") - if "List" not in ref_type: + ref_from = valid_refs.get(v).get("From") + if ref_from == "Pseudo": + if v in PSEUDOPARAMS_SINGLE: + return + if ref_from == "Resources": + # Ref to resource are always singular + return + if ( + "List" not in ref_type + and ref_from == "Parameters" + ): return if v not in valid_refs: # Picked up by another rule diff --git a/src/cfnlint/rules/resources/properties/ValueRefGetAtt.py b/src/cfnlint/rules/resources/properties/ValueRefGetAtt.py deleted file mode 100644 index b1775fb3b2..0000000000 --- a/src/cfnlint/rules/resources/properties/ValueRefGetAtt.py +++ /dev/null @@ -1,26 +0,0 @@ -""" -Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -SPDX-License-Identifier: MIT-0 -""" -from cfnlint.rules import CloudFormationLintRule - - -class ValueRefGetAtt(CloudFormationLintRule): - """Check if Resource Properties are correct""" - - id = "E3008" - shortdesc = "Check values of properties for valid Refs and GetAtts" - description = "Checks resource properties for Ref and GetAtt values" - tags = ["resources", "ref", "getatt"] - - def __init__(self): - super().__init__() - self.cfn = None - - def initialize(self, cfn): - self.cfn = cfn - return super().initialize(cfn) - - # pylint: disable=unused-argument - def awsType(self, validator, uI, instance, schema): - pass diff --git a/src/cfnlint/template/template.py b/src/cfnlint/template/template.py index 12ecb3cb60..0e301e36ef 100644 --- a/src/cfnlint/template/template.py +++ b/src/cfnlint/template/template.py @@ -217,7 +217,7 @@ def get_valid_refs(self): for pseudoparam in cfnlint.PSEUDOPARAMS: element = {} element["Type"] = "Pseudo" - element["From"] = "Pseduo" + element["From"] = "Pseudo" results[pseudoparam] = element return results diff --git a/test/fixtures/templates/bad/functions/dynamic_reference.yaml b/test/fixtures/templates/bad/functions/dynamic_reference.yaml index 7b11998bfc..07a99e3e99 100644 --- a/test/fixtures/templates/bad/functions/dynamic_reference.yaml +++ b/test/fixtures/templates/bad/functions/dynamic_reference.yaml @@ -31,3 +31,10 @@ Resources: # DB User Not Supported for secure strings DbUser: !Sub '{{resolve:ssm-secure:MyUsername:1}}' RdsDbInstanceArn: String + Bucket: + Type: AWS::S3::Bucket + Properties: + BucketName: !Sub '{{resolve:ssm-secure:MyUsername:1}}' +Outputs: + Test: + Value: !Sub '{{resolve:ssm-secure:MyUsername:1}}' \ No newline at end of file diff --git a/test/fixtures/templates/bad/properties_az.yaml b/test/fixtures/templates/bad/properties_az.yaml deleted file mode 100644 index a20b36bb90..0000000000 --- a/test/fixtures/templates/bad/properties_az.yaml +++ /dev/null @@ -1,38 +0,0 @@ ---- -AWSTemplateFormatVersion: "2010-09-09" -Description: > - AWS EC2 Good Template -Parameters: - myEnvironment: - Type: String - AllowedValues: - - prod - - dev -Conditions: - isProd: - "Fn::Equals": - - !Ref myEnvironment - - "prod" -Resources: - MyLaunchConfig: - Type: AWS::AutoScaling::LaunchConfiguration - Properties: - ImageId: "ami-2f726546" - InstanceType: t2.micro - myLoadBalancer: - Type: "AWS::AutoScaling::AutoScalingGroup" - Properties: - AvailabilityZones: - # Hardcoded AZ's - - !If [isProd, "eu-west-1a", !Ref "AWS::NoValue"] - - "eu-west-1b" - MaxSize: 3 - MinSize: 1 - LaunchConfigurationName: !Ref MyLaunchConfig - mySubnet2-2: - Type: AWS::EC2::Subnet - Properties: - # Hardcoded AZ - AvailabilityZone: us-east-1a - CidrBlock: "172.31.0.0/16" - VpcId: vpc-1234567 diff --git a/test/fixtures/templates/good/resources/properties/az.yaml b/test/fixtures/templates/good/resources/properties/az.yaml deleted file mode 100644 index fdcb97fbf1..0000000000 --- a/test/fixtures/templates/good/resources/properties/az.yaml +++ /dev/null @@ -1,24 +0,0 @@ ---- -AWSTemplateFormatVersion: "2010-09-09" -Resources: - DefaultTargetGroup: - Type: "AWS::ElasticLoadBalancingV2::TargetGroup" - Properties: - VpcId: hello - Port: 80 - Protocol: HTTP - HealthCheckIntervalSeconds: 30 - HealthCheckPath: "/" - HealthCheckPort: "80" - HealthCheckProtocol: "HTTP" - HealthCheckTimeoutSeconds: 5 - HealthyThresholdCount: 5 - TargetType: ip - Targets: - - Id: "10.31.33.28" - AvailabilityZone: all - Matcher: - HttpCode: "200" - TargetGroupAttributes: - - Key: deregistration_delay.timeout_seconds - Value: "20" diff --git a/test/integration/test_patches.py b/test/integration/test_patches.py new file mode 100644 index 0000000000..e02ba2baf1 --- /dev/null +++ b/test/integration/test_patches.py @@ -0,0 +1,27 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +import json +import os +from test.integration import BaseCliTestCase + +from jsonpatch import InvalidJsonPatch, JsonPatch + + +class TestPatches(BaseCliTestCase): + """Test Patches""" + + def test_patches(self): + """Test ignoring certain rules""" + for root, _, files in os.walk( + "src/cfnlint/data/ExtendedProviderSchema", topdown=True + ): + for name in files: + if name.endswith(".json"): + with open(os.path.join(root, name)) as fh: + patches = json.load(fh) + try: + JsonPatch(patches) + except InvalidJsonPatch: + raise Exception(f"Invalid patch: {name}") diff --git a/test/unit/module/helpers/test_get_url_retrieve.py b/test/unit/module/helpers/test_get_url_retrieve.py new file mode 100644 index 0000000000..9bbdfeaf07 --- /dev/null +++ b/test/unit/module/helpers/test_get_url_retrieve.py @@ -0,0 +1,54 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +try: + import StringIO +except: + pass +from test.testlib.testcase import BaseTestCase +from unittest.mock import MagicMock, patch + +import cfnlint.helpers + + +class TestGetUrlRetrieve(BaseTestCase): + """Test Get URL Retrieve""" + + @patch("cfnlint.helpers.urlretrieve") + def test_get_url_retrieve(self, mocked_urlretrieve): + """Test the basics of URL retrieve""" + + mocked_urlretrieve.return_value = ("file/path", None) + + url = "http://foo.com" + result = cfnlint.helpers.get_url_retrieve(url) + mocked_urlretrieve.assert_called_with(url) + self.assertEqual(result, "file/path") + + @patch("cfnlint.helpers.urlopen") + @patch("cfnlint.helpers.load_metadata") + @patch("cfnlint.helpers.save_metadata") + @patch("cfnlint.helpers.urlretrieve") + def test_get_url_retrieve_cached( + self, mocked_urlretrieve, mock_save_metadata, mock_load_metadata, mocked_urlopen + ): + """Test the basics of URL retrieve""" + etag = "ETAG_ONE" + url = "http://foo.com" + + mock_load_metadata.return_value = {} + + cm = MagicMock() + cm.getcode.return_value = 200 + cm.info.return_value = {"Content-Encoding": "gzip", "ETag": etag} + cm.__enter__.return_value = cm + mocked_urlopen.return_value = cm + + mocked_urlretrieve.return_value = ("file/path", None) + + result = cfnlint.helpers.get_url_retrieve(url, caching=True) + mocked_urlretrieve.assert_called_with(url) + mock_load_metadata.assert_called_once() + mock_save_metadata.assert_called_once() + self.assertEqual(result, "file/path") diff --git a/test/unit/module/helpers/test_regex_dict.py b/test/unit/module/helpers/test_regex_dict.py index 9ec2f97bf8..1ddb870e09 100644 --- a/test/unit/module/helpers/test_regex_dict.py +++ b/test/unit/module/helpers/test_regex_dict.py @@ -31,3 +31,12 @@ def test_return_longest(self): obj["^TestLonger.*"] = True self.assertTrue(obj["TestLongerObject"]) + + def test_contains_object(self): + obj = RegexDict() + obj["^Test"] = False + obj["^TestLonger.*"] = True + + self.assertTrue("TestLongerObject" in obj) + self.assertFalse("NotIn" in obj) + self.assertFalse({"a": "b"} in obj) diff --git a/test/unit/module/rule/test_matching.py b/test/unit/module/rule/test_matching.py index b5eb6cbb1b..f3f43edf3f 100644 --- a/test/unit/module/rule/test_matching.py +++ b/test/unit/module/rule/test_matching.py @@ -19,9 +19,6 @@ class rule(CloudFormationLintRule): class TestMatching(BaseTestCase): """Test Matching Wrapper""" - def setUp(self) -> None: - return super().setUp() - def example(self: Any, *args: Any, **kwargs: Any): return [rule_match] @@ -30,3 +27,17 @@ def test_matching_location(self): r = f(self.example) t = r(self, "", None) self.assertEqual(t, [Match(1, 1, 1, 1, "", rule(), "Message", rule_match)]) + + def test_compare(self): + rule_match_1 = RuleMatch( + ["path"], "Message", rule=rule(), location=(0, 0, 0, 0) + ) + rule_match_2 = RuleMatch( + ["path"], "Message", rule=rule(), location=(0, 0, 0, 0) + ) + self.assertEqual(rule_match_1, rule_match_2) + + rule_match_3 = RuleMatch( + ["path"], "New Message", rule=rule(), location=(1, 1, 1, 1) + ) + self.assertNotEqual(rule_match_1, rule_match_3) diff --git a/test/unit/rules/functions/test_dynamic_reference.py b/test/unit/rules/functions/test_dynamic_reference.py index 1495b5cf89..6a728d67f6 100644 --- a/test/unit/rules/functions/test_dynamic_reference.py +++ b/test/unit/rules/functions/test_dynamic_reference.py @@ -27,5 +27,5 @@ def test_file_positive(self): def test_file_negative(self): """Test failure""" self.helper_file_negative( - "test/fixtures/templates/bad/functions/dynamic_reference.yaml", 2 + "test/fixtures/templates/bad/functions/dynamic_reference.yaml", 4 ) diff --git a/test/unit/rules/parameters/test_allowed_value.py b/test/unit/rules/parameters/test_allowed_value.py index 3e84f1a4e9..593485d4b8 100644 --- a/test/unit/rules/parameters/test_allowed_value.py +++ b/test/unit/rules/parameters/test_allowed_value.py @@ -34,6 +34,8 @@ def setUp(self): 2, ], }, + "5": [], + "6:": {"Type": "String", "AllowedValues": {"foo": "bar"}}, } }, regions=["us-east-1"], @@ -50,3 +52,7 @@ def test_validate(self): self.assertEqual(len(list(self.rule.validate("3", ["B", "C"]))), 1) self.assertEqual(len(list(self.rule.validate("4", [1, 2]))), 0) self.assertEqual(len(list(self.rule.validate("4", [2, 3]))), 1) + + # bad structure on parameter + self.assertEqual(len(list(self.rule.validate("5", ["foo"]))), 0) + self.assertEqual(len(list(self.rule.validate("6", ["foo"]))), 0) diff --git a/test/unit/rules/resources/properties/test_allowed_pattern.py b/test/unit/rules/resources/properties/test_allowed_pattern.py index 995a55829e..e6cce5b2aa 100644 --- a/test/unit/rules/resources/properties/test_allowed_pattern.py +++ b/test/unit/rules/resources/properties/test_allowed_pattern.py @@ -4,6 +4,30 @@ """ from test.unit.rules import BaseRuleTestCase +from jsonschema import Draft7Validator + +from cfnlint.rules.resources.properties.AllowedPattern import AllowedPattern + class TestAllowedPattern(BaseRuleTestCase): - """Test Allowed Value Property Configuration""" + """Test allowed pattern Property Configuration""" + + def setUp(self): + """Setup""" + self.rule = AllowedPattern() + + def test_allowed_pattern(self): + validator = Draft7Validator({"type": "string"}) + + self.assertEqual(len(list(self.rule.pattern(validator, ".*", "foo", {}))), 0) + self.assertEqual(len(list(self.rule.pattern(validator, "foo", "bar", {}))), 1) + self.assertEqual( + len( + list( + self.rule.pattern( + validator, "foo", "{{resolve:ssm:S3AccessControl:2}}", {} + ) + ) + ), + 0, + ) diff --git a/test/unit/rules/resources/properties/test_allowed_value.py b/test/unit/rules/resources/properties/test_allowed_value.py index 237a29db6f..7ecbff51a5 100644 --- a/test/unit/rules/resources/properties/test_allowed_value.py +++ b/test/unit/rules/resources/properties/test_allowed_value.py @@ -6,10 +6,10 @@ from jsonschema import Draft7Validator +from cfnlint.rules.parameters.AllowedValue import AllowedValue as ParameterAllowedValue from cfnlint.rules.resources.properties.AllowedValue import ( AllowedValue, # pylint: disable=E0401 ) -from cfnlint.rules.parameters.AllowedValue import AllowedValue as ParameterAllowedValue from cfnlint.template import Template @@ -34,7 +34,7 @@ def setUp(self): def test_allowed_value(self): """Test Positive""" - + validator = Draft7Validator({"type": "string", "enum": ["a", "b"]}) self.assertEqual(len(list(self.rule.enum(validator, ["a", "b"], "a", {}))), 0) self.assertEqual(len(list(self.rule.enum(validator, ["a", "b"], "c", {}))), 1) @@ -44,4 +44,36 @@ def test_allowed_value(self): self.assertEqual(len(list(self.rule.enum(validator, [0], 0, {}))), 0) self.assertEqual(len(list(self.rule.enum(validator, [1], 0, {}))), 1) - self.assertEqual(len(list(self.rule.enum(validator, [0], {"Ref": "Foo"}, {}))), 1) + self.assertEqual(len(list(self.rule.enum(validator, [True], 0, {}))), 1) + self.assertEqual(len(list(self.rule.enum(validator, [True], 1, {}))), 1) + self.assertEqual(len(list(self.rule.enum(validator, [False], 0, {}))), 1) + self.assertEqual(len(list(self.rule.enum(validator, [False], 1, {}))), 1) + + self.assertEqual( + len(list(self.rule.enum(validator, [0], {"Ref": "Foo"}, {}))), 1 + ) + + ## fall back to enum validation and not doing Ref + self.assertEqual( + len( + list( + self.rule.enum( + validator, + ["Bar"], + {"Ref": "Foo", "Fn::GetAtt": "ResourceName"}, + {}, + ) + ) + ), + 1, + ) + self.assertEqual( + len( + list( + self.rule.enum( + validator, ["Bar"], {"Fn::GetAtt": "ResourceName"}, {} + ) + ) + ), + 1, + ) diff --git a/test/unit/rules/resources/properties/test_availability_zone.py b/test/unit/rules/resources/properties/test_availability_zone.py index 14ef49fc5d..0b0c0d30ae 100644 --- a/test/unit/rules/resources/properties/test_availability_zone.py +++ b/test/unit/rules/resources/properties/test_availability_zone.py @@ -4,26 +4,70 @@ """ from test.unit.rules import BaseRuleTestCase +from jsonschema import Draft7Validator + from cfnlint.rules.resources.properties.AvailabilityZone import ( AvailabilityZone, # pylint: disable=E0401 ) -class TestPropertyAvailabilityZone(BaseRuleTestCase): +class TestAvailabilityZone(BaseRuleTestCase): """Test Password Property Configuration""" def setUp(self): """Setup""" - super(TestPropertyAvailabilityZone, self).setUp() - self.collection.register(AvailabilityZone()) - self.success_templates = [ - "test/fixtures/templates/good/resources/properties/az.yaml" - ] - - def test_file_positive(self): - """Success test""" - self.helper_file_positive() - - def test_file_negative(self): - """Failure test""" - self.helper_file_negative("test/fixtures/templates/bad/properties_az.yaml", 3) + self.rule = AvailabilityZone() + + def test_availability_zones(self): + validator = Draft7Validator({}) + + # hard coded string + self.assertEqual( + len(list(self.rule.availabilityzones(validator, {}, ["us-east-1"], {}))), 1 + ) + + # proper function + self.assertEqual( + len( + list( + self.rule.availabilityzones( + validator, {}, {"Fn::GetAZs": "us-east-1"}, {} + ) + ) + ), + 0, + ) + + # not a string + self.assertEqual( + len(list(self.rule.availabilityzones(validator, {}, True, {}))), 0 + ) + + # not a string + self.assertEqual( + len( + list( + self.rule.availabilityzones( + validator, {}, [{"Ref": "Parameter"}], {} + ) + ) + ), + 0, + ) + + # exception + self.assertEqual( + len(list(self.rule.availabilityzones(validator, {}, ["all"], {}))), 0 + ) + + # more than 1 + self.assertEqual( + len( + list( + self.rule.availabilityzones( + validator, {}, ["us-east-1", "us-west-2"], {} + ) + ) + ), + 2, + ) diff --git a/test/unit/rules/resources/properties/test_aws_type.py b/test/unit/rules/resources/properties/test_aws_type.py new file mode 100644 index 0000000000..f123b41b69 --- /dev/null +++ b/test/unit/rules/resources/properties/test_aws_type.py @@ -0,0 +1,36 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +from test.unit.rules import BaseRuleTestCase + +from jsonschema import Draft7Validator + +from cfnlint.jsonschema import ValidationError +from cfnlint.rules import CloudFormationLintRule +from cfnlint.rules.resources.properties.AwsType import AwsType # pylint: disable=E0401 + + +class ValidType(CloudFormationLintRule): + id = "AnId" + + def foo(self, validator, awsType, instance, schema): + if instance == "bar": + return + yield ValidationError("Not bar") + + +class TestAwsType(BaseRuleTestCase): + """Test AWS Types""" + + def setUp(self): + """Setup""" + super(TestAwsType, self).setUp() + self.rule = AwsType() + self.rule.child_rules = {"AnId": ValidType()} + self.rule.types = {"foo": "AnId"} + + def test_aws_type(self): + validator = Draft7Validator({}) + self.assertEqual(len(list(self.rule.awsType(validator, "foo", "bar", {}))), 0) + self.assertEqual(len(list(self.rule.awsType(validator, "foo", "foo", {}))), 1) diff --git a/test/unit/rules/resources/properties/test_list_duplicates.py b/test/unit/rules/resources/properties/test_list_duplicates.py index 39eb58a1e6..2b75b7bc5f 100644 --- a/test/unit/rules/resources/properties/test_list_duplicates.py +++ b/test/unit/rules/resources/properties/test_list_duplicates.py @@ -65,6 +65,9 @@ def test_duplicate(self): self.assertEqual( len(list(rule.uniqueItems(validator, True, [["a"], ["a"]], {}))), 1 ) + self.assertEqual( + len(list(rule.uniqueItems(validator, True, [["a"], ["a", "b"]], {}))), 0 + ) self.assertEqual( len(list(rule.uniqueItems(validator, True, [["a"], ["b"]], {}))), 0 ) diff --git a/test/unit/rules/resources/properties/test_properties.py b/test/unit/rules/resources/properties/test_properties.py index 702a3fb79b..d22392c87a 100644 --- a/test/unit/rules/resources/properties/test_properties.py +++ b/test/unit/rules/resources/properties/test_properties.py @@ -14,6 +14,114 @@ class TestAdditionalProperties(BaseRuleTestCase): """Test Allowed Value Property Configuration""" + def test_properties(self): + """Test Properties""" + rule = Properties() + validator = Draft7Validator({}) + + base_object = { + "a": "a", + "b": "b", + } + + schema = { + "type": "object", + "properties": { + "a": {"type": "string"}, + "b": {"type": "string"}, + }, + "additionalProperties": False, + } + + self.assertEqual( + len( + list( + rule.properties( + validator, + schema, + { + "Fn::If": [ + "Condition", + {**base_object}, + {**base_object}, + ] + }, + schema, + ) + ) + ), + 0, + ) + + self.assertEqual( + len( + list( + rule.properties( + validator, + schema, + { + "Fn::If": [ + "Condition", + base_object, + {**base_object, **{"c": "c"}}, + ] + }, + schema, + ) + ) + ), + 1, + ) + + self.assertEqual( + len( + list( + rule.properties( + validator, + schema, + "test", + schema, + ) + ) + ), + 0, + ) + + self.assertEqual( + len( + list( + rule.properties( + validator, + schema, + { + "Fn::If": [ + "Condition", + base_object, + base_object, + {**base_object, **{"c": "c"}}, + ] + }, + schema, + ) + ) + ), + 0, + ) + + self.assertEqual( + len( + list( + rule.properties( + validator, + schema, + {"Fn::If": {"foo": "bar"}}, + schema, + ) + ) + ), + 0, + ) + def test_additional_properties(self): """Test Positive""" rule = Properties() diff --git a/test/unit/rules/resources/properties/test_string_size.py b/test/unit/rules/resources/properties/test_string_size.py index 29bee5a666..7c16d577a1 100644 --- a/test/unit/rules/resources/properties/test_string_size.py +++ b/test/unit/rules/resources/properties/test_string_size.py @@ -12,6 +12,11 @@ ) +class Unserializable: + def __init__(self) -> None: + self.foo = "bar" + + class TestStringSize(BaseRuleTestCase): """Test List Size Property Configuration""" @@ -32,5 +37,11 @@ def test_file_positive(self): len(list(rule.maxLength(validator, 10, {"a": {"Fn::Sub": "b"}}, {}))), 0 ) self.assertEqual( - len(list(rule.maxLength(validator, 10, {"a": {"Fn::Sub": "bcd"}}, {}))), 1 + len(list(rule.maxLength(validator, 3, {"Fn::Sub": "bcd"}, {}))), 1 ) + self.assertEqual( + len(list(rule.maxLength(validator, 3, {"Fn::Sub": ["abcd", {}]}, {}))), 1 + ) + + with self.assertRaises(TypeError): + list(rule.maxLength(validator, 10, {"foo": Unserializable()}, {})) diff --git a/test/unit/rules/resources/properties/test_value_primitive_type.py b/test/unit/rules/resources/properties/test_value_primitive_type.py index 65e25905eb..dca6734db7 100644 --- a/test/unit/rules/resources/properties/test_value_primitive_type.py +++ b/test/unit/rules/resources/properties/test_value_primitive_type.py @@ -10,6 +10,7 @@ ValidationError, ValuePrimitiveType, ) +from cfnlint.template import Template class TestResourceValuePrimitiveTypeCheckValue(BaseRuleTestCase): @@ -155,9 +156,32 @@ def setUp(self): "string": jsonschema._types.is_string, }, ) + template = Template( + "", + { + "Parameters": { + "aString": { + "Type": "String", + }, + "aList": { + "Type": "List", + }, + }, + "Resources": { + "aResource": { + "Type": "Custom::Resource", + }, + "aModule": { + "Type": "A::Module", + }, + }, + }, + regions=["us-east-1"], + ) + self.rule.validate_configure(template) def test_validation(self): - """Test Positive""" + """Test type function for json schema""" # sub is a string boolean self.assertEqual( len( @@ -191,6 +215,11 @@ def test_validation(self): ), 0, ) + # array type with singular value + self.assertEqual( + len(list(self.rule.type(self.validator, ["array"], {"Fn::Sub": ""}, {}))), + 1, + ) # two types the second being valid self.assertEqual( len( @@ -213,3 +242,146 @@ def test_validation(self): ), 1, ) + + def test_validation_with_condition(self): + """Test type function for json schema""" + self.assertEqual( + len( + list( + self.rule.type( + self.validator, + ["string"], + {"Fn::If": ["Condition", "foo", "bar"]}, + {}, + ) + ) + ), + 0, + ) + self.assertEqual( + len( + list( + self.rule.type( + self.validator, + ["array"], + {"Fn::If": ["Condition", "foo", "bar"]}, + {}, + ) + ) + ), + 2, + ) + self.assertEqual( + len( + list( + self.rule.type( + self.validator, + ["array"], + {"Fn::If": ["Condition", "foo", "bar", "extra"]}, + {}, + ) + ) + ), + 0, + ) + + def test_validation_ref(self): + """Test type function for json schema""" + self.assertEqual( + len( + list( + self.rule.type( + self.validator, + ["string"], + {"Ref": ["Condition", "foo", "bar"]}, + {}, + ) + ) + ), + 0, + ) + self.assertEqual( + len(list(self.rule.type(self.validator, ["string"], {"Ref": "aList"}, {}))), + 1, + ) + self.assertEqual( + len( + list(self.rule.type(self.validator, ["string"], {"Ref": "aString"}, {})) + ), + 0, + ) + + self.assertEqual( + len(list(self.rule.type(self.validator, ["array"], {"Ref": "aList"}, {}))), + 0, + ) + self.assertEqual( + len( + list(self.rule.type(self.validator, ["array"], {"Ref": "aString"}, {})) + ), + 1, + ) + + self.assertEqual( + len( + list( + self.rule.type( + self.validator, ["array"], {"Ref": "AWS::AccountId"}, {} + ) + ) + ), + 1, + ) + + self.assertEqual( + list( + self.rule.type( + self.validator, ["array"], {"Ref": "AWS::NotificationARNs"}, {} + ) + ), + [], + ) + + self.assertEqual( + len( + list( + self.rule.type( + self.validator, ["string"], {"Ref": "AWS::NotificationARNs"}, {} + ) + ) + ), + 1, + ) + + self.assertEqual( + len( + list( + self.rule.type( + self.validator, ["string"], {"Ref": "AWS::AccountId"}, {} + ) + ) + ), + 0, + ) + + self.assertEqual( + len( + list( + self.rule.type( + self.validator, ["object"], {"Ref": "AWS::NotificationARNs"}, {} + ) + ) + ), + 0, + ) + + self.assertEqual( + len( + list( + self.rule.type( + self.validator, ["object"], {"Ref": "AWS::AccountId"}, {} + ) + ) + ), + 0, + )