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>

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

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

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
 

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
-

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():