Repository: ansible/ansible
Pull request: first_found lookup, let lookup handle templating errors
Avoids case in which TE was not sending valid and templatable entries to the lookup The lookup already handles the case TE was attempting to itself, so no need for this code anymore.
fixes #70772

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
lookups/first_found

Problem statement:
with_first_found fails to find a file if an undefined variable is used in the list since ansible 2.9
##### SUMMARY
<!--- Explain the problem briefly below -->
with_first_found fails "No file was found when using first_found. ..." even if a file matches search conditions, when an undefined variable is used in files parameter.
This is started after I've upgraded from ansible 2.8 to 2.9, so I suppose it is a bug.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
first_found

##### ANSIBLE VERSION
```paste below
ansible 2.9.10
  config file = None
  configured module search path = ['/Users/afunix/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/ansible/2.9.10/libexec/lib/python3.8/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.8.4 (default, Jul 14 2020, 02:58:48) [Clang 11.0.3 (clang-1103.0.32.62)]
```

##### CONFIGURATION
<!--- Paste verbatim output from "ansible-config dump --only-changed" between quotes -->
```paste below
DEFAULT_VAULT_PASSWORD_FILE(env: ANSIBLE_VAULT_PASSWORD_FILE) = /Users/afunix/.vault
```

##### OS / ENVIRONMENT
Host: macOS 10.15.5
Target: CentOS 7.8


##### STEPS TO REPRODUCE

playbook.yml
```yaml
---
- hosts: all
  roles:
      - test

```

roles/test/tasks/main.yml
```yaml
---
- debug:
      msg: "path: {{role_path}}/vars"

- debug:
    msg: "file: {{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"

- name: Load variables specific for OS family
  include_vars: "{{item}}"
  with_first_found:
    - files:
        - "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml"
        - "{{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"
      paths:
        - "{{role_path}}/vars"

```


roles/test/vars/CentOS-7.yml
```yaml
---
test: test
```


##### EXPECTED RESULTS
ansible-playbook is able to find CentOS-7.yml when running against CentOS 7.8 system, since the file exists.


##### ACTUAL RESULTS
ansible-playbook cannot find CentOS-7.yml when running against CentOS 7.8 system.

The issue goes away once I remove "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml" from files.

Checked ansible -m setup for the host and ansible_lsb is empty.

```paste below
$ ansible-playbook -i inventory playbook.yml 

PLAY [all] *****************************************************************************************************************************************************************************************************

TASK [Gathering Facts] *****************************************************************************************************************************************************************************************
ok: [alice]

TASK [test : debug] ********************************************************************************************************************************************************************************************
ok: [alice] => {
    "msg": "path: /Users/afunix/tmp/ansible-with-first-found/roles/test/vars"
}

TASK [test : debug] ********************************************************************************************************************************************************************************************
ok: [alice] => {
    "msg": "file: CentOS-7.yml"
}

TASK [test : Load variables specific for OS family] ************************************************************************************************************************************************************
fatal: [alice]: FAILED! => {"msg": "No file was found when using first_found. Use errors='ignore' to allow this task to be skipped if no files are found"}

PLAY RECAP *****************************************************************************************************************************************************************************************************
alice                      : ok=3    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

$ stat /Users/afunix/tmp/ansible-with-first-found/roles/test/vars/CentOS-7.yml
16777220 38020305 -rw-r--r-- 1 afunix staff 0 15 "Jul 20 18:11:55 2020" "Jul 20 18:11:38 2020" "Jul 20 18:11:38 2020" "Jul 20 18:11:38 2020" 4096 8 0 /Users/afunix/tmp/ansible-with-first-found/roles/test/vars/CentOS-7.yml

$ ansible -i inventories/production/hosts -m setup alice | grep ansible_lsb
        "ansible_lsb": {},
```

Hints from issue discussion:
Files identified in the description:
None

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

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
Files identified in the description:
* [`lib/ansible/plugins/lookup/first_found.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/lookup/first_found.py)

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

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
The issue here, is in filtering out untemplated terms in the search.  We only account for strings in the terms in this code path.

The following diff should resolve the issue:

```diff
diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index 5034bbc862..373f47e082 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -19,6 +19,7 @@ from ansible import constants as C
 from ansible.errors import AnsibleError, AnsibleParserError, AnsibleUndefinedVariable, AnsibleConnectionFailure, AnsibleActionFail, AnsibleActionSkip
 from ansible.executor.task_result import TaskResult
 from ansible.executor.module_common import get_action_args_with_defaults
+from ansible.module_utils.common._collections_compat import Mapping
 from ansible.module_utils.parsing.convert_bool import boolean
 from ansible.module_utils.six import iteritems, string_types, binary_type
 from ansible.module_utils.six.moves import xrange
@@ -223,8 +224,17 @@ class TaskExecutor:
 
                 loop_terms = listify_lookup_plugin_terms(terms=self._task.loop, templar=templar, loader=self._loader, fail_on_undefined=fail,
                                                          convert_bare=False)
+
                 if not fail:
-                    loop_terms = [t for t in loop_terms if not templar.is_template(t)]
+                    intermediate = []
+                    for term in loop_terms:
+                        if isinstance(term, Mapping) and 'files' in term:
+                            term['files'] = [t for t in term['files'] if not templar.is_template(t)]
+                            intermediate.append(term)
+                        else:
+                            if not templar.is_template(term):
+                                intermediate.append(term)
+                    loop_terms = intermediate
 
                 # get lookup
                 mylookup = self._shared_loader_obj.lookup_loader.get(self._task.loop_with, loader=self._loader, templar=templar)
```

If anyone is interested in testing this further, and creating tests to validate it, please feel free to do so.
Hey @afunix 

Are you able to test the fix and provide some feedback ?
@afunix I eventually took the patch and did a PR.
I confirm the change fixes my issue.
needs_verified (first_found input was revamped and that might have fixed this issue)
I can still reproduce the issue on ansible 2.12.5 (MacOSX, installed with brew)
I'm also still hitting this issue and wondering if there is a fix for this
This is my work-around, instead of sth like this:

```
  with_first_found:
    - files:
        - "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml"
        - "{{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"
      paths:
        - "{{role_path}}/vars"
```

instead, you can use:

```
  with_first_found:
    - files:
        - "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml"
      paths:
        - "{{role_path}}/vars"
    - files:
        - "{{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"
      paths:
        - "{{role_path}}/vars"
```

Commit message: restore first_found fail for lookup terms

Patch to review:
diff --git a/changelogs/fragments/first_found_template_fix.yml b/changelogs/fragments/first_found_template_fix.yml
new file mode 100644
index 00000000000000..70fe6b58628e1c
--- /dev/null
+++ b/changelogs/fragments/first_found_template_fix.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - first_found lookup now gets 'untemplated' loop entries and handles templating itself as task_executor was removing even 'templatable' entries and breaking functionality. https://github.com/ansible/ansible/issues/70772
diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index bc08c6791ae9dd..1e475335b3b115 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -224,14 +224,11 @@ def _get_loop_items(self):
         items = None
         if self._task.loop_with:
             if self._task.loop_with in self._shared_loader_obj.lookup_loader:
-                fail = True
-                if self._task.loop_with == 'first_found':
-                    # first_found loops are special. If the item is undefined then we want to fall through to the next value rather than failing.
-                    fail = False
 
+                # TODO: hardcoded so it fails for non first_found lookups, but thhis shoudl be generalized for those that don't do their own templating
+                # lookup prop/attribute?
+                fail = bool(self._task.loop_with == 'first_found')
                 loop_terms = listify_lookup_plugin_terms(terms=self._task.loop, templar=templar, fail_on_undefined=fail, convert_bare=False)
-                if not fail:
-                    loop_terms = [t for t in loop_terms if not templar.is_template(t)]
 
                 # get lookup
                 mylookup = self._shared_loader_obj.lookup_loader.get(self._task.loop_with, loader=self._loader, templar=templar)
diff --git a/lib/ansible/plugins/lookup/first_found.py b/lib/ansible/plugins/lookup/first_found.py
index 888370b8e37b14..990693c5e36169 100644
--- a/lib/ansible/plugins/lookup/first_found.py
+++ b/lib/ansible/plugins/lookup/first_found.py
@@ -226,6 +226,8 @@ def run(self, terms, variables, **kwargs):
             try:
                 fn = self._templar.template(fn)
             except (AnsibleUndefinedVariable, UndefinedError):
+                # NOTE: backwards compat ff behaviour is to ignore errors when vars are undefined.
+                #       moved here from task_executor.
                 continue
 
             # get subdir if set by task executor, default to files otherwise
diff --git a/test/integration/targets/lookup_first_found/tasks/main.yml b/test/integration/targets/lookup_first_found/tasks/main.yml
index d0ca91eaf54415..d6942b937f37fd 100644
--- a/test/integration/targets/lookup_first_found/tasks/main.yml
+++ b/test/integration/targets/lookup_first_found/tasks/main.yml
@@ -102,3 +102,41 @@
 
 - assert:
     that: "no_terms|first == '/etc/hosts'"
+
+- name: handle templatable dictionary entries
+  block:
+
+  - name: Load variables specific for OS family
+    assert:
+      that:
+        - "{{item}} is file"
+    with_first_found:
+      - files:
+          - "{{ansible_id}}-{{ansible_lsb.major_release}}.yml"  # invalid var, should be skipped
+          - "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml"
+          - "{{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"
+          - itworks.yml
+        paths:
+          - "{{role_path}}/vars"
+
+  - name: Load variables specific for OS family, but now as list of dicts
+    assert:
+      that:
+        - "{{item}} is file"
+    with_first_found:
+      - files:
+          - "{{ansible_id}}-{{ansible_lsb.major_release}}.yml"  # invalidvar, should be skipped
+        paths:
+          - "{{role_path}}/vars"
+      - files:
+          - "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml"
+        paths:
+          - "{{role_path}}/vars"
+      - files:
+          - "{{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"
+        paths:
+          - "{{role_path}}/vars"
+      - files:
+          - itworks.yml
+        paths:
+          - "{{role_path}}/vars"
diff --git a/test/integration/targets/lookup_first_found/vars/itworks.yml b/test/integration/targets/lookup_first_found/vars/itworks.yml
new file mode 100644
index 00000000000000..8f8a21a46d7314
--- /dev/null
+++ b/test/integration/targets/lookup_first_found/vars/itworks.yml
@@ -0,0 +1 @@
+doesit: yes it does


Code review comments:
diff_hunk: @@ -224,14 +224,11 @@ def _get_loop_items(self):
         items = None
         if self._task.loop_with:
             if self._task.loop_with in self._shared_loader_obj.lookup_loader:
-                fail = True
-                if self._task.loop_with == 'first_found':
-                    # first_found loops are special. If the item is undefined then we want to fall through to the next value rather than failing.
-                    fail = False
 
+                # TODO: hardcoded so it fails for non first_found lookups, but thhis shoudl be generalized for those that don't do their own templating
+                # lookup prop/attribute?
+                fail = bool(self._task.loop_with == 'first_found')
path: lib/ansible/executor/task_executor.py
text: @user1:
lgtm

```suggestion
                fail = bool(self._task.loop_with != 'first_found')
```

@author:
duh! `<slaps forhead>`