From 9d68a0e5adb045379bf62012019aaffd3d48ab8f Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Mon, 10 Oct 2022 15:41:31 -0700 Subject: [PATCH] Merge upstream changes --- src/cfnlint/helpers.py | 17 ++++-- src/cfnlint/maintenance.py | 34 ++++++++---- src/cfnlint/rules/resources/ResourceSchema.py | 52 ------------------- ...dOnResourceTypesWithAutoExpiringContent.py | 2 +- .../rules/resources/properties/JsonSchema.py | 51 ++++++++++++++---- .../properties/ValuePrimitiveType.py | 31 ++++++++--- .../resources/properties/ValueRefGetAtt.py | 1 - src/cfnlint/template.py | 5 +- test/integration/test_patched_specs.py | 1 - .../properties/test_value_primitive_type.py | 1 - 10 files changed, 104 insertions(+), 91 deletions(-) delete mode 100644 src/cfnlint/rules/resources/ResourceSchema.py diff --git a/src/cfnlint/helpers.py b/src/cfnlint/helpers.py index c796af96a9..8e7a45ce95 100644 --- a/src/cfnlint/helpers.py +++ b/src/cfnlint/helpers.py @@ -318,7 +318,8 @@ def get_url_content(url, caching=False): return content -def get_url_retrieve(url: str, caching: bool=False) -> str: + +def get_url_retrieve(url: str, caching: bool = False) -> str: """Get the contents of a zip file and returns a string representing the file @@ -338,7 +339,7 @@ def get_url_retrieve(url: str, caching: bool=False) -> str: # Load in all existing values metadata = load_metadata(metadata_filename) metadata['etag'] = res.info().get('ETag') - metadata['url'] = url # To make it obvious which url the Tag relates to + metadata['url'] = url # To make it obvious which url the Tag relates to save_metadata(metadata, metadata_filename) fileobject, _ = urlretrieve(url) @@ -467,6 +468,7 @@ def initialize_specs(): for reg in REGIONS: RESOURCE_SPECS[reg] = load_resource(CloudSpecs, filename=(f'{reg}.json')) + def initialize_provider_spec_modules() -> None: """Initialize the provider spec variables and modules. To be dynamic we import the modules from the regions variable @@ -480,7 +482,10 @@ def initialize_provider_spec_modules() -> None: PROVIDER_SCHEMAS_AWS[reg] = {} # provider specs don't exist for me-central-1 yet if reg != 'me-central-1': - __provider_schema_modules[reg] = __import__(f'cfnlint.data.ProviderSchemas.{reg}', fromlist=['']) + __provider_schema_modules[reg] = __import__( + f'cfnlint.data.ProviderSchemas.{reg}', fromlist=[''] + ) + def get_provider_schema(region: str, resource_type: str) -> dict: """Get the provider resource shcema and cache it to speed up future lookups @@ -494,13 +499,17 @@ def get_provider_schema(region: str, resource_type: str) -> dict: rt = resource_type.replace('::', '-').lower() schema = PROVIDER_SCHEMAS_AWS[region].get(resource_type) if schema is None: - PROVIDER_SCHEMAS_AWS[region][resource_type] = load_resource(__provider_schema_modules[region], filename=(f'{rt}.json')) + PROVIDER_SCHEMAS_AWS[region][resource_type] = load_resource( + __provider_schema_modules[region], filename=(f'{rt}.json') + ) return PROVIDER_SCHEMAS_AWS[region][resource_type] return schema + initialize_specs() initialize_provider_spec_modules() + def format_json_string(json_string): """Format the given JSON string""" diff --git a/src/cfnlint/maintenance.py b/src/cfnlint/maintenance.py index 4202201dc0..cc06d3822d 100644 --- a/src/cfnlint/maintenance.py +++ b/src/cfnlint/maintenance.py @@ -42,7 +42,9 @@ def update_resource_specs(force: bool = False): # Patch from registry schema pool_tuple = [(k, v, schema_cache, force) for k, v in SPEC_REGIONS.items()] pool.starmap(update_resource_spec, pool_tuple) - provider_pool_tuple = [(k, force) for k, v in SPEC_REGIONS.items() if k != 'us-east-1'] + provider_pool_tuple = [ + (k, force) for k, v in SPEC_REGIONS.items() if k != 'us-east-1' + ] pool.starmap(update_provider_schema, provider_pool_tuple) except AttributeError: # Do it the long, slow way @@ -153,7 +155,7 @@ def search_and_replace_botocore_types(obj): json.dump(spec, f, indent=1, sort_keys=True, separators=(',', ': ')) -def update_provider_schema(region, force: bool=False) -> None: +def update_provider_schema(region, force: bool = False) -> None: """Update the provider schemas from the AWS websites Args: @@ -166,7 +168,9 @@ def update_provider_schema(region, force: bool=False) -> None: suffix = '.cn' if region in ['cn-north-1', 'cn-northwest-1'] else '' url = f'https://schema.cloudformation.{region}.amazonaws.com{suffix}/CloudformationSchema.zip' - directory = os.path.join(os.path.dirname(cfnlint.__file__), f'data/ProviderSchemas/{region}/') + directory = os.path.join( + os.path.dirname(cfnlint.__file__), f'data/ProviderSchemas/{region}/' + ) multiprocessing_logger = multiprocessing.log_to_stderr() @@ -184,18 +188,30 @@ def update_provider_schema(region, force: bool=False) -> None: # if the region is not us-east-1 compare the files to those in us-east-1 # symlink if the files are the same if region != 'us-east-1': - directory_us_east_1 = os.path.join(os.path.dirname(cfnlint.__file__), 'data/ProviderSchemas/us-east-1/') + directory_us_east_1 = os.path.join( + os.path.dirname(cfnlint.__file__), 'data/ProviderSchemas/us-east-1/' + ) for filename in os.listdir(directory): if filename != '__init__.py': try: - if filecmp.cmp(f'{directory}{filename}', f'{directory_us_east_1}{filename}'): + if filecmp.cmp( + f'{directory}{filename}', f'{directory_us_east_1}{filename}' + ): os.remove(f'{directory}{filename}') - os.symlink(f'{directory_us_east_1}{filename}', f'{directory}{filename}') - except Exception as e: #pylint: disable=broad-except + os.symlink( + f'{directory_us_east_1}{filename}', + f'{directory}{filename}', + ) + except Exception as e: # pylint: disable=broad-except # Exceptions will typically be the file doesn't exist in us-east-1 - multiprocessing_logger.debug('Issuing comparing %s into %s: %s', f'{directory}{filename}', f'{directory_us_east_1}{filename}', e) + multiprocessing_logger.debug( + 'Issuing comparing %s into %s: %s', + f'{directory}{filename}', + f'{directory_us_east_1}{filename}', + e, + ) - except Exception as e: #pylint: disable=broad-except + except Exception as e: # pylint: disable=broad-except multiprocessing_logger.debug('Issuing updating specs for %s', region) diff --git a/src/cfnlint/rules/resources/ResourceSchema.py b/src/cfnlint/rules/resources/ResourceSchema.py deleted file mode 100644 index c914b8746f..0000000000 --- a/src/cfnlint/rules/resources/ResourceSchema.py +++ /dev/null @@ -1,52 +0,0 @@ -""" -Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -SPDX-License-Identifier: MIT-0 -""" -import re -from jsonschema import validate, ValidationError -from cfnlint.helpers import ( - REGEX_DYN_REF, - PSEUDOPARAMS, - FN_PREFIX, - UNCONVERTED_SUFFIXES, - REGISTRY_SCHEMAS, -) -from cfnlint.rules import CloudFormationLintRule -from cfnlint.rules import RuleMatch - - -class ResourceSchema(CloudFormationLintRule): - id = 'E3000' - shortdesc = 'Resource schema' - description = 'CloudFormation Registry resource schema validation' - source_url = ( - 'https://github.com/aws-cloudformation/aws-cloudformation-resource-schema/' - ) - tags = ['resources'] - - def match(self, cfn): - matches = [] - for schema in REGISTRY_SCHEMAS: - resource_type = schema['typeName'] - for resource_name, resource_values in cfn.get_resources( - [resource_type] - ).items(): - properties = resource_values.get('Properties', {}) - # ignoring resources with CloudFormation template syntax in Properties - if ( - not re.match(REGEX_DYN_REF, str(properties)) - and not any( - x in str(properties) - for x in PSEUDOPARAMS + UNCONVERTED_SUFFIXES - ) - and FN_PREFIX not in str(properties) - ): - try: - validate(properties, schema) - except ValidationError as e: - matches.append( - RuleMatch( - ['Resources', resource_name, 'Properties'], e.message - ) - ) - return matches diff --git a/src/cfnlint/rules/resources/RetentionPeriodOnResourceTypesWithAutoExpiringContent.py b/src/cfnlint/rules/resources/RetentionPeriodOnResourceTypesWithAutoExpiringContent.py index 7ca54d8db1..1cbd32a8fa 100644 --- a/src/cfnlint/rules/resources/RetentionPeriodOnResourceTypesWithAutoExpiringContent.py +++ b/src/cfnlint/rules/resources/RetentionPeriodOnResourceTypesWithAutoExpiringContent.py @@ -23,7 +23,7 @@ class RetentionPeriodOnResourceTypesWithAutoExpiringContent(CloudFormationLintRu tags = ['resources', 'retentionperiod'] def _check_ref(self, value, parameters, resources, path): # pylint: disable=W0613 - print(value) + pass def match(self, cfn): """Check for RetentionPeriod""" diff --git a/src/cfnlint/rules/resources/properties/JsonSchema.py b/src/cfnlint/rules/resources/properties/JsonSchema.py index 7ac5a94ec2..fbbcb946fe 100644 --- a/src/cfnlint/rules/resources/properties/JsonSchema.py +++ b/src/cfnlint/rules/resources/properties/JsonSchema.py @@ -10,13 +10,18 @@ from cfnlint.rules import CloudFormationLintRule from cfnlint.rules import RuleMatch import cfnlint.helpers -from cfnlint.helpers import REGEX_DYN_REF, PSEUDOPARAMS, FN_PREFIX, UNCONVERTED_SUFFIXES, REGISTRY_SCHEMAS +from cfnlint.helpers import ( + REGEX_DYN_REF, + PSEUDOPARAMS, + FN_PREFIX, + UNCONVERTED_SUFFIXES, + REGISTRY_SCHEMAS, +) LOGGER = logging.getLogger('cfnlint.rules.resources.properties.JsonSchema') # pylint: disable=too-many-instance-attributes class RuleSet(object): - def __init__(self): self.additionalProperties = 'E3002' self.required = 'E3003' @@ -34,8 +39,10 @@ def __init__(self): self.pattern = 'E3031' self.oneOf = 'E2523' + class JsonSchema(CloudFormationLintRule): """Check Base Resource Configuration""" + id = 'E3000' shortdesc = 'Parent rule for doing JSON schema validation' description = 'Making sure that resources properties comply with their JSON schema' @@ -62,7 +69,7 @@ def __init__(self): self.rules = RuleSet() self.config_definition = {'enabled': {'default': False, 'type': 'boolean'}} - #pylint: disable=unused-argument + # pylint: disable=unused-argument def validate_required(self, validator, required, instance, schema): if not validator.is_type(instance, 'object'): return @@ -119,7 +126,13 @@ def json_schema_validate(self, validator, properties, path): if hasattr(e, 'extra_args'): kwargs = getattr(e, 'extra_args') matches.append( - RuleMatch(path + list(e.path), e.message, rule=self.child_rules[getattr(self.rules, e.validator)], **kwargs)) + RuleMatch( + path + list(e.path), + e.message, + rule=self.child_rules[getattr(self.rules, e.validator)], + **kwargs, + ) + ) return matches @@ -160,14 +173,27 @@ def match(self, cfn): for schema in REGISTRY_SCHEMAS: resource_type = schema['typeName'] - for resource_name, resource_values in cfn.get_resources([resource_type]).items(): + for resource_name, resource_values in cfn.get_resources( + [resource_type] + ).items(): properties = resource_values.get('Properties', {}) # ignoring resources with CloudFormation template syntax in Properties - if not re.match(REGEX_DYN_REF, str(properties)) and not any(x in str(properties) for x in PSEUDOPARAMS + UNCONVERTED_SUFFIXES) and FN_PREFIX not in str(properties): + if ( + not re.match(REGEX_DYN_REF, str(properties)) + and not any( + x in str(properties) + for x in PSEUDOPARAMS + UNCONVERTED_SUFFIXES + ) + and FN_PREFIX not in str(properties) + ): try: jsonschema.validate(properties, schema) except jsonschema.ValidationError as e: - matches.append(RuleMatch(['Resources', resource_name, 'Properties'], e.message)) + matches.append( + RuleMatch( + ['Resources', resource_name, 'Properties'], e.message + ) + ) if not self.config.get('enabled'): return matches @@ -191,9 +217,14 @@ def match(self, cfn): schema = self.clense_schema(deepcopy(schema)) cfn_validator = self.validator(schema) path = ['Resources', n, 'Properties'] - for scenario in cfn.get_object_without_nested_conditions(p, path): - matches.extend(self.json_schema_validate( - cfn_validator, scenario.get('Object'), path)) + for scenario in cfn.get_object_without_nested_conditions( + p, path + ): + matches.extend( + self.json_schema_validate( + cfn_validator, scenario.get('Object'), path + ) + ) return matches diff --git a/src/cfnlint/rules/resources/properties/ValuePrimitiveType.py b/src/cfnlint/rules/resources/properties/ValuePrimitiveType.py index ef51e36f1f..50b8ffb692 100644 --- a/src/cfnlint/rules/resources/properties/ValuePrimitiveType.py +++ b/src/cfnlint/rules/resources/properties/ValuePrimitiveType.py @@ -234,8 +234,10 @@ def match_resource_properties(self, properties, resource_type, path, cfn): return matches - #pylint: disable=too-many-return-statements - def _schema_value_check(self, value: Any, item_type: str, strict_check: bool) -> bool: + # pylint: disable=too-many-return-statements + def _schema_value_check( + self, value: Any, item_type: str, strict_check: bool + ) -> bool: """Checks non strict""" if not strict_check: try: @@ -296,7 +298,7 @@ def _schema_check_primitive_type(self, value: Any, types: List[str]) -> bool: return result - #pylint: disable=unused-argument + # pylint: disable=unused-argument def validate(self, validator, types, instance, schema): types = ensure_list(types) reprs = ', '.join(repr(type) for type in types) @@ -308,20 +310,28 @@ def validate(self, validator, types, instance, schema): for t in types: if t == 'array': return - yield ValidationError(f'{instance!r} is not of type {reprs}', extra_args={}) + yield ValidationError( + f'{instance!r} is not of type {reprs}', extra_args={} + ) elif k in FUNCTIONS: for t in types: if t in ['string', 'integer', 'boolane']: return - yield ValidationError(f'{instance!r} is not of type {reprs}', extra_args={}) + yield ValidationError( + f'{instance!r} is not of type {reprs}', extra_args={} + ) else: - yield ValidationError(f'{instance!r} is not of type {reprs}', extra_args={}) + yield ValidationError( + f'{instance!r} is not of type {reprs}', extra_args={} + ) if not self._schema_check_primitive_type(instance, types): extra_args = { 'actual_type': type(instance).__name__, 'expected_type': reprs, } - yield ValidationError(f'{instance!r} is not of type {reprs}', extra_args=extra_args) + yield ValidationError( + f'{instance!r} is not of type {reprs}', extra_args=extra_args + ) class ValidationError(jsonschema.ValidationError): @@ -329,8 +339,13 @@ def __init__(self, message, extra_args): super(ValidationError, self).__init__(message) self.extra_args = extra_args + def ensure_list(thing): """ Wrap ``thing`` in a list if it's a single str. Otherwise, return it unchanged. - """ \ No newline at end of file + """ + + if isinstance(thing, str): + return [thing] + return thing diff --git a/src/cfnlint/rules/resources/properties/ValueRefGetAtt.py b/src/cfnlint/rules/resources/properties/ValueRefGetAtt.py index 0ed655ad62..701581bd9a 100644 --- a/src/cfnlint/rules/resources/properties/ValueRefGetAtt.py +++ b/src/cfnlint/rules/resources/properties/ValueRefGetAtt.py @@ -221,7 +221,6 @@ def check_value_getatt(self, value, path, **kwargs): ) ) elif '.'.join(map(str, resource_attribute)) != specs[resource_type]: - print(resource_type) message = ( 'Property "{0}" can Fn::GetAtt to a resource attribute "{1}" at {2}' ) diff --git a/src/cfnlint/template.py b/src/cfnlint/template.py index a1e12c303d..0e4b9f1ac4 100644 --- a/src/cfnlint/template.py +++ b/src/cfnlint/template.py @@ -596,7 +596,6 @@ def get_location_yaml(self, text, path): """ LOGGER.debug('Get location of path %s', path) result = None - error = False if not path: result = self._loc(text) elif len(path) > 1: @@ -622,7 +621,6 @@ def get_location_yaml(self, text, path): try: result = self._loc(text[path[0]]) except AttributeError as err: - error = True LOGGER.debug(err) else: try: @@ -630,10 +628,9 @@ def get_location_yaml(self, text, path): if key == path[0]: result = self._loc(key) except AttributeError as err: - error = True LOGGER.debug(err) - return result, error + return result def check_resource_property( self, diff --git a/test/integration/test_patched_specs.py b/test/integration/test_patched_specs.py index fa6a950a45..2b3d8b8e50 100644 --- a/test/integration/test_patched_specs.py +++ b/test/integration/test_patched_specs.py @@ -84,7 +84,6 @@ def test_sub_properties(self): if resource_name: v_value_properties = v_values.get('Properties', {}) if v_value_properties is None: - print(v_values) self._test_sub_properties(resource_name, '', v_values) else: for p_name, p_values in v_value_properties.items(): 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 f924c60410..c23cc8ca94 100644 --- a/test/unit/rules/resources/properties/test_value_primitive_type.py +++ b/test/unit/rules/resources/properties/test_value_primitive_type.py @@ -200,7 +200,6 @@ def test_file_positive(self): self.assertEqual( self.rule._schema_check_primitive_type("1.2", ["integer"]), False ) - print('in', self.rule._schema_check_primitive_type(True, ["integer"])) self.assertEqual(self.rule._schema_check_primitive_type(True, ["integer"]), False) self.assertEqual( self.rule._schema_check_primitive_type("test", ["integer"]), False