From 39945b85707490dbd9eb814641991277be1f7e2e Mon Sep 17 00:00:00 2001
From: Ian Wienand <iwienand@redhat.com>
Date: Thu, 30 May 2019 03:23:03 +1000
Subject: [PATCH] Make query with errors='ignore' return a blank list (#57038)

The jinja2 query() function (or lookup with wantslist=True, which is
the same thing) should always return a list.

However, if you combine a query with errors='ignore' and take the
error path, the current code returns a None value.  This is important
in a case such as

 - name: Conditional include file
   import_tasks: '{{ item }}'
   vars:
     params:
       files:
         - path/file1.yaml
         - path/file2.yaml
   loop: "{{ q('first_found', params, errors='ignore') }}"

If neither file1.yaml or file2.yaml exist, this should do nothing by
returning an empty list to the loop.  Currently if you run the above
task you'll get a rather unhelpful:

 Invalid data passed to 'loop', it requires a list, got this instead: .

This change ensures that when a query ignores an error, it returns a
empty list.  The errors='ignore' case is tested in several variants
with first_found.  The extant (but deprecated) "skip: True" for
first_found doesn't seem to be explicitly tested; a test is added here
to avoid regressions before removal in 2.12.

This fixes a regression you'll hit if you follow the suggestion in the
deprecation message included with
e17a2b502d6601be53c60d7ba1c627df419460c9 to use errors=ignore over
"skip: True" for first_found.  This change adds an example that points
out the query/lookup difference and also fixes the error message to
not mention the now deprecated "skip: True".

Closes #56775
---
 lib/ansible/plugins/lookup/first_found.py     | 13 ++++-
 lib/ansible/template/__init__.py              |  2 +-
 .../targets/iterators/tasks/main.yml          | 56 +++++++++++++++++++
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/lib/ansible/plugins/lookup/first_found.py b/lib/ansible/plugins/lookup/first_found.py
index ef4b045649a..a6fb1b7b8ff 100644
--- a/lib/ansible/plugins/lookup/first_found.py
+++ b/lib/ansible/plugins/lookup/first_found.py
@@ -44,6 +44,17 @@ EXAMPLES = """
       - "bar.txt"  # will be looked in files/ dir relative to role and/or play
       - "/path/to/biz.txt"
 
+- name: |
+        include tasks only if files exist.  Note the use of query() to return
+        a blank list for the loop if no files are found.
+  import_tasks: '{{ item }}'
+  vars:
+    params:
+      files:
+        - path/tasks.yaml
+        - path/other_tasks.yaml
+  loop: "{{ q('first_found', params, errors='ignore') }}"
+
 - name: |
         copy first existing file found to /some/file,
         looking in relative directories from where the task is defined and
@@ -166,5 +177,5 @@ class LookupModule(LookupBase):
                 return [path]
         if skip:
             return []
-        raise AnsibleLookupError("No file was found when using first_found. Use the 'skip: true' option to allow this task to be skipped if no "
+        raise AnsibleLookupError("No file was found when using first_found. Use errors='ignore' to allow this task to be skipped if no "
                                  "files are found")
diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py
index 2d168e48f3f..f88b7165db1 100644
--- a/lib/ansible/template/__init__.py
+++ b/lib/ansible/template/__init__.py
@@ -706,7 +706,7 @@ class Templar:
                         display.display(msg, log_only=True)
                     else:
                         raise AnsibleError(to_native(msg))
-                ran = None
+                ran = [] if wantlist else None
 
             if ran and not allow_unsafe:
                 if wantlist:
diff --git a/test/integration/targets/iterators/tasks/main.yml b/test/integration/targets/iterators/tasks/main.yml
index 917d97357c8..c7438ed338b 100644
--- a/test/integration/targets/iterators/tasks/main.yml
+++ b/test/integration/targets/iterators/tasks/main.yml
@@ -270,3 +270,59 @@
         - "b__ == 'flattened'"
         - "c__ == 'flattened'"
         - "d__ == 'flattened'"
+
+
+# q(FIRST_FOUND)
+- name: test q(first_found) with no files produces empty list
+  set_fact:
+    first_found_var: "{{ q('first_found', params, errors='ignore') }}"
+  vars:
+    params:
+      files: "not_a_file.yaml"
+
+- name: verify q(first_found) result
+  assert:
+    that:
+      - "first_found_var == []"
+
+- name: test lookup(first_found) with no files produces empty string
+  set_fact:
+    first_found_var: "{{ lookup('first_found', params, errors='ignore') }}"
+  vars:
+    params:
+      files: "not_a_file.yaml"
+
+- name: verify lookup(first_found) result
+  assert:
+    that:
+      - "first_found_var == ''"
+
+# NOTE: skip: True deprecated e17a2b502d6601be53c60d7ba1c627df419460c9, remove 2.12
+- name: test first_found with no matches and skip=True does nothing
+  set_fact: "this_not_set={{ item }}"
+  vars:
+    params:
+      files:
+        - not/a/file.yaml
+        - another/non/file.yaml
+      skip: True
+  loop: "{{ q('first_found', params) }}"
+
+- name: verify skip
+  assert:
+    that:
+      - "this_not_set is not defined"
+
+- name: test first_found with no matches and errors='ignore' skips in a loop
+  set_fact: "this_not_set={{ item }}"
+  vars:
+    params:
+      files:
+        - not/a/file.yaml
+        - another/non/file.yaml
+  loop: "{{ query('first_found', params, errors='ignore') }}"
+
+- name: verify errors=ignore
+  assert:
+    that:
+      - "this_not_set is not defined"