From f8caa43dc7a7beaf66e719e1e5f7bc6ff70fa7e6 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Sun, 4 Apr 2021 19:16:27 -0700 Subject: add optional module_utils import support (#73832) (#73916) Treat core and collections module_utils imports nested within any Python block statement (eg, `try`, `if`) as optional. This allows Ansible modules to implement runtime fallback behavior for missing module_utils (eg from a newer version of ansible-core), where previously, the module payload builder would always fail when unable to locate a module_util (regardless of any runtime behavior the module may implement). * sanity test fixes ci_complete (cherry-picked from 3e1f6484d77f2d7546952cfa22a8534d74ed3dc6) --- changelogs/fragments/optional_module_utils.yml | 4 ++ lib/ansible/executor/module_common.py | 36 +++++++++- .../testns/testcoll/plugins/module_utils/legit.py | 6 ++ .../targets/module_utils/library/test_failure.py | 8 +-- .../targets/module_utils/library/test_optional.py | 84 ++++++++++++++++++++++ .../targets/module_utils/module_utils_test.yml | 11 ++- 6 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/optional_module_utils.yml create mode 100644 test/integration/targets/module_utils/collections/ansible_collections/testns/testcoll/plugins/module_utils/legit.py create mode 100644 test/integration/targets/module_utils/library/test_optional.py diff --git a/changelogs/fragments/optional_module_utils.yml b/changelogs/fragments/optional_module_utils.yml new file mode 100644 index 0000000000..e9ff22c4c5 --- /dev/null +++ b/changelogs/fragments/optional_module_utils.yml @@ -0,0 +1,4 @@ +minor_changes: +- module payload builder - module_utils imports in any nested block (eg, ``try``, ``if``) are treated as optional during + module payload builds; this allows modules to implement runtime fallback behavior for module_utils that do not exist + in older versions of Ansible. diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 65a52a7064..3235d186b1 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -29,6 +29,7 @@ import shlex import zipfile import re import pkgutil +from ast import AST, Import, ImportFrom from io import BytesIO from ansible.release import __version__, __author__ @@ -437,7 +438,7 @@ NEW_STYLE_PYTHON_MODULE_RE = re.compile( class ModuleDepFinder(ast.NodeVisitor): - def __init__(self, module_fqn, *args, **kwargs): + def __init__(self, module_fqn, tree, *args, **kwargs): """ Walk the ast tree for the python module. :arg module_fqn: The fully qualified name to reach this module in dotted notation. @@ -462,6 +463,27 @@ class ModuleDepFinder(ast.NodeVisitor): self.submodules = set() self.module_fqn = module_fqn + self._tree = tree # squirrel this away so we can compare node parents to it + self.submodules = set() + self.required_imports = set() + + self._visit_map = { + Import: self.visit_Import, + ImportFrom: self.visit_ImportFrom, + } + + self.generic_visit(tree) + + def generic_visit(self, node): + for field, value in ast.iter_fields(node): + if isinstance(value, list): + for item in value: + if isinstance(item, (Import, ImportFrom)): + item.parent = node + self._visit_map[item.__class__](item) + elif isinstance(item, AST): + self.generic_visit(item) + def visit_Import(self, node): """ Handle import ansible.module_utils.MODLIB[.MODLIBn] [as asname] @@ -474,6 +496,8 @@ class ModuleDepFinder(ast.NodeVisitor): alias.name.startswith('ansible_collections.')): py_mod = tuple(alias.name.split('.')) self.submodules.add(py_mod) + if node.parent == self._tree: + self.required_imports.add(py_mod) self.generic_visit(node) def visit_ImportFrom(self, node): @@ -533,6 +557,8 @@ class ModuleDepFinder(ast.NodeVisitor): if py_mod: for alias in node.names: self.submodules.add(py_mod + (alias.name,)) + if node.parent == self._tree: + self.required_imports.add(py_mod + (alias.name,)) self.generic_visit(node) @@ -683,8 +709,7 @@ def recursive_finder(name, module_fqn, data, py_module_names, py_module_cache, z except (SyntaxError, IndentationError) as e: raise AnsibleError("Unable to import %s due to %s" % (name, e.msg)) - finder = ModuleDepFinder(module_fqn) - finder.visit(tree) + finder = ModuleDepFinder(module_fqn, tree) # # Determine what imports that we've found are modules (vs class, function. @@ -751,6 +776,11 @@ def recursive_finder(name, module_fqn, data, py_module_names, py_module_cache, z # Could not find the module. Construct a helpful error message. if module_info is None: + # is this import optional? if so, just skip it for this module + if py_module_name not in finder.required_imports: + continue + + # not an optional import, so it's a fatal error... msg = ['Could not find imported module support code for %s. Looked for' % (name,)] if idx == 2: msg.append('either %s.py or %s.py' % (py_module_name[-1], py_module_name[-2])) diff --git a/test/integration/targets/module_utils/collections/ansible_collections/testns/testcoll/plugins/module_utils/legit.py b/test/integration/targets/module_utils/collections/ansible_collections/testns/testcoll/plugins/module_utils/legit.py new file mode 100644 index 0000000000..b9d63482a1 --- /dev/null +++ b/test/integration/targets/module_utils/collections/ansible_collections/testns/testcoll/plugins/module_utils/legit.py @@ -0,0 +1,6 @@ +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +def importme(): + return "successfully imported from testns.testcoll" diff --git a/test/integration/targets/module_utils/library/test_failure.py b/test/integration/targets/module_utils/library/test_failure.py index e1a87c2e71..e5257aefc9 100644 --- a/test/integration/targets/module_utils/library/test_failure.py +++ b/test/integration/targets/module_utils/library/test_failure.py @@ -4,11 +4,9 @@ results = {} # Test that we are rooted correctly # Following files: # module_utils/yak/zebra/foo.py -try: - from ansible.module_utils.zebra import foo - results['zebra'] = foo.data -except ImportError: - results['zebra'] = 'Failed in module as expected but incorrectly did not fail in AnsiballZ construction' +from ansible.module_utils.zebra import foo + +results['zebra'] = foo.data from ansible.module_utils.basic import AnsibleModule AnsibleModule(argument_spec=dict()).exit_json(**results) diff --git a/test/integration/targets/module_utils/library/test_optional.py b/test/integration/targets/module_utils/library/test_optional.py new file mode 100644 index 0000000000..4d0225d96f --- /dev/null +++ b/test/integration/targets/module_utils/library/test_optional.py @@ -0,0 +1,84 @@ +#!/usr/bin/python +# Most of these names are only available via PluginLoader so pylint doesn't +# know they exist +# pylint: disable=no-name-in-module +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +from ansible.module_utils.basic import AnsibleModule + +# internal constants to keep pylint from griping about constant-valued conditionals +_private_false = False +_private_true = True + +# module_utils import statements nested below any block are considered optional "best-effort" for AnsiballZ to include. +# test a number of different import shapes and nesting types to exercise this... + +# first, some nested imports that should succeed... +try: + from ansible.module_utils.urls import fetch_url as yep1 +except ImportError: + yep1 = None + +try: + import ansible.module_utils.common.text.converters as yep2 +except ImportError: + yep2 = None + +try: + # optional import from a legit collection + from ansible_collections.testns.testcoll.plugins.module_utils.legit import importme as yep3 +except ImportError: + yep3 = None + +# and a bunch that should fail to be found, but not break the module_utils payload build in the process... +try: + from ansible.module_utils.bogus import fromnope1 +except ImportError: + fromnope1 = None + +if _private_false: + from ansible.module_utils.alsobogus import fromnope2 +else: + fromnope2 = None + +try: + import ansible.module_utils.verybogus + nope1 = ansible.module_utils.verybogus +except ImportError: + nope1 = None + +# deepish nested with multiple block types- make sure the AST walker made it all the way down +try: + if _private_true: + if _private_true: + if _private_true: + if _private_true: + try: + import ansible.module_utils.stillbogus as nope2 + except ImportError: + raise +except ImportError: + nope2 = None + +try: + # optional import from a valid collection with an invalid package + from ansible_collections.testns.testcoll.plugins.module_utils.bogus import collnope1 +except ImportError: + collnope1 = None + +try: + # optional import from a bogus collection + from ansible_collections.bogusns.boguscoll.plugins.module_utils.bogus import collnope2 +except ImportError: + collnope2 = None + +module = AnsibleModule(argument_spec={}) + +if not all([yep1, yep2, yep3]): + module.fail_json(msg='one or more existing optional imports did not resolve') + +if any([fromnope1, fromnope2, nope1, nope2, collnope1, collnope2]): + module.fail_json(msg='one or more missing optional imports resolved unexpectedly') + +module.exit_json(msg='all missing optional imports behaved as expected') diff --git a/test/integration/targets/module_utils/module_utils_test.yml b/test/integration/targets/module_utils/module_utils_test.yml index cf0672bc30..6cad19c26b 100644 --- a/test/integration/targets/module_utils/module_utils_test.yml +++ b/test/integration/targets/module_utils/module_utils_test.yml @@ -43,9 +43,18 @@ ignore_errors: True register: result - - debug: var=result - name: Make sure we failed in AnsiBallZ assert: that: - result is failed - result['msg'] == "Could not find imported module support code for test_failure. Looked for either foo.py or zebra.py" + + + - name: Test that optional imports behave properly + test_optional: + register: optionaltest + + - assert: + that: + - optionaltest is success + - optionaltest.msg == 'all missing optional imports behaved as expected' \ No newline at end of file -- cgit v1.2.1