search

ansible / ansible-lint

ansible-lint checks playbooks for practices and behavior that could potentially be improved and can fix some of the most common ones for you
https://ansible.readthedocs.io/projects/lint/
GNU General Public License v3.0
3.45k stars 654 forks source link

New rule to require using pipefail on shell scripts using PIPE #442

Closed ssbarnea closed 5 years ago

ssbarnea commented 5 years ago

Issue Type

Desired Behaviour

Ansible lint should warning user that he needs to use set -o pipefail when he is using PIPE (|) in shells scripts as otherwise the command error code could be lost.

We have being succesfully using such extra test for a very long period of time and we would be happy to make it part of default ansible-lint rules:

See https://github.com/openstack/tripleo-quickstart-extras/blob/master/ci-scripts/ansible_rules/ShellPipefail.py

Example:

- shell: |
    df | grep '/dev'

This code may fail to return a non zero exit code from df if grep will still have a match.

The safer way to write this code would be

- shell: |
    set -o pipefail
    df | grep '/dev'
ssbarnea commented 5 years ago

Interesting surprise to see that there is a two years old PR adding this at https://github.com/ansible/ansible-lint/pull/199 -- not sure how it compares with the rule we have but I don't care which one will be merged as long it will be, hopefully soon.

awcrosby commented 5 years ago

Closing as #199 as been merged.

ssbarnea commented 5 years ago

@awcrosby Thanks! Can't wait to see the new release.

gmr commented 5 years ago

FWIW I think this assumes a bit about the shell being used. Dash (Debian Almquist Shell) does not support -o pipefail and is the default shell in Ubuntu Xenial.