From 2769f5621b1d91a6eceb14d671ba9f0d07db6825 Mon Sep 17 00:00:00 2001 From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> Date: Wed, 23 Mar 2022 16:55:37 -0400 Subject: [PATCH] Add find_spec and exec_module to RestrictedModuleLoader (#76427) * Add find_spec and exec_module to RestrictedModuleLoader * Fix getting new loader with correct path Fix pep8 errors * Use convert_ansible_name_to_absolute_paths instead of the loader path * Apply suggestions from code review * Fix type hints and test them in CI * Fix error message for ansible.module_utils.basic if it's missing Add mypy ignored missing imports for controller sanity tests * Add mypy attr-defined ignore entries for python 3.8, 3.9, 3.10 for vendored six Add mypy attr-defined ignore for python 2.7 in lib/ansible/utils/collection_loader/_collection_finder.py * Just test controller python versions to simplify ignoring mypy errors --- .../_internal/commands/sanity/mypy.py | 5 +- .../controller/sanity/mypy/ansible-test.ini | 3 + .../_util/target/sanity/import/importer.py | 98 +++++++++++-------- test/sanity/ignore.txt | 3 + 4 files changed, 67 insertions(+), 42 deletions(-) diff --git a/test/lib/ansible_test/_internal/commands/sanity/mypy.py b/test/lib/ansible_test/_internal/commands/sanity/mypy.py index c384e7e422c..5b83aa8b9f1 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/mypy.py +++ b/test/lib/ansible_test/_internal/commands/sanity/mypy.py @@ -63,7 +63,8 @@ class MypyTest(SanityMultipleVersion): def filter_targets(self, targets): # type: (t.List[TestTarget]) -> t.List[TestTarget] """Return the given list of test targets, filtered to include only those relevant for the test.""" return [target for target in targets if os.path.splitext(target.path)[1] == '.py' and ( - target.path.startswith('lib/ansible/') or target.path.startswith('test/lib/ansible_test/_internal/'))] + target.path.startswith('lib/ansible/') or target.path.startswith('test/lib/ansible_test/_internal/') + or target.path.startswith('test/lib/ansible_test/_util/target/sanity/import/'))] @property def error_code(self): # type: () -> t.Optional[str] @@ -90,6 +91,7 @@ class MypyTest(SanityMultipleVersion): return SanitySkipped(self.name, python.version) contexts = ( + MyPyContext('ansible-test', ['test/lib/ansible_test/_util/target/sanity/import/'], CONTROLLER_PYTHON_VERSIONS), MyPyContext('ansible-test', ['test/lib/ansible_test/_internal/'], CONTROLLER_PYTHON_VERSIONS), MyPyContext('ansible-core', ['lib/ansible/'], CONTROLLER_PYTHON_VERSIONS), MyPyContext('modules', ['lib/ansible/modules/', 'lib/ansible/module_utils/'], REMOTE_ONLY_PYTHON_VERSIONS), @@ -171,6 +173,7 @@ class MypyTest(SanityMultipleVersion): display.info(f'Checking context "{context.name}"', verbosity=1) env = ansible_environment(args, color=False) + env['MYPYPATH'] = env['PYTHONPATH'] # The --no-site-packages option should not be used, as it will prevent loading of type stubs from the sanity test virtual environment. diff --git a/test/lib/ansible_test/_util/controller/sanity/mypy/ansible-test.ini b/test/lib/ansible_test/_util/controller/sanity/mypy/ansible-test.ini index 6e756c0dc7c..190e9529a0a 100644 --- a/test/lib/ansible_test/_util/controller/sanity/mypy/ansible-test.ini +++ b/test/lib/ansible_test/_util/controller/sanity/mypy/ansible-test.ini @@ -19,3 +19,6 @@ ignore_missing_imports = True [mypy-ansible_release] ignore_missing_imports = True + +[mypy-StringIO] +ignore_missing_imports = True diff --git a/test/lib/ansible_test/_util/target/sanity/import/importer.py b/test/lib/ansible_test/_util/target/sanity/import/importer.py index 5afb5dc8e1f..3dcb8bf9340 100644 --- a/test/lib/ansible_test/_util/target/sanity/import/importer.py +++ b/test/lib/ansible_test/_util/target/sanity/import/importer.py @@ -54,6 +54,14 @@ def main(): except ImportError: from io import StringIO + try: + from importlib.util import spec_from_loader, module_from_spec + from importlib.machinery import SourceFileLoader, ModuleSpec # pylint: disable=unused-import + except ImportError: + has_py3_loader = False + else: + has_py3_loader = True + if collection_full_name: # allow importing code from collections when testing a collection from ansible.module_utils.common.text.converters import to_bytes, to_text, to_native, text_type @@ -149,12 +157,25 @@ def main(): self.loaded_modules = set() self.restrict_to_module_paths = restrict_to_module_paths + def find_spec(self, fullname, path=None, target=None): # pylint: disable=unused-argument + # type: (RestrictedModuleLoader, str, list[str], types.ModuleType | None ) -> ModuleSpec | None | ImportError + """Return the spec from the loader or None""" + loader = self._get_loader(fullname, path=path) + if loader is not None: + if has_py3_loader: + # loader is expected to be Optional[importlib.abc.Loader], but RestrictedModuleLoader does not inherit from importlib.abc.Loder + return spec_from_loader(fullname, loader) # type: ignore[arg-type] + raise ImportError("Failed to import '%s' due to a bug in ansible-test. Check importlib imports for typos." % fullname) + return None + def find_module(self, fullname, path=None): - """Return self if the given fullname is restricted, otherwise return None. - :param fullname: str - :param path: str - :return: RestrictedModuleLoader | None - """ + # type: (RestrictedModuleLoader, str, list[str]) -> RestrictedModuleLoader | None + """Return self if the given fullname is restricted, otherwise return None.""" + return self._get_loader(fullname, path=path) + + def _get_loader(self, fullname, path=None): + # type: (RestrictedModuleLoader, str, list[str]) -> RestrictedModuleLoader | None + """Return self if the given fullname is restricted, otherwise return None.""" if fullname in self.loaded_modules: return None # ignore modules that are already being loaded @@ -191,27 +212,49 @@ def main(): # not a namespace we care about return None + def create_module(self, spec): # pylint: disable=unused-argument + # type: (RestrictedModuleLoader, ModuleSpec) -> None + """Return None to use default module creation.""" + return None + + def exec_module(self, module): + # type: (RestrictedModuleLoader, types.ModuleType) -> None | ImportError + """Execute the module if the name is ansible.module_utils.basic and otherwise raise an ImportError""" + fullname = module.__spec__.name + if fullname == 'ansible.module_utils.basic': + self.loaded_modules.add(fullname) + for path in convert_ansible_name_to_absolute_paths(fullname): + if not os.path.exists(path): + continue + loader = SourceFileLoader(fullname, path) + spec = spec_from_loader(fullname, loader) + real_module = module_from_spec(spec) + loader.exec_module(real_module) + real_module.AnsibleModule = ImporterAnsibleModule # type: ignore[attr-defined] + real_module._load_params = lambda *args, **kwargs: {} # type: ignore[attr-defined] # pylint: disable=protected-access + sys.modules[fullname] = real_module + return None + raise ImportError('could not find "%s"' % fullname) + raise ImportError('import of "%s" is not allowed in this context' % fullname) + def load_module(self, fullname): - """Raise an ImportError. - :type fullname: str - """ + # type: (RestrictedModuleLoader, str) -> types.ModuleType | ImportError + """Return the module if the name is ansible.module_utils.basic and otherwise raise an ImportError.""" if fullname == 'ansible.module_utils.basic': module = self.__load_module(fullname) # stop Ansible module execution during AnsibleModule instantiation - module.AnsibleModule = ImporterAnsibleModule + module.AnsibleModule = ImporterAnsibleModule # type: ignore[attr-defined] # no-op for _load_params since it may be called before instantiating AnsibleModule - module._load_params = lambda *args, **kwargs: {} # pylint: disable=protected-access + module._load_params = lambda *args, **kwargs: {} # type: ignore[attr-defined] # pylint: disable=protected-access return module raise ImportError('import of "%s" is not allowed in this context' % fullname) def __load_module(self, fullname): - """Load the requested module while avoiding infinite recursion. - :type fullname: str - :rtype: module - """ + # type: (RestrictedModuleLoader, str) -> types.ModuleType + """Load the requested module while avoiding infinite recursion.""" self.loaded_modules.add(fullname) return import_module(fullname) @@ -514,33 +557,6 @@ def main(): "ignore", "Python 3.5 support will be dropped in the next release of cryptography. Please upgrade your Python.") - if sys.version_info >= (3, 10): - # Temporary solution for Python 3.10 until find_spec is implemented in RestrictedModuleLoader. - # That implementation is dependent on find_spec being added to the controller's collection loader first. - # The warning text is: main..RestrictedModuleLoader.find_spec() not found; falling back to find_module() - warnings.filterwarnings( - "ignore", - r"main\.\.RestrictedModuleLoader\.find_spec\(\) not found; falling back to find_module\(\)", - ) - # Temporary solution for Python 3.10 until exec_module is implemented in RestrictedModuleLoader. - # That implementation is dependent on exec_module being added to the controller's collection loader first. - # The warning text is: main..RestrictedModuleLoader.exec_module() not found; falling back to load_module() - warnings.filterwarnings( - "ignore", - r"main\.\.RestrictedModuleLoader\.exec_module\(\) not found; falling back to load_module\(\)", - ) - - # Temporary solution for Python 3.10 until find_spec is implemented in the controller's collection loader. - warnings.filterwarnings( - "ignore", - r"_Ansible.*Finder\.find_spec\(\) not found; falling back to find_module\(\)", - ) - # Temporary solution for Python 3.10 until exec_module is implemented in the controller's collection loader. - warnings.filterwarnings( - "ignore", - r"_Ansible.*Loader\.exec_module\(\) not found; falling back to load_module\(\)", - ) - try: yield finally: diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index e99ce827cd8..005475736a6 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -303,8 +303,11 @@ lib/ansible/module_utils/six/__init__.py mypy-3.6:attr-defined # vendored code lib/ansible/module_utils/six/__init__.py mypy-3.7:var-annotated # vendored code lib/ansible/module_utils/six/__init__.py mypy-3.7:attr-defined # vendored code lib/ansible/module_utils/six/__init__.py mypy-3.8:var-annotated # vendored code +lib/ansible/module_utils/six/__init__.py mypy-3.8:attr-defined # vendored code lib/ansible/module_utils/six/__init__.py mypy-3.9:var-annotated # vendored code +lib/ansible/module_utils/six/__init__.py mypy-3.9:attr-defined # vendored code lib/ansible/module_utils/six/__init__.py mypy-3.10:var-annotated # vendored code +lib/ansible/module_utils/six/__init__.py mypy-3.10:attr-defined # vendored code lib/ansible/module_utils/distro/_distro.py mypy-2.7:arg-type # vendored code lib/ansible/module_utils/distro/_distro.py mypy-3.5:valid-type # vendored code lib/ansible/module_utils/distro/_distro.py mypy-3.6:valid-type # vendored code