From 78e9fa96152d186ec7086d741f5f8ddaba1315e8 Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Sun, 7 Jul 2024 10:03:26 -0700 Subject: [PATCH] Allow Transform section to have the full transform def (#3470) * Allow Transform section to have the full transform def * Switch transform validation to rule E1005 --- src/cfnlint/context/context.py | 2 +- .../schemas/other/template/configuration.json | 10 +- .../data/schemas/other/transforms/__init__.py | 0 .../other/transforms/configuration.json | 38 ++++++++ src/cfnlint/rules/transforms/Configuration.py | 31 ++++++ src/cfnlint/rules/transforms/__init__.py | 0 test/integration/test_schema_files.py | 1 + test/unit/module/context/test_transforms.py | 17 +--- test/unit/rules/__init__.py | 4 +- .../rules/templates/test_base_template.py | 4 +- test/unit/rules/transforms/__init__.py | 0 .../rules/transforms/test_configuration.py | 95 +++++++++++++++++++ 12 files changed, 177 insertions(+), 25 deletions(-) create mode 100644 src/cfnlint/data/schemas/other/transforms/__init__.py create mode 100644 src/cfnlint/data/schemas/other/transforms/configuration.json create mode 100644 src/cfnlint/rules/transforms/Configuration.py create mode 100644 src/cfnlint/rules/transforms/__init__.py create mode 100644 test/unit/rules/transforms/__init__.py create mode 100644 test/unit/rules/transforms/test_configuration.py diff --git a/src/cfnlint/context/context.py b/src/cfnlint/context/context.py index e3aaeba8db..8bfb407a18 100644 --- a/src/cfnlint/context/context.py +++ b/src/cfnlint/context/context.py @@ -38,7 +38,7 @@ def __post_init__(self, transforms) -> None: for transform in transforms: if not isinstance(transform, str): - raise ValueError("Transform must be a string") + continue self._transforms.append(transform) def has_language_extensions_transform(self): diff --git a/src/cfnlint/data/schemas/other/template/configuration.json b/src/cfnlint/data/schemas/other/template/configuration.json index c7b825e88b..afc9203b28 100644 --- a/src/cfnlint/data/schemas/other/template/configuration.json +++ b/src/cfnlint/data/schemas/other/template/configuration.json @@ -22,15 +22,7 @@ "Rules": { "type": "object" }, - "Transform": { - "items": { - "type": "string" - }, - "type": [ - "string", - "array" - ] - } + "Transform": {} }, "required": [ "Resources" diff --git a/src/cfnlint/data/schemas/other/transforms/__init__.py b/src/cfnlint/data/schemas/other/transforms/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/cfnlint/data/schemas/other/transforms/configuration.json b/src/cfnlint/data/schemas/other/transforms/configuration.json new file mode 100644 index 0000000000..da56c0d3cb --- /dev/null +++ b/src/cfnlint/data/schemas/other/transforms/configuration.json @@ -0,0 +1,38 @@ +{ + "additionalProperties": false, + "items": { + "$ref": "#/", + "type": [ + "string", + "object" + ] + }, + "properties": { + "Name": { + "type": "string" + }, + "Parameters": { + "patternProperties": { + ".*": { + "type": [ + "string", + "array", + "boolean", + "object", + "number", + "integer" + ] + } + }, + "type": "object" + } + }, + "required": [ + "Name" + ], + "type": [ + "string", + "array", + "object" + ] +} diff --git a/src/cfnlint/rules/transforms/Configuration.py b/src/cfnlint/rules/transforms/Configuration.py new file mode 100644 index 0000000000..489c4d832a --- /dev/null +++ b/src/cfnlint/rules/transforms/Configuration.py @@ -0,0 +1,31 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from __future__ import annotations + +import cfnlint.data.schemas.other.transforms +from cfnlint.rules.jsonschema.CfnLintJsonSchema import CfnLintJsonSchema, SchemaDetails + + +class Configuration(CfnLintJsonSchema): + """Check if Parameters are configured correctly""" + + id = "E1005" + shortdesc = "Validate Transform configuration" + description = ( + "Validate that the transforms section of a template is properly configured" + ) + source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/transform-section-structure.html" + tags = ["transform"] + + def __init__(self): + """Init""" + super().__init__( + keywords=["Transform"], + schema_details=SchemaDetails( + cfnlint.data.schemas.other.transforms, "configuration.json" + ), + all_matches=True, + ) diff --git a/src/cfnlint/rules/transforms/__init__.py b/src/cfnlint/rules/transforms/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/integration/test_schema_files.py b/test/integration/test_schema_files.py index e00bf3805a..8aa44d2e1e 100644 --- a/test/integration/test_schema_files.py +++ b/test/integration/test_schema_files.py @@ -51,6 +51,7 @@ class TestSchemaFiles(TestCase): "Resources/*/Type", "Resources/*/UpdatePolicy", "Resources/*/UpdateReplacePolicy", + "Transform", ] def setUp(self) -> None: diff --git a/test/unit/module/context/test_transforms.py b/test/unit/module/context/test_transforms.py index a852cf7c42..451e9c8566 100644 --- a/test/unit/module/context/test_transforms.py +++ b/test/unit/module/context/test_transforms.py @@ -12,7 +12,11 @@ "name,instance,expected", [ ("Valid transforms", "AWS::LanguageExtensions", True), - ("Valid transforms lists", ["AWS::LanguageExtensions"], True), + ( + "Valid transforms lists", + ["AWS::LanguageExtensions", {"Name": "Include"}], + True, + ), ("Valid transforms lists", None, False), ("Valid transforms lists", "", False), ], @@ -23,14 +27,3 @@ def test_transforms(name, instance, expected): assert ( expected == transforms.has_language_extensions_transform() ), f"{name!r} test got {transforms.has_language_extensions_transform()}" - - -@pytest.mark.parametrize( - "name,instance", - [ - ("Invalid Type", {}), - ], -) -def test_errors(name, instance): - with pytest.raises(ValueError): - Transforms(instance) diff --git a/test/unit/rules/__init__.py b/test/unit/rules/__init__.py index c1970fd040..ba28f38cb9 100644 --- a/test/unit/rules/__init__.py +++ b/test/unit/rules/__init__.py @@ -56,5 +56,7 @@ def helper_file_negative(self, filename, err_count, config=None): self.assertEqual( err_count, len(failures), - "Expected {} failures but got {} on {}".format(0, failures, filename), + "Expected {} failures but got {} on {}".format( + err_count, failures, filename + ), ) diff --git a/test/unit/rules/templates/test_base_template.py b/test/unit/rules/templates/test_base_template.py index 7194606d3a..3dee201c9b 100644 --- a/test/unit/rules/templates/test_base_template.py +++ b/test/unit/rules/templates/test_base_template.py @@ -40,7 +40,7 @@ def test_file_negative(self): def test_file_base(self): """Test failure""" - self.helper_file_negative("test/fixtures/templates/bad/templates/base.yaml", 2) + self.helper_file_negative("test/fixtures/templates/bad/templates/base.yaml", 1) def test_file_base_date(self): """Test failure""" @@ -51,5 +51,5 @@ def test_file_base_date(self): def test_file_base_null(self): """Test failure""" self.helper_file_negative( - "test/fixtures/templates/bad/templates/base_null.yaml", 3 + "test/fixtures/templates/bad/templates/base_null.yaml", 2 ) diff --git a/test/unit/rules/transforms/__init__.py b/test/unit/rules/transforms/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unit/rules/transforms/test_configuration.py b/test/unit/rules/transforms/test_configuration.py new file mode 100644 index 0000000000..c55cd87def --- /dev/null +++ b/test/unit/rules/transforms/test_configuration.py @@ -0,0 +1,95 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from collections import deque + +import pytest + +from cfnlint.jsonschema import ValidationError +from cfnlint.rules.transforms.Configuration import Configuration + + +@pytest.fixture(scope="module") +def rule(): + rule = Configuration() + yield rule + + +@pytest.mark.parametrize( + "name,instance,expected", + [ + ( + "Empty list is ok", + [], + [], + ), + ( + "String is ok", + "Foo", + [], + ), + ( + "List is ok", + ["Foo", "Bar"], + [], + ), + ( + "Object is ok", + {"Name": "Foo", "Parameters": {"Bar": "Test"}}, + [], + ), + ( + "Array of objects is ok", + [ + {"Name": "Foo", "Parameters": {"Bar": "Test"}}, + "Foo", + ], + [], + ), + ( + "Missing required Name", + {"Parameters": {"Bar": "Test"}}, + [ + ValidationError( + "'Name' is a required property", + validator="required", + rule=Configuration(), + path=deque([]), + schema_path=deque(["required"]), + ) + ], + ), + ( + "No additional property names are allowed", + {"Name": "Foo", "Foo": "Bar", "Parameters": {"Bar": "Test"}}, + [ + ValidationError( + "Additional properties are not allowed ('Foo' was unexpected)", + validator="additionalProperties", + rule=Configuration(), + path=deque(["Foo"]), + schema_path=deque(["additionalProperties"]), + ) + ], + ), + ( + "Null is not ok", + None, + [ + ValidationError( + "None is not of type 'string', 'array', 'object'", + validator="type", + rule=Configuration(), + path=deque([]), + schema_path=deque(["type"]), + ) + ], + ), + ], +) +def test_validate(name, instance, expected, rule, validator): + errs = list(rule.validate(validator, {}, instance, {})) + + assert errs == expected, f"Test {name!r} got {errs!r}"