include_vars should not error on files with non-matching extensions

[gregswift]

ISSUE TYPE

• Bug Report

COMPONENT NAME

include_vars

ANSIBLE VERSION

$ ansible --version
ansible 2.4.2.0
  config file = /home/xaeth/.ansible.cfg
  configured module search path = [u'/home/xaeth/.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, Jan 17 2018, 14:28:32) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]

CONFIGURATION

$ ansible-config dump --only-changed
ANSIBLE_SSH_ARGS(/home/xaeth/.ansible.cfg) = -o ForwardAgent=yes -o PubKeyAuthentication=yes -o ControlMaster=auto -o ControlPersist=120s -o ConnectTimeout=15
ANSIBLE_SSH_CONTROL_PATH(/home/xaeth/.ansible.cfg) = ~/.ssh/sock/%%h:%%p
DEFAULT_HOST_LIST(env: ANSIBLE_INVENTORY) = [u'/home/xaeth/ansible/inventory']
DEFAULT_TRANSPORT(/home/xaeth/.ansible.cfg) = ssh

OS / ENVIRONMENT

N/A

SUMMARY

When using the include_vars module if the directory or any of its nested directories contain a file that doesn't match the default or defined extension the task will fail. By default include_vars looks for yml, yaml, and json files. If we add the files to the ignore_files parameter explicitly then things work. Seems like this shouldn't be necessary.

STEPS TO REPRODUCE

While this will happen regardless of how you manage the directory, I think my example shows a good use case for the why. In this case I'm using is checking out a git repo that has files that need to be included in. That repository has a README.md, Jenkinsfile, and Makefile along with the var files ending in a .yml extension. The playbook checks out the git repo, and then tries to load all included files.

---
- hosts: localhost
  tasks:
  - name: Fetch answer data
    git:
      repo: "https://github.com/myrepo/myvars"
      version: master
      dest: vars/myvars/
      force: yes
  - include_vars:
      name: collected
      dir: vars/myvars
      depth: 1
  - debug:
       var: collected

EXPECTED RESULTS

TASK [debug] *****************************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "collected": {
        "mysql": {
            "groups": [
                {
                    "name": "mysql-admin"
                }
            ], 
            "remote": [
                {
.. snip ..

ACTUAL RESULTS

TASK [include_vars] **********************************************************************************************************************************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"ansible_facts": {"collected": {}}, "ansible_included_var_files": [], "changed": false, "message": "/home/xaeth/Development/ansible/playbooks/vars/myvars/README.md does not have a valid extension: json, yaml, yml"}
	to retry, use: --limit @/home/xaeth/Development/ansible/playbooks/test-answer.retry

[ansibot]

Files identified in the description:

• lib/ansible/modules/utilities/logic/include_vars.py

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibot]

cc @linuxdynasty
click here for bot help

[bcoca]

It functions as intended and designed, this 'feature' was only thing you could not do already in loops.

- include_vars:
     name: collected
     file: '{{item}}'
  loop: "{{q('fileglob', 'vars/myvars/*' }}"

I actually recommend using loop instead in almost any case, even when you need the 'foreign file alert', its better to just have a find task that fails when they are present.

[gregswift]

@bcoca

So, generally I get how maybe the new loop stuff provides an alternate path. However, i just went to implement new something using this, and again i'm reading through the examples. These examples just don't make sense in a scenario where non-matching files throw an error.

- name: Include all .json and .jsn files in vars/all and all nested directories (2.3)
  include_vars:
    dir: 'vars/all'
    extensions:
        - json
        - jsn

- name: Include all default extension files in vars/all and all nested directories and save the output in test. (2.2)
  include_vars:
    dir: 'vars/all'
    name: test

- name: Include default extension files in vars/services (2.2)
  include_vars:
    dir: 'vars/services'
    depth: 1

Just reading the names (which i get are overly descriptive for the sake of documentation) my gut just screams... what about the non-matching extensions?!

Additionally, nothing in the module documentation states that this is expected behavior.

Finally, using the module as defined by its help documentation and its defined parameters is significantly more discoverable than using lookups in inline jinja.

oh wait.. and it doesnt help that it doesnt work in 2.4 ?

TASK [include_vars] ************************************************************************************************************************
fatal: [127.0.0.1]: FAILED! => {"msg": "Unexpected failure in finding the lookup named '{{ q('fileglob', 'customers/*' }}' in the available lookup plugins"}

TASK [include_vars] ************************************************************************************************************************
fatal: [127.0.0.1]: FAILED! => {"msg": "Unexpected failure in finding the lookup named '{{ lookup('fileglob', 'customers/*' }}' in the available lookup plugins"}

of course if i use with_fileglobs it works

[bcoca]

Not just 'new loop', old with_ loops provide the same i.e with_fileglob.

I cannot speak to docs but to discussions when the 'feature' was being developed, I even 'fixed' this at one point as part of removing restrictions on the 'naked' include_vars: filename that this 'feature' had accidentally added and was asked to 'unfix' for the directory option.

[linuxdynasty]

@gregswift when I initially updated the module with the current help it did work as expected, but some of the features I added were taken out if I am not mistaken, shortly after being merged. Since I label all of my vars with yml or json all worked as expected as I didn't realize not everyone gives extensions to the vars files. I have a locally modified version that I use that is essentially the initial revision that was merged in. That being said I do not have a good answer for you, but I can send you the version I am using and that is 100% compliant with the documentation string. I currently use it with Ansible 2.4.2

[gregswift]

So far in my playing with_fileglobs is actually worse in behavior than what I'd expect.

    - include_vars:
        file: "{{ item }}"
        name: "test"
      with_fileglob: "files/*.yml"

Returns only the last file parsed inside the variable 'test' (yes i'm aware of the other discussions around this). However, using include_vars by itself, like this, brings them all together just fine:

- include_vars:
    name: "test"
    dir: "files"
    depth: "1"

Frankly, the reason this has been an annoyance is that I like to include README's in the base of my repositories, and ansible just really seems to hate that concept (inventory throws a warning and per this ticket, include_vars errors).

While I get that the docs work 100% as they read, it is also possible to read the extension parameter docs as implying that this is to limit what is parsed, not just extending.

There seems to be 3 paths afaict:

• State ' Hey... if there are any other file types we blow up. Don't do that. Its a feature not a bug '.
• Respect the extensions parameter and ignore non-matching files in the directory (also making this behavior explicit in the docs would be good)
• Provide a 'ignore_other_extensions' that supports this behavior

[rackergs]

that commit looks great. thanks @maxamillion

[gregswift]

🎉 👯‍♂️ 👯‍♀️ 🎉

thanks for the work @maxamillion

[maxamillion]

@gregswift anytime, happy to help!