From 85e37931160d7e2446c834cfba8811cce35e3fec Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Mon, 13 Feb 2023 16:08:36 -0800 Subject: [PATCH] Update coverage on some of the converted rules (#2586) --- .../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/jsonschema/_utils.py | 8 ++ src/cfnlint/jsonschema/_validators.py | 2 +- src/cfnlint/rules/functions/GetAtt.py | 13 +-- src/cfnlint/rules/resources/Configuration.py | 4 +- .../resources/properties/AllowedValue.py | 13 +-- .../resources/properties/AvailabilityZone.py | 98 +++---------------- .../rules/resources/properties/AwsType.py | 42 ++++++++ .../resources/properties/ListDuplicates.py | 15 +-- .../resources/properties/ValueRefGetAtt.py | 26 ----- .../fixtures/templates/bad/properties_az.yaml | 38 ------- .../good/resources/properties/az.yaml | 24 ----- test/integration/test_patches.py | 24 +++++ .../properties/test_allowed_pattern.py | 26 ++++- .../properties/test_availability_zone.py | 72 +++++++++++--- .../resources/properties/test_string_size.py | 13 ++- 32 files changed, 323 insertions(+), 222 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 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/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/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/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_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()}, {}))