Skip to content

Commit

Permalink
Allow subdirectories to be part of the task prefix (ansible#4066)
Browse files Browse the repository at this point in the history
  • Loading branch information
cavcrosby committed May 8, 2024
1 parent 262c02c commit b685341
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 7 deletions.
10 changes: 10 additions & 0 deletions examples/playbooks/tasks/partial_prefix/foo.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
- name: foo | This prefix is incomplete
ansible.builtin.assert:
that: true
- name: partial_prefix | This prefix is incomplete
ansible.builtin.assert:
that: true
- name: partial_prefix | foo | This is correct
ansible.builtin.assert:
that: true
5 changes: 5 additions & 0 deletions src/ansiblelint/rules/name.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ For example, if you have a task named `Restart server` inside a file named
`tasks/deploy.yml`, this rule suggests renaming it to `deploy | Restart server`,
so it would be easier to identify where it comes from.

For task files that are embedded within subdirectories, these subdirectories
will also be appended as part of the prefix. For example, if you have a task
named `Terminate server` inside a file named `tasks/foo/destroy.yml`, this rule
suggests renaming it to `foo | destroy | Terminate server`.

For the moment, this sub-rule is just an **opt-in**, so you need to add it to
your `enable_list` to activate it.

Expand Down
57 changes: 54 additions & 3 deletions src/ansiblelint/rules/name.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import sys
from typing import TYPE_CHECKING, Any

import wcmatch.pathlib
import wcmatch.wcmatch

from ansiblelint.constants import LINE_NUMBER_KEY
from ansiblelint.file_utils import Lintable
from ansiblelint.rules import AnsibleLintRule, TransformMixin
Expand Down Expand Up @@ -121,9 +124,13 @@ def _check_name(
# stage one check prefix
effective_name = name
if self._collection and lintable:
prefix = self._collection.options.task_name_prefix.format(
stem=lintable.path.stem,
)
stems = [
self._collection.options.task_name_prefix.format(stem=stem)
for stem in wcmatch.pathlib.PurePath(
self._find_full_stem(lintable),
).parts
]
prefix = "".join(stems)
if lintable.kind == "tasks" and lintable.path.stem != "main":
if not name.startswith(prefix):
# For the moment in order to raise errors this rule needs to be
Expand Down Expand Up @@ -166,6 +173,35 @@ def _check_name(
)
return results

def _find_full_stem(self, lintable: Lintable) -> str:
lintable_dir = wcmatch.pathlib.PurePath(lintable.dir)
stem = lintable.path.stem
kind = str(lintable.kind)

stems = [lintable_dir.name]
lintable_dir = lintable_dir.parent
pathex = lintable_dir / stem
for entry in self.options.kinds:
for key, value in entry.items():
if kind == key:
glob = value

while pathex.globmatch(
glob,
flags=(
wcmatch.pathlib.GLOBSTAR
| wcmatch.pathlib.BRACE
| wcmatch.pathlib.DOTGLOB
),
):
stems.insert(0, lintable_dir.name)
lintable_dir = lintable_dir.parent
pathex = lintable_dir / stem

if stems[0].startswith(kind):
del stems[0]
return str(wcmatch.pathlib.PurePath(*stems, stem))

def transform(
self,
match: MatchError,
Expand Down Expand Up @@ -257,6 +293,21 @@ def test_name_prefix_negative(config_options: Options) -> None:
assert results[1].tag == "name[prefix]"
assert results[2].tag == "name[prefix]"

def test_name_prefix_negative_2(config_options: Options) -> None:
"""Negative test for name[prefix]."""
config_options.enable_list = ["name[prefix]"]
collection = RulesCollection(options=config_options)
collection.register(NameRule())
failure = Lintable(
"examples/playbooks/tasks/partial_prefix/foo.yml",
kind="tasks",
)
bad_runner = Runner(failure, rules=collection)
results = bad_runner.run()
assert len(results) == 2
assert results[0].tag == "name[prefix]"
assert results[1].tag == "name[prefix]"

def test_rule_name_lowercase() -> None:
"""Negative test for a task that starts with lowercase."""
collection = RulesCollection()
Expand Down
8 changes: 4 additions & 4 deletions test/test_file_path_evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@
"tasks/subtasks/subtask_1.yml": textwrap.dedent(
"""\
---
- name: subtask_1 | From subtask 1 import subtask 2
- name: subtasks | subtask_1 | From subtask 1 import subtask 2
ansible.builtin.import_tasks: tasks/subtasks/subtask_2.yml
""",
),
"tasks/subtasks/subtask_2.yml": textwrap.dedent(
"""\
---
- name: subtask_2 | From subtask 2 do something
- name: subtasks | subtask_2 | From subtask 2 do something
debug: # <-- expected to raise fqcn[action-core]
msg: |
Something...
Expand Down Expand Up @@ -87,14 +87,14 @@
"tasks/subtasks/subtask_1.yml": textwrap.dedent(
"""\
---
- name: subtask_1 | From subtask 1 import subtask 2
- name: subtasks | subtask_1 | From subtask 1 import subtask 2
ansible.builtin.include_tasks: tasks/subtasks/subtask_2.yml
""",
),
"tasks/subtasks/subtask_2.yml": textwrap.dedent(
"""\
---
- name: subtask_2 | From subtask 2 do something
- name: subtasks | subtask_2 | From subtask 2 do something
debug: # <-- expected to raise fqcn[action-core]
msg: |
Something...
Expand Down

0 comments on commit b685341

Please sign in to comment.