Merge pull request #839 from dhutchison/task/update-cfn-lint

fix(deps): upgrade cfn-lint range #838

.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

.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

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.78.1,<2.0"
 setuptools = ">=40.4.3"
 boto3 = ">=1.9.21,<2.0"
 botocore = ">=1.12.21,<2.0"

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.78.1,<2.0
 setuptools>=40.4.3
 boto3>=1.9.21,<2.0
 botocore>=1.12.21,<2.0

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,
@@ -47,7 +75,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)
@@ -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
 

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"

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.78.1,<2.0
 setuptools>=40.4.3
 boto3>=1.9.21,<2.0
 botocore>=1.12.21,<2.0