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..4f4dce7ec6 100644 --- a/src/cfnlint/helpers.py +++ b/src/cfnlint/helpers.py @@ -110,10 +110,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 +119,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": { 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/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..58969ce3ce 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,13 +18,6 @@ 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): @@ -34,8 +27,8 @@ def enum(self, validator, enums, instance, schema): 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..2ff50beb85 100644 --- a/src/cfnlint/rules/resources/properties/AvailabilityZone.py +++ b/src/cfnlint/rules/resources/properties/AvailabilityZone.py @@ -2,6 +2,7 @@ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 """ +from cfnlint.jsonschema import ValidationError from cfnlint.rules import CloudFormationLintRule, RuleMatch @@ -9,7 +10,7 @@ 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,23 @@ 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", - ] + 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)) + 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..fd540f6539 --- /dev/null +++ b/src/cfnlint/rules/resources/properties/AwsType.py @@ -0,0 +1,42 @@ +""" +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( + 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..4b8bb3e98b 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 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 d68fa167d3..5c2287e58e 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/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..375f4b7d92 --- /dev/null +++ b/test/integration/test_patches.py @@ -0,0 +1,24 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +import json +from test.integration import BaseCliTestCase +import os +from jsonpatch import JsonPatch, InvalidJsonPatch + +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/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_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, + )