From 4de1b71cc3335672105474a7300b7e294df01983 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Mon, 3 May 2021 14:24:09 +0100 Subject: [PATCH] Add support for acceptable command options (#1544) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * create get_second_cmd_arg function Signed-off-by: Thomas Sjögren * add support for acceptable command options Signed-off-by: Thomas Sjögren * add CommandsInsteadOfModulesRule tests Signed-off-by: Thomas Sjögren * ignore PT009, add docstrings Signed-off-by: Thomas Sjögren * hopefully fix D400 Signed-off-by: Thomas Sjögren * remove playbooks and place tasks in test file Signed-off-by: Thomas Sjögren * move tests into src Signed-off-by: Thomas Sjögren * systemd module status isnt systemctl status Signed-off-by: Thomas Sjögren * Fix lint * Fix test Co-authored-by: Thomas Sjögren --- .../rules/CommandsInsteadOfModulesRule.py | 128 +++++++++++++++++- src/ansiblelint/utils.py | 11 ++ test/TestSkipInsideYaml.py | 14 +- 3 files changed, 145 insertions(+), 8 deletions(-) diff --git a/src/ansiblelint/rules/CommandsInsteadOfModulesRule.py b/src/ansiblelint/rules/CommandsInsteadOfModulesRule.py index 1a76f6a00b..6079651330 100644 --- a/src/ansiblelint/rules/CommandsInsteadOfModulesRule.py +++ b/src/ansiblelint/rules/CommandsInsteadOfModulesRule.py @@ -19,10 +19,11 @@ # THE SOFTWARE. import os +import sys from typing import TYPE_CHECKING, Any, Dict, Union from ansiblelint.rules import AnsibleLintRule -from ansiblelint.utils import convert_to_boolean, get_first_cmd_arg +from ansiblelint.utils import convert_to_boolean, get_first_cmd_arg, get_second_cmd_arg if TYPE_CHECKING: from typing import Optional @@ -64,20 +65,145 @@ class CommandsInsteadOfModulesRule(AnsibleLintRule): 'yum': 'yum', } + _executable_options = { + 'git': ['branch', 'log'], + 'systemctl': ['set-default', 'show-environment', 'status'], + } + def matchtask( self, task: Dict[str, Any], file: 'Optional[Lintable]' = None ) -> Union[bool, str]: + if task['action']['__ansible_module__'] not in self._commands: return False first_cmd_arg = get_first_cmd_arg(task) + second_cmd_arg = get_second_cmd_arg(task) + if not first_cmd_arg: return False executable = os.path.basename(first_cmd_arg) + + if ( + second_cmd_arg + and executable in self._executable_options + and second_cmd_arg in self._executable_options[executable] + ): + return False + if executable in self._modules and convert_to_boolean( task['action'].get('warn', True) ): message = '{0} used in place of {1} module' return message.format(executable, self._modules[executable]) return False + + +if "pytest" in sys.modules: + import pytest + + APT_GET = ''' +- hosts: all + tasks: + - name: run apt-get update + command: apt-get update +''' + + GIT_BRANCH = ''' +- hosts: all + tasks: + - name: print current git branch + command: git branch +''' + + GIT_LOG = ''' +- hosts: all + tasks: + - name: print git log + command: git log +''' + + RESTART_SSHD = ''' +- hosts: all + tasks: + - name: restart sshd + command: systemctl restart sshd +''' + + SYSTEMCTL_STATUS = ''' +- hosts: all + tasks: + - name: show systemctl service status + command: systemctl status systemd-timesyncd +''' + + SYSTEMD_ENVIRONMENT = ''' +- hosts: all + tasks: + - name: show systemd environment + command: systemctl show-environment +''' + + SYSTEMD_RUNLEVEL = ''' +- hosts: all + tasks: + - name: set systemd runlevel + command: systemctl set-default multi-user.target +''' + + @pytest.mark.parametrize( + 'rule_runner', (CommandsInsteadOfModulesRule,), indirect=['rule_runner'] + ) + def test_apt_get(rule_runner: Any) -> None: + """The apt module supports update.""" + results = rule_runner.run_playbook(APT_GET) + assert len(results) == 1 + + @pytest.mark.parametrize( + 'rule_runner', (CommandsInsteadOfModulesRule,), indirect=['rule_runner'] + ) + def test_restart_sshd(rule_runner: Any) -> None: + """Restarting services is supported by the systemd module.""" + results = rule_runner.run_playbook(RESTART_SSHD) + assert len(results) == 1 + + @pytest.mark.parametrize( + 'rule_runner', (CommandsInsteadOfModulesRule,), indirect=['rule_runner'] + ) + def test_git_log(rule_runner: Any) -> None: + """The git log command is not supported by the git module.""" + results = rule_runner.run_playbook(GIT_LOG) + assert len(results) == 0 + + @pytest.mark.parametrize( + 'rule_runner', (CommandsInsteadOfModulesRule,), indirect=['rule_runner'] + ) + def test_git_branch(rule_runner: Any) -> None: + """The git branch command is not supported by the git module.""" + results = rule_runner.run_playbook(GIT_BRANCH) + assert len(results) == 0 + + @pytest.mark.parametrize( + 'rule_runner', (CommandsInsteadOfModulesRule,), indirect=['rule_runner'] + ) + def test_systemd_status(rule_runner: Any) -> None: + """Set-default is not supported by the systemd module.""" + results = rule_runner.run_playbook(SYSTEMCTL_STATUS) + assert len(results) == 0 + + @pytest.mark.parametrize( + 'rule_runner', (CommandsInsteadOfModulesRule,), indirect=['rule_runner'] + ) + def test_systemd_environment(rule_runner: Any) -> None: + """Showing the environment is not supported by the systemd module.""" + results = rule_runner.run_playbook(SYSTEMD_ENVIRONMENT) + assert len(results) == 0 + + @pytest.mark.parametrize( + 'rule_runner', (CommandsInsteadOfModulesRule,), indirect=['rule_runner'] + ) + def test_systemd_runlevel(rule_runner: Any) -> None: + """Set-default is not supported by the systemd module.""" + results = rule_runner.run_playbook(SYSTEMD_RUNLEVEL) + assert len(results) == 0 diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 6d76f6a7bf..70aea18a24 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -730,6 +730,17 @@ def get_first_cmd_arg(task: Dict[str, Any]) -> Any: return first_cmd_arg +def get_second_cmd_arg(task: Dict[str, Any]) -> Any: + try: + if 'cmd' in task['action']: + second_cmd_arg = task['action']['cmd'].split()[1] + else: + second_cmd_arg = task['action']['__ansible_arguments__'][1] + except IndexError: + return None + return second_cmd_arg + + def is_playbook(filename: str) -> bool: """ Check if the file is a playbook. diff --git a/test/TestSkipInsideYaml.py b/test/TestSkipInsideYaml.py index e4feb87700..e7207e1651 100644 --- a/test/TestSkipInsideYaml.py +++ b/test/TestSkipInsideYaml.py @@ -2,12 +2,10 @@ ROLE_TASKS = '''\ --- -- name: test command-instead-of-module - command: git log - changed_when: false -- name: test command-instead-of-module (skipped) - command: git log # noqa command-instead-of-module - changed_when: false +- debug: + msg: this should fail linting due lack of name +- debug: # noqa unnamed-task + msg: this should pass due to noqa comment ''' ROLE_TASKS_WITH_BLOCK = '''\ @@ -91,7 +89,9 @@ def test_role_tasks(default_text_runner): results = default_text_runner.run_role_tasks_main(ROLE_TASKS) - assert len(results) == 1 + assert len(results) == 1, results + assert results[0].linenumber == 2 + assert results[0].rule.id == "unnamed-task" def test_role_tasks_with_block(default_text_runner):