From 0d7e95ddd4d6edeec43b2a3beae057fe229c0491 Mon Sep 17 00:00:00 2001 From: David Hutchison Date: Wed, 25 Sep 2024 22:28:22 +0100 Subject: [PATCH 1/5] fix(deps): upgrade cfn-lint range #838 --- pyproject.toml | 2 +- requirements.txt | 2 +- travis-specific-requirements.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 18f5c3f1..d5e95f08 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ name = "taskcat" pathspec = '0.10.3' reprint = "*" tabulate = ">=0.8.2, <1.0" -cfn_lint = ">=0.72.0,<1.0" +cfn_lint = ">=0.72.0,<2.0" setuptools = ">=40.4.3" boto3 = ">=1.9.21,<2.0" botocore = ">=1.12.21,<2.0" diff --git a/requirements.txt b/requirements.txt index e65c80d4..74af45ce 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ pathspec==0.10.3 reprint tabulate>=0.8.2,<1.0 -cfn_lint>=0.72.0,<1.0 +cfn_lint>=0.72.0,<2.0 setuptools>=40.4.3 boto3>=1.9.21,<2.0 botocore>=1.12.21,<2.0 diff --git a/travis-specific-requirements.txt b/travis-specific-requirements.txt index e89acc13..12f5b901 100644 --- a/travis-specific-requirements.txt +++ b/travis-specific-requirements.txt @@ -1,6 +1,6 @@ reprint tabulate>=0.8.2,<1.0 -cfn_lint>=0.13.0,<1.0 +cfn_lint>=0.13.0,<2.0 setuptools>=40.4.3 boto3>=1.9.21,<2.0 botocore>=1.12.21,<2.0 From b9b2314662ed68e03f9a828b13e92d8efb676c15 Mon Sep 17 00:00:00 2001 From: David Hutchison Date: Wed, 25 Sep 2024 23:18:01 +0100 Subject: [PATCH 2/5] fix(tests): support newer cfn-lint versions cfnlint.core no longer exposes REGION, but it has always been available through helpers --- taskcat/_cfn_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taskcat/_cfn_lint.py b/taskcat/_cfn_lint.py index 613702db..2d655a61 100644 --- a/taskcat/_cfn_lint.py +++ b/taskcat/_cfn_lint.py @@ -47,7 +47,7 @@ def __init__(self, config: Config, templates: Templates, strict: bool = False): @staticmethod def _filter_unsupported_regions(regions): - lint_regions = set(cfnlint.core.REGIONS) + lint_regions = set(cfnlint.helpers.REGIONS) if set(regions).issubset(lint_regions): return regions supported = set(regions).intersection(lint_regions) From 6e380bbe1620a9b0295d7da8960ac82a86fec516 Mon Sep 17 00:00:00 2001 From: David Hutchison Date: Wed, 25 Sep 2024 23:29:26 +0100 Subject: [PATCH 3/5] fix(deps): set minimum cfn-lint version to one the tests pass for #838 --- pyproject.toml | 2 +- requirements.txt | 2 +- travis-specific-requirements.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d5e95f08..f181f1cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ name = "taskcat" pathspec = '0.10.3' reprint = "*" tabulate = ">=0.8.2, <1.0" -cfn_lint = ">=0.72.0,<2.0" +cfn_lint = ">=0.78.1,<2.0" setuptools = ">=40.4.3" boto3 = ">=1.9.21,<2.0" botocore = ">=1.12.21,<2.0" diff --git a/requirements.txt b/requirements.txt index 74af45ce..6b57c50a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ pathspec==0.10.3 reprint tabulate>=0.8.2,<1.0 -cfn_lint>=0.72.0,<2.0 +cfn_lint>=0.78.1,<2.0 setuptools>=40.4.3 boto3>=1.9.21,<2.0 botocore>=1.12.21,<2.0 diff --git a/travis-specific-requirements.txt b/travis-specific-requirements.txt index 12f5b901..622b7abc 100644 --- a/travis-specific-requirements.txt +++ b/travis-specific-requirements.txt @@ -1,6 +1,6 @@ reprint tabulate>=0.8.2,<1.0 -cfn_lint>=0.13.0,<2.0 +cfn_lint>=0.78.1,<2.0 setuptools>=40.4.3 boto3>=1.9.21,<2.0 botocore>=1.12.21,<2.0 From 2f5de08ace98adcaff6c8e69728724642f1b4ba1 Mon Sep 17 00:00:00 2001 From: David Hutchison Date: Thu, 26 Sep 2024 23:18:50 +0100 Subject: [PATCH 4/5] fix(pylint): bypass existing failures --- .pre-commit-config.yaml | 2 +- .pylintrc | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4d17c354..a12f0d31 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -84,7 +84,7 @@ repos: - id: pylint-local name: pylint-local description: Run pylint in the local virtualenv - entry: pylint "--disable=C0209" "setup.py" "taskcat/" "bin/" + entry: pylint "--disable=C0209" "taskcat/" "bin/" language: system pass_filenames: false always_run: true diff --git a/.pylintrc b/.pylintrc index 7bcef657..9893de74 100644 --- a/.pylintrc +++ b/.pylintrc @@ -78,7 +78,10 @@ disable=bad-inline-option, logging-format-interpolation, unnecessary-dunder-call, implicit-str-concat, - R0801 + R0801, + too-many-positional-arguments, + broad-exception-raised, + broad-exception-caught # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option # multiple time (only on the command line, not in the configuration file where @@ -509,5 +512,5 @@ min-public-methods=2 # Exceptions that will emit a warning when being caught. Defaults to # "BaseException, Exception". -overgeneral-exceptions=BaseException, - Exception +overgeneral-exceptions=builtins.BaseException, + builtins.Exception From e2ac81556c0717663ba40290faeffb6c81b13d89 Mon Sep 17 00:00:00 2001 From: David Hutchison Date: Thu, 3 Oct 2024 23:14:37 +0100 Subject: [PATCH 5/5] fix(cfn-lint): backwards compatible support for cfn-lint 1.x --- taskcat/_cfn_lint.py | 42 ++++++++++++++++++++++++++++++++---------- tests/test_cfn_lint.py | 24 ++++++++++++++++++------ 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/taskcat/_cfn_lint.py b/taskcat/_cfn_lint.py index 2d655a61..a895293a 100644 --- a/taskcat/_cfn_lint.py +++ b/taskcat/_cfn_lint.py @@ -2,14 +2,26 @@ import re import textwrap +import cfnlint.config import cfnlint.core import cfnlint.helpers +import cfnlint.version from cfnlint.config import ConfigMixIn as CfnLintConfig from jsonschema.exceptions import ValidationError from taskcat._common_utils import neglect_submodule_templates from taskcat._config import Config from taskcat._dataclasses import Templates +# Ignoring linting errors here as pylint doesn't seem to handle the conditional nature +if cfnlint.version.__version__.startswith("0."): + from cfnlint.core import ( # pylint:disable=no-name-in-module, ungrouped-imports + CfnLintExitException, + ) +else: + from cfnlint.runner import ( # pylint:disable=no-name-in-module, ungrouped-imports + CfnLintExitException, + ) + LOG = logging.getLogger(__name__) @@ -32,8 +44,24 @@ def __init__(self, config: Config, templates: Templates, strict: bool = False): except ValidationError as e: LOG.error("Error parsing cfn-lint config file: %s", str(e)) raise + + # There is a change in the way that the cfn lint config class functions between the 0.x and 1.x versions. + # In 1.x, the append_rules property getter includes the default rule set along with the loaded configuration + # https://github.com/aws-cloudformation/cfn-lint/blob/23ee527fadb43e4fd54238eeea5bc3a169175c91/src/cfnlint/config.py#L773 + # In 0.x, it only returned the loaded configuration. + # https://github.com/aws-cloudformation/cfn-lint/blob/f006cb5d8c7056923f3f21b31c14edfeed3804b5/src/cfnlint/config.py#L730 + # + # This causes issues for us as the get_rules method combines the supplied value with the default rule list, + # resulting in a duplicate rule error. get_rules has always behaved this way though, so not sure if we have just missed something + # in the intended approach to calling this. + if cfnlint.version.__version__.startswith("0."): + append_rules = self._cfnlint_config.append_rules + else: + append_rules = self._cfnlint_config.append_rules + append_rules.remove(cfnlint.config._DEFAULT_RULESDIR) + self._rules = cfnlint.core.get_rules( - self._cfnlint_config.append_rules, + append_rules, self._cfnlint_config.ignore_checks, self._cfnlint_config.include_checks, self._cfnlint_config.configure_rules, @@ -91,17 +119,11 @@ def _run_checks(self, template, name, lint_errors, lints): tpath = str(template.template_path) results = [] try: - (_, rules, template_matches) = cfnlint.core.get_template_rules( - tpath, self._cfnlint_config + results = cfnlint.core.run_checks( + tpath, template.template, self._rules, lints[name]["regions"] ) - if template_matches: - results = template_matches - else: - results = cfnlint.core.run_checks( - tpath, template.template, rules, lints[name]["regions"] - ) lints[name]["results"][tpath] = results - except cfnlint.core.CfnLintExitException as e: + except CfnLintExitException as e: lint_errors.add(str(e)) lints[name]["results"][tpath] = results diff --git a/tests/test_cfn_lint.py b/tests/test_cfn_lint.py index b38286d5..75cd1e47 100644 --- a/tests/test_cfn_lint.py +++ b/tests/test_cfn_lint.py @@ -6,6 +6,7 @@ import yaml +import cfnlint.version from taskcat._cfn_lint import Lint from taskcat._config import Config @@ -30,6 +31,22 @@ def __init__(self): "(This code may only work with `package` cli command as the property" "(Resources/Name/Properties/TemplateURL) is a string) matched /private/tmp/lint_test/test-config-three/templates/taskcat_test_template_test1:1" ) +if cfnlint.version.__version__.startswith("0."): + invalid_type_error = [ + f"[E3001: Basic CloudFormation Resource Check] (Invalid or " + f"unsupported Type AWS::Not::Exist for resource Name in " + f"eu-west-1) matched " + f"{test_two_path}:1" + ] +else: + # The format of this message seems to change in 1.3.0 onwards + # (prior 1.x versions were pre-releases so not supporting them) + invalid_type_error = [ + f"[E3006: Validate the CloudFormation resource type] (Resource " + f"type 'AWS::Not::Exist' does not exist in " + f"'eu-west-1') matched " + f"{test_two_path}:1" + ] test_cases = [ { "config": { @@ -71,12 +88,7 @@ def __init__(self): "/tmp/lint_test/test-config-two/templates/taskcat_test_" "template_test1" ).resolve() - ): [ - f"[E3001: Basic CloudFormation Resource Check] (Invalid or " - f"unsupported Type AWS::Not::Exist for resource Name in " - f"eu-west-1) matched " - f"{test_two_path}:1" - ] + ): invalid_type_error }, "template": Path( "/tmp/lint_test/test-config-two/templates/taskcat_test_template"