From 27e4ae138521ab3f87e189d4b405b3f7f309e231 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Thu, 9 Mar 2023 17:52:51 +0000 Subject: [PATCH] Ignore risky-shell-pipe with pwsh (#3166) --- .config/dictionary.txt | 1 + .../playbooks/rule-risky-shell-pipe-pass.yml | 8 ++++++++ src/ansiblelint/rules/risky_shell_pipe.md | 18 +++++++++++------- src/ansiblelint/rules/risky_shell_pipe.py | 4 ++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/.config/dictionary.txt b/.config/dictionary.txt index 898c56b8f4..1b9bb1b491 100644 --- a/.config/dictionary.txt +++ b/.config/dictionary.txt @@ -255,6 +255,7 @@ prerun prettierignore programoutput psutil +pwsh pyargs pycache pycharm diff --git a/examples/playbooks/rule-risky-shell-pipe-pass.yml b/examples/playbooks/rule-risky-shell-pipe-pass.yml index 78c32ed144..386e8e8128 100644 --- a/examples/playbooks/rule-risky-shell-pipe-pass.yml +++ b/examples/playbooks/rule-risky-shell-pipe-pass.yml @@ -60,3 +60,11 @@ set -o pipefail df | grep '/dev' changed_when: false + + - name: "PowerShell with pipefail should be ok, bug #3161" + # https://github.com/ansible/ansible-lint/issues/3161 + ansible.builtin.shell: + executable: /bin/pwsh + cmd: | + $ProgressPreference = 'this | that' + changed_when: false diff --git a/src/ansiblelint/rules/risky_shell_pipe.md b/src/ansiblelint/rules/risky_shell_pipe.md index 0c222a953b..302d0d9f87 100644 --- a/src/ansiblelint/rules/risky_shell_pipe.md +++ b/src/ansiblelint/rules/risky_shell_pipe.md @@ -2,9 +2,13 @@ This rule checks for the bash `pipefail` option with the Ansible `shell` module. -You should always set `pipefail` when piping output from a command to another. -The return status of a pipeline is the exit status of the command. -The `pipefail` option ensures that tasks fail as expected if the first command fails. +You should always set `pipefail` when piping output from one command to another. +The return status of a pipeline is the exit status of the command. The +`pipefail` option ensures that tasks fail as expected if the first command +fails. + +As this requirement does apply to PowerShell, for shell commands that have +`pwsh` inside `executable` attribute, this rule will not trigger. ## Problematic Code @@ -14,7 +18,7 @@ The `pipefail` option ensures that tasks fail as expected if the first command f hosts: localhost tasks: - name: Pipeline without pipefail - shell: false | cat + ansible.builtin.shell: false | cat ``` ## Correct Code @@ -23,13 +27,13 @@ The `pipefail` option ensures that tasks fail as expected if the first command f --- - name: Example playbook hosts: localhost - become: no + become: false tasks: - name: Pipeline with pipefail - shell: set -o pipefail && false | cat + ansible.builtin.shell: set -o pipefail && false | cat - name: Pipeline with pipefail, multi-line - shell: | + ansible.builtin.shell: | set -o pipefail # <-- adding this will prevent surprises false | cat ``` diff --git a/src/ansiblelint/rules/risky_shell_pipe.py b/src/ansiblelint/rules/risky_shell_pipe.py index bace0a7cf8..92b44bdbba 100644 --- a/src/ansiblelint/rules/risky_shell_pipe.py +++ b/src/ansiblelint/rules/risky_shell_pipe.py @@ -45,6 +45,10 @@ def matchtask( jinja_stripped_cmd = self.unjinja(get_cmd_args(task)) + # https://github.com/ansible/ansible-lint/issues/3161 + if "pwsh" in task["action"].get("executable", ""): + return False + return bool( self._pipe_re.search(jinja_stripped_cmd) and not self._pipefail_re.search(jinja_stripped_cmd)