diff --git a/src/cfnlint/config.py b/src/cfnlint/config.py index 33ad3ff411..63db113e63 100644 --- a/src/cfnlint/config.py +++ b/src/cfnlint/config.py @@ -327,6 +327,64 @@ def __call__(self, parser, namespace, values, option_string=None): parser.exit() +class ExtendKeyValuePairs(argparse.Action): + def __init__( + self, + option_strings, + dest, + nargs=None, + const=None, + default=None, + type=None, + choices=None, + required=False, + help=None, + metavar=None, + ): # pylint: disable=W0622 + super().__init__( + option_strings=option_strings, + dest=dest, + nargs=nargs, + const=const, + default=default, + type=type, + choices=choices, + required=required, + help=help, + metavar=metavar, + ) + + def __call__(self, parser, namespace, values, option_string=None): + try: + items = {} + for value in values: + # split it into key and value + key, value = value.split("=", 1) + items[key.strip()] = value.strip() + + result = getattr(namespace, self.dest) + [items] + setattr(namespace, self.dest, result) + except Exception: # pylint: disable=W0703 + parser.print_help() + parser.exit() + + +class ExtendAction(argparse.Action): + """Support argument types that are lists and can + be specified multiple times. + """ + + def __call__(self, parser, namespace, values, option_string=None): + items = getattr(namespace, self.dest) + items = [] if items is None else items + for value in values: + if isinstance(value, list): + items.extend(value) + else: + items.append(value) + setattr(namespace, self.dest, items) + + class CliArgs: """Base Args class""" @@ -344,21 +402,6 @@ def error(self, message): self.print_help(sys.stderr) self.exit(32, f"{self.prog}: error: {message}\n") - class ExtendAction(argparse.Action): - """Support argument types that are lists and can - be specified multiple times. - """ - - def __call__(self, parser, namespace, values, option_string=None): - items = getattr(namespace, self.dest) - items = [] if items is None else items - for value in values: - if isinstance(value, list): - items.extend(value) - else: - items.append(value) - setattr(namespace, self.dest, items) - usage = ( "\nBasic: cfn-lint test.yaml\n" "Ignore a rule: cfn-lint -i E3012 -- test.yaml\n" @@ -368,10 +411,15 @@ def __call__(self, parser, namespace, values, option_string=None): parser = ArgumentParser(description="CloudFormation Linter", usage=usage) parser.register("action", "extend", ExtendAction) + parser.register("action", "rule_configuration", RuleConfigurationAction) + parser.register("action", "extend_key_value", ExtendKeyValuePairs) standard = parser.add_argument_group("Standard") advanced = parser.add_argument_group("Advanced / Debugging") + validation_group = standard.add_mutually_exclusive_group() + parameter_group = standard.add_mutually_exclusive_group() + # Allow the template to be passes as an optional or a positional argument standard.add_argument( "templates", @@ -379,7 +427,7 @@ def __call__(self, parser, namespace, values, option_string=None): nargs="*", help="The CloudFormation template to be linted", ) - standard.add_argument( + validation_group.add_argument( "-t", "--template", metavar="TEMPLATE", @@ -403,7 +451,7 @@ def __call__(self, parser, namespace, values, option_string=None): default=[], action="extend", ) - standard.add_argument( + validation_group.add_argument( "--deployment-files", dest="deployment_files", help="Deployment files", @@ -411,14 +459,13 @@ def __call__(self, parser, namespace, values, option_string=None): default=[], action="extend", ) - standard.add_argument( + parameter_group.add_argument( "-tp", "--template-parameters", dest="template_parameters", - metavar="KEY=VALUE", nargs="+", - default={}, - action=key_value, + default=[], + action="extend_key_value", help="only check rules whose id do not match these values", ) advanced.add_argument( @@ -509,7 +556,7 @@ def __call__(self, parser, namespace, values, option_string=None): dest="configure_rules", nargs="+", default={}, - action=RuleConfigurationAction, + action="rule_configuration", help=( "Provide configuration for a rule. Format RuleId:key=value. Example:" " E3012:strict=true" @@ -614,15 +661,15 @@ def set_template_args(self, template): if isinstance(configs, dict): for key, value in { - "ignore_checks": (list), - "regions": (list), "append_rules": (list), - "override_spec": (str), + "configure_rules": (dict), "custom_rules": (str), "ignore_bad_template": (bool), + "ignore_checks": (list), "include_checks": (list), - "configure_rules": (dict), "include_experimental": (bool), + "override_spec": (str), + "regions": (list), }.items(): if key in configs: if isinstance(configs[key], value): @@ -635,17 +682,18 @@ def set_template_args(self, template): class ManualArgs(TypedDict, total=False): configure_rules: dict[str, dict[str, Any]] - include_checks: list[str] - ignore_checks: list[str] - template_parameters: dict[str, Any] - mandatory_checks: list[str] - include_experimental: bool + deployment_files: list[str] ignore_bad_template: bool + ignore_checks: list[str] ignore_templates: list + include_checks: list[str] + include_experimental: bool + mandatory_checks: list[str] merge_configs: bool non_zero_exit_code: str output_file: str regions: list + template_parameters: list[dict[str, Any]] # pylint: disable=too-many-public-methods @@ -665,24 +713,25 @@ def __init__(self, cli_args: list[str] | None = None, **kwargs: Unpack[ManualArg def __repr__(self): return format_json_string( { + "append_rules": self.append_rules, + "config_file": self.config_file, + "configure_rules": self.configure_rules, + "custom_rules": self.custom_rules, + "debug": self.debug, + "deployment_files": self.deployment_files, + "format": self.format, + "ignore_bad_template": self.ignore_bad_template, "ignore_checks": self.ignore_checks, "include_checks": self.include_checks, - "mandatory_checks": self.mandatory_checks, - "template_parameters": self.template_parameters, "include_experimental": self.include_experimental, - "configure_rules": self.configure_rules, - "regions": self.regions, - "ignore_bad_template": self.ignore_bad_template, - "debug": self.debug, "info": self.info, - "format": self.format, - "templates": self.templates, - "append_rules": self.append_rules, - "override_spec": self.override_spec, - "custom_rules": self.custom_rules, - "config_file": self.config_file, + "mandatory_checks": self.mandatory_checks, "merge_configs": self.merge_configs, "non_zero_exit_code": self.non_zero_exit_code, + "override_spec": self.override_spec, + "regions": self.regions, + "template_parameters": self.template_parameters, + "templates": self.templates, } ) @@ -845,7 +894,7 @@ def template_parameters(self): return self._get_argument_value("template_parameters", True, True) @template_parameters.setter - def template_parameters(self, template_parameters: dict[str, Any]): + def template_parameters(self, template_parameters: list[dict[str, Any]]): self._manual_args["template_parameters"] = template_parameters @property diff --git a/src/cfnlint/data/CfnLintCli/config/schema.json b/src/cfnlint/data/CfnLintCli/config/schema.json index f72fc6189f..a32cc88b4b 100644 --- a/src/cfnlint/data/CfnLintCli/config/schema.json +++ b/src/cfnlint/data/CfnLintCli/config/schema.json @@ -58,6 +58,12 @@ "description": "custom rule file to use", "type": "string" }, + "deployment_files": { + "items": { + "type": "string" + }, + "type": "array" + }, "ignore_bad_template": { "description": "Ignore bad templates", "type": "boolean" @@ -116,6 +122,22 @@ }, "type": "array" }, + "template_parameters": { + "items": { + "patternProperties": { + "^.*$": { + "type": [ + "string", + "integer", + "boolean", + "number" + ] + } + }, + "type": "object" + }, + "type": "array" + }, "templates": { "description": "Templates to lint", "items": { diff --git a/src/cfnlint/data/schemas/other/deployment_files/git_sync.json b/src/cfnlint/data/schemas/other/deployment_files/git_sync.json new file mode 100644 index 0000000000..c0c3f7d867 --- /dev/null +++ b/src/cfnlint/data/schemas/other/deployment_files/git_sync.json @@ -0,0 +1,28 @@ +{ + "additionalProperties": false, + "properties": { + "parameters": { + "patternProperties": { + "^.+$": { + "type": "string" + } + }, + "type": "object" + }, + "tags": { + "patternProperties": { + "^.+$": { + "type": "string" + } + }, + "type": "object" + }, + "template-file-path": { + "type": "string" + } + }, + "required": [ + "template-file-path" + ], + "type": "object" +} diff --git a/src/cfnlint/rules/_rule.py b/src/cfnlint/rules/_rule.py index 631b40903a..f340315356 100644 --- a/src/cfnlint/rules/_rule.py +++ b/src/cfnlint/rules/_rule.py @@ -94,6 +94,16 @@ def __init__(self, path: Path, message: str, **kwargs): for k, v in kwargs.items(): setattr(self, k, v) + def __repr__(self): + return cfnlint.helpers.format_json_string( + { + "path": self.path, + "path_string": self.path_string, + "message": self.message, + "context": self.context, + } + ) + def __eq__(self, item): """ Override the equality comparison operator to compare rule diff --git a/src/cfnlint/rules/deployment_files/Configuration.py b/src/cfnlint/rules/deployment_files/Configuration.py new file mode 100644 index 0000000000..90c1f3854a --- /dev/null +++ b/src/cfnlint/rules/deployment_files/Configuration.py @@ -0,0 +1,33 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from __future__ import annotations + +from cfnlint._typing import Any, RuleMatches +from cfnlint.jsonschema import StandardValidator +from cfnlint.rules.jsonschema.Base import BaseJsonSchema + + +class Configuration(BaseJsonSchema): + + id = "E0100" + shortdesc = "Validate deployment file configuration" + description = ( + "Validate if a deployment file has the correct syntax " + "for one of the supported formats" + ) + source_url = "https://github.com/aws-cloudformation/cfn-lint" + tags = ["base"] + + def validate_deployment_file( + self, data: dict[str, Any], schema: dict[str, Any] + ) -> RuleMatches: + matches = [] + + validator = StandardValidator(schema) + + matches.extend(self.json_schema_validate(validator, data, [])) + + return matches diff --git a/src/cfnlint/rules/parameters/DeploymentParameters.py b/src/cfnlint/rules/deployment_files/Parameters.py similarity index 97% rename from src/cfnlint/rules/parameters/DeploymentParameters.py rename to src/cfnlint/rules/deployment_files/Parameters.py index 8c9dcfe6da..ed031aabd2 100644 --- a/src/cfnlint/rules/parameters/DeploymentParameters.py +++ b/src/cfnlint/rules/deployment_files/Parameters.py @@ -11,9 +11,7 @@ from cfnlint.rules.jsonschema.CfnLintJsonSchema import CfnLintJsonSchema -class DeploymentParameters(CfnLintJsonSchema): - """Check if Parameters are configured correctly""" - +class Parameters(CfnLintJsonSchema): id = "E2900" shortdesc = ( "Validate deployment file parameters are valid against template parameters" diff --git a/src/cfnlint/rules/deployment_files/__init__.py b/src/cfnlint/rules/deployment_files/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/cfnlint/runner/deployment_file/deployment_types/git_sync.py b/src/cfnlint/runner/deployment_file/deployment_types/git_sync.py index f73ca2cde6..6af58ebd92 100644 --- a/src/cfnlint/runner/deployment_file/deployment_types/git_sync.py +++ b/src/cfnlint/runner/deployment_file/deployment_types/git_sync.py @@ -7,16 +7,28 @@ from typing import Any +import cfnlint.data.schemas.other.deployment_files +from cfnlint._typing import RuleMatches +from cfnlint.helpers import load_resource +from cfnlint.rules.deployment_files.Configuration import Configuration from cfnlint.runner.deployment_file.deployment import Deployment -def create_deployment_from_git_sync(data: dict[str, Any]) -> Deployment: +def create_deployment_from_git_sync( + data: dict[str, Any] +) -> tuple[Deployment | None, RuleMatches | None]: - template_file_path = data.get("template-file-path") - if not template_file_path: - raise ValueError("template-file-path is required") - parameters = data.get("parameters", {}) - tags = data.get("tags", {}) - return Deployment( - template_file_path=template_file_path, parameters=parameters, tags=tags + schema = load_resource(cfnlint.data.schemas.other.deployment_files, "git_sync.json") + matches = Configuration().validate_deployment_file(data, schema) + if matches: + return None, matches + + template_file_path: str = data.get("template-file-path", "") + parameters: dict[str, Any] = data.get("parameters", {}) + tags: dict[str, Any] = data.get("tags", {}) + return ( + Deployment( + template_file_path=template_file_path, parameters=parameters, tags=tags + ), + None, ) diff --git a/src/cfnlint/runner/deployment_file/runner.py b/src/cfnlint/runner/deployment_file/runner.py index bbc5064416..3d296fdcab 100644 --- a/src/cfnlint/runner/deployment_file/runner.py +++ b/src/cfnlint/runner/deployment_file/runner.py @@ -13,8 +13,8 @@ import cfnlint.runner.deployment_file.deployment_types from cfnlint.config import ConfigMixIn from cfnlint.decode import decode -from cfnlint.rules import Match, Rules -from cfnlint.runner.exceptions import CfnLintExitException +from cfnlint.rules import Match, RuleMatch, Rules +from cfnlint.rules.deployment_files.Configuration import Configuration from cfnlint.runner.template import run_template_by_file_path LOGGER = logging.getLogger(__name__) @@ -45,26 +45,52 @@ def run_deployment_file( if config.ignore_bad_template: ignore_bad_template = True + all_matches: list[RuleMatch] = [] for plugin in cfnlint.runner.deployment_file.deployment_types.__all__: + deployment_data, deployment_matches = getattr( + cfnlint.runner.deployment_file.deployment_types, plugin + )(data) + if deployment_matches: + all_matches.extend(deployment_matches) + continue try: - deployment_data = getattr( - cfnlint.runner.deployment_file.deployment_types, plugin - )(data) - template_path = Path(filename).parent / deployment_data.template_file_path - template_config = deepcopy(config) - template_config.template_parameters = deployment_data.parameters - - yield from run_template_by_file_path( - template_path, template_config, rules, ignore_bad_template + template_path = ( + (Path(filename).parent / deployment_data.template_file_path) + .resolve() + .relative_to(Path.cwd()) ) - return - except Exception as e: - LOGGER.info(e) - continue + except ValueError: + LOGGER.debug( + ( + f"Template file path {deployment_data.template_file_path!r} " + "is not relative to the current working directory" + ) + ) + template_path = Path(filename).parent / deployment_data.template_file_path + template_config = deepcopy(config) + template_config.template_parameters = [deployment_data.parameters] + + yield from run_template_by_file_path( + filename=template_path, + config=template_config, + rules=rules, + ignore_bad_template=ignore_bad_template, + ) + return - raise CfnLintExitException( - f"Deployment file {filename} didn't meet any supported deployment file format", - 1, + for match in all_matches: + LOGGER.debug( + f"While tring to process deployment file got error: {match.message}" + ) + + yield Match( + linenumber=1, + columnnumber=1, + linenumberend=1, + columnnumberend=1, + filename=filename, + message=f"Deployment file {filename!r} is not supported", + rule=Configuration(), ) diff --git a/src/cfnlint/runner/template/runner.py b/src/cfnlint/runner/template/runner.py index 884543c460..666236c7e1 100644 --- a/src/cfnlint/runner/template/runner.py +++ b/src/cfnlint/runner/template/runner.py @@ -71,13 +71,10 @@ def _check_metadata_directives( yield match -def _run_template( - filename: str | None, template: Any, config: ConfigMixIn, rules: Rules +def _run_template_per_config( + cfn: Template, config: ConfigMixIn, rules: Rules ) -> Iterator[Match]: - config.set_template_args(template) - cfn = Template(filename, template, config.regions, config.template_parameters) - LOGGER.info("Run scan of template %s", cfn.filename) if not set(config.regions).issubset(set(REGIONS)): unsupported_regions = list(set(config.regions).difference(set(REGIONS))) @@ -98,15 +95,29 @@ def _run_template( if cfn.template is not None: if config.build_graph: cfn.build_graph() - yield from _dedup( - _check_metadata_directives( - rules.run(filename=cfn.filename, cfn=cfn, config=config), - cfn=cfn, - config=config, - ) + yield from _check_metadata_directives( + rules.run(filename=cfn.filename, cfn=cfn, config=config), + cfn=cfn, + config=config, ) +def _run_template( + filename: str | None, template: Any, config: ConfigMixIn, rules: Rules +) -> Iterator[Match]: + + config.set_template_args(template) + if config.template_parameters: + matches: list[Match] = [] + for template_parameters in config.template_parameters: + cfn = Template(filename, template, config.regions, template_parameters) + matches.extend(list(_run_template_per_config(cfn, config, rules))) + yield from _dedup(iter(matches)) + else: + cfn = Template(filename, template, config.regions) + yield from _dedup(_run_template_per_config(cfn, config, rules)) + + def run_template_by_file_path( filename: str | None, config: ConfigMixIn, rules: Rules, ignore_bad_template: bool ) -> Iterator[Match]: diff --git a/test/unit/module/runner/deployment_file/__init__.py b/test/unit/module/runner/deployment_file/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unit/module/runner/deployment_file/deployment_types/__init__.py b/test/unit/module/runner/deployment_file/deployment_types/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unit/module/runner/deployment_file/deployment_types/test_git_sync.py b/test/unit/module/runner/deployment_file/deployment_types/test_git_sync.py new file mode 100644 index 0000000000..e35b18f123 --- /dev/null +++ b/test/unit/module/runner/deployment_file/deployment_types/test_git_sync.py @@ -0,0 +1,127 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +import pytest + +from cfnlint.rules import RuleMatch +from cfnlint.runner.deployment_file.deployment import Deployment +from cfnlint.runner.deployment_file.deployment_types import ( + create_deployment_from_git_sync, +) + + +@pytest.mark.parametrize( + "name, instance, expected", + [ + ( + "A standard git sync file", + { + "template-file-path": "../a/path", + "parameters": { + "Foo": "Bar", + }, + "tags": { + "Key": "Value", + }, + }, + ( + Deployment( + template_file_path="../a/path", + parameters={ + "Foo": "Bar", + }, + tags={ + "Key": "Value", + }, + ), + None, + ), + ), + ( + "Bad template-file-path type", + { + "template-file-path": ["../a/path"], + "parameters": { + "Foo": "Bar", + }, + "tags": { + "Key": "Value", + }, + }, + ( + None, + [ + RuleMatch( + message="['../a/path'] is not of type 'string'", + path=["template-file-path"], + ) + ], + ), + ), + ( + "No template file path", + { + "parameters": { + "Foo": "Bar", + }, + "tags": { + "Key": "Value", + }, + }, + ( + None, + [ + RuleMatch( + message="'template-file-path' is a required property", + path=[], + ) + ], + ), + ), + ( + "Bad type on parameters", + { + "template-file-path": "../a/path", + "parameters": ["Foo=Bar"], + "tags": { + "Key": "Value", + }, + }, + ( + None, + [ + RuleMatch( + message="['Foo=Bar'] is not of type 'object'", + path=["parameters"], + ) + ], + ), + ), + ( + "Bad type on tags", + { + "template-file-path": "../a/path", + "parameters": { + "Foo": "Bar", + }, + "tags": ["Foo=Bar"], + }, + ( + None, + [ + RuleMatch( + message="['Foo=Bar'] is not of type 'object'", + path=["tags"], + ) + ], + ), + ), + ], +) +def test_git_sync(name, instance, expected): + + results = create_deployment_from_git_sync(instance) + + assert results == expected, f"{name}: {results} != {expected}" diff --git a/test/unit/module/runner/deployment_file/test_runner.py b/test/unit/module/runner/deployment_file/test_runner.py new file mode 100644 index 0000000000..c33701fe96 --- /dev/null +++ b/test/unit/module/runner/deployment_file/test_runner.py @@ -0,0 +1,142 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from pathlib import Path +from unittest.mock import patch + +import pytest + +from cfnlint.config import ConfigMixIn +from cfnlint.rules import Match +from cfnlint.rules.deployment_files.Configuration import Configuration +from cfnlint.runner.deployment_file import run_deployment_file + +_filename = "deployment-file.yaml" + + +@pytest.mark.parametrize( + "name, decode, validate_template_parameters, validate_template_return, expected", + [ + ( + "A standard git sync file", + ( + { + "template-file-path": "../a/path", + "parameters": { + "Foo": "Bar", + }, + "tags": { + "Key": "Value", + }, + }, + [], + ), + { + "filename": Path("../a/path"), + "template_parameters": [{"Foo": "Bar"}], + }, + [], + [], + ), + ( + "Bad template-file-path type", + ( + { + "template-file-path": ["../a/path"], + "parameters": { + "Foo": "Bar", + }, + "tags": { + "Key": "Value", + }, + }, + [], + ), + {}, + [], + [ + Match( + linenumber=1, + columnnumber=1, + linenumberend=1, + columnnumberend=1, + filename=_filename, + message=f"Deployment file {_filename!r} is not supported", + rule=Configuration(), + ) + ], + ), + ( + "Bad template-file-path type", + ( + { + "template-file-path": "../a/path", + "parameters": { + "Foo": "Bar", + }, + "tags": { + "Key": "Value", + }, + }, + [], + ), + { + "filename": Path("../a/path"), + "template_parameters": [{"Foo": "Bar"}], + }, + iter( + [ + Match( + linenumber=1, + columnnumber=1, + linenumberend=1, + columnnumberend=1, + filename=_filename, + message=f"Deployment file {_filename!r} is not supported", + rule=Configuration(), + ) + ] + ), + [ + Match( + linenumber=1, + columnnumber=1, + linenumberend=1, + columnnumberend=1, + filename=_filename, + message=f"Deployment file {_filename!r} is not supported", + rule=Configuration(), + ) + ], + ), + ], +) +def test_runner( + name, decode, validate_template_parameters, validate_template_return, expected +): + + with patch( + "cfnlint.runner.deployment_file.runner.decode", return_value=decode + ) as mock_decode: + with patch( + "cfnlint.runner.deployment_file.runner.run_template_by_file_path", + return_value=validate_template_return, + ) as mock_run_template_by_file_path: + deployment = list(run_deployment_file(_filename, ConfigMixIn(), None)) + + mock_decode.assert_called_once() + if validate_template_parameters: + mock_run_template_by_file_path.assert_called_once() + config = mock_run_template_by_file_path.call_args_list + assert config[0].kwargs.get( + "config" + ).template_parameters == validate_template_parameters.get( + "template_parameters" + ) + assert config[0].kwargs.get( + "filename" + ) == validate_template_parameters.get("filename") + + assert deployment == expected, f"{name}: {deployment} != {expected}" diff --git a/test/unit/module/runner/template/__init__.py b/test/unit/module/runner/template/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unit/module/runner/test_template_runner.py b/test/unit/module/runner/template/test_template_runner.py similarity index 100% rename from test/unit/module/runner/test_template_runner.py rename to test/unit/module/runner/template/test_template_runner.py diff --git a/test/unit/rules/resources/ec2/test_private_ip_with_network_interafce.py b/test/unit/rules/resources/ec2/test_private_ip_with_network_interafce.py index 23f087af94..ea3615250c 100644 --- a/test/unit/rules/resources/ec2/test_private_ip_with_network_interafce.py +++ b/test/unit/rules/resources/ec2/test_private_ip_with_network_interafce.py @@ -190,11 +190,6 @@ def rule(): def test_validate(name, instance, expected, rule, validator): errs = list(rule.validate(validator, "", instance, {})) - for err in errs: - print(err.validator) - print(err.path) - print(err.schema_path) - assert ( errs == expected ), f"Expected test {name!r} to have {expected!r} but got {errs!r}"