Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -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
Expand Down
44 changes: 33 additions & 11 deletions taskcat/_cfn_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)


Expand All @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

# 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,
Expand All @@ -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)
Expand Down Expand Up @@ -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

Expand Down
24 changes: 18 additions & 6 deletions tests/test_cfn_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import yaml

import cfnlint.version
from taskcat._cfn_lint import Lint
from taskcat._config import Config

Expand All @@ -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": {
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion travis-specific-requirements.txt
Original file line number Diff line number Diff line change
@@ -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
Expand Down