[306] Shells that use pipes should set the pipefail option - hard to comply on non-RHEL

[geerlingguy]

Issue Type

• Bug report

Ansible and Ansible Lint details

$ ansible --version
ansible 2.7.9
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/Users/jgeerling/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python2.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.15 (default, Jan 18 2019, 23:17:44) [GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.5)]

$ ansible-lint --version
ansible-lint 4.1.0

• ansible installation method: pip
• ansible-lint installation method: pip

Desired Behaviour

When using the shell module with pipes, sometimes it is desirable to allow early commands to have failures but continue through the rest of the pipes.

Additionally, the default shell interpreter on current versions of Debian and Ubuntu is dash, which does not have the -o pipefail option for set.

Actual Behaviour (Bug report only)

Example task:

- name: Confirm PHP version is correct.
  shell: |
    set -o pipefail
    php -v | grep -F '{{ php_version }}'
  changed_when: false

When this is run on Ubuntu 16.04 or 18.04, it returns:

TASK [Confirm PHP version is correct.] *****************************************
    fatal: [instance]: FAILED! => {"changed": false, "cmd": "set -o pipefail\n php -v | grep -F '7.2'", "delta": "0:00:00.035653", "end": "2019-03-24 00:23:44.382720", "msg": "non-zero return code", "rc": 2, "start": "2019-03-24 00:23:44.347067", "stderr": "/bin/sh: 1: set: Illegal option -o pipefail", "stderr_lines": ["/bin/sh: 1: set: Illegal option -o pipefail"], "stdout": "", "stdout_lines": []}

For now, for the affected roles (which I support on RHEL, CentOS, Ubuntu, Debian, Fedora, and Amazon Linux... so I have to have a solution which is able to be used cross-platform) I have simply ignored the rule in the global .ansible-lint:

skip_list:
  - '306'

[TomaszKlosinski]

I found workaround for this:

  shell: |
    set -o pipefail
    php -v | grep -F '{{ php_version }}'
  args:
    executable: /bin/bash

[odyssey4me]

Perhaps the solution here would be to only check this if the executable is bash.

[brunswyck]

I have simply ignored the rule in the global .ansible-lint:

skip_list:
  - '306'

Define "global .ansible-lint" please

[ssbarnea]

@brunswyck Create this file in the root of your project. I doubt user config makes much sense for a linter anyway.

I was inclined to close this ticket as there is a known way to avoid it with config or inline skips.

Still, by looking twice at it, I still think that forcing users to add skips for shells that do not support it generates bad experiences.

I wonder if someone could find a solution that is more cross platform that the current one that is bash-centric. Removing the rule completely would also be bad, as I know that this rule saved us multiple times from introducing bugs, almost every month I see a change that would be slipped an unchecked pipe usage into the code-base.

[hashar]

The Ansible shell module default executable is the Ansible configuration setting executable which defaults to /bin/sh and is apparently set to /bin/bash on RHEL. The execution outcome thus depends on the Ansible configuration and that should be explicitly checked.

One might well write a playbook with the bash idiom, test it locally and have a pleasant output simply cause the executable has been set to bash. Then when running the task on a Debian production host that has sh the task ends up failing.

The linter has no way to know which executable will end up being used. When it detects a bash only syntax and the executable is not explicitly set to /.*\/bash/, it should flag it as an error and request the executable to be explicitly mentioned:

shell: |
  set -o pipefail
  foo | bar
args:
  executable: /bin/bash

This way we ensure that the pipefail option is properly recognized regardless of the Ansible configuration.

[ssbarnea]

There is no way for the linter to know which shell will be used on the host. There is nothing specific to RHEL in this rule as the lack of pipefail applies to other distributions as well.

To make the situation even worse, /bin/sh is usually a symlink that points to something else. I kinda feel is impossible to guess if is not bash. Maybe only a rule that looks if executable is mentioned and is not (bash|zsh|sh), practically sorting almost zero false positives.

I am more than happy to reduce the number of false-positive matches on this rule but the question is how? How about just to disable the rule if you don't like it? In fact that is one of the reasons that made me create #971

[odyssey4me]

The only things I can think of are as follows:

1. This is a problem best solved in ansible's shell module, not the linter.
2. If not solved in ansible, then activate the rule if the executable is set to .*/bash. Yes, this doesn't solve the problem if /bin/sh is a symlink to /bin/bash but at least it will catch those who are setting the executable. Perhaps the linter could just warn anyone using the default that this might be a problem.

This does make me think that ansible-lint could do with more than just on/off for rules. Perhaps it should have warn to, and allow people to move any rules into any one of the rule sets (on, off or warn).

[ssbarnea]

I am going to close this bug as we already have the warn_list, so users can easily move this rule to it, order even directly to skip_list. In the absence of any support from ansible core modules we cannot guess when this rule is relevant or not.

[brlin-tw]

It might be feasible to do the shell detection at runtime to workaround this problem, id. est.:

- name: Set pipefail option only when shell is Bash
  shell: >-
    if test -v BASH; then set -o pipefail; fi

    my | shell | pipeline

However, this also triggers the risky-shell-pipe violation, one may consider loosening the detection pattern to allow such workarounds.

[ssbarnea]

@brlin-tw We are open to suggestions here, clearly the patterns can be improved. I think that probably if we identify the use of | and at the same time we find pipefail in any place inside the text, to ignore the rule, assuming that the user knew about pipefail.

The main goal of the rule is to tell users using pipe, that they might want to set the pipefail, how they set it, is up to them. Even adding a comment as # pipefail may be able to disable that feature.

Real shell detection is not possible as this it is determined at runtime by factor that are not limited to the task definition inside the playbook.

WDYT?

[reitzig]

FWIW, I think the rule has merit. ansible.builtin.shell should have a parameter/arg to enabile pipefail (and probably other options, as well). That would make acting on the linter error a lot clearer, and the linter would have an easier time checking.

[MohammedNoureldin]

Selecting bash as a shell is not always an option, sometimes we use a very minimal version of Linux, where bash is not available. Could it be solved in another way without ignoring the rule?

[marek22k]

Why is the issue closed when it is not resolved? I think it would make more sense if it is closed as not planed. I have the same problem :-(

[tubemeister]

Well that's half an hour of my life I'm not getting back... ;-)

Lint says, use pipefail
Ansible run now breaks, because dash

On the ignore list it goes...

[hashar]

Indeed, that lint rule is irrelevant unless Ansible is configured with executable=/bin/bash and I guess the linter doesn't have access to that configuration setting.

The rule should be enhanced to also requires the executable to be explicitly set, then there are surely a lot of use cases for NOT using bash.

I guess it is broken by design (and I blame the shell module for that).

[zoredache]

Indeed, that lint rule is irrelevant unless Ansible is configured with executable=/bin/bash and I guess the linter doesn't have access to that configuration setting.

The shell that is something on the target side, and can be different for different targets. It wouldn't really make sense for a linter to be connecting to all the targets in your inventory to see if a particular usage would be broken.

I really wish ansible had an easy way to set module_defaults globally. That way you could just set the module_defaults for shell to be the path to your bash binary.

[mcandre]

Note that bash is not the only sh implementation that supports pipefail. zsh and certain ash versions do. dash does not. posh may not. yash does not. toybox sh, unknown. busybox sh, unknown. Notably, ksh93 invented the flag, so ksh implementations >= the 93 release support it as well. Unknown of the ksh variants (mksh, oksh, pdksh, rksh) which version they are based on. A lot of this stuff unfortunately comes down to the version of the operating system that provides the shell distribution package.

Someone should probably publish a table of POSIX 2022 sh features to implementation names, similar to the giant tables of JavaScript 6 features to browsers, and C++23 features to compilers. Then we would have a common document to reference whenever discussing the subtle, dark arts of shell coding.