From a3a7982d27ff507da1cd372eb46f12595f64cd9a Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Thu, 9 Feb 2023 17:40:59 +0000 Subject: [PATCH] Load rule ignores from external text file Fixes: #1584 Fixes: #3012 --- .ansible-lint | 8 +++++- .ansible-lint-ignore | 3 +++ .github/workflows/tox.yml | 2 +- docs/configuring.md | 16 ++++++++++++ docs/usage.md | 8 +++++- src/ansiblelint/__main__.py | 13 +++++++++- src/ansiblelint/app.py | 34 ++++++++++++++------------ src/ansiblelint/cli.py | 7 ++++++ src/ansiblelint/formatters/__init__.py | 2 ++ src/ansiblelint/loaders.py | 32 +++++++++++++++++++++++- test/test_app.py | 18 ++++++++++++++ test/test_loaders.py | 8 ++++++ 12 files changed, 130 insertions(+), 21 deletions(-) create mode 100644 .ansible-lint-ignore create mode 100644 test/test_app.py create mode 100644 test/test_loaders.py diff --git a/.ansible-lint b/.ansible-lint index d6ade300f26..dbbdd43a54f 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -44,7 +44,13 @@ use_default_rules: true # rulesdir: # - ./rule/directory/ -# Ansible-lint completely ignores rules or tags listed below +# Ansible-lint is able to recognize and load skip rules stored inside +# `.ansible-lint-ignore` files. To skip a rule just enter filename and tag, +# like "playbook.yml package-latest" on a new line. Optionally you can add +# comments after the tag, prefixed by "#". We discourage the use of skip_list +# below because that will hide violations from the output. When putting ignores +# inside the ignore file, they are marked as ignored, but still visible, making +# it easier to address later. skip_list: - skip_this_tag diff --git a/.ansible-lint-ignore b/.ansible-lint-ignore new file mode 100644 index 00000000000..dae2afe3190 --- /dev/null +++ b/.ansible-lint-ignore @@ -0,0 +1,3 @@ +# See https://ansible-lint.readthedocs.io/configuring/#ignoring-rules-for-entire-files +playbook2.yml package-latest # comment +playbook2.yml foo-bar diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index e4d37f2a4d8..fc128aa47e8 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -70,7 +70,7 @@ jobs: WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY # Number of expected test passes, safety measure for accidental skip of # tests. Update value if you add/remove tests. - PYTEST_REQPASS: 788 + PYTEST_REQPASS: 790 steps: - name: Activate WSL1 diff --git a/docs/configuring.md b/docs/configuring.md index 4355be2fad4..4210e9eaaa7 100644 --- a/docs/configuring.md +++ b/docs/configuring.md @@ -38,6 +38,22 @@ counterparts: --8<-- ".ansible-lint" ``` +## Ignoring rules for entire files + +Ansible-lint will load skip rules from `.ansible-lint-ignore` file that is +adjacent to its config file. The file format is very simple, containing the +filename and the rule to be ignored. It also supports comments starting with +`#`. + +```yaml title=".ansible-lint-ignore" +# this is just a comment +playbook.yml package-latest # disable package-latest rule for playbook.yml +playbook.yml deprecated-module +``` + +The file can also be created by adding `--generate-ignore` to the command line. +Keep in mind that this will override any existing file content. + ## Pre-commit setup To use Ansible-lint with [pre-commit], add the following to the diff --git a/docs/usage.md b/docs/usage.md index e27f5a62d4d..bca27691126 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -40,7 +40,13 @@ Ansible-lint creates a new cache on the next invocation. You should add the `.cache` folder to the `.gitignore` file in your git repositories. -## Using progressive mode +## Using progressive mode (deprecated) + +!!! warning + + This feature is deprecated and will be removed in the next major release. + We encourage you to use [ignore file](configuring.md#ignoring-rules-for-entire-files) + instead. For easier adoption, Ansible-lint can alert for rule violations that occur since the last commit. This allows new code to be merged without any rule violations diff --git a/src/ansiblelint/__main__.py b/src/ansiblelint/__main__.py index a0d60b1eadd..f3f215bc15d 100755 --- a/src/ansiblelint/__main__.py +++ b/src/ansiblelint/__main__.py @@ -50,6 +50,7 @@ from ansiblelint.config import get_version_warning, options from ansiblelint.constants import EXIT_CONTROL_C_RC, GIT_CMD, LOCK_TIMEOUT_RC from ansiblelint.file_utils import abspath, cwd, normpath +from ansiblelint.loaders import load_ignore_txt from ansiblelint.skip_utils import normalize_tag from ansiblelint.version import __version__ @@ -216,6 +217,11 @@ def main(argv: list[str] | None = None) -> int: # noqa: C901 _logger.debug("Options: %s", options) _logger.debug(os.getcwd()) + if options.progressive: + _logger.warning( + "Progressive mode is deprecated and will be removed in next major version, use ignore files instead: https://ansible-lint.readthedocs.io/configuring/#ignoring-rules-for-entire-files" + ) + if not options.offline: # pylint: disable=import-outside-toplevel from ansiblelint.schemas import refresh_schemas @@ -282,8 +288,13 @@ def main(argv: list[str] | None = None) -> int: # noqa: C901 if options.strict and result.matches: mark_as_success = False - # Remove skipped list items from the result + # Remove skipped_list items from the result result.matches = [m for m in result.matches if m.tag not in app.options.skip_list] + # Mark matches as ignored inside ignore file + ignore_map = load_ignore_txt() + for match in result.matches: + if match.tag in ignore_map[match.filename]: + match.ignored = True app.render_matches(result.matches) diff --git a/src/ansiblelint/app.py b/src/ansiblelint/app.py index ad71f42a9a0..1aed9a27729 100644 --- a/src/ansiblelint/app.py +++ b/src/ansiblelint/app.py @@ -18,6 +18,7 @@ from ansiblelint.config import options as default_options from ansiblelint.constants import RULE_DOC_URL, SUCCESS_RC, VIOLATIONS_FOUND_RC from ansiblelint.errors import MatchError +from ansiblelint.loaders import IGNORE_TXT from ansiblelint.stats import SummarizedResults, TagStats if TYPE_CHECKING: @@ -101,6 +102,10 @@ def count_results(self, matches: list[MatchError]) -> SummarizedResults: result = SummarizedResults() for match in matches: + # any ignores match counts as a warning + if match.ignored: + result.warnings += 1 + continue # tag can include a sub-rule id: `yaml[document-start]` # rule.id is the generic rule id: `yaml` # *rule.tags is the list of the rule's tags (categories): `style` @@ -162,24 +167,21 @@ def report_outcome(self, result: LintResult, mark_as_success: bool = False) -> i matched_rules = self._get_matched_skippable_rules(result.matches) - entries = [] - for key in sorted(matched_rules.keys()): - if {key, *matched_rules[key].tags}.isdisjoint(self.options.warn_list): - entries.append(f" - {key} # {matched_rules[key].shortdesc}\n") - for match in result.matches: - if "experimental" in match.rule.tags: - entries.append(" - experimental # all rules tagged as experimental\n") - break - if entries and not self.options.quiet: + if matched_rules and self.options.generate_ignore: + console_stderr.print(f"Writing ignore file to {IGNORE_TXT}") + lines = set() + for rule in result.matches: + lines.add(f"{rule.filename} {rule.tag}") + with open(IGNORE_TXT, "w", encoding="utf-8") as ignore_file: + ignore_file.write( + "# This file contains ignores rule violations for ansible-lint\n" + ) + ignore_file.writelines(sorted(list(lines))) + ignore_file.write("\n") + elif matched_rules and not self.options.quiet: console_stderr.print( - "You can skip specific rules or tags by adding them to your " - "configuration file:" + "Read [link=https://ansible-lint.readthedocs.io/configuring/#ignoring-rules-for-entire-files]documentation[/link] for instructions on how to ignore specific rule violations." ) - msg += """\ -# .config/ansible-lint.yml -warn_list: # or 'skip_list' to silence them completely -""" - msg += "".join(sorted(entries)) # Do not deprecate the old tags just yet. Why? Because it is not currently feasible # to migrate old tags to new tags. There are a lot of things out there that still diff --git a/src/ansiblelint/cli.py b/src/ansiblelint/cli.py index 628c9b48640..9540d233a63 100644 --- a/src/ansiblelint/cli.py +++ b/src/ansiblelint/cli.py @@ -368,6 +368,13 @@ def get_cli_parser() -> argparse.ArgumentParser: help="only check rules whose id/tags do not match these values. \ e.g: --skip-list=name,run-once", ) + parser.add_argument( + "--generate-ignore", + dest="generate_ignore", + action="store_true", + default=False, + help="Generate a text file '.ansible-lint-ignore' that ignores all found violations. Each line contains filename and rule id separated by a space.", + ) parser.add_argument( "-w", "--warn-list", diff --git a/src/ansiblelint/formatters/__init__.py b/src/ansiblelint/formatters/__init__.py index 71d4012112d..2914f6d640a 100644 --- a/src/ansiblelint/formatters/__init__.py +++ b/src/ansiblelint/formatters/__init__.py @@ -61,6 +61,8 @@ def format(self, match: MatchError) -> str: result = f"[{match.level}][bold][link={match.rule.url}]{self.escape(match.tag)}[/link][/][/][dim]:[/] [{match.level}]{self.escape(match.message)}[/]" if match.level != "error": result += f" [dim][{match.level}]({match.level})[/][/]" + if match.ignored: + result += " [dim]# ignored[/]" result += ( "\n" f"[filename]{self._format_path(match.filename or '')}[/]:{match.position}" diff --git a/src/ansiblelint/loaders.py b/src/ansiblelint/loaders.py index 431b79c1275..61087018fbb 100644 --- a/src/ansiblelint/loaders.py +++ b/src/ansiblelint/loaders.py @@ -1,6 +1,9 @@ """Utilities for loading various files.""" from __future__ import annotations +import logging +import os +from collections import defaultdict from functools import partial from pathlib import Path from typing import Any @@ -14,8 +17,10 @@ except (ImportError, AttributeError): from yaml import FullLoader, SafeLoader # type: ignore +IGNORE_TXT = ".ansible-lint-ignore" yaml_load = partial(yaml.load, Loader=FullLoader) yaml_load_safe = partial(yaml.load, Loader=SafeLoader) +_logger = logging.getLogger(__name__) def yaml_from_file(filepath: str | Path) -> Any: @@ -24,4 +29,29 @@ def yaml_from_file(filepath: str | Path) -> Any: return yaml_load(content) -__all__ = ["yaml_from_file", "yaml_load", "yaml_load_safe", "YAMLError"] +def load_ignore_txt(filepath: str | Path = IGNORE_TXT) -> dict[str, set[str]]: + """Return a list of rules to ignore.""" + result = defaultdict(set) + if os.path.isfile(filepath): + with open(str(filepath), encoding="utf-8") as content: + _logger.debug("Loading ignores from %s", filepath) + for line in content: + entry = line.split("#")[0].rstrip() + if entry: + try: + path, rule = entry.split() + except ValueError as exc: + raise RuntimeError( + f"Unable to parse line '{line}' from {filepath} file." + ) from exc + result[path].add(rule) + return result + + +__all__ = [ + "load_ignore_txt", + "yaml_from_file", + "yaml_load", + "yaml_load_safe", + "YAMLError", +] diff --git a/test/test_app.py b/test/test_app.py new file mode 100644 index 00000000000..0dc31cb69b6 --- /dev/null +++ b/test/test_app.py @@ -0,0 +1,18 @@ +"""Test for app module.""" +from pathlib import Path + +from ansiblelint.file_utils import Lintable +from ansiblelint.testing import run_ansible_lint + + +def test_generate_ignore(tmp_path: Path) -> None: + """Validate that --generate-ignore dumps expected ignore to the file.""" + lintable = Lintable(tmp_path / "playbook.yaml") + lintable.content = "foo: bar" + lintable.write(force=True) + assert not (tmp_path / ".ansible-lint-ignore").exists() + result = run_ansible_lint(lintable.filename, "--generate-ignore", cwd=str(tmp_path)) + assert result.returncode == 2 + assert (tmp_path / ".ansible-lint-ignore").exists() + with open(tmp_path / ".ansible-lint-ignore", encoding="utf-8") as f: + assert "playbook.yaml syntax-check[specific]\n" in f.readlines() diff --git a/test/test_loaders.py b/test/test_loaders.py new file mode 100644 index 00000000000..d07c26e92f0 --- /dev/null +++ b/test/test_loaders.py @@ -0,0 +1,8 @@ +"""Tests for loaders submodule.""" +from ansiblelint.loaders import load_ignore_txt + + +def test_load_ignore_txt() -> None: + """Test load_ignore_txt.""" + result = load_ignore_txt(".ansible-lint-ignore") + assert result == {"playbook2.yml": {"foo-bar", "package-latest"}}