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

Make command instead of shell rule recognize fqcn #1580

Merged
merged 1 commit into from May 23, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.