Ignore non-valid file extensions for include_vars dirs

[maxamillion]

Signed-off-by: Adam Miller admiller@redhat.com

SUMMARY

Fixes #35745

Previously, if the dir contained any non-valid varfiles file extensions the module/task would error.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

plugins/action/include_vars.py

ANSIBLE VERSION

ansible 2.5.0 (include_vars 2dcfbd679a) last updated 2018/02/06 15:05:47 (GMT -500)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/admiller/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/admiller/src/dev/ansible/lib/ansible
  executable location = /home/admiller/src/dev/ansible/bin/ansible
  python version = 2.7.14 (default, Jan 17 2018, 14:28:32) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]

ADDITIONAL INFORMATION

Using the playbook:

---
- hosts: localhost
  tasks:
  - name: Fetch answer data
    git:
      repo: "https://github.com/maxamillion/ansible-bug-reproducers.git"
      version: master
      dest: /tmp/myvars/
      force: yes
  - include_vars:
      name: collected
      dir: "/tmp/myvars/35745-include_vars-file-extensions/vars"
      depth: 1
  - debug:
       var: collected

Before:

$(ansible)  ansible-playbook ~/src/abr/35745-include_vars-file-extensions/repro.yml
 [WARNING]: Could not match supplied host pattern, ignoring: all        
                                                                        
 [WARNING]: provided hosts list is empty, only localhost is available
                                                                        
                                                                                                                                                 
PLAY [localhost] ******************************************************************************************************************************  
                                                                                                                                                 
TASK [Gathering Facts] ************************************************************************************************************************
ok: [localhost]                                                         
                                                                                                                                                 
TASK [Fetch answer data] **********************************************************************************************************************
changed: [localhost]                                                                                                                             
                                                                        
TASK [include_vars] ***************************************************************************************************************************  
fatal: [localhost]: FAILED! => {"ansible_facts": {"collected": {}}, "ansible_included_var_files": [], "changed": false, "message": "/tmp/myvars/35745-include_vars-file-extensions/vars/README.md does not have a valid extension: yaml, yml, json"}
        to retry, use: --limit @/home/admiller/src/abr/35745-include_vars-file-extensions/repro.retry
                                                                        
PLAY RECAP ************************************************************************************************************************************
localhost                  : ok=2    changed=1    unreachable=0    failed=1  

After:

$(ansible)  ansible-playbook ~/src/abr/35745-include_vars-file-extensions/repro.yml
 [WARNING]: Could not match supplied host pattern, ignoring: all         
                                                                                                                                                 
 [WARNING]: provided hosts list is empty, only localhost is available   
                                                                                                                                                 
                                                                        
PLAY [localhost] ******************************************************************************************************************************  
                                                                                                                                                 
TASK [Gathering Facts] ************************************************************************************************************************
ok: [localhost]                                                                                                                                  
                                                                                                                                                 
TASK [Fetch answer data] **********************************************************************************************************************
ok: [localhost]

TASK [include_vars] ***************************************************************************************************************************
ok: [localhost]

TASK [debug] **********************************************************************************************************************************
ok: [localhost] => {
    "collected": {
        "testing": "it works!"
    }
}

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

[samdoran]

Further discussion is planned for the next meeting.

cc @bcoca

[abadger]

We discussed at today's meeting but there was no maxamillion or bcoca. This seems like there's no downside (currently users with files in vars with unknown extensions will break. After this, the files will be ignored).

We'd like input from @bcoca or @maxamillion as to what the cons might be otherwise we're inclined to say go ahead and merge.

[abadger]

bcoca arrived during open floor and we talked this over. Decided that changing behaviour needs a toggle for backwards compat. We disagreed on whether the toggle could itself be part of a deprecation cycle to change the default behaviour (and potentially remove the failure as an option in the future). bcoca thinks that using a dir with filters should be done in jinja2 with a loop instead. I think that using these parameters as filters for which files to consider (which is what the documentation shows) is fine. The problem to me is that they're not filters, they're statements of files to error on.

Implementing a toggle is enough to move this forward.

[bcoca]

fyi, jinja is not needed, that was one example, using the find module can get same results w/o having to deal with jinja

[maxamillion]

sorry for the lag time on this, now with a new option to keep backwards compat

[ansibot]

cc @linuxdynasty
click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/include_vars.py:0:0: E309 version_added for new option (ignore_unkown_extensions) should be 2.6. Currently 2.7

click here for bot help

[maxamillion]

@abadger @nitzmahone ping - I was going to set the version of this to 2.7 and defer until then because 2.6 is a bugfix only and this is technically a feature but the sanity test is angry about that. Thoughts on how bets to handle this?

[mattclay]

@maxamillion There's no way to avoid the error prior to merging until devel is for 2.7. Once stable-2.6 is branched this will pass CI, assuming no new issues come up.

[maxamillion]

rebuild_merge

[maxamillion]

Oh bleh, nvm ... the branch hasn't happened yet. Tabling for now.