From 27df905b2c026deaa2824e72d7301c8c66e996eb Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Thu, 27 Feb 2025 12:43:15 +1000 Subject: [PATCH 1/5] Fix collection loader iterator for ext modules Fixes the collection loader iter_modules logic to return compiled Python extension modules. This loader is used when attempting to call `pkgutil.iter_modules` on a package that is contained inside the `ansible_collections` path. Before extension modules were skipped which would break imports of 3rd party modules that used compiled extensions if it was installed in a Python environment located inside a collection. --- .../collection-loader-extensions.yml | 6 +++ .../collection_loader/_collection_finder.py | 52 +++++++++++++------ 2 files changed, 42 insertions(+), 16 deletions(-) create mode 100644 changelogs/fragments/collection-loader-extensions.yml diff --git a/changelogs/fragments/collection-loader-extensions.yml b/changelogs/fragments/collection-loader-extensions.yml new file mode 100644 index 00000000000..0d784a2af43 --- /dev/null +++ b/changelogs/fragments/collection-loader-extensions.yml @@ -0,0 +1,6 @@ +bugfixes: + - >- + collection loader - Fix the collection loader logic to correctly return Python module when calling + ``pkgutil.iter_modules`` with a package that is inside a collection path and contains compiled Python extension + modules. + diff --git a/lib/ansible/utils/collection_loader/_collection_finder.py b/lib/ansible/utils/collection_loader/_collection_finder.py index 7e788808fb0..753a4c80b12 100644 --- a/lib/ansible/utils/collection_loader/_collection_finder.py +++ b/lib/ansible/utils/collection_loader/_collection_finder.py @@ -6,6 +6,7 @@ from __future__ import annotations +import inspect import itertools import os import os.path @@ -1237,23 +1238,42 @@ def _iter_modules_impl(paths, prefix=''): prefix = _to_text(prefix) # yield (module_loader, name, ispkg) for each module/pkg under path # TODO: implement ignore/silent catch for unreadable? - for b_path in map(_to_bytes, paths): - if not os.path.isdir(b_path): + # Mostly based off the logic in the builtin pkgutil.iter_modules importer + # https://github.com/python/cpython/blob/fda056e64bdfcac3dd3d13eebda0a24994d83cb8/Lib/pkgutil.py#L130-L168 + for path in paths: + if not os.path.isdir(path): continue - for b_basename in sorted(os.listdir(b_path)): - b_candidate_module_path = os.path.join(b_path, b_basename) - if os.path.isdir(b_candidate_module_path): - # exclude things that obviously aren't Python package dirs - # FIXME: this dir is adjustable in py3.8+, check for it - if b'.' in b_basename or b_basename == b'__pycache__': - continue - - # TODO: proper string handling? - yield prefix + _to_text(b_basename), True - else: - # FIXME: match builtin ordering for package/dir/file, support compiled? - if b_basename.endswith(b'.py') and b_basename != b'__init__.py': - yield prefix + _to_text(os.path.splitext(b_basename)[0]), False + + yielded = set() + for basename in sorted(os.listdir(path)): + modname = inspect.getmodulename(basename) + if modname == '__init__' or modname in yielded: + continue + + mod_path = os.path.join(path, basename) + ispkg = False + + ispkg = ( + not modname and + os.path.isdir(mod_path) and + '.' not in basename and + _is_dir_a_pkg(mod_path) + ) + + if modname and '.' not in modname: + yielded.add(modname) + yield prefix + modname, ispkg + + +def _is_dir_a_pkg(path: str) -> bool: + """Checks if the directory is a Python package (contains __init__).""" + with os.scandir(path) as scandir_it: + for entry in scandir_it: + subname = inspect.getmodulename(entry.name) + if subname == '__init__': + return True + + return False def _get_collection_metadata(collection_name): From 108922a0736d6aef7129d217fe8302c0ec2a7d8d Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Thu, 27 Feb 2025 13:28:36 +1000 Subject: [PATCH 2/5] Fix up pkg logic --- .../collection-loader-extensions.yml | 1 - .../collection_loader/_collection_finder.py | 27 +++++++------------ 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/changelogs/fragments/collection-loader-extensions.yml b/changelogs/fragments/collection-loader-extensions.yml index 0d784a2af43..db523179f9e 100644 --- a/changelogs/fragments/collection-loader-extensions.yml +++ b/changelogs/fragments/collection-loader-extensions.yml @@ -3,4 +3,3 @@ bugfixes: collection loader - Fix the collection loader logic to correctly return Python module when calling ``pkgutil.iter_modules`` with a package that is inside a collection path and contains compiled Python extension modules. - diff --git a/lib/ansible/utils/collection_loader/_collection_finder.py b/lib/ansible/utils/collection_loader/_collection_finder.py index 753a4c80b12..9bc08960732 100644 --- a/lib/ansible/utils/collection_loader/_collection_finder.py +++ b/lib/ansible/utils/collection_loader/_collection_finder.py @@ -1238,7 +1238,8 @@ def _iter_modules_impl(paths, prefix=''): prefix = _to_text(prefix) # yield (module_loader, name, ispkg) for each module/pkg under path # TODO: implement ignore/silent catch for unreadable? - # Mostly based off the logic in the builtin pkgutil.iter_modules importer + # Mostly based off the logic in the builtin pkgutil.iter_modules importer. + # Only difference is we don't check if dirs contains __init__. # https://github.com/python/cpython/blob/fda056e64bdfcac3dd3d13eebda0a24994d83cb8/Lib/pkgutil.py#L130-L168 for path in paths: if not os.path.isdir(path): @@ -1252,30 +1253,20 @@ def _iter_modules_impl(paths, prefix=''): mod_path = os.path.join(path, basename) ispkg = False + if not modname and os.path.isdir(mod_path) and '.' not in basename: + # exclude things that obviously aren't Python package dirs + # FIXME: this dir is adjustable in py3.8+, check for it + if basename == '__pycache__': + continue - ispkg = ( - not modname and - os.path.isdir(mod_path) and - '.' not in basename and - _is_dir_a_pkg(mod_path) - ) + modname = basename + ispkg = True if modname and '.' not in modname: yielded.add(modname) yield prefix + modname, ispkg -def _is_dir_a_pkg(path: str) -> bool: - """Checks if the directory is a Python package (contains __init__).""" - with os.scandir(path) as scandir_it: - for entry in scandir_it: - subname = inspect.getmodulename(entry.name) - if subname == '__init__': - return True - - return False - - def _get_collection_metadata(collection_name): collection_name = _to_text(collection_name) if not collection_name or not isinstance(collection_name, str) or len(collection_name.split('.')) != 2: From ae22804cc94734f6b8dcfbd26b28f2338e6b6b02 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Wed, 5 Mar 2025 06:41:55 +1000 Subject: [PATCH 3/5] Added integration test --- .../integration/targets/collection_loader/aliases | 3 +++ .../col/plugins/action/test_collection_loader.py | 13 +++++++++++++ .../ns/col/temp_pythonpath/my_module/__init__.py | 9 +++++++++ .../col/temp_pythonpath/my_module/sub/__init__.py | 0 .../targets/collection_loader/main.yml | 11 +++++++++++ .../targets/collection_loader/runme.sh | 15 +++++++++++++++ 6 files changed, 51 insertions(+) create mode 100644 test/integration/targets/collection_loader/aliases create mode 100644 test/integration/targets/collection_loader/ansible_collections/ns/col/plugins/action/test_collection_loader.py create mode 100644 test/integration/targets/collection_loader/ansible_collections/ns/col/temp_pythonpath/my_module/__init__.py create mode 100644 test/integration/targets/collection_loader/ansible_collections/ns/col/temp_pythonpath/my_module/sub/__init__.py create mode 100644 test/integration/targets/collection_loader/main.yml create mode 100755 test/integration/targets/collection_loader/runme.sh diff --git a/test/integration/targets/collection_loader/aliases b/test/integration/targets/collection_loader/aliases new file mode 100644 index 00000000000..98b09ad6985 --- /dev/null +++ b/test/integration/targets/collection_loader/aliases @@ -0,0 +1,3 @@ +shippable/posix/group3 +context/controller +needs/target/collection \ No newline at end of file diff --git a/test/integration/targets/collection_loader/ansible_collections/ns/col/plugins/action/test_collection_loader.py b/test/integration/targets/collection_loader/ansible_collections/ns/col/plugins/action/test_collection_loader.py new file mode 100644 index 00000000000..ec8f75490a2 --- /dev/null +++ b/test/integration/targets/collection_loader/ansible_collections/ns/col/plugins/action/test_collection_loader.py @@ -0,0 +1,13 @@ +from __future__ import annotations + +from ansible.plugins.action import ActionBase + + +class ActionModule(ActionBase): + def run(self, tmp=None, task_vars=None): + import my_module + + result = super(ActionModule, self).run(tmp, task_vars) + result['modules'] = my_module.run() + + return result diff --git a/test/integration/targets/collection_loader/ansible_collections/ns/col/temp_pythonpath/my_module/__init__.py b/test/integration/targets/collection_loader/ansible_collections/ns/col/temp_pythonpath/my_module/__init__.py new file mode 100644 index 00000000000..56889ba0ccb --- /dev/null +++ b/test/integration/targets/collection_loader/ansible_collections/ns/col/temp_pythonpath/my_module/__init__.py @@ -0,0 +1,9 @@ +from __future__ import annotations + +import pkgutil + +from my_module import sub + + +def run() -> list[str]: + return [m[1] for m in pkgutil.iter_modules(sub.__path__)] diff --git a/test/integration/targets/collection_loader/ansible_collections/ns/col/temp_pythonpath/my_module/sub/__init__.py b/test/integration/targets/collection_loader/ansible_collections/ns/col/temp_pythonpath/my_module/sub/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/integration/targets/collection_loader/main.yml b/test/integration/targets/collection_loader/main.yml new file mode 100644 index 00000000000..fd0f28fbd27 --- /dev/null +++ b/test/integration/targets/collection_loader/main.yml @@ -0,0 +1,11 @@ +- name: run collection loader tests + hosts: localhost + tasks: + - name: run module which imports Python library inside collection dir + ns.col.test_collection_loader: + register: result + + - name: assert module loader results + assert: + that: + - result.modules == ['foo'] diff --git a/test/integration/targets/collection_loader/runme.sh b/test/integration/targets/collection_loader/runme.sh new file mode 100755 index 00000000000..56c498bb5ec --- /dev/null +++ b/test/integration/targets/collection_loader/runme.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash + +set -eux + +PLAYBOOK_DIR="${PWD}" + +source ../collection/setup.sh + +EXT=$( python -c 'import importlib.machinery as i; print(next(iter([s for s in i.all_suffixes() if s != ".py"])))' ) +touch "temp_pythonpath/my_module/sub/foo${EXT}" + +ANSIBLE_COLLECTIONS_PATH="${PWD}../../../" \ + ANSIBLE_LOCALHOST_WARNING=false \ + PYTHONPATH="./temp_pythonpath:${PYTHONPATH}" \ + ansible-playbook "${PLAYBOOK_DIR}/main.yml" "$@" From c761a71cea3b03e29f94a8027a2a38b216199e32 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Thu, 6 Mar 2025 09:10:57 +1000 Subject: [PATCH 4/5] Add newline to alias file --- test/integration/targets/collection_loader/aliases | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/targets/collection_loader/aliases b/test/integration/targets/collection_loader/aliases index 98b09ad6985..6982add4de5 100644 --- a/test/integration/targets/collection_loader/aliases +++ b/test/integration/targets/collection_loader/aliases @@ -1,3 +1,3 @@ shippable/posix/group3 context/controller -needs/target/collection \ No newline at end of file +needs/target/collection From ab9cfe9c613e206abb75b7a0fc6dc1dbac224bde Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Fri, 7 Mar 2025 04:34:13 +1000 Subject: [PATCH 5/5] Update lib/ansible/utils/collection_loader/_collection_finder.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) --- lib/ansible/utils/collection_loader/_collection_finder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/utils/collection_loader/_collection_finder.py b/lib/ansible/utils/collection_loader/_collection_finder.py index 9bc08960732..e58fc29ece7 100644 --- a/lib/ansible/utils/collection_loader/_collection_finder.py +++ b/lib/ansible/utils/collection_loader/_collection_finder.py @@ -1245,10 +1245,10 @@ def _iter_modules_impl(paths, prefix=''): if not os.path.isdir(path): continue - yielded = set() + yielded = {'__init__'} for basename in sorted(os.listdir(path)): modname = inspect.getmodulename(basename) - if modname == '__init__' or modname in yielded: + if modname in yielded: continue mod_path = os.path.join(path, basename)