From 556a1daa335f6762a9a25db51c1bb23ce206119b Mon Sep 17 00:00:00 2001 From: Pilou Date: Thu, 20 Jul 2017 16:26:13 +0200 Subject: [PATCH] fix searched paths in DataLoader.path_dwim_relative (avoid AnsibleFileNotFound) (#26729) * add unit test: nested dynamic includes * nested dynamic includes: avoid AnsibleFileNotFound error Error was: Unable to retrieve file contents Could not find or access 'include2.yml' Before 8f758204cf379a743332bc9865fb3c002b6c3a6a, at the end of 'path_dwim_relative' method, the 'search' variable contained amongst others paths: '/tmp/roles/testrole/tasks/tasks/included.yml' and '/tmp/roles/testrole/tasks/included.yml'. The commit mentioned before removed the last one despite the method docstrings specify 'with or without explicitly named dirname subdirs'. * add integration test: nested includes --- lib/ansible/parsing/dataloader.py | 1 + .../test_includes/tasks/branch_toplevel.yml | 9 ++++ .../test_includes/tasks/leaf_sublevel.yml | 2 + .../roles/test_includes/tasks/main.yml | 24 ++++++++++ test/units/parsing/test_dataloader.py | 44 +++++++++++++++++++ 5 files changed, 80 insertions(+) create mode 100644 test/integration/targets/includes/roles/test_includes/tasks/branch_toplevel.yml create mode 100644 test/integration/targets/includes/roles/test_includes/tasks/leaf_sublevel.yml diff --git a/lib/ansible/parsing/dataloader.py b/lib/ansible/parsing/dataloader.py index 3fefafb6af5..27d624bf1d4 100644 --- a/lib/ansible/parsing/dataloader.py +++ b/lib/ansible/parsing/dataloader.py @@ -304,6 +304,7 @@ class DataLoader: search.append(unfrackpath(os.path.join(basedir, 'tasks', source), follow=False)) # try to create absolute path for loader basedir + templates/files/vars + filename + search.append(unfrackpath(os.path.join(basedir, source), follow=False)) search.append(unfrackpath(os.path.join(dirname, source), follow=False)) search.append(self.path_dwim(os.path.join(dirname, source))) diff --git a/test/integration/targets/includes/roles/test_includes/tasks/branch_toplevel.yml b/test/integration/targets/includes/roles/test_includes/tasks/branch_toplevel.yml new file mode 100644 index 00000000000..624167057b3 --- /dev/null +++ b/test/integration/targets/includes/roles/test_includes/tasks/branch_toplevel.yml @@ -0,0 +1,9 @@ +# 'canary2' used instead of 'canary', otherwise a "recursive loop detected in +# template string" occurs when both includes use static=yes +- include: 'leaf_sublevel.yml canary2={{ canary }}' + static: yes + when: 'nested_include_static|bool' # value for 'static' can not be a variable, hence use 'when' + +- include: 'leaf_sublevel.yml canary2={{ canary }}' + static: no + when: 'not nested_include_static|bool' diff --git a/test/integration/targets/includes/roles/test_includes/tasks/leaf_sublevel.yml b/test/integration/targets/includes/roles/test_includes/tasks/leaf_sublevel.yml new file mode 100644 index 00000000000..06632017d95 --- /dev/null +++ b/test/integration/targets/includes/roles/test_includes/tasks/leaf_sublevel.yml @@ -0,0 +1,2 @@ +- set_fact: + canary_fact: '{{ canary2 }}' diff --git a/test/integration/targets/includes/roles/test_includes/tasks/main.yml b/test/integration/targets/includes/roles/test_includes/tasks/main.yml index 3add6721c5c..6fcac9ebe51 100644 --- a/test/integration/targets/includes/roles/test_includes/tasks/main.yml +++ b/test/integration/targets/includes/roles/test_includes/tasks/main.yml @@ -80,3 +80,27 @@ # both these via a handler include - included_handler - verify_handler + +- include: branch_toplevel.yml canary=value1 nested_include_static=no + static: no +- assert: + that: + - 'canary_fact == "value1"' + +- include: branch_toplevel.yml canary=value2 nested_include_static=yes + static: no +- assert: + that: + - 'canary_fact == "value2"' + +- include: branch_toplevel.yml canary=value3 nested_include_static=no + static: yes +- assert: + that: + - 'canary_fact == "value3"' + +- include: branch_toplevel.yml canary=value4 nested_include_static=yes + static: yes +- assert: + that: + - 'canary_fact == "value4"' diff --git a/test/units/parsing/test_dataloader.py b/test/units/parsing/test_dataloader.py index 5ff9108c5a4..92d930cfbf2 100644 --- a/test/units/parsing/test_dataloader.py +++ b/test/units/parsing/test_dataloader.py @@ -19,12 +19,17 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import os + from ansible.compat.tests import unittest from ansible.compat.tests.mock import patch, mock_open from ansible.errors import AnsibleParserError, yaml_strings +from ansible.module_utils._text import to_text from ansible.module_utils.six import PY3 from ansible.parsing.dataloader import DataLoader +from units.mock.path import mock_unfrackpath_noop + class TestDataLoader(unittest.TestCase): @@ -69,6 +74,45 @@ class TestDataLoader(unittest.TestCase): self.assertIn(yaml_strings.YAML_COMMON_LEADING_TAB_ERROR, str(cm.exception)) self.assertIn('foo: bar', str(cm.exception)) + @patch('ansible.parsing.dataloader.unfrackpath', mock_unfrackpath_noop) + @patch.object(DataLoader, '_is_role') + def test_path_dwim_relative(self, mock_is_role): + """ + simulate a nested dynamic include: + + playbook.yml: + - hosts: localhost + roles: + - { role: 'testrole' } + + testrole/tasks/main.yml: + - include: "include1.yml" + static: no + + testrole/tasks/include1.yml: + - include: include2.yml + static: no + + testrole/tasks/include2.yml: + - debug: msg="blah" + """ + mock_is_role.return_value = False + with patch('os.path.exists') as mock_os_path_exists: + mock_os_path_exists.return_value = False + self._loader.path_dwim_relative('/tmp/roles/testrole/tasks', 'tasks', 'included2.yml') + + # Fetch first args for every call + # mock_os_path_exists.assert_any_call isn't used because os.path.normpath must be used in order to compare paths + called_args = [os.path.normpath(to_text(call[0][0])) for call in mock_os_path_exists.call_args_list] + + # 'path_dwim_relative' docstrings say 'with or without explicitly named dirname subdirs': + self.assertIn('/tmp/roles/testrole/tasks/included2.yml', called_args) + self.assertIn('/tmp/roles/testrole/tasks/tasks/included2.yml', called_args) + + # relative directories below are taken in account too: + self.assertIn('tasks/included2.yml', called_args) + self.assertIn('included2.yml', called_args) + class TestDataLoaderWithVault(unittest.TestCase):