Skip to content

Commit

Permalink
Make command instead of shell rule recognize fqcn (#1580)
Browse files Browse the repository at this point in the history
- refactor rule testing
- make rule recognize newer fqcn calls

Fixes: #1573
  • Loading branch information
ssbarnea committed May 23, 2021
1 parent c4016a5 commit 254f9b2
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 71 deletions.
8 changes: 0 additions & 8 deletions examples/playbooks/command-instead-of-shell-failure.yml

This file was deleted.

37 changes: 0 additions & 37 deletions examples/playbooks/command-instead-of-shell-success.yml

This file was deleted.

90 changes: 88 additions & 2 deletions src/ansiblelint/rules/UseCommandInsteadOfShellRule.py
Expand Up @@ -17,7 +17,7 @@
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

import sys
from typing import TYPE_CHECKING, Any, Dict, Union

from ansiblelint.rules import AnsibleLintRule
Expand All @@ -28,6 +28,74 @@
from ansiblelint.file_utils import Lintable


FAIL_PLAY = """---
- hosts: localhost
tasks:
- name: shell no pipe
shell: echo hello
changed_when: false
- name: shell with jinja filter
shell: echo {{ "hello"|upper }}
changed_when: false
- name: shell with jinja filter (fqcn)
ansible.builtin.shell: echo {{ "hello"|upper }}
changed_when: false
"""

SUCCESS_PLAY = """---
- hosts: localhost
tasks:
- name: shell with pipe
shell: echo hello | true # noqa: risky-shell-pipe
changed_when: false
- name: shell with redirect
shell: echo hello > /tmp/hello
changed_when: false
- name: chain two shell commands
shell: echo hello && echo goodbye
changed_when: false
- name: run commands in succession
shell: echo hello ; echo goodbye
changed_when: false
- name: use variables
shell: echo $HOME $USER
changed_when: false
- name: use * for globbing
shell: ls foo*
changed_when: false
- name: use ? for globbing
shell: ls foo?
changed_when: false
- name: use [] for globbing
shell: ls foo[1,2,3]
changed_when: false
- name: use shell generator
shell: ls foo{.txt,.xml}
changed_when: false
- name: use backticks
shell: ls `ls foo*`
changed_when: false
- name: use shell with cmd
shell:
cmd: |
set -x
ls foo?
changed_when: false
"""


class UseCommandInsteadOfShellRule(AnsibleLintRule):
id = 'command-instead-of-shell'
shortdesc = 'Use shell only when shell functionality is required'
Expand All @@ -45,7 +113,7 @@ def matchtask(
) -> Union[bool, str]:
# Use unjinja so that we don't match on jinja filters
# rather than pipes
if task["action"]["__ansible_module__"] == 'shell':
if task["action"]["__ansible_module__"] in ['shell', 'ansible.builtin.shell']:
if 'cmd' in task['action']:
unjinjad_cmd = self.unjinja(task["action"].get("cmd", []))
else:
Expand All @@ -54,3 +122,21 @@ def matchtask(
)
return not any(ch in unjinjad_cmd for ch in '&|<>;$\n*[]{}?`')
return False


# testing code to be loaded only with pytest or when executed the rule file
if "pytest" in sys.modules:

import pytest

from ansiblelint.testing import RunFromText # pylint: disable=ungrouped-imports

@pytest.mark.parametrize(('text', 'expected'), ((SUCCESS_PLAY, 0), (FAIL_PLAY, 3)))
def test_rule_command_instead_of_shell(
default_text_runner: RunFromText, text: str, expected: int
) -> None:
"""Validate that rule works as intended."""
results = default_text_runner.run_playbook(text)
for result in results:
assert result.rule.id == UseCommandInsteadOfShellRule.id, result
assert len(results) == expected
24 changes: 0 additions & 24 deletions test/TestUseCommandInsteadOfShell.py

This file was deleted.

0 comments on commit 254f9b2

Please sign in to comment.