Skip to content

Commit

Permalink
Limit the maximum block depth (#3602)
Browse files Browse the repository at this point in the history
Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>
  • Loading branch information
Ruchip16 and ssbarnea authored Aug 21, 2023
1 parent f6961ff commit 385ad82
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 18 deletions.
4 changes: 4 additions & 0 deletions .ansible-lint
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,7 @@ kinds:

# Allow setting custom prefix for name[prefix] rule
task_name_prefix: "{stem} | "
# Complexity related settings

# Limit the depth of the nested blocks:
# max_block_depth: 20
14 changes: 14 additions & 0 deletions examples/playbooks/rule-complexity-fail.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,17 @@
- name: Task 6
ansible.builtin.debug:
msg: "This is task 6"

- name: Block Task 7
block:
- name: 2nd level block
block:
- name: 3rd level block
block:
- name: 4th level block
block:
- name: 5th level block
block:
- name: Nested Task 1
ansible.builtin.debug:
msg: "This is nested task 1"
16 changes: 14 additions & 2 deletions examples/playbooks/rule-complexity-pass.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,17 @@
msg: "This is task 4"

- name: Task 5
ansible.builtin.debug:
msg: "This is task 5"
block:
- name: Include under block level 1
ansible.builtin.debug:
msg: "This is nested block"
- name: Block level 2
block:
- name: Include under block level 2
ansible.builtin.debug:
msg: "This is block 2"
- name: Block level 3
block:
- name: INCLUDE under block level 3
ansible.builtin.debug:
msg: "This is block 3"
1 change: 1 addition & 0 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ class Options: # pylint: disable=too-many-instance-attributes,too-few-public-me
list_profiles: bool = False # display profiles command
ignore_file: Path | None = None
max_tasks: int = 100
max_block_depth: int = 20


options = Options()
Expand Down
12 changes: 11 additions & 1 deletion src/ansiblelint/rules/complexity.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,17 @@
This rule aims to warn about Ansible content that seems to be overly complex,
suggesting refactoring for better readability and maintainability.

## complexity[tasks]

`complexity[tasks]` will be triggered if the total number of tasks inside a file
is above 100. If encountered, you are should consider using
is above 100. If encountered, you should consider using
[`ansible.builtin.include_tasks`](https://docs.ansible.com/ansible/latest/collections/ansible/builtin/include_tasks_module.html)
to split your tasks into smaller files.

## complexity[nesting]

`complexity[nesting]` will appear when a block contains too many tasks, by
default that number is 20 but it can be changed inside the configuration file by
defining `max_block_depth` value.

Replace nested block with an include_tasks to make code easier to maintain. Maximum block depth allowed is ...
60 changes: 45 additions & 15 deletions src/ansiblelint/rules/complexity.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
import sys
from typing import TYPE_CHECKING, Any

from ansiblelint.config import options
from ansiblelint.constants import LINE_NUMBER_KEY
from ansiblelint.rules import AnsibleLintRule
from ansiblelint.rules import AnsibleLintRule, RulesCollection

if TYPE_CHECKING:
from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable
from ansiblelint.utils import Task


class ComplexityRule(AnsibleLintRule):
Expand All @@ -31,52 +31,82 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
if file.kind != "playbook":
return []
tasks = data.get("tasks", [])
if len(tasks) > options.max_tasks:
if not isinstance(self._collection, RulesCollection):
msg = "Rules cannot be run outside a rule collection."
raise RuntimeError(msg)
if len(tasks) > self._collection.options.max_tasks:
results.append(
self.create_matcherror(
message=f"Maximum tasks allowed in a play is {options.max_tasks}.",
message=f"Maximum tasks allowed in a play is {self._collection.options.max_tasks}.",
lineno=data[LINE_NUMBER_KEY],
tag=f"{self.id}[play]",
filename=file,
),
)
return results

def matchtask(self, task: Task, file: Lintable | None = None) -> list[MatchError]:
"""Check if the task is a block and count the number of items inside it."""
results: list[MatchError] = []

if not isinstance(self._collection, RulesCollection):
msg = "Rules cannot be run outside a rule collection."
raise RuntimeError(msg)

if task.action == "block/always/rescue":
block_depth = self.calculate_block_depth(task)
if block_depth > self._collection.options.max_block_depth:
results.append(
self.create_matcherror(
message=f"Replace nested block with an include_tasks to make code easier to maintain. Maximum block depth allowed is {self._collection.options.max_block_depth}.",
lineno=task[LINE_NUMBER_KEY],
tag=f"{self.id}[nesting]",
filename=file,
),
)
return results

def calculate_block_depth(self, task: Task) -> int:
"""Recursively calculate the block depth of a task."""
if not isinstance(task.position, str):
raise NotImplementedError
return task.position.count(".block")


if "pytest" in sys.modules:
import pytest

from ansiblelint.rules import RulesCollection # pylint: disable=ungrouped-imports
from ansiblelint.config import options # pylint: disable=ungrouped-imports
from ansiblelint.runner import Runner # pylint: disable=ungrouped-imports

@pytest.mark.parametrize(
("file", "expected"),
("file", "expected_results"),
(
pytest.param(
"examples/playbooks/rule-complexity-pass.yml",
0,
[],
id="pass",
),
pytest.param(
"examples/playbooks/rule-complexity-fail.yml",
1,
["complexity[play]", "complexity[nesting]"],
id="fail",
),
),
)
def test_complexity(
default_rules_collection: RulesCollection,
file: str,
expected: int,
expected_results: list[str],
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Test rule."""
monkeypatch.setattr(options, "max_tasks", 5)
collection = RulesCollection()
monkeypatch.setattr(options, "max_block_depth", 3)
collection = RulesCollection(options=options)
collection.register(ComplexityRule())
results = Runner(file, rules=default_rules_collection).run()
results = Runner(file, rules=collection).run()

for result in results:
assert len(results) == len(expected_results)
for i, result in enumerate(results):
assert result.rule.id == ComplexityRule.id, result
assert result.tag == "complexity[play]"
assert len(results) == expected
assert result.tag == expected_results[i]
5 changes: 5 additions & 0 deletions src/ansiblelint/schemas/ansible-lint-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@
"title": "Loop Var Prefix",
"type": "string"
},
"max_block_depth": {
"title": "Maximum Block Depth",
"type": "integer",
"default": 20
},
"mock_modules": {
"items": {
"type": "string"
Expand Down

0 comments on commit 385ad82

Please sign in to comment.