From 73df410f372af8d5fcb16fc2459fd91ab2cd53b9 Mon Sep 17 00:00:00 2001 From: Pat Myron Date: Mon, 3 May 2021 11:55:01 -0700 Subject: [PATCH 1/2] removing duplicate module validation caught by cfn-lint https://github.com/aws-cloudformation/cloudformation-cli/pull/634 https://github.com/aws-cloudformation/cloudformation-cli/pull/644 --- src/rpdk/core/fragment/generator.py | 96 +------------------ .../core/fragment/module_fragment_reader.py | 4 - 2 files changed, 1 insertion(+), 99 deletions(-) diff --git a/src/rpdk/core/fragment/generator.py b/src/rpdk/core/fragment/generator.py index 6d240b2a..d0cfaf37 100644 --- a/src/rpdk/core/fragment/generator.py +++ b/src/rpdk/core/fragment/generator.py @@ -15,18 +15,13 @@ from rpdk.core.exceptions import FragmentValidationError from .lint_warning_printer import print_cfn_lint_warnings -from .module_fragment_reader import get_template_file_size_in_bytes, read_raw_fragments +from .module_fragment_reader import read_raw_fragments LOG = logging.getLogger(__name__) FRAGMENT_DIR = "fragments" SAMPLE_FRAGMENT_OUTPUT = "sample.json" SCHEMA_NAME = "schema.json" SAMPLE_FRAGMENT = "../data/examples/module/sample.json" -RESOURCE_LIMIT = 500 -OUTPUT_LIMIT = 200 -MAPPING_LIMIT = 200 -MAPPING_ATTRIBUTE_LIMIT = 200 -TEMPLATE_FILE_SIZE_IN_BYTES_LIMIT = 1500000 class TemplateFragment: # pylint: disable=too-many-instance-attributes @@ -34,11 +29,6 @@ def __init__(self, type_name, root=None): self.root = Path(root) if root else Path.cwd() self.fragment_dir = self.root / FRAGMENT_DIR self.type_name = type_name - self.resource_limit = RESOURCE_LIMIT - self.output_limit = OUTPUT_LIMIT - self.mapping_limit = MAPPING_LIMIT - self.mapping_attribute_limit = MAPPING_ATTRIBUTE_LIMIT - self.template_file_size_in_bytes_limit = TEMPLATE_FILE_SIZE_IN_BYTES_LIMIT LOG.debug("Fragment directory: %s", self.fragment_dir) @@ -69,17 +59,13 @@ def validate_fragments(self): since it can occur anywhere in the template. """ raw_fragments = read_raw_fragments(self.fragment_dir) - self.__validate_file_size_limit() self.__validate_resources(raw_fragments) - self.__validate_parameters(raw_fragments) self.__validate_no_transforms_present(raw_fragments) self.__validate_outputs(raw_fragments) - self.__validate_mappings(raw_fragments) print_cfn_lint_warnings(self.fragment_dir) def __validate_outputs(self, raw_fragments): self.__validate_no_exports_present(raw_fragments) - self.__validate_output_limit(raw_fragments) @staticmethod def __validate_no_exports_present(raw_fragments): @@ -91,24 +77,7 @@ def __validate_no_exports_present(raw_fragments): "Found an Export statement in Output: " + _output_name ) - def __validate_output_limit(self, raw_fragments): - if "Outputs" in raw_fragments: - output_count = len(raw_fragments["Outputs"].items()) - if output_count > self.output_limit: - raise FragmentValidationError( - "The Module template fragment has " - + str(output_count) - + " outputs but must not exceed the limit of " - + str(self.output_limit) - + " outputs" - ) - def __validate_resources(self, raw_fragments): - if "Resources" not in raw_fragments: - raise FragmentValidationError( - "A Module template fragment must have a Resources section" - ) - self.__validate_resource_limit(raw_fragments) for _resource_name, resource in raw_fragments["Resources"].items(): if "Type" in resource: self.__validate_no_nested_stacks(resource) @@ -118,10 +87,6 @@ def __validate_resources(self, raw_fragments): raise FragmentValidationError( "Resource '" + _resource_name + "' is invalid" ) - else: - raise FragmentValidationError( - "Resource '" + _resource_name + "' has neither Type nor Name" - ) @staticmethod def __validate_no_include(resource): @@ -142,26 +107,6 @@ def __validate_no_nested_stacks(resource): "Template fragment can't contain nested stack." ) - def __validate_resource_limit(self, raw_fragments): - resource_count = len(raw_fragments["Resources"].items()) - if resource_count > self.resource_limit: - raise FragmentValidationError( - "The Module template fragment has " - + str(resource_count) - + " resources but must not exceed the limit of " - + str(self.resource_limit) - + " resources" - ) - - @staticmethod - def __validate_parameters(raw_fragments): - if "Parameters" in raw_fragments: - for _parameter_name, parameter in raw_fragments["Parameters"].items(): - if "Type" not in parameter: - raise FragmentValidationError( - "Parameter '" + _parameter_name + "' must have a Type" - ) - @staticmethod def __validate_no_transforms_present(raw_fragments): if "transform" in raw_fragments or "Transform" in raw_fragments: @@ -173,45 +118,6 @@ def __validate_no_transforms_present(raw_fragments): "Template fragment can't contain any transform." ) - def __validate_mappings(self, raw_fragments): - self.__validate_mapping_limit(raw_fragments) - self.__validate_mapping_attribute_limit(raw_fragments) - - def __validate_mapping_limit(self, raw_fragments): - if "Mappings" in raw_fragments: - mapping_count = len(raw_fragments["Mappings"].items()) - if mapping_count > self.mapping_limit: - raise FragmentValidationError( - "The Module template fragment has " - + str(mapping_count) - + " mappings but must not exceed the limit of " - + str(self.output_limit) - + " mappings" - ) - - def __validate_mapping_attribute_limit(self, raw_fragments): - if "Mappings" in raw_fragments: - for _mapping_name, mapping in raw_fragments["Mappings"].items(): - attribute_count = len(mapping.items()) - if attribute_count > self.mapping_attribute_limit: - raise FragmentValidationError( - "The mapping " - + _mapping_name - + " has " - + str(attribute_count) - + " attributes but must not exceed the limit of " - + str(self.output_limit) - + " mapping attributes" - ) - - def __validate_file_size_limit(self): - total_size = get_template_file_size_in_bytes(self.fragment_dir) - if total_size > self.template_file_size_in_bytes_limit: - raise FragmentValidationError( - "The total file size of the template" - " fragments exceeds the CloudFormation Template size limit" - ) - @staticmethod def __build_resources(raw_fragments): raw_resources = {} diff --git a/src/rpdk/core/fragment/module_fragment_reader.py b/src/rpdk/core/fragment/module_fragment_reader.py index 01040da9..cb270514 100644 --- a/src/rpdk/core/fragment/module_fragment_reader.py +++ b/src/rpdk/core/fragment/module_fragment_reader.py @@ -14,10 +14,6 @@ def read_raw_fragments(fragment_dir): return _load_fragment(_get_fragment_file(fragment_dir)) -def get_template_file_size_in_bytes(fragment_dir): - return os.stat(_get_fragment_file(fragment_dir)).st_size - - def _load_fragment(fragment_file): try: with open(fragment_file, "r", encoding="utf-8") as f: From 5520e0563cccf003cb395f9839c67c10c2af7f67 Mon Sep 17 00:00:00 2001 From: Pat Myron Date: Mon, 3 May 2021 12:31:29 -0700 Subject: [PATCH 2/2] removing no longer needed tests --- ...ragment_mapping_with_three_attributes.json | 21 ------ .../fragment_three_mappings.json | 25 ------- .../fragment_three_outputs.json | 19 ----- .../fragment_three_resources.json | 14 ---- tests/data/sample_fragments/noresources.json | 10 --- .../parameter_without_type.json | 23 ------ .../resource_without_type_or_name.json | 13 ---- tests/fragments/test_generator.py | 71 ------------------- 8 files changed, 196 deletions(-) delete mode 100644 tests/data/sample_fragments/fragment_mapping_with_three_attributes.json delete mode 100644 tests/data/sample_fragments/fragment_three_mappings.json delete mode 100644 tests/data/sample_fragments/fragment_three_outputs.json delete mode 100644 tests/data/sample_fragments/fragment_three_resources.json delete mode 100644 tests/data/sample_fragments/noresources.json delete mode 100644 tests/data/sample_fragments/parameter_without_type.json delete mode 100644 tests/data/sample_fragments/resource_without_type_or_name.json diff --git a/tests/data/sample_fragments/fragment_mapping_with_three_attributes.json b/tests/data/sample_fragments/fragment_mapping_with_three_attributes.json deleted file mode 100644 index 9ad409c7..00000000 --- a/tests/data/sample_fragments/fragment_mapping_with_three_attributes.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "AWSTemplateFormatVersion": "2010-09-09", - "Resources": { - "Wait1": { - "Type": "AWS::CloudFormation::WaitConditionHandle" - } - }, - "Mappings": { - "Mapping01": { - "Key01": { - "Name": "Value01" - }, - "Key02": { - "Name": "Value01" - }, - "Key03": { - "Name": "Value01" - } - } - } -} diff --git a/tests/data/sample_fragments/fragment_three_mappings.json b/tests/data/sample_fragments/fragment_three_mappings.json deleted file mode 100644 index 36008480..00000000 --- a/tests/data/sample_fragments/fragment_three_mappings.json +++ /dev/null @@ -1,25 +0,0 @@ -{ - "AWSTemplateFormatVersion": "2010-09-09", - "Resources": { - "Wait1": { - "Type": "AWS::CloudFormation::WaitConditionHandle" - } - }, - "Mappings": { - "Mapping01": { - "Key01": { - "Name": "Value01" - } - }, - "Mapping02": { - "Key01": { - "Name": "Value01" - } - }, - "Mapping03": { - "Key01": { - "Name": "Value01" - } - } - } -} diff --git a/tests/data/sample_fragments/fragment_three_outputs.json b/tests/data/sample_fragments/fragment_three_outputs.json deleted file mode 100644 index e381eeb3..00000000 --- a/tests/data/sample_fragments/fragment_three_outputs.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "AWSTemplateFormatVersion": "2010-09-09", - "Resources": { - "Wait1": { - "Type": "AWS::CloudFormation::WaitConditionHandle" - } - }, - "Outputs": { - "Output1": { - "Value": "Value 1" - }, - "Output2": { - "Value": "Value 2" - }, - "Output3": { - "Value": "Value 3" - } - } -} diff --git a/tests/data/sample_fragments/fragment_three_resources.json b/tests/data/sample_fragments/fragment_three_resources.json deleted file mode 100644 index 4039812f..00000000 --- a/tests/data/sample_fragments/fragment_three_resources.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "AWSTemplateFormatVersion": "2010-09-09", - "Resources": { - "Wait1": { - "Type": "AWS::CloudFormation::WaitConditionHandle" - }, - "Wait2": { - "Type": "AWS::CloudFormation::WaitConditionHandle" - }, - "Wait3": { - "Type": "AWS::CloudFormation::WaitConditionHandle" - } - } -} diff --git a/tests/data/sample_fragments/noresources.json b/tests/data/sample_fragments/noresources.json deleted file mode 100644 index d6c652fb..00000000 --- a/tests/data/sample_fragments/noresources.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "AWSTemplateFormatVersion": "2010-09-09", - "Description": "A secure S3 Bucket. The features are Versioning and DeletionPolicy.", - "Parameters": { - "BucketName": { - "Description": "Name for the bucket", - "Type": "String" - } - } -} diff --git a/tests/data/sample_fragments/parameter_without_type.json b/tests/data/sample_fragments/parameter_without_type.json deleted file mode 100644 index b354a19d..00000000 --- a/tests/data/sample_fragments/parameter_without_type.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "AWSTemplateFormatVersion": "2010-09-09", - "Description": "A secure S3 Bucket. The features are Versioning and DeletionPolicy.", - "Parameters": { - "BucketName": { - "Description": "Name for the bucket" - } - }, - "Resources": { - "S3Bucket": { - "Type": "AWS::S3::Bucket", - "DeletionPolicy": "Retain", - "Properties": { - "BucketName": { - "Ref": "BucketName" - }, - "VersioningConfiguration": { - "Status": "Enabled" - } - } - } - } -} diff --git a/tests/data/sample_fragments/resource_without_type_or_name.json b/tests/data/sample_fragments/resource_without_type_or_name.json deleted file mode 100644 index 4785eaa0..00000000 --- a/tests/data/sample_fragments/resource_without_type_or_name.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "AWSTemplateFormatVersion": "2010-09-09", - "Description": "A secure S3 Bucket. The features are Versioning and DeletionPolicy.", - "Parameters": { - "BucketName": { - "Description": "Name for the bucket", - "Type": "String" - } - }, - "Resources": { - "S3Bucket": {} - } -} diff --git a/tests/fragments/test_generator.py b/tests/fragments/test_generator.py index 9296af28..a000c96e 100644 --- a/tests/fragments/test_generator.py +++ b/tests/fragments/test_generator.py @@ -208,14 +208,6 @@ def test_template_fragments_invalid_transform(template_fragment): ) -def test_template_fragments_resource_without_type(template_fragment): - __assert_throws_validation_error( - "resource_without_type_or_name.json", - template_fragment, - "has neither Type nor Name", - ) - - def test_template_fragments_macros(template_fragment): __assert_throws_validation_error( "macros.yaml", template_fragment, "can't contain any macro" @@ -228,12 +220,6 @@ def test_template_fragments_nested_stack(template_fragment): ) -def test_template_fragments_parameter_without_type(template_fragment): - __assert_throws_validation_error( - "parameter_without_type.json", template_fragment, "must have a Type" - ) - - def test_template_fragments_transform(template_fragment): __assert_throws_validation_error( "transform.json", template_fragment, "can't contain transform section" @@ -246,12 +232,6 @@ def test_template_fragments_transform_section(template_fragment): ) -def test_template_fragments_without_resources(template_fragment): - __assert_throws_validation_error( - "noresources.json", template_fragment, "must have a Resources section" - ) - - def test_template_fragments_with_json_syntax_error(template_fragment): __assert_throws_validation_error( "syntax_error.json", template_fragment, "line 15, column 24" @@ -272,57 +252,6 @@ def test_template_fragments_with_sub_is_valid(template_fragment): __assert_validation_throws_no_error("ec2withsub.yaml", template_fragment) -def test_template_exceeding_resource_limit(template_fragment): - template_fragment.resource_limit = 2 - __assert_throws_validation_error( - "fragment_three_resources.json", - template_fragment, - "has 3 resources but must not exceed the limit of 2", - ) - - -def test_template_exceeding_output_limit(template_fragment): - template_fragment.output_limit = 2 - __assert_throws_validation_error( - "fragment_three_outputs.json", - template_fragment, - "has 3 outputs but must not exceed the limit of 2", - ) - - -def test_template_exceeding_mapping_limit(template_fragment): - template_fragment.mapping_limit = 2 - __assert_throws_validation_error( - "fragment_three_mappings.json", - template_fragment, - "has 3 mappings but must not exceed the limit of 2", - ) - - -def test_template_exceeding_mapping_attribute_limit(template_fragment): - template_fragment.mapping_attribute_limit = 2 - __assert_throws_validation_error( - "fragment_mapping_with_three_attributes.json", - template_fragment, - "has 3 attributes but must not exceed the limit of 2", - ) - - -def test_template_mappings_dont_exceed_any_limit(template_fragment): - __assert_validation_throws_no_error( - "fragment_mapping_with_three_attributes.json", template_fragment - ) - - -def test_template_exceeding_file_size_limit(template_fragment): - template_fragment.template_file_size_in_bytes_limit = 300 - __assert_throws_validation_error( - "sample.json", - template_fragment, - "exceeds the CloudFormation Template size limit", - ) - - def test_template_folder_with_multiple_fragment_files(): template_fragment = TemplateFragment( type_name,