Repository: ansible/ansible
Pull request: Properly handle user selection of `None` as vars_files
In a playbook, if a user has a playbook like:

```
- hosts: localhost
  connection: local
  vars_files:
  tasks:
  - ....
```

Then `vars_files` will be none, and cause a `TypeError` in vars-manager when it
tries to iterate over them. To avoid this, I changed the getter to either send
back the vars files from the user, or an empty list when the user passed
`None`.

##### SUMMARY
<!--- Describe the change, including rationale and design decisions -->

<!---
If you are fixing an existing issue, please include "Fixes #nnn" in your
commit message and your description; but you should still explain what
the change does.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->

 - Bugfix Pull Request

##### COMPONENT NAME
<!--- Name of the module/plugin/module/task -->
vars manager
 
##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.0 (devel bba941cd5b) last updated 2017/10/03 15:59:31 (GMT -400)
  config file = None
  configured module search path = [u'/home/ryansb/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  python version = 2.7.12 (default, Sep 27 2016, 18:08:33) [GCC 6.2.1 20160916 (Red Hat 6.2.1-2)]
```


##### ADDITIONAL INFORMATION
<!---
Include additional information to help people understand the change here.
For bugs that don't have a linked bug report, a step-by-step reproduction
of the problem is helpful.
  -->
This fixes #31249 

<!--- Paste verbatim command output below, e.g. before and after your change -->
```
BEFORE:


PLAYBOOK: lol.yml *******************************************************************************
1 plays in /tmp/lol.yml
ERROR! Unexpected Exception, this is probably a bug: 'NoneType' object is not iterable
the full traceback was:

Traceback (most recent call last):
  File "/tmp/ansible/bin/ansible-playbook", line 109, in <module>
    exit_code = cli.run()
  File "/tmp/ansible/lib/ansible/cli/playbook.py", line 130, in run
    results = pbex.run()
  File "/tmp/ansible/lib/ansible/executor/playbook_executor.py", line 126, in run
    all_vars = self._variable_manager.get_vars(play=play)
  File "/tmp/ansible/lib/ansible/vars/manager.py", line 348, in get_vars
    for vars_file_item in play.get_vars_files():
TypeError: 'NoneType' object is not iterable


AFTER


TASK [debug] ************************************************************************************
task path: /tmp/lol.yml:5
ok: [localhost] => {
    "msg": "hi"
}
META: ran handlers
META: ran handlers

PLAY RECAP **************************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0  
```


Problem statement:
Unexpected Exception, this is probably a bug: 'NoneType' object is not iterable
<!---
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release, and master branch are affected too.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bug Report

##### COMPONENT NAME
<!--- Name of the module/plugin/task/feature -->
ansible-playbook
##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.0 (devel 101377768b) last updated 2017/10/03 14:54:07 (GMT +000)
  config file = /sbux_ansible/ansible.cfg
  configured module search path = [u'/sbux_ansible/library', u'/usr/share/ansible']
  ansible python module location = /home/ubuntu/ansible/lib/ansible
  executable location = /home/ubuntu/ansible/bin/ansible
  python version = 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609]
```
also in 2.4, and observed in 2.3
```
ansible 2.3.1.0
  config file = /home/administrator/ansible/playbooks/ansible.cfg
  configured module search path = [u'/usr/share/ansible/']
  python version = 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609]
```

##### CONFIGURATION
<!---
If using Ansible 2.4 or above, paste the results of "ansible-config dump --only-changed"

Otherwise, mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).

-->
```
DEFAULT_HOST_LIST(/sbux_ansible/ansible.cfg) = [u'/sbux_ansible/inventory.txt']
DEFAULT_MODULE_PATH(/sbux_ansible/ansible.cfg) = [u'/sbux_ansible/library', u'/usr/share/ansible']
RETRY_FILES_ENABLED(/sbux_ansible/ansible.cfg) = False
```
##### OS / ENVIRONMENT
<!---
Mention the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.
-->
`ubuntu/xenial64`
##### SUMMARY
<!--- Explain the problem briefly -->
Method `play.get_vars_files()` returns None rather than an empty list. In very large playbooks, it is not obvious what is causing the exception.
##### STEPS TO REPRODUCE
<!---
For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used.
-->
If `vars_files` has no entries (note the comment), the play fails with `TypeError: 'NoneType' object is not iterable`
<!--- Paste example playbooks or commands between quotes below -->
```yaml
#!/usr/bin/ansible-playbook
---
#
#
#      Copyright (c) 2017 World Wide Technology, Inc.
#      All rights reserved.
#
#      author: Joel W. King,  World Wide Technology
#
#
- hosts: localhost
  connection: local
  user: administrator

  vars_files:
   #  - "{{playbook_dir}}/passwords.yml"

  tasks:
  - name: debug the output
    debug: msg="{{f5.username}}"

```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
Ignore the empty list or issue a warning to the user.
##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->

<!--- Paste verbatim command output between quotes below -->
```
ubuntu@ubuntu-xenial:~/ansible/playbooks$ ansible-playbook ./debug_vars_files.yml -vvv
ansible-playbook 2.5.0 (devel 101377768b) last updated 2017/10/03 14:54:07 (GMT +000)
  config file = /sbux_ansible/ansible.cfg
  configured module search path = [u'/sbux_ansible/library', u'/usr/share/ansible']
  ansible python module location = /home/ubuntu/ansible/lib/ansible
  executable location = /home/ubuntu/ansible/bin/ansible-playbook
  python version = 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609]
Using /sbux_ansible/ansible.cfg as config file
Parsed /sbux_ansible/inventory.txt inventory source with ini plugin

PLAYBOOK: debug_vars_files.yml *****************************************************************************************************
1 plays in ./debug_vars_files.yml
ERROR! Unexpected Exception, this is probably a bug: 'NoneType' object is not iterable
the full traceback was:

Traceback (most recent call last):
  File "/home/ubuntu/ansible/bin/ansible-playbook", line 109, in <module>
    exit_code = cli.run()
  File "/home/ubuntu/ansible/lib/ansible/cli/playbook.py", line 130, in run
    results = pbex.run()
  File "/home/ubuntu/ansible/lib/ansible/executor/playbook_executor.py", line 126, in run
    all_vars = self._variable_manager.get_vars(play=play)
  File "/home/ubuntu/ansible/lib/ansible/vars/manager.py", line 348, in get_vars
    for vars_file_item in play.get_vars_files():
TypeError: 'NoneType' object is not iterable
```

Hints from issue discussion:
The workaround here is, of course, to supply an empty list instead of a `None`. I notice that the vars file attributes are defaulted to `[]`, but won't override if the use supplies `None` which we could definitely handle more gracefully.

Commit message: Catch error when vars_files isn't iterable

Patch to review:
diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py
index 2fe4a9020def71..ccd3b323a1d7ef 100644
--- a/lib/ansible/playbook/play.py
+++ b/lib/ansible/playbook/play.py
@@ -277,6 +277,8 @@ def get_vars(self):
         return self.vars.copy()
 
     def get_vars_files(self):
+        if self.vars_files is None:
+            return []
         return self.vars_files
 
     def get_handlers(self):
diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py
index a29b186db614a2..93a601fc027743 100644
--- a/lib/ansible/vars/manager.py
+++ b/lib/ansible/vars/manager.py
@@ -345,7 +345,12 @@ def plugins_by_groups():
         if play:
             all_vars = combine_vars(all_vars, play.get_vars())
 
-            for vars_file_item in play.get_vars_files():
+            vars_files = play.get_vars_files()
+            if not hasattr(vars_files, '__iter__'):
+                raise AnsibleParserError("Error while reading vars files - please supply a list of file names. "
+                                         "Got %s" % type(vars_files))
+
+            for vars_file_item in vars_files:
                 # create a set of temporary vars here, which incorporate the extra
                 # and magic vars so we can properly template the vars_files entries
                 temp_vars = combine_vars(all_vars, self._extra_vars)


Code review comments:
diff_hunk: @@ -345,7 +345,12 @@ def plugins_by_groups():
         if play:
             all_vars = combine_vars(all_vars, play.get_vars())
 
-            for vars_file_item in play.get_vars_files():
+            vars_files = play.get_vars_files()
+            if not hasattr(vars_files, '__iter__'):
path: lib/ansible/vars/manager.py
text: @user1:
bikeshed: its normally cheaper to catch the exception vs testing for attribute every time

@author:
Yeah, I just wasn't sure if we wanted to nest the whole loop in a try/except. Would that be preferable to having this check? 

@user1:
yeah, as long as you are capturing the specific exception

@author:
Done