Skip to content

Commit

Permalink
Refactor command-instead-of-shell fixtures in standalone examples (#3210
Browse files Browse the repository at this point in the history
)

Co-authored-by: Ajinkya Udgirkar <audgirka@redhat.com>
  • Loading branch information
audgirka and audgirka committed Mar 24, 2023
1 parent 73af468 commit 0fd5900
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 97 deletions.
25 changes: 25 additions & 0 deletions examples/playbooks/rule-command-instead-of-shell-fail.yml
@@ -0,0 +1,25 @@
---
- name: Fixture
hosts: localhost
tasks:
- name: Shell no pipe
ansible.builtin.shell:
cmd: echo hello
changed_when: false

- name: Shell with jinja filter
ansible.builtin.shell:
cmd: echo {{ "hello" | upper }}
changed_when: false

- name: Shell with jinja filter (fqcn)
ansible.builtin.shell:
cmd: echo {{ "hello" | upper }}
changed_when: false

- name: Command with executable parameter
ansible.builtin.shell:
cmd: clear
args:
executable: /bin/bash
changed_when: false
60 changes: 60 additions & 0 deletions examples/playbooks/rule-command-instead-of-shell-pass.yml
@@ -0,0 +1,60 @@
---
- name: Fixture
hosts: localhost
tasks:
- name: Shell with pipe
ansible.builtin.shell:
cmd: echo hello | true # noqa: risky-shell-pipe
changed_when: false

- name: Shell with redirect
ansible.builtin.shell:
cmd: echo hello > /tmp/hello
changed_when: false

- name: Chain two shell commands
ansible.builtin.shell:
cmd: echo hello && echo goodbye
changed_when: false

- name: Run commands in succession
ansible.builtin.shell:
cmd: echo hello ; echo goodbye
changed_when: false

- name: Use variables
ansible.builtin.shell:
cmd: echo $HOME $USER
changed_when: false

- name: Use * for globbing
ansible.builtin.shell:
cmd: ls foo*
changed_when: false

- name: Use ? for globbing
ansible.builtin.shell:
cmd: ls foo?
changed_when: false

- name: Use [] for globbing
ansible.builtin.shell:
cmd: ls foo[1,2,3]
changed_when: false

- name: Use shell generator
ansible.builtin.shell:
cmd: ls foo{.txt,.xml}
changed_when: false

- name: Use backticks
ansible.builtin.shell:
cmd: ls `ls foo*`
changed_when: false

- name: Use shell with cmd
ansible.builtin.shell:
cmd: |
set -x
ls foo?
changed_when: false
109 changes: 13 additions & 96 deletions src/ansiblelint/rules/command_instead_of_shell.py
Expand Up @@ -30,96 +30,6 @@
from ansiblelint.file_utils import Lintable


FAIL_PLAY = """---
- name: Fixture
hosts: localhost
tasks:
- name: Shell no pipe
ansible.builtin.shell:
cmd: echo hello
changed_when: false
- name: Shell with jinja filter
ansible.builtin.shell:
cmd: echo {{ "hello" | upper }}
changed_when: false
- name: Shell with jinja filter (fqcn)
ansible.builtin.shell:
cmd: echo {{ "hello" | upper }}
changed_when: false
- name: Command with executable parameter
ansible.builtin.shell:
cmd: clear
args:
executable: /bin/bash
changed_when: false
"""

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


class UseCommandInsteadOfShellRule(AnsibleLintRule):
"""Use shell only when shell functionality is required."""

Expand Down Expand Up @@ -154,20 +64,27 @@ def matchtask(
if "pytest" in sys.modules:
import pytest

from ansiblelint.testing import RunFromText # pylint: disable=ungrouped-imports
from ansiblelint.rules import RulesCollection # pylint: disable=ungrouped-imports
from ansiblelint.runner import Runner # pylint: disable=ungrouped-imports

@pytest.mark.parametrize(
("text", "expected"),
("file", "expected"),
(
pytest.param(SUCCESS_PLAY, 0, id="good"),
pytest.param(FAIL_PLAY, 3, id="bad"),
pytest.param(
"examples/playbooks/rule-command-instead-of-shell-pass.yml",
0,
id="good",
),
pytest.param(
"examples/playbooks/rule-command-instead-of-shell-fail.yml", 3, id="bad"
),
),
)
def test_rule_command_instead_of_shell(
default_text_runner: RunFromText, text: str, expected: int
default_rules_collection: RulesCollection, file: str, expected: int
) -> None:
"""Validate that rule works as intended."""
results = default_text_runner.run_playbook(text)
results = Runner(file, rules=default_rules_collection).run()
for result in results:
assert result.rule.id == UseCommandInsteadOfShellRule.id, result
assert len(results) == expected
2 changes: 1 addition & 1 deletion src/ansiblelint/schemas/__store__.json
Expand Up @@ -4,7 +4,7 @@
"url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/ansible-lint-config.json"
},
"ansible-navigator-config": {
"etag": "c1038568788867f861cdba2aefde88a71b8ab1a6d874ecfa84eb14c110787677",
"etag": "ddcaa4cc87f2fe9ea0dde101954c674222662b91c159d3e3b28bb18095af676b",
"url": "https://raw.githubusercontent.com/ansible/ansible-navigator/main/src/ansible_navigator/data/ansible-navigator.json"
},
"arg_specs": {
Expand Down
9 changes: 9 additions & 0 deletions src/ansiblelint/schemas/ansible-navigator-config.json
Expand Up @@ -158,6 +158,15 @@
"description": "The directory path to store artifacts generated by ansible-runner",
"type": "string"
},
"job-events": {
"default": false,
"description": "Write ansible-runner job_events in the artifact directory",
"enum": [
true,
false
],
"type": "boolean"
},
"rotate-artifacts-count": {
"description": "Keep ansible-runner artifact directories, for last n runs, if set to 0 artifact directories won't be deleted",
"type": "integer"
Expand Down

0 comments on commit 0fd5900

Please sign in to comment.