Include parent actions (block, include_tasks, ...) in matchtask()

[maunzCache]

Summary

"Parent actions" - in lack for a better name - such as block, always, rescue, include, include_tasks, import_playbook and import_tasks are removed from the list of tasks available in AnsibleLintRule.matchtasks() and AnsibleLintRule.matchtask(). This requires custom implementation to check generic parameters such as when, become, ignore_errors, vars, ...
This happens due to removal in ansiblelint.utils.get_action_tasks() . The removal of those keywords seems to be intentional as the comment blocks suggests.

Issue Type

• Ideas

Ansible and Ansible Lint details

ansible --version
ansible 2.10.7
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/opt/ansible-custom/modules']
  ansible python module location = /home/.../ansible-lint-rules/venv/lib/python3.8/site-packages/ansible
  executable location = /home/.../ansible-lint-rules/venv/bin/ansible
  python version = 3.8.5 (default, Jan 27 2021, 15:41:15) [GCC 9.3.0]

ansible-lint --version
ansible-lint 5.0.6 using ansible 2.10.7

• ansible installation method: pip
• ansible-lint installation method: pip

OS / ENVIRONMENT

uname -a 
Linux 5.4.0-70-generic #78-Ubuntu SMP Fri Mar 19 13:29:52 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

STEPS TO REPRODUCE

Consider the following lint rule:

from typing import Any, Dict, Union

from ansiblelint.rules import AnsibleLintRule


class TaskPreventsUnitTestSkip(AnsibleLintRule):
    id = 'task-prevents-unit-test-skip'
    shortdesc = 'Prevent usage of "when: not unit_test".'
    description = (
        'Modules must not use "when: not unit_test" to skip tasks '
        'which are used for code structure.'
        'Tasks should only be skipped by variables created by modules '
        'e.g. via "assert" or "set_fact".'
    )
    severity = 'LOW'
    tags = ['task']

    modules_prevents_unit_test = ['assert', 'block', 'import_role',
                                  'import_tasks', 'include_role',
                                  'include_tasks', 'include_vars', 'lineinfile',
                                  'set_fact', 'template']

    def matchtask(self, task: Dict[str, Any]) -> Union[bool, str]:
        # Does not match block -> assert instead
        # Does not match import_tasks
        # Does not match include_tasks
        print(task)

        if task.get('action')[
            '__ansible_module__'] in self.modules_prevents_unit_test:
            if ('when' in task) and ('not unit_test' in task.get('when')):
                return True
        return False

Asserting on this implementation i get only 7 of 10 checks by checking the tasks from the modules_prevents_unit_test variable. The commentary shows which are missing.

Assertion

def test_task_prevents_unit_test_skip_fails():
    collection = RulesCollection()
    collection.register(TaskPreventsUnitTestSkip())
    tasks = Lintable('examples/task-prevents-unit-test-skip.yml', kind='tasks')

    bad_runner = Runner(tasks, rules=collection)
    errs = bad_runner.run()

    assert len(errs) == 10

Snipped from the test tasks

---
# Erroneous tasks marked by linting rule
- name: "[task-prevents-unit-test-skip] Prevent usage of 'when: not unit_test'."
  assert:
    that:
      - True != False
  when: not unit_test

# False-negative. block is 'skipped' and not tested by the rule, however, assert is available.
- name: "[task-prevents-unit-test-skip] Prevent usage of 'when: not unit_test'."
  block:
    - name: "block | [task-prevents-unit-test-skip] Prevent usage of 'when: not unit_test'."
      assert:
        that:
          - True != False
  when: not unit_test

# ...

Desired Behaviour

I would like to be able to also check "parent actions" as described in the summary but without implementing additional or redundant got already present in ansible.utils. There is a set of customer linting rules in my environment which i'd like to apply to all kinds of tasks even though these are merely "wrapper" (such as block). Still i am aware that those keywords are removed from the output as loops may appear when collection the "child actions" from the "parent action".

Actual Behaviour

The current implementation will remove the desired actions from the output. My test environment cannot rely on linting as there may be false-negatives when applying those rules.

There is a workaround for the situation but requires somewhat of an overhead. I would be able to implement the lookup myself when overriding matchtasks().