From d5e3c96583bfdcbd30f36cf3b5c6bc4087668734 Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Fri, 7 Apr 2023 10:36:09 -0700 Subject: [PATCH] Validate conditions when checking resource types for a region --- .../resource/configuration.json | 2 +- .../rules/parameters/AllowedPattern.py | 6 +- src/cfnlint/rules/resources/Configuration.py | 31 +- .../maintenance/test_update_resource_specs.py | 374 ------------------ 4 files changed, 25 insertions(+), 388 deletions(-) delete mode 100644 test/unit/module/maintenance/test_update_resource_specs.py diff --git a/src/cfnlint/data/AdditionalSchemas/resource/configuration.json b/src/cfnlint/data/AdditionalSchemas/resource/configuration.json index a9a907dd27..7dd3bd9b03 100644 --- a/src/cfnlint/data/AdditionalSchemas/resource/configuration.json +++ b/src/cfnlint/data/AdditionalSchemas/resource/configuration.json @@ -1,5 +1,6 @@ { "additionalProperties": false, + "awsType": true, "else": { "not": { "required": [ @@ -55,7 +56,6 @@ "type": "object" }, "Type": { - "awsType": true, "type": "string" }, "UpdatePolicy": { diff --git a/src/cfnlint/rules/parameters/AllowedPattern.py b/src/cfnlint/rules/parameters/AllowedPattern.py index d9c49d9182..cf54fb198c 100644 --- a/src/cfnlint/rules/parameters/AllowedPattern.py +++ b/src/cfnlint/rules/parameters/AllowedPattern.py @@ -2,7 +2,7 @@ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 """ -from typing import Generator, Iterable +from typing import Generator, List, Union import regex as re @@ -29,8 +29,8 @@ def initialize(self, cfn: Template): self.parameters = cfn.get_parameters() def _pattern( - self, instance: str, patrn: str, path: Iterable[str] - ) -> Generator[ValidationError]: + self, instance: str, patrn: str, path: List[Union[str, int]] + ) -> Generator[ValidationError, None, None]: if not re.search(patrn, instance): yield ValidationError( f"{instance!r} does not match {patrn!r}", diff --git a/src/cfnlint/rules/resources/Configuration.py b/src/cfnlint/rules/resources/Configuration.py index 20a4624968..7f9d8cc54a 100644 --- a/src/cfnlint/rules/resources/Configuration.py +++ b/src/cfnlint/rules/resources/Configuration.py @@ -32,27 +32,40 @@ def __init__(self): cfn=None, rules=None, )(schema=schema) + self.cfn = None def initialize(self, cfn): super().initialize(cfn) self.regions = cfn.regions + self.cfn = cfn # pylint: disable=unused-argument def _awsType(self, validator, iT, instance, schema): - if not validator.is_type(instance, "string"): + resource_type = instance.get("Type") + if not validator.is_type(resource_type, "string"): return + + resource_condition = instance.get("Condition") + for region in self.regions: - if instance in PROVIDER_SCHEMA_MANAGER.get_resource_types(region=region): - return - if not instance.startswith( + if validator.is_type(resource_condition, "string"): + if False in self.cfn.conditions.build_scenerios_on_region( + resource_condition, region + ): + continue + if resource_type in PROVIDER_SCHEMA_MANAGER.get_resource_types( + region=region + ): + continue + if not resource_type.startswith( ("Custom::", "AWS::Serverless::") - ) and not instance.endswith("::MODULE"): + ) and not resource_type.endswith("::MODULE"): yield ValidationError( - f"Resource type `{instance}` does not exist in '{region}'" + f"Resource type `{resource_type}` does not exist in '{region}'" ) # pylint: disable=unused-argument - def _check_resource(self, cfn, resource_name, resource_values): + def _check_resource(self, resource_name, resource_values): """Check Resource""" matches = [] @@ -97,8 +110,6 @@ def match(self, cfn): self.logger.debug( "Validating resource %s base configuration", resource_name ) - matches.extend( - self._check_resource(cfn, resource_name, resource_values) - ) + matches.extend(self._check_resource(resource_name, resource_values)) return matches diff --git a/test/unit/module/maintenance/test_update_resource_specs.py b/test/unit/module/maintenance/test_update_resource_specs.py deleted file mode 100644 index 1f2b8177e2..0000000000 --- a/test/unit/module/maintenance/test_update_resource_specs.py +++ /dev/null @@ -1,374 +0,0 @@ -""" -Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -SPDX-License-Identifier: MIT-0 -""" -import logging -import sys -from test.testlib.testcase import BaseTestCase -from unittest.mock import ANY, MagicMock, Mock, call, patch - -import cfnlint.maintenance - -LOGGER = logging.getLogger("cfnlint.maintenance") -LOGGER.addHandler(logging.NullHandler()) - - -def patch_spec_sideffect(content, region, patch_types="ExtendedSpecs"): - if region == "all" and patch_types == "ExtendedSpecs": - return { - "PropertyTypes": { - "AWS::Lambda::CodeSigningConfig.AllowedPublishers": { - "Properties": { - "SigningProfileVersionArns": {}, - } - }, - "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies": { - "Properties": { - "UntrustedArtifactOnDeployment": {}, - } - }, - }, - "ResourceTypes": { - "AWS::Lambda::CodeSigningConfig": { - "Attributes": { - "CodeSigningConfigArn": { - "PrimitiveType": "String", - }, - "CodeSigningConfigId": { - "PrimitiveType": "String", - }, - }, - "Properties": { - "AllowedPublishers": {}, - "CodeSigningPolicies": {}, - "Description": {}, - }, - } - }, - "ValueTypes": {}, - } - if region in ["us-east-1", "us-west-2"] and patch_types == "ExtendedSpecs": - return { - "PropertyTypes": { - "AWS::Lambda::CodeSigningConfig.AllowedPublishers": { - "Properties": { - "SigningProfileVersionArns": {}, - } - }, - "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies": { - "Properties": { - "UntrustedArtifactOnDeployment": {}, - } - }, - }, - "ResourceTypes": { - "AWS::Lambda::CodeSigningConfig": { - "Attributes": { - "CodeSigningConfigArn": { - "PrimitiveType": "String", - }, - "CodeSigningConfigId": { - "PrimitiveType": "String", - }, - }, - "Properties": { - "AllowedPublishers": {}, - "CodeSigningPolicies": {}, - "Description": {}, - }, - } - }, - "ValueTypes": { - "AWS::EC2::Instance.Types": ["m2.medium"], - }, - } - - return content - - -class TestUpdateResourceSpecs(BaseTestCase): - """Used for Testing Resource Specs""" - - @patch("cfnlint.maintenance.url_has_newer_version") - @patch("cfnlint.maintenance.get_url_content") - @patch("cfnlint.maintenance.json.dump") - @patch("cfnlint.maintenance.patch_spec") - @patch("cfnlint.maintenance.SPEC_REGIONS", {"us-east-1": "http://foo.badurl"}) - @patch("cfnlint.maintenance.urlopen") - def test_update_resource_spec( - self, - mock_urlopen, - mock_patch_spec, - mock_json_dump, - mock_content, - mock_url_newer_version, - ): - """Success update resource spec""" - - mock_url_newer_version.return_value = True - mock_content.return_value = '{"PropertyTypes": {}, "ResourceTypes": {}}' - mock_patch_spec.side_effect = patch_spec_sideffect - - cm = MagicMock() - cm.getcode.return_value = 200 - cm.__enter__.return_value = cm - - with open("test/fixtures/registry/schema.zip", "rb") as f: - byte = f.read() - cm.read.return_value = byte - - mock_urlopen.return_value = cm - schema_cache = cfnlint.maintenance.get_schema_value_types() - - builtin_module_name = "builtins" - - with patch("{}.open".format(builtin_module_name)) as mock_builtin_open: - cfnlint.maintenance.update_resource_spec( - "us-east-1", "http://foo.badurl", schema_cache - ) - mock_json_dump.assert_called_with( - { - "PropertyTypes": { - "AWS::Lambda::CodeSigningConfig.AllowedPublishers": { - "Properties": { - "SigningProfileVersionArns": { - "Value": { - "ValueType": "AWS::Lambda::CodeSigningConfig.AllowedPublishers.SigningProfileVersionArns", - } - }, - } - }, - "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies": { - "Properties": { - "UntrustedArtifactOnDeployment": { - "Value": { - "ValueType": "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies.UntrustedArtifactOnDeployment", - } - }, - } - }, - }, - "ResourceTypes": { - "AWS::Lambda::CodeSigningConfig": { - "Attributes": { - "CodeSigningConfigArn": { - "PrimitiveType": "String", - }, - "CodeSigningConfigId": { - "PrimitiveType": "String", - }, - }, - "Properties": { - "AllowedPublishers": {}, - "CodeSigningPolicies": {}, - "Description": {}, - }, - } - }, - "ValueTypes": { - "AWS::EC2::Instance.Types": ["m2.medium"], - "AWS::Lambda::CodeSigningConfig.AllowedPublishers.SigningProfileVersionArns": { - "AllowedPatternRegex": "arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\\-])+:([a-z]{2}(-gov)?-[a-z]+-\\d{1})?:(\\d{12})?:(.*)", - "StringMin": 12, - "StringMax": 1024, - }, - "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies.UntrustedArtifactOnDeployment": { - "AllowedValues": ["Warn", "Enforce"] - }, - }, - }, - mock_builtin_open.return_value.__enter__.return_value, - indent=1, - separators=(",", ": "), - sort_keys=True, - ) - - @patch("cfnlint.maintenance.url_has_newer_version") - @patch("cfnlint.maintenance.get_url_content") - @patch("cfnlint.maintenance.json.dump") - @patch("cfnlint.maintenance.patch_spec") - @patch("cfnlint.maintenance.SPEC_REGIONS", {"us-east-1": "http://foo.badurl"}) - @patch("cfnlint.maintenance.urlopen") - @patch("cfnlint.maintenance.cfnlint.helpers.load_resource") - def test_update_resource_spec_cache( - self, - mock_load_resource, - mock_urlopen, - mock_patch_spec, - mock_json_dump, - mock_content, - mock_url_newer_version, - ): - """Success update resource spec with cache""" - - mock_url_newer_version.return_value = True - mock_content.return_value = '{"PropertyTypes": {}, "ResourceTypes": {}}' - mock_patch_spec.side_effect = patch_spec_sideffect - - cm = MagicMock() - cm.getcode.return_value = 200 - cm.__enter__.return_value = cm - - with open("test/fixtures/registry/schema.zip", "rb") as f: - byte = f.read() - cm.read.return_value = byte - - mock_urlopen.return_value = cm - schema_cache = cfnlint.maintenance.get_schema_value_types() - - builtin_module_name = "builtins" - - mock_load_resource.return_value = { - "PropertyTypes": { - "AWS::Lambda::CodeSigningConfig.AllowedPublishers": { - "Properties": { - "SigningProfileVersionArns": { - "Value": { - "ValueType": "AWS::Lambda::CodeSigningConfig.AllowedPublishers.SigningProfileVersionArns", - } - }, - } - }, - "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies": { - "Properties": { - "UntrustedArtifactOnDeployment": { - "Value": { - "ValueType": "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies.UntrustedArtifactOnDeployment", - } - }, - } - }, - }, - "ResourceTypes": { - "AWS::Lambda::CodeSigningConfig": { - "Attributes": { - "CodeSigningConfigArn": { - "PrimitiveType": "String", - }, - "CodeSigningConfigId": { - "PrimitiveType": "String", - }, - }, - "Properties": { - "AllowedPublishers": {}, - "CodeSigningPolicies": {}, - "Description": {}, - }, - } - }, - "ValueTypes": { - "AWS::EC2::Instance.Types": [ - "m2.medium", - "m2.large", - ], - "AWS::Lambda::CodeSigningConfig.AllowedPublishers.SigningProfileVersionArns": { - "AllowedPatternRegex": "arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\\-])+:([a-z]{2}(-gov)?-[a-z]+-\\d{1})?:(\\d{12})?:(.*)", - "StringMin": 12, - "StringMax": 1024, - }, - "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies.UntrustedArtifactOnDeployment": { - "AllowedValues": ["Warn", "Enforce"] - }, - }, - } - - with patch("{}.open".format(builtin_module_name)) as mock_builtin_open: - cfnlint.maintenance.update_resource_spec( - "us-west-2", "http://foo.badurl", schema_cache - ) - mock_json_dump.assert_called_with( - { - "PropertyTypes": { - "AWS::Lambda::CodeSigningConfig.AllowedPublishers": "CACHED", - "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies": "CACHED", - }, - "ResourceTypes": { - "AWS::Lambda::CodeSigningConfig": "CACHED", - }, - "ValueTypes": { - # does not match above - "AWS::EC2::Instance.Types": [ - "m2.medium", - ], - "AWS::Lambda::CodeSigningConfig.AllowedPublishers.SigningProfileVersionArns": "CACHED", - "AWS::Lambda::CodeSigningConfig.CodeSigningPolicies.UntrustedArtifactOnDeployment": "CACHED", - }, - }, - mock_builtin_open.return_value.__enter__.return_value, - indent=1, - separators=(",", ": "), - sort_keys=True, - ) - - @patch("cfnlint.maintenance.url_has_newer_version") - @patch("cfnlint.maintenance.get_url_content") - @patch("cfnlint.maintenance.json.dump") - @patch("cfnlint.maintenance.patch_spec") - @patch("cfnlint.maintenance.SPEC_REGIONS", {"us-east-1": "http://foo.badurl"}) - def test_do_not_update_resource_spec( - self, mock_patch_spec, mock_json_dump, mock_content, mock_url_newer_version - ): - """Success update resource spec""" - - mock_url_newer_version.return_value = False - - result = cfnlint.maintenance.update_resource_spec( - "us-east-1", "http://foo.badurl", ANY - ) - self.assertIsNone(result) - mock_content.assert_not_called() - mock_patch_spec.assert_not_called() - mock_json_dump.assert_not_called() - - @patch("cfnlint.maintenance.url_has_newer_version") - @patch("cfnlint.maintenance.get_url_content") - @patch("cfnlint.maintenance.json.dump") - @patch("cfnlint.maintenance.patch_spec") - @patch("cfnlint.maintenance.SPEC_REGIONS", {"us-east-1": "http://foo.badurl"}) - @patch("cfnlint.maintenance.urlopen") - def test_update_resource_spec_force( - self, - mock_urlopen, - mock_patch_spec, - mock_json_dump, - mock_content, - mock_url_newer_version, - ): - """Success update resource spec""" - - mock_url_newer_version.return_value = False - mock_content.return_value = '{"PropertyTypes": {}, "ResourceTypes": {}}' - mock_patch_spec.side_effect = patch_spec_sideffect - - mock_urlresponse = Mock() - cm = MagicMock() - cm.getcode.return_value = 200 - cm.__enter__.return_value = cm - - with open("test/fixtures/registry/schema.zip", "rb") as f: - byte = f.read() - cm.read.return_value = byte - - mock_urlopen.return_value = cm - schema_cache = cfnlint.maintenance.get_schema_value_types() - - builtin_module_name = "builtins" - - with patch("{}.open".format(builtin_module_name)) as mock_builtin_open: - cfnlint.maintenance.update_resource_spec( - "us-east-1", "http://foo.badurl", schema_cache, True - ) - mock_content.assert_called_once() - mock_patch_spec.assert_called() - mock_json_dump.assert_called_once() - - @patch("cfnlint.maintenance.multiprocessing.Pool") - @patch("cfnlint.maintenance.update_resource_spec") - @patch("cfnlint.maintenance.SPEC_REGIONS", {"us-east-1": "http://foo.badurl"}) - def test_update_resource_specs_python_3(self, mock_update_resource_spec, mock_pool): - fake_pool = MagicMock() - mock_pool.return_value.__enter__.return_value = fake_pool - - cfnlint.maintenance.update_resource_specs() - - fake_pool.starmap.assert_called_once()