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

Add action and args properties to task class #3576

Merged
merged 2 commits into from
Jun 22, 2023
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
3 changes: 2 additions & 1 deletion examples/playbooks/rule-args-module-fail.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
masked: false

- name: Enable service httpd and ensure it is not masked
# module should produce: 'Unsupported parameters for ansible.builtin.systemd module: foo. Supported parameters include: no_block, state, daemon_reload (daemon-reload), name (service, unit), force, masked, daemon_reexec (daemon-reexec), scope, enabled.'
# module should produce: 'Unsupported parameters for ansible.builtin.systemd module"
ansible.builtin.systemd:
foo: true

Expand All @@ -34,3 +34,4 @@
ansible.builtin.file:
path: /opt/software/deployment
state: away
mode: "0600"
11 changes: 7 additions & 4 deletions examples/playbooks/rule-risky-file-permissions-fail.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
path: foo
create: true
mode: preserve
section: bar

- name: FAIL_INI_PERMISSION
hosts: all
Expand All @@ -16,6 +17,7 @@
community.general.ini_file:
path: foo
create: true
section: bar

- name: FAIL_PRESERVE_MODE
hosts: all
Expand All @@ -28,7 +30,7 @@
- name: FAIL_MISSING_PERMISSIONS_TOUCH
hosts: all
tasks:
- name: Permissions missing and might create file
- name: Permissions missing and might create file # noqa: fqcn[action-core]
file:
path: foo
state: touch
Expand All @@ -40,7 +42,7 @@
- name: FAIL_MISSING_PERMISSIONS_DIRECTORY
hosts: all
tasks:
- name: Permissions missing and might create directory
- name: Permissions missing and might create directory # noqa: fqcn[action-core]
file:
path: foo
state: directory
Expand Down Expand Up @@ -71,15 +73,16 @@
- name: FAIL_REPLACE_PRESERVE
hosts: all
tasks:
- name: Replace does not allow preserve mode
- name: Replace does not allow preserve mode # noqa: fqcn[action-core]
replace:
path: foo
mode: preserve
regexp: foo

- name: FAIL_PERMISSION_COMMENT
hosts: all
tasks:
- name: Permissions is only a comment
- name: Permissions is only a comment # noqa: fqcn[action-core]
file:
path: foo
owner: root
Expand Down
2 changes: 2 additions & 0 deletions examples/playbooks/rule-risky-file-permissions-pass.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
- name: Replace should not require mode
ansible.builtin.replace:
path: foo
regexp: foo

- name: SUCCESS_RECURSE
hosts: all
Expand All @@ -64,6 +65,7 @@
ansible.builtin.file:
state: directory
recurse: true
path: foo
- name: Permissions not missing and numeric (fqcn)
ansible.builtin.file:
path: bar
Expand Down
9 changes: 7 additions & 2 deletions src/ansiblelint/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,13 @@ def main():
SKIPPED_RULES_KEY = "__skipped_rules__"
LINE_NUMBER_KEY = "__line__"
FILENAME_KEY = "__file__"
ANNOTATION_KEYS = [FILENAME_KEY, LINE_NUMBER_KEY, SKIPPED_RULES_KEY]

ANNOTATION_KEYS = [
FILENAME_KEY,
LINE_NUMBER_KEY,
SKIPPED_RULES_KEY,
"__ansible_module__",
"__ansible_module_original__",
]
INCLUSION_ACTION_NAMES = {
"include",
"include_tasks",
Expand Down
15 changes: 7 additions & 8 deletions src/ansiblelint/rules/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,10 @@ def _parse_failed_msg(

from ansiblelint.runner import Runner # pylint: disable=ungrouped-imports

def test_args_module_fail() -> None:
def test_args_module_fail(default_rules_collection: RulesCollection) -> None:
"""Test rule invalid module options."""
collection = RulesCollection()
collection.register(ArgsRule())
success = "examples/playbooks/rule-args-module-fail.yml"
results = Runner(success, rules=collection).run()
results = Runner(success, rules=default_rules_collection).run()
assert len(results) == 5
assert results[0].tag == "args[module]"
assert "missing required arguments" in results[0].message
Expand All @@ -300,12 +298,13 @@ def test_args_module_fail() -> None:
assert results[4].tag == "args[module]"
assert "value of state must be one of" in results[4].message

def test_args_module_pass(caplog: pytest.LogCaptureFixture) -> None:
def test_args_module_pass(
default_rules_collection: RulesCollection,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Test rule valid module options."""
collection = RulesCollection()
collection.register(ArgsRule())
success = "examples/playbooks/rule-args-module-pass.yml"
with caplog.at_level(logging.WARNING):
results = Runner(success, rules=collection).run()
results = Runner(success, rules=default_rules_collection).run()
assert len(results) == 0, results
assert len(caplog.records) == 0, caplog.records
14 changes: 10 additions & 4 deletions src/ansiblelint/rules/risky_file_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ def matchtask(
module = task["action"]["__ansible_module__"]
mode = task["action"].get("mode", None)

if not isinstance(task.args, dict):
# We are unable to check args when using jinja templating
return False

if module not in self._modules and module not in self._modules_with_create:
return False

Expand Down Expand Up @@ -151,11 +155,13 @@ def matchtask(
),
),
)
def test_risky_file_permissions(file: str, expected: int) -> None:
def test_risky_file_permissions(
file: str,
expected: int,
default_rules_collection: RulesCollection,
) -> None:
"""The ini_file module does not accept preserve mode."""
collection = RulesCollection()
collection.register(MissingFilePermissionsRule())
runner = RunFromText(collection)
runner = RunFromText(default_rules_collection)
results = runner.run(Path(file))
assert len(results) == expected
for result in results:
Expand Down
25 changes: 25 additions & 0 deletions src/ansiblelint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
from ansiblelint.app import get_app
from ansiblelint.config import Options, options
from ansiblelint.constants import (
ANNOTATION_KEYS,
FILENAME_KEY,
INCLUSION_ACTION_NAMES,
LINE_NUMBER_KEY,
Expand Down Expand Up @@ -788,6 +789,30 @@ def name(self) -> str | None:
"""Return the name of the task."""
return self.raw_task.get("name", None)

@property
def action(self) -> str:
"""Return the resolved action name."""
action_name = self.normalized_task["action"]["__ansible_module_original__"]
if not isinstance(action_name, str):
msg = "Task actions can only be strings."
raise RuntimeError(msg)
return action_name

@property
def args(self) -> Any:
"""Return the arguments passed to the task action.

While we usually expect to return a dictionary, it can also
return a templated string when jinja is used.
"""
if "args" in self.raw_task:
return self.raw_task["args"]
result = {}
for k, v in self.normalized_task["action"].items():
if k not in ANNOTATION_KEYS:
result[k] = v
return result

@property
def normalized_task(self) -> dict[str, Any]:
"""Return the name of the task."""
Expand Down