Add support for acceptable command options (#1544)

* create get_second_cmd_arg function Signed-off-by: Thomas Sjögren <[email protected]> * add support for acceptable command options Signed-off-by: Thomas Sjögren <[email protected]> * add CommandsInsteadOfModulesRule tests Signed-off-by: Thomas Sjögren <[email protected]> * ignore PT009, add docstrings Signed-off-by: Thomas Sjögren <[email protected]> * hopefully fix D400 Signed-off-by: Thomas Sjögren <[email protected]> * remove playbooks and place tasks in test file Signed-off-by: Thomas Sjögren <[email protected]> * move tests into src Signed-off-by: Thomas Sjögren <[email protected]> * systemd module status isnt systemctl status Signed-off-by: Thomas Sjögren <[email protected]> * Fix lint * Fix test Co-authored-by: Thomas Sjögren <[email protected]>
ansible · May 3, 2021 · 4de1b71 · 4de1b71
1 parent 7a3fcc8
commit 4de1b71
Show file tree

Hide file tree

Showing 3 changed files with 145 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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):