Repository: ansible/ansible
Pull request: Fix ansible.builtin.include_vars - depth
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Changed vars.py, so it starts from depth 1 as root, as suggested by @sivel 
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
Fixes #80987 

##### 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 -->
vars.py

##### ADDITIONAL INFORMATION
<!--- Include additional information to help people understand the change here -->
<!--- A step-by-step reproduction of the problem is helpful if there is no related issue -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```paste below

```


Problem statement:
ansible.builtin.include_vars - depth doesn't work as described
### Summary

Not all var files which are located in the specified depth of a "dir" are included.

### Issue Type

Bug Report

### Component Name

ansible.builtin.include_vars

### Ansible Version

```console
$ ansible --version
ansible [core 2.14.6]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/kpog/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3/dist-packages/ansible
  ansible collection location = /home/kpog/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.10.6 (main, May 29 2023, 11:10:38) [GCC 11.3.0] (/usr/bin/python3)
  jinja version = 3.0.3
  libyaml = True
```


### Configuration

```console
# if using a version older than ansible-core 2.12 you should omit the '-t all'
$ ansible-config dump --only-changed -t all
CONFIG_FILE() = /etc/ansible/ansible.cfg
```


### OS / Environment

Ubuntu22.04 - desktop

### Steps to Reproduce

<!--- Paste example playbooks or commands between quotes below -->
```yaml (paste below)
- name: Include vars example
  hosts: localhost

  tasks:
    - name: Include all yml configuration files from folder "{{ my_config_folder }}"
      vars:
        my_config_folder: "/home/kpog/WORK/TMP/include_vars/config"
      ansible.builtin.include_vars:
        dir: "{{ my_config_folder }}"
        extensions:
          - 'yml'
          - 'yaml'
        ignore_unknown_extensions: 'yes'
        depth: 2
        name: my_include_vars

    - name: Print included params
      debug:
        var: my_include_vars

```


### Expected Results

Config folder structure:
```
config
├── config0.yml
├── sub1
│   ├── config1.yml
│   └── sub11
│       ├── config11.yml
│       └── sub111
│           └── config111.yml
├── sub2
│   ├── config2.yml
│   └── sub21
│       ├── config21.yml
│       └── sub211
│           └── config211.yml
└── sub3
    └── config3.yml
```
On each configX.yml there is one parameter my_varX: "{{ folder of the file }}"

When using for example depth: "2" the config files:
config0.yml
config1.yml
config2.yml
config3.yml
should be included.





### Actual Results

```console
When using depth: "0" all configX.yml files are included. Result of playbook is:

ASK [Print included params] *******************************************************************************************************************************
ok: [localhost] => {
    "my_include_vars": {
        "my_name0": "config/config0.yml",
        "my_name1": "config/sub1/config1.yml",
        "my_name11": "config/sub1/sub11/config11.yml",
        "my_name111": "config/sub1/sub11/sub111/config111.yml",
        "my_name2": "config/sub2/config2.yml",
        "my_name21": "config/sub2/sub21/config21.yml",
        "my_name211": "config/sub2/sub21/sub211/config211.yml",
        "my_name3": "config/sub3/config3.yml"
    }
}

When using depth: "2" only config0.yml and config1.yml are included, but not config2.yml and config3.yml Result of playbook is:
TASK [Print included params] *******************************************************************************************************************************
ok: [localhost] => {
    "my_include_vars": {
        "my_name0": "config/config0.yml",
        "my_name1": "config/sub1/config1.yml"
    }
}


When using depth: "6" result of playbook is:
TASK [Print included params] *******************************************************************************************************************************
ok: [localhost] => {
    "my_include_vars": {
        "my_name0": "config/config0.yml",
        "my_name1": "config/sub1/config1.yml",
        "my_name11": "config/sub1/sub11/config11.yml",
        "my_name111": "config/sub1/sub11/sub111/config111.yml",
        "my_name2": "config/sub2/config2.yml",
        "my_name21": "config/sub2/sub21/config21.yml"
    }
}

It seems that count counter is just always incremented for the each subfolder and doesn't take account of an actual folder structure.
```


### Code of Conduct

- [X] I agree to follow the Ansible Code of Conduct

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/modules/include_vars.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/include_vars.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/devel/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
The current code makes some assumptions about sort order for knowing it should stop iterating, but there are some inherent problems with that.  Something like this I believe works as expected:

```diff
diff --git a/lib/ansible/plugins/action/include_vars.py b/lib/ansible/plugins/action/include_vars.py
index bff2985134..83835b3734 100644
--- a/lib/ansible/plugins/action/include_vars.py
+++ b/lib/ansible/plugins/action/include_vars.py
@@ -6,6 +6,7 @@ __metaclass__ = type
 
 from os import path, walk
 import re
+import pathlib
 
 import ansible.constants as C
 from ansible.errors import AnsibleError
@@ -182,16 +183,15 @@ class ActionModule(ActionBase):
             alphabetical order. Do not iterate pass the set depth.
             The default depth is unlimited.
         """
-        current_depth = 0
         sorted_walk = list(walk(self.source_dir, onerror=self._log_walk, followlinks=True))
         sorted_walk.sort(key=lambda x: x[0])
         for current_root, current_dir, current_files in sorted_walk:
-            current_depth += 1
-            if current_depth <= self.depth or self.depth == 0:
-                current_files.sort()
-                yield (current_root, current_files)
-            else:
-                break
+            # Depth 1 is the root, relative_to omits the root
+            current_depth = len(pathlib.Path(current_root).relative_to(self.source_dir).parts) + 1
+            if self.depth != 0 and current_depth > self.depth:
+                continue
+            current_files.sort()
+            yield (current_root, current_files)
 
     def _ignore_file(self, filename):
         """ Return True if a file matches the list of ignore_files.
```
I checked your changes and works fine on my playbook. Thanks.

Commit message: Adding changelog fragment and tests

Patch to review:
diff --git a/changelogs/fragments/80995-include-all-var-files.yml b/changelogs/fragments/80995-include-all-var-files.yml
new file mode 100644
index 00000000000000..1847f48322752f
--- /dev/null
+++ b/changelogs/fragments/80995-include-all-var-files.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- include_vars.py - Ensured that all files specified in the directory are included and that we iterate over all files (https://github.com/ansible/ansible/issues/80987).
diff --git a/lib/ansible/plugins/action/include_vars.py b/lib/ansible/plugins/action/include_vars.py
index bff29851344f69..83835b373409b3 100644
--- a/lib/ansible/plugins/action/include_vars.py
+++ b/lib/ansible/plugins/action/include_vars.py
@@ -6,6 +6,7 @@
 
 from os import path, walk
 import re
+import pathlib
 
 import ansible.constants as C
 from ansible.errors import AnsibleError
@@ -182,16 +183,15 @@ def _traverse_dir_depth(self):
             alphabetical order. Do not iterate pass the set depth.
             The default depth is unlimited.
         """
-        current_depth = 0
         sorted_walk = list(walk(self.source_dir, onerror=self._log_walk, followlinks=True))
         sorted_walk.sort(key=lambda x: x[0])
         for current_root, current_dir, current_files in sorted_walk:
-            current_depth += 1
-            if current_depth <= self.depth or self.depth == 0:
-                current_files.sort()
-                yield (current_root, current_files)
-            else:
-                break
+            # Depth 1 is the root, relative_to omits the root
+            current_depth = len(pathlib.Path(current_root).relative_to(self.source_dir).parts) + 1
+            if self.depth != 0 and current_depth > self.depth:
+                continue
+            current_files.sort()
+            yield (current_root, current_files)
 
     def _ignore_file(self, filename):
         """ Return True if a file matches the list of ignore_files.
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11.yml b/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11.yml
new file mode 100644
index 00000000000000..1615a93e96b203
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11.yml
@@ -0,0 +1 @@
+var1: sub1var1
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11/config11-2.yml b/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11/config11-2.yml
new file mode 100644
index 00000000000000..912bb8f880083a
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11/config11-2.yml
@@ -0,0 +1 @@
+insidevar11-2: config11-2
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11/config11.yml b/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11/config11.yml
new file mode 100644
index 00000000000000..93bb06d264055e
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub11/config11.yml
@@ -0,0 +1 @@
+insidevar1: config11
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub12.yml b/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub12.yml
new file mode 100644
index 00000000000000..5ca10304fcb0ff
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/sub1/sub12.yml
@@ -0,0 +1 @@
+var12: sub1var2
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21.yml b/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21.yml
new file mode 100644
index 00000000000000..f4491269888972
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21.yml
@@ -0,0 +1 @@
+var21: sub21
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21/config211.yml b/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21/config211.yml
new file mode 100644
index 00000000000000..36eab40b59a591
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21/config211.yml
@@ -0,0 +1 @@
+insidevar2: config211
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21/config212.yml b/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21/config212.yml
new file mode 100644
index 00000000000000..322ce81b3d03e8
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/sub2/sub21/config212.yml
@@ -0,0 +1 @@
+insidervariable: config212
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/sub3/config3.yml b/test/integration/targets/include_vars-ad-hoc/dir/sub3/config3.yml
new file mode 100644
index 00000000000000..1c3a6093d7d082
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/sub3/config3.yml
@@ -0,0 +1 @@
+variable3: config3
diff --git a/test/integration/targets/include_vars-ad-hoc/playbook.yml b/test/integration/targets/include_vars-ad-hoc/playbook.yml
new file mode 100644
index 00000000000000..301147c565f05a
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/playbook.yml
@@ -0,0 +1,19 @@
+- name: Include vars example
+  hosts: localhost
+
+  tasks:
+    - name: Include all yml configuration files from folder "{{ my_config_folder }}"
+      vars:
+        my_config_folder: "dir/"
+      ansible.builtin.include_vars:
+        dir: "{{ my_config_folder }}"
+        extensions:
+          - 'yml'
+          - 'yaml'
+        ignore_unknown_extensions: 'yes'
+        depth: 3
+        name: my_include_vars
+
+    - name: Print included params
+      debug:
+        var: my_include_vars
diff --git a/test/integration/targets/include_vars-ad-hoc/runme.sh b/test/integration/targets/include_vars-ad-hoc/runme.sh
index 51b68d213417c1..8440485214f462 100755
--- a/test/integration/targets/include_vars-ad-hoc/runme.sh
+++ b/test/integration/targets/include_vars-ad-hoc/runme.sh
@@ -4,3 +4,21 @@ set -eux
 
 ansible testhost -i ../../inventory -m include_vars -a 'dir/inc.yml' "$@"
 ansible testhost -i ../../inventory -m include_vars -a 'dir=dir' "$@"
+
+ansible-playbook playbook.yml>output.txt
+if grep -q '"insidervariable": "config212",' output.txt && \
+   grep -q '"insidevar1": "config11",' output.txt && \
+   grep -q '"insidevar11-2": "config11-2",' output.txt && \
+   grep -q '"insidevar2": "config211",' output.txt && \
+   grep -q '"porter": "cable",' output.txt && \
+   grep -q '"var1": "sub1var1",' output.txt && \
+   grep -q '"var12": "sub1var2",' output.txt && \
+   grep -q '"var21": "sub21",' output.txt && \
+   grep -q '"variable3": "config3"' output.txt; then
+	echo "All lines are present in output.txt"
+	rm output.txt
+	exit 0
+fi
+echo "One or more lines are missing from output.txt"
+rm output.txt
+exit 1


Code review comments:
diff_hunk: @@ -4,3 +4,21 @@ set -eux
 
 ansible testhost -i ../../inventory -m include_vars -a 'dir/inc.yml' "$@"
 ansible testhost -i ../../inventory -m include_vars -a 'dir=dir' "$@"
+
+ansible-playbook playbook.yml>output.txt
path: test/integration/targets/include_vars-ad-hoc/runme.sh
text: @user1:
This should go in the normal include_vars test suite since it's not testing something adhoc specific (and is not using adhoc). Rather than grepping for defined variables, you could do this in the test play with an assert task. Other than that, LGTM.