Skip to content
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

Make sure all tasks get evaluated by matchtask including block/always/rescue and nested tasks #2031

Merged
merged 45 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
48ebe2f
Update var-naming rule to evaluate block vars
nishipy Mar 22, 2022
54d7743
Merge branch 'main' into fix-1895
ssbarnea Mar 22, 2022
d88559d
Roll back previous commit 48ebe2f7172a0a20115bbf2ed49250a7c97d1499
nishipy Mar 26, 2022
1041231
Update var-naming rule to evaluate block vars in pre_tasks/tasks/post…
nishipy Mar 26, 2022
4d2c4fa
Merge branch 'ansible:main' into fix-1895
nishipy Mar 26, 2022
bbd6e51
Merge branch 'fix-1895' of https://github.com/nishipy/ansible-lint in…
nishipy Mar 26, 2022
e210011
Delete unused package
nishipy Mar 26, 2022
9c02dcc
Fix lint error
nishipy Mar 26, 2022
efd8c70
Fix lint error
nishipy Mar 26, 2022
38ce3a5
Merge branch 'ansible:main' into fix-1895
nishipy Mar 29, 2022
edd1e47
Fix tox errors
nishipy Mar 29, 2022
6bce654
Fix errors on eco and docs
nishipy Mar 29, 2022
2ddb9bc
Fix typo
nishipy Mar 29, 2022
8e94643
Merge branch 'main' into fix-1895
ssbarnea Apr 6, 2022
f0c2ab1
Update to evaluate nested task with block/always/rescue
nishipy Apr 17, 2022
760067f
Remove unnecessary comments
nishipy Apr 17, 2022
b77ebc4
Remove unused lines
nishipy Apr 17, 2022
22655ca
Update src/ansiblelint/utils.py
nishipy Apr 18, 2022
61f00cf
Update src/ansiblelint/rules/unnamed_task.py
nishipy Apr 18, 2022
2314eba
Remove unused variable
nishipy Apr 18, 2022
557a530
Roll back src/ansiblelint/rules/__init__.py
nishipy Apr 18, 2022
661ccae
Update src/ansiblelint/utils.py
nishipy Apr 18, 2022
fd7dd9c
Update src/ansiblelint/rules/unnamed_task.py
nishipy Apr 18, 2022
d642f59
Update src/ansiblelint/utils.py
nishipy Apr 18, 2022
d36c3df
Update src/ansiblelint/utils.py
nishipy Apr 18, 2022
436f1b6
Update normalize_task_v2
nishipy Apr 18, 2022
fff8973
Update normalize_task_v2 to return earlier
nishipy Apr 18, 2022
f0c431b
Update _extract_ansible_parsed_keys_from_task()
nishipy Apr 18, 2022
b027ecc
Update src/ansiblelint/utils.py
nishipy Apr 23, 2022
8e91224
Update src/ansiblelint/utils.py
nishipy Apr 23, 2022
30538aa
Update src/ansiblelint/utils.py
nishipy Apr 23, 2022
793ebfa
chore: auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 23, 2022
c68314f
Merge branch 'ansible:main' into fix-1895
nishipy Apr 29, 2022
eb2343a
Merge branch 'ansible:main' into fix-1895
nishipy Apr 29, 2022
c40c7bc
Add test_is_nested_task()
nishipy Apr 29, 2022
cbac936
Add test_normalize_task_v2()
nishipy May 3, 2022
50e57b9
Merge main branch
nishipy May 3, 2022
6644dd9
Update PYTEST_REQPASS for test_utils
nishipy May 3, 2022
f0194f0
Resolve conflict with PYTEST_REQPASS in tox.yml
nishipy May 3, 2022
4b148fe
Resolve conflict with PYTEST_REQPASS in tox.yml
nishipy May 3, 2022
832f125
Fix error on lint
nishipy May 3, 2022
a7474b3
Merge branch 'main' into fix-1895
ssbarnea May 4, 2022
206f1d0
add a var-naming test using block task
cognifloyd May 5, 2022
d05fdb4
update test count
cognifloyd May 5, 2022
503e3e4
update eco test results
cognifloyd May 5, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 611
PYTEST_REQPASS: 615
steps:
- name: Activate WSL1
if: "contains(matrix.shell, 'wsl')"
Expand Down
4 changes: 3 additions & 1 deletion src/ansiblelint/rules/unnamed_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from typing import TYPE_CHECKING, Any, Dict, Union

from ansiblelint.rules import AnsibleLintRule
from ansiblelint.utils import is_nested_task

if TYPE_CHECKING:
from typing import Optional
Expand All @@ -45,4 +46,5 @@ class TaskHasNameRule(AnsibleLintRule):
def matchtask(
self, task: Dict[str, Any], file: "Optional[Lintable]" = None
) -> Union[bool, str]:
return not task.get("name")
# Ignore nonamed block/always/rescue
return not (task.get("name") or is_nested_task(task))
49 changes: 40 additions & 9 deletions src/ansiblelint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -521,12 +521,39 @@ def _sanitize_task(task: Dict[str, Any]) -> Dict[str, Any]:
return result


def _extract_ansible_parsed_keys_from_task(
result: Dict[str, Any],
task: Dict[str, Any],
keys: Tuple[str, ...],
) -> Dict[str, Any]:
"""Return a dict with existing key in task."""
for (k, v) in list(task.items()):
if k in keys:
# we don't want to re-assign these values, which were
# determined by the ModuleArgsParser() above
continue
result[k] = v
return result


def normalize_task_v2(task: Dict[str, Any]) -> Dict[str, Any]:
"""Ensure tasks have a normalized action key and strings are converted to python objects."""
result = {}
result: Dict[str, Any] = {}
ansible_parsed_keys = ("action", "local_action", "args", "delegate_to")

if is_nested_task(task):
_extract_ansible_parsed_keys_from_task(result, task, ansible_parsed_keys)
# Add dummy action for block/always/rescue statements
result["action"] = dict(
__ansible_module__="block/always/rescue",
__ansible_module_original__="block/always/rescue",
)
cognifloyd marked this conversation as resolved.
Show resolved Hide resolved

return result

sanitized_task = _sanitize_task(task)
mod_arg_parser = ModuleArgsParser(sanitized_task)

try:
action, arguments, result["delegate_to"] = mod_arg_parser.parse(
skip_action_validation=options.skip_action_validation
Expand All @@ -545,12 +572,9 @@ def normalize_task_v2(task: Dict[str, Any]) -> Dict[str, Any]:
action = "shell"
del arguments["_uses_shell"]

for (k, v) in list(task.items()):
if k in ("action", "local_action", "args", "delegate_to") or k == action:
# we don't want to re-assign these values, which were
# determined by the ModuleArgsParser() above
continue
result[k] = v
_extract_ansible_parsed_keys_from_task(
result, task, ansible_parsed_keys + (action,)
)

if not isinstance(action, str):
raise RuntimeError(f"Task actions can only be strings, got {action}")
Expand Down Expand Up @@ -660,8 +684,6 @@ def get_action_tasks(data: AnsibleBaseYAMLObject, file: Lintable) -> List[Any]:

# Add sub-elements of block/rescue/always to tasks list
tasks.extend(extract_from_list(tasks, NESTED_TASK_KEYS, recursive=True))
# Remove block/rescue/always elements from tasks list
tasks[:] = [task for task in tasks if all(k not in task for k in NESTED_TASK_KEYS)]

# Include the FQCN task names as this happens before normalize
return [
Expand Down Expand Up @@ -875,3 +897,12 @@ def nested_items(
yield "list-item", item, parent
for k, v, returned_parent in nested_items(item):
yield k, v, returned_parent


def is_nested_task(task: Dict[str, Any]) -> bool:
"""Check if task includes block/always/rescue."""
for key in NESTED_TASK_KEYS:
if task.get(key):
return True

return False
92 changes: 92 additions & 0 deletions test/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from _pytest.capture import CaptureFixture
from _pytest.logging import LogCaptureFixture
from _pytest.monkeypatch import MonkeyPatch
from ansible.utils.sentinel import Sentinel

from ansiblelint import cli, constants, utils
from ansiblelint.__main__ import initialize_logger
Expand Down Expand Up @@ -130,6 +131,59 @@ def test_normalize_complex_command() -> None:
)


@pytest.mark.parametrize(
("task", "expected_form"),
(
pytest.param(
dict(
name="ensure apache is at the latest version",
yum={"name": "httpd", "state": "latest"},
),
dict(
delegate_to=Sentinel,
name="ensure apache is at the latest version",
action={
"__ansible_module__": "yum",
"__ansible_module_original__": "yum",
"__ansible_arguments__": [],
"name": "httpd",
"state": "latest",
},
),
),
pytest.param(
dict(
name="Attempt and graceful roll back",
block=[
{
"name": "Install httpd and memcached",
"ansible.builtin.yum": ["httpd", "memcached"],
"state": "present",
}
],
),
dict(
name="Attempt and graceful roll back",
block=[
{
"name": "Install httpd and memcached",
"ansible.builtin.yum": ["httpd", "memcached"],
"state": "present",
}
],
action={
"__ansible_module__": "block/always/rescue",
"__ansible_module_original__": "block/always/rescue",
},
),
),
),
)
def test_normalize_task_v2(task: Dict[str, Any], expected_form: Dict[str, Any]) -> None:
"""Check that it normalizes task and returns the expected form."""
assert utils.normalize_task_v2(task) == expected_form


def test_extract_from_list() -> None:
"""Check that tasks get extracted from blocks if present."""
block = {
Expand Down Expand Up @@ -353,3 +407,41 @@ def test_nested_items() -> None:
match=r"Call to deprecated function ansiblelint\.utils\.nested_items.*"
):
assert list(utils.nested_items(data)) == items


@pytest.mark.parametrize(
("task", "expected"),
(
pytest.param(
dict(
name="ensure apache is at the latest version",
yum={"name": "httpd", "state": "latest"},
),
False,
),
pytest.param(
dict(
name="Attempt and graceful roll back",
block=[
{"name": "Force a failure", "ansible.builtin.command": "/bin/false"}
],
rescue=[
{
"name": "Force a failure in middle of recovery!",
"ansible.builtin.command": "/bin/false",
}
],
always=[
{
"name": "Always do this",
"ansible.builtin.debug": {"msg": "This always executes"},
}
],
),
True,
),
),
)
def test_is_nested_task(task: Dict[str, Any], expected: bool) -> None:
"""Test is_nested_task() returns expected bool."""
assert utils.is_nested_task(task) == expected