From d22804c4fbb85010c4589836cd59284c2cf11f9e Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 14 Dec 2020 19:30:13 -0500 Subject: [PATCH] saner path dir management (#72648) * saner path dir management fixes #72628 ensure we always store paths w/o a_c Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> --- .../colleciton_flex_ac_dir_paths.yml | 2 + lib/ansible/collections/list.py | 47 ++++++++++--------- .../collection_loader/_collection_finder.py | 35 +++++++++----- test/integration/targets/collections/runme.sh | 8 +++- .../test_collection_loader.py | 21 +++++---- 5 files changed, 67 insertions(+), 46 deletions(-) create mode 100644 changelogs/fragments/colleciton_flex_ac_dir_paths.yml diff --git a/changelogs/fragments/colleciton_flex_ac_dir_paths.yml b/changelogs/fragments/colleciton_flex_ac_dir_paths.yml new file mode 100644 index 00000000000..bd595197854 --- /dev/null +++ b/changelogs/fragments/colleciton_flex_ac_dir_paths.yml @@ -0,0 +1,2 @@ +bugfixes: + - Be smarter about collection paths ending with ansible_collections, emulating a-galaxy behaviour. Issue 72628 diff --git a/lib/ansible/collections/list.py b/lib/ansible/collections/list.py index a1d99017067..c6af77a3647 100644 --- a/lib/ansible/collections/list.py +++ b/lib/ansible/collections/list.py @@ -69,33 +69,34 @@ def list_collection_dirs(search_paths=None, coll_filter=None): collections = defaultdict(dict) for path in list_valid_collection_paths(search_paths): - b_path = to_bytes(path) - if os.path.isdir(b_path): - b_coll_root = to_bytes(os.path.join(path, 'ansible_collections')) + if os.path.basename(path) != 'ansible_collections': + path = os.path.join(path, 'ansible_collections') - if os.path.exists(b_coll_root) and os.path.isdir(b_coll_root): + b_coll_root = to_bytes(path, errors='surrogate_or_strict') - if namespace is None: - namespaces = os.listdir(b_coll_root) - else: - namespaces = [namespace] + if os.path.exists(b_coll_root) and os.path.isdir(b_coll_root): - for ns in namespaces: - b_namespace_dir = os.path.join(b_coll_root, to_bytes(ns)) + if namespace is None: + namespaces = os.listdir(b_coll_root) + else: + namespaces = [namespace] - if os.path.isdir(b_namespace_dir): + for ns in namespaces: + b_namespace_dir = os.path.join(b_coll_root, to_bytes(ns)) - if collection is None: - colls = os.listdir(b_namespace_dir) - else: - colls = [collection] + if os.path.isdir(b_namespace_dir): - for mycoll in colls: + if collection is None: + colls = os.listdir(b_namespace_dir) + else: + colls = [collection] - # skip dupe collections as they will be masked in execution - if mycoll not in collections[ns]: - b_coll = to_bytes(mycoll) - b_coll_dir = os.path.join(b_namespace_dir, b_coll) - if is_collection_path(b_coll_dir): - collections[ns][mycoll] = b_coll_dir - yield b_coll_dir + for mycoll in colls: + + # skip dupe collections as they will be masked in execution + if mycoll not in collections[ns]: + b_coll = to_bytes(mycoll) + b_coll_dir = os.path.join(b_namespace_dir, b_coll) + if is_collection_path(b_coll_dir): + collections[ns][mycoll] = b_coll_dir + yield b_coll_dir diff --git a/lib/ansible/utils/collection_loader/_collection_finder.py b/lib/ansible/utils/collection_loader/_collection_finder.py index 3ce077d3545..5f5b0dbb681 100644 --- a/lib/ansible/utils/collection_loader/_collection_finder.py +++ b/lib/ansible/utils/collection_loader/_collection_finder.py @@ -61,19 +61,22 @@ class _AnsibleCollectionFinder: # expand any placeholders in configured paths paths = [os.path.expanduser(to_native(p, errors='surrogate_or_strict')) for p in paths] + # add syspaths if needed if scan_sys_paths: - # append all sys.path entries with an ansible_collections package - for path in sys.path: - if ( - path not in paths and - os.path.isdir(to_bytes( - os.path.join(path, 'ansible_collections'), - errors='surrogate_or_strict', - )) - ): - paths.append(path) + paths.extend(sys.path) - self._n_configured_paths = paths + good_paths = [] + # expand any placeholders in configured paths + for p in paths: + + # ensure we alway shave ansible_collections + if os.path.basename(p) == 'ansible_collections': + p = os.path.dirname(p) + + if p not in good_paths and os.path.isdir(to_bytes(os.path.join(p, 'ansible_collections'), errors='surrogate_or_strict')): + good_paths.append(p) + + self._n_configured_paths = good_paths self._n_cached_collection_paths = None self._n_cached_collection_qualified_paths = None @@ -111,8 +114,14 @@ class _AnsibleCollectionFinder: path = to_native(path) interesting_paths = self._n_cached_collection_qualified_paths if not interesting_paths: - interesting_paths = [os.path.join(p, 'ansible_collections') for p in - self._n_collection_paths] + interesting_paths = [] + for p in self._n_collection_paths: + if os.path.basename(p) != 'ansible_collections': + p = os.path.join(p, 'ansible_collections') + + if p not in interesting_paths: + interesting_paths.append(p) + interesting_paths.insert(0, self._ansible_pkg_path) self._n_cached_collection_qualified_paths = interesting_paths diff --git a/test/integration/targets/collections/runme.sh b/test/integration/targets/collections/runme.sh index a0359e2183f..a9b05447449 100755 --- a/test/integration/targets/collections/runme.sh +++ b/test/integration/targets/collections/runme.sh @@ -12,6 +12,13 @@ export ANSIBLE_COLLECTIONS_ON_ANSIBLE_VERSION_MISMATCH=0 ipath=../../$(basename "${INVENTORY_PATH:-../../inventory}") export INVENTORY_PATH="$ipath" +# ensure we can call collection module +ansible localhost -m testns.testcoll.testmodule + +# ensure we can call collection module with ansible_collections in path +ANSIBLE_COLLECTIONS_PATH=$PWD/collection_root_sys/ansible_collections ansible localhost -m testns.testcoll.testmodule + + echo "--- validating callbacks" # validate FQ callbacks in ansible-playbook ANSIBLE_CALLBACKS_ENABLED=testns.testcoll.usercallback ansible-playbook noop.yml | grep "usercallback says ok" @@ -127,4 +134,3 @@ if [[ "$(grep -wc "dynamic_host_a" "$CACHEFILE")" -ne "0" ]]; then fi ./vars_plugin_tests.sh - diff --git a/test/units/utils/collection_loader/test_collection_loader.py b/test/units/utils/collection_loader/test_collection_loader.py index d5489cbf3b1..a2bee819a13 100644 --- a/test/units/utils/collection_loader/test_collection_loader.py +++ b/test/units/utils/collection_loader/test_collection_loader.py @@ -35,19 +35,22 @@ def test_finder_setup(): # ensure sys.path paths that have an ansible_collections dir are added to the end of the collections paths with patch.object(sys, 'path', ['/bogus', default_test_collection_paths[1], '/morebogus', default_test_collection_paths[0]]): - f = _AnsibleCollectionFinder(paths=['/explicit', '/other']) - assert f._n_collection_paths == ['/explicit', '/other', default_test_collection_paths[1], default_test_collection_paths[0]] + with patch('os.path.isdir', side_effect=lambda x: b'bogus' not in x): + f = _AnsibleCollectionFinder(paths=['/explicit', '/other']) + assert f._n_collection_paths == ['/explicit', '/other', default_test_collection_paths[1], default_test_collection_paths[0]] configured_paths = ['/bogus'] playbook_paths = ['/playbookdir'] - f = _AnsibleCollectionFinder(paths=configured_paths) - assert f._n_collection_paths == configured_paths - f.set_playbook_paths(playbook_paths) - assert f._n_collection_paths == extend_paths(playbook_paths, 'collections') + configured_paths + with patch.object(sys, 'path', ['/bogus', '/playbookdir']) and patch('os.path.isdir', side_effect=lambda x: b'bogus' in x): + f = _AnsibleCollectionFinder(paths=configured_paths) + assert f._n_collection_paths == configured_paths - # ensure scalar playbook_paths gets listified - f.set_playbook_paths(playbook_paths[0]) - assert f._n_collection_paths == extend_paths(playbook_paths, 'collections') + configured_paths + f.set_playbook_paths(playbook_paths) + assert f._n_collection_paths == extend_paths(playbook_paths, 'collections') + configured_paths + + # ensure scalar playbook_paths gets listified + f.set_playbook_paths(playbook_paths[0]) + assert f._n_collection_paths == extend_paths(playbook_paths, 'collections') + configured_paths def test_finder_not_interested():