Skip to content

Commit

Permalink
add optional module_utils import support (#73832) (#73916)
Browse files Browse the repository at this point in the history
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 3e1f648)
  • Loading branch information
nitzmahone committed Apr 5, 2021
1 parent 1592c8b commit f8caa43
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 9 deletions.
4 changes: 4 additions & 0 deletions 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.
36 changes: 33 additions & 3 deletions lib/ansible/executor/module_common.py
Expand Up @@ -29,6 +29,7 @@
import zipfile
import re
import pkgutil
from ast import AST, Import, ImportFrom
from io import BytesIO

from ansible.release import __version__, __author__
Expand Down Expand Up @@ -437,7 +438,7 @@ def _strip_comments(source):

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.
Expand All @@ -462,6 +463,27 @@ def __init__(self, module_fqn, *args, **kwargs):
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]
Expand All @@ -474,6 +496,8 @@ def visit_Import(self, node):
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):
Expand Down Expand Up @@ -533,6 +557,8 @@ def visit_ImportFrom(self, node):
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)

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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]))
Expand Down
@@ -0,0 +1,6 @@
from __future__ import absolute_import, division, print_function
__metaclass__ = type


def importme():
return "successfully imported from testns.testcoll"
8 changes: 3 additions & 5 deletions test/integration/targets/module_utils/library/test_failure.py
Expand Up @@ -4,11 +4,9 @@
# 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)
84 changes: 84 additions & 0 deletions 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')
11 changes: 10 additions & 1 deletion test/integration/targets/module_utils/module_utils_test.yml
Expand Up @@ -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'

0 comments on commit f8caa43

Please sign in to comment.