Unexpected behavior in when: conditions

[wknapik]

ISSUE TYPE

Bug Report

COMPONENT NAME

core

ANSIBLE VERSION

ansible 2.5.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/foo/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.14 (default, Sep 20 2017, 01:25:59) [GCC 7.2.0]

2.5.0.34173.3d63ecb6f3-1

CONFIGURATION

DEFAULT_STDOUT_CALLBACK(/etc/ansible/ansible.cfg) = debug

OS / ENVIRONMENT

N/A

SUMMARY

Unexpected behavior in when conditions.

STEPS TO REPRODUCE

- set_fact:
    foo: []
- debug: msg="foo"
  when: foo
- debug: msg="bar"
  when: []
- debug: msg="baz"
  when: ([])

EXPECTED RESULTS

I expected the same behavior in each case - skip/skip/skip.

ACTUAL RESULTS

Got different behaviors - skip/run/skip.

[wknapik]

Maybe I'm misunderstanding something and it's supposed to work this way, but it is counterintuitive and difficult to debug.

[alikins]

reproducer:

---
- hosts: localhost
  gather_facts: false
  vars:
    bar: []
  tasks:
  - set_fact:
      foo: []
  - debug:
      msg: "foo (empty list fact)"
    when: foo
  - debug:
      msg: "bar (empty list var)"
    when: bar
  - debug:
      msg: "(empty list inline)"
    when: []
  - debug:
      msg: "baz (empty list in a tuple inline)"
    when: ([])

result:

[newswoop:F27:ansible (devel % u=)]$ ansible-playbook -vvvvv ping_when_list.yml 
ansible-playbook 2.5.0 (devel 7b19c28438) last updated 2017/11/27 12:32:25 (GMT -400)
PLAYBOOK: ping_when_list.yml ***************************************************************************************************************************************************************************************
1 plays in ping_when_list.yml

PLAY [localhost] ***************************************************************************************************************************************************************************************************
META: ran handlers

TASK [set_fact] ****************************************************************************************************************************************************************************************************
task path: /home/adrian/src/ansible/ping_when_list.yml:7
ok: [localhost] => {
    "ansible_facts": {
        "foo": []
    }, 
    "changed": false
}

TASK [debug] *******************************************************************************************************************************************************************************************************
task path: /home/adrian/src/ansible/ping_when_list.yml:9
skipping: [localhost] => {
    "changed": false, 
    "skip_reason": "Conditional result was False"
}

TASK [debug] *******************************************************************************************************************************************************************************************************
task path: /home/adrian/src/ansible/ping_when_list.yml:12
skipping: [localhost] => {
    "changed": false, 
    "skip_reason": "Conditional result was False"
}

TASK [debug] *******************************************************************************************************************************************************************************************************
task path: /home/adrian/src/ansible/ping_when_list.yml:15
ok: [localhost] => {
    "msg": "(empty list inline)"
}

TASK [debug] *******************************************************************************************************************************************************************************************************
task path: /home/adrian/src/ansible/ping_when_list.yml:18
skipping: [localhost] => {
    "changed": false, 
    "skip_reason": "Conditional result was False"
}
META: ran handlers
META: ran handlers

PLAY RECAP *********************************************************************************************************************************************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0  

[rpijlman]

I wonder what Ansible's specification says about these cases. Is the behaviour wel-defined or unspecified?

[sivel]

I believe this is a side affect of the fact that a when statement can be a list, and when specified as a list the individual elements are implicitly ANDed. For example:

when:
   - a is defined
   - b is defined

is equivalent to:

when: a is defined and b is defined

So effectively when: [] becomes when: None

In ansible.playbook.conditional.Conditional.evaluate_conditional, we have logic that if no conditional was actually provided, it should return True

        try:
            # this allows for direct boolean assignments to conditionals "when: False"
            if isinstance(self.when, bool):
                return self.when

            for conditional in self.when:
                if not self._check_conditional(conditional, templar, all_vars):
                    return False
        except Exception as e:
            raise AnsibleError(
                "The conditional check '%s' failed. The error was: %s" % (to_native(conditional), to_native(e)), obj=ds
            )

        return True

I'm not sure what should be done here. As far as I know this logic has always been present.

[wknapik]

A list with one element evaluates to that element.
A list with multiple elements is ANDed.
But an empty list of conditions makes no sense - there's no way to evaluate it to either true, or false.

[sivel]

I'm not necessarily saying it makes sense, but it evaluates to the same thing as no condition, because a list of no conditions was provided. And historically, no condition is treated as True. Changing the behavior of this now, could have far reaching negative ramifications for backwards compatibility.

[wknapik]

An empty list should trigger an error, or at least a warning, if an explicit cast to bool is not used.

As a bool, it's false; as a list of conditions, it makes no sense and the current behavior is to return true, when it's a literal (unless it's wrapped in (), in which case it's false) and false, when it's a variable (regardless of whether () is used).

How many users, do you think, understand this behavior ?

[wknapik]

In this major version, the warning seems like the only viable option.

[sivel]

Let's take a step back and talk about the use case. What use case do you have that you need to explicitly specify when: []?

The more I think about it, this seems like an extreme edge case, as I can't think of a time when a person would explicitly use when: []. I'm not sure what that achieves or why you would do it in the first place.

needs_info

[wknapik]

Well, I wasn't fuzzing ansible. I ran into this in the course of regular usage.

I was debugging the conditions in a few tasks and figured I needed to simplify them to the point where I could be absolutely certain I understood what was going on and then build them back up towards what I wanted. That's how I ended up with a when: [] condition. When I re-ran the tasks I was even more confused, because [] evaluates to false in basically every language, that accepts lists as conditions, including ansible and python.

I don't see how adding a warning could be anything but good in this situation, especially that there's already a warning, in the same function, about using jinja templates (even though it's a legitimate use case). And the new warning could be trivially muted by an explicit cast to bool, while the jinja warning is impossible to get rid of (afaik).

The jinja warning is there because the user might be doing something that will not work the way they expect, which is exactly what this issue is about.

But I think this discussion is moving away from the more significant point, which is that this implementation is simply wrong. It makes no mathematical sense.
And treating literals and variables holding those same literals differently is also utterly wrong.

But hey, if you want to sweep the problem under the carpet and pretend it doesn't exist - go ahead. I've exhausted the energy I could/would devote to this.

[sivel]

We decided in todays core meeting, to change the default to prevent when: [] from evaluating as True.

@bcoca will be providing a pull request to address.

[samdoran]

After further discussion and attempting to fix this, we have decided to not fix this. This is only triggered when setting when to a literal empty list, not a variable that evaluates to an empty list.

For further help with the issue, please reach out on IRC or the mailing list:

   * IRC: #ansible on irc.freenode.net  
   * mailing list: https://groups.google.com/forum/#!forum/ansible-project