Support relative imports in AnsiballZ. (#61196)

changelogs/fragments/relative-import.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+- Ansible now supports relative imports of module_utils files in modules and module_utils.

docs/docsite/rst/porting_guides/porting_guide_2.9.rst

@@ -63,6 +63,38 @@ Modules
 * The ``win_get_url`` and ``win_uri`` module now sends requests with a default ``User-Agent`` of ``ansible-httpget``. This can be changed by using the ``http_agent`` key.
 
 
+Writing modules
+---------------
+
+* Module and module_utils files can now use relative imports to include other module_utils files.
+  This is useful for shortening long import lines, especially in collections.
+
+  Example of using a relative import in collections:
+
+  .. code-block:: python
+
+    # File: ansible_collections/my_namespace/my_collection/plugins/modules/my_module.py
+    # Old way to use an absolute import to import module_utils from the collection:
+    from ansible_collections.my_namespace.my_collection.plugins.module_utils import my_util
+    # New way using a relative import:
+    from ..module_utils import my_util
+
+  Modules and module_utils shipped with Ansible can use relative imports as well but the savings
+  are smaller:
+
+  .. code-block:: python
+
+    # File: ansible/modules/system/ping.py
+    # Old way to use an absolute import to import module_utils from core:
+    from ansible.module_utils.basic import AnsibleModule
+    # New way using a relative import:
+    from ...module_utils.basic import AnsibleModule
+
+  Each single dot (``.``) represents one level of the tree (equivalent to ``../`` in filesystem relative links).
+
+  .. seealso:: `The Python Relative Import Docs <https://www.python.org/dev/peps/pep-0328/#guido-s-decision>`_ go into more detail of how to write relative imports.
+
+
 Modules removed
 ---------------
 

hacking/test-module.py

@@ -28,6 +28,7 @@
 #    ./hacking/test-module.py -m lib/ansible/modules/files/lineinfile.py -a "dest=/etc/exports line='/srv/home hostname1(rw,sync)'" --check
 #    ./hacking/test-module.py -m lib/ansible/modules/commands/command.py -a "echo hello" -n -o "test_hello"
 
+import glob
 import optparse
 import os
 import subprocess
@@ -193,8 +194,19 @@ def ansiballz_setup(modfile, modname, interpreters):
         sys.exit(err)
     debug_dir = lines[1].strip()
 
+    # All the directories in an AnsiBallZ that modules can live
+    core_dirs = glob.glob(os.path.join(debug_dir, 'ansible/modules'))
+    collection_dirs = glob.glob(os.path.join(debug_dir, 'ansible_collections/*/*/plugins/modules'))
+
+    # There's only one module in an AnsiBallZ payload so look for the first module and then exit
+    for module_dir in core_dirs + collection_dirs:
+        for dirname, directories, filenames in os.walk(module_dir):
+            for filename in filenames:
+                if filename == modname + '.py':
+                    modfile = os.path.join(dirname, filename)
+                    break
+
     argsfile = os.path.join(debug_dir, 'args')
-    modfile = os.path.join(debug_dir, '__main__.py')
 
     print("* ansiballz module detected; extracted module source to: %s" % debug_dir)
     return modfile, argsfile

lib/ansible/executor/module_common.py

@@ -152,20 +152,14 @@ def _ansiballz_main():
         sys.path = [p for p in sys.path if p != scriptdir]
 
     import base64
+    import runpy
     import shutil
     import tempfile
     import zipfile
 
     if sys.version_info < (3,):
-        # imp is used on Python<3
-        import imp
-        bytes = str
-        MOD_DESC = ('.py', 'U', imp.PY_SOURCE)
         PY3 = False
     else:
-        # importlib is only used on Python>=3
-        import importlib.util
-        unicode = str
         PY3 = True
 
     ZIPDATA = """%(zipdata)s"""
@@ -187,13 +181,6 @@ def _ansiballz_main():
         zinfo.filename = 'sitecustomize.py'
         zinfo.date_time = ( %(year)i, %(month)i, %(day)i, %(hour)i, %(minute)i, %(second)i)
         z.writestr(zinfo, sitecustomize)
-        # Note: Remove the following section when we switch to zipimport
-        # Write the module to disk for imp.load_module
-        module = os.path.join(temp_path, '__main__.py')
-        with open(module, 'wb') as f:
-            f.write(z.read('__main__.py'))
-            f.close()
-        # End pre-zipimport section
         z.close()
 
         # Put the zipped up module_utils we got from the controller first in the python path so that we
@@ -205,13 +192,7 @@ def _ansiballz_main():
         basic._ANSIBLE_ARGS = json_params
 %(coverage)s
         # Run the module!  By importing it as '__main__', it thinks it is executing as a script
-        if sys.version_info >= (3,):
-            spec = importlib.util.spec_from_file_location('__main__', module)
-            module = importlib.util.module_from_spec(spec)
-            spec.loader.exec_module(module)
-        else:
-            with open(module, 'rb') as mod:
-                imp.load_module('__main__', mod, module, MOD_DESC)
+        runpy.run_module(mod_name='%(module_fqn)s', init_globals=None, run_name='__main__', alter_sys=False)
 
         # Ansible modules must exit themselves
         print('{"msg": "New-style module did not handle its own exit", "failed": true}')
@@ -253,7 +234,6 @@ def _ansiballz_main():
         # Okay to use __file__ here because we're running from a kept file
         basedir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'debug_dir')
         args_path = os.path.join(basedir, 'args')
-        script_path = os.path.join(basedir, '__main__.py')
 
         if command == 'excommunicate':
             print('The excommunicate debug command is deprecated and will be removed in 2.11.  Use execute instead.')
@@ -306,15 +286,7 @@ def _ansiballz_main():
             basic._ANSIBLE_ARGS = json_params
 
             # Run the module!  By importing it as '__main__', it thinks it is executing as a script
-            if PY3:
-                import importlib.util
-                spec = importlib.util.spec_from_file_location('__main__', script_path)
-                module = importlib.util.module_from_spec(spec)
-                spec.loader.exec_module(module)
-            else:
-                import imp
-                with open(script_path, 'r') as f:
-                    imp.load_module('__main__', f, script_path, ('.py', 'r', imp.PY_SOURCE))
+            runpy.run_module(mod_name='%(module_fqn)s', init_globals=None, run_name='__main__', alter_sys=False)
 
             # Ansible modules must exit themselves
             print('{"msg": "New-style module did not handle its own exit", "failed": true}')
@@ -394,9 +366,11 @@ ANSIBALLZ_COVERAGE_TEMPLATE = '''
 ANSIBALLZ_COVERAGE_CHECK_TEMPLATE = '''
         try:
             if PY3:
+                import importlib.util
                 if importlib.util.find_spec('coverage') is None:
                     raise ImportError
             else:
+                import imp
                 imp.find_module('coverage')
         except ImportError:
             print('{"msg": "Could not find `coverage` module.", "failed": true}')
@@ -439,73 +413,131 @@ else:
     # ANSIBALLZ_TEMPLATE stripped of comments for smaller over the wire size
     ACTIVE_ANSIBALLZ_TEMPLATE = _strip_comments(ANSIBALLZ_TEMPLATE)
 
+# dirname(dirname(dirname(site-packages/ansible/executor/module_common.py) == site-packages
+# Do this instead of getting site-packages from distutils.sysconfig so we work when we
+# haven't been installed
+site_packages = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
+CORE_LIBRARY_PATH_RE = re.compile(r'%s/(?P<path>ansible/modules/.*)\.py$' % site_packages)
+COLLECTION_PATH_RE = re.compile(r'/(?P<path>ansible_collections/[^/]+/[^/]+/plugins/modules/.*)\.py$')
+
+# Detect new-style Python modules by looking for required imports:
+# import ansible_collections.[my_ns.my_col.plugins.module_utils.my_module_util]
+# from ansible_collections.[my_ns.my_col.plugins.module_utils import my_module_util]
+# import ansible.module_utils[.basic]
+# from ansible.module_utils[ import basic]
+# from ansible.module_utils[.basic import AnsibleModule]
+# from ..module_utils[ import basic]
+# from ..module_utils[.basic import AnsibleModule]
+NEW_STYLE_PYTHON_MODULE_RE = re.compile(
+    # Relative imports
+    br'(?:from +\.{2,} *module_utils.* +import |'
+    # Collection absolute imports:
+    br'from +ansible_collections\.[^.]+\.[^.]+\.plugins\.module_utils.* +import |'
+    br'import +ansible_collections\.[^.]+\.[^.]+\.plugins\.module_utils.*|'
+    # Core absolute imports
+    br'from +ansible\.module_utils.* +import |'
+    br'import +ansible\.module_utils\.)'
+)
+
 
 class ModuleDepFinder(ast.NodeVisitor):
-    # Caveats:
-    # This code currently does not handle:
-    # * relative imports from py2.6+ from . import urls
-    IMPORT_PREFIX_SIZE = len('ansible.module_utils.')
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, module_fqn, *args, **kwargs):
         """
         Walk the ast tree for the python module.
+        :arg module_fqn: The fully qualified name to reach this module in dotted notation.
+            example: ansible.module_utils.basic
 
         Save submodule[.submoduleN][.identifier] into self.submodules
+        when they are from ansible.module_utils or ansible_collections packages
 
         self.submodules will end up with tuples like:
-          - ('basic',)
-          - ('urls', 'fetch_url')
-          - ('database', 'postgres')
-          - ('database', 'postgres', 'quote')
+          - ('ansible', 'module_utils', 'basic',)
+          - ('ansible', 'module_utils', 'urls', 'fetch_url')
+          - ('ansible', 'module_utils', 'database', 'postgres')
+          - ('ansible', 'module_utils', 'database', 'postgres', 'quote')
+          - ('ansible', 'module_utils', 'database', 'postgres', 'quote')
+          - ('ansible_collections', 'my_ns', 'my_col', 'plugins', 'module_utils', 'foo')
 
         It's up to calling code to determine whether the final element of the
-        dotted strings are module names or something else (function, class, or
-        variable names)
+        tuple are module names or something else (function, class, or variable names)
+        .. seealso:: :python3:class:`ast.NodeVisitor`
         """
         super(ModuleDepFinder, self).__init__(*args, **kwargs)
         self.submodules = set()
+        self.module_fqn = module_fqn
 
     def visit_Import(self, node):
-        # import ansible.module_utils.MODLIB[.MODLIBn] [as asname]
+        """
+        Handle import ansible.module_utils.MODLIB[.MODLIBn] [as asname]
+
+        We save these as interesting submodules when the imported library is in ansible.module_utils
+        or ansible.collections
+        """
         for alias in node.names:
-            if alias.name.startswith('ansible.module_utils.'):
-                py_mod = alias.name[self.IMPORT_PREFIX_SIZE:]
-                py_mod = tuple(py_mod.split('.'))
+            if (alias.name.startswith('ansible.module_utils.') or
+                    alias.name.startswith('ansible_collections.')):
+                py_mod = tuple(alias.name.split('.'))
                 self.submodules.add(py_mod)
-            elif alias.name.startswith('ansible_collections.'):
-                # keep 'ansible_collections.' as a sentinel prefix to trigger collection-loaded MU path
-                self.submodules.add(tuple(alias.name.split('.')))
         self.generic_visit(node)
 
     def visit_ImportFrom(self, node):
+        """
+        Handle from ansible.module_utils.MODLIB import [.MODLIBn] [as asname]
+
+        Also has to handle relative imports
+
+        We save these as interesting submodules when the imported library is in ansible.module_utils
+        or ansible.collections
+        """
+
+        # FIXME: These should all get skipped:
+        # from ansible.executor import module_common
+        # from ...executor import module_common
+        # from ... import executor (Currently it gives a non-helpful error)
+        if node.level > 0:
+            if self.module_fqn:
+                parts = tuple(self.module_fqn.split('.'))
+                if node.module:
+                    # relative import: from .module import x
+                    node_module = '.'.join(parts[:-node.level] + (node.module,))
+                else:
+                    # relative import: from . import x
+                    node_module = '.'.join(parts[:-node.level])
+            else:
+                # fall back to an absolute import
+                node_module = node.module
+        else:
+            # absolute import: from module import x
+            node_module = node.module
+
         # Specialcase: six is a special case because of its
         # import logic
+        py_mod = None
         if node.names[0].name == '_six':
             self.submodules.add(('_six',))
-        elif node.module.startswith('ansible.module_utils'):
-            where_from = node.module[self.IMPORT_PREFIX_SIZE:]
-            if where_from:
-                # from ansible.module_utils.MODULE1[.MODULEn] import IDENTIFIER [as asname]
-                # from ansible.module_utils.MODULE1[.MODULEn] import MODULEn+1 [as asname]
-                # from ansible.module_utils.MODULE1[.MODULEn] import MODULEn+1 [,IDENTIFIER] [as asname]
-                py_mod = tuple(where_from.split('.'))
-                for alias in node.names:
-                    self.submodules.add(py_mod + (alias.name,))
-            else:
-                # from ansible.module_utils import MODLIB [,MODLIB2] [as asname]
-                for alias in node.names:
-                    self.submodules.add((alias.name,))
+        elif node_module.startswith('ansible.module_utils'):
+            # from ansible.module_utils.MODULE1[.MODULEn] import IDENTIFIER [as asname]
+            # from ansible.module_utils.MODULE1[.MODULEn] import MODULEn+1 [as asname]
+            # from ansible.module_utils.MODULE1[.MODULEn] import MODULEn+1 [,IDENTIFIER] [as asname]
+            # from ansible.module_utils import MODULE1 [,MODULEn] [as asname]
+            py_mod = tuple(node_module.split('.'))
 
-        elif node.module.startswith('ansible_collections.'):
-            # TODO: finish out the subpackage et al cases
-            if node.module.endswith('plugins.module_utils'):
+        elif node_module.startswith('ansible_collections.'):
+            if node_module.endswith('plugins.module_utils') or '.plugins.module_utils.' in node_module:
                 # from ansible_collections.ns.coll.plugins.module_utils import MODULE [as aname] [,MODULE2] [as aname]
-                py_mod = tuple(node.module.split('.'))
-                for alias in node.names:
-                    self.submodules.add(py_mod + (alias.name,))
-            else:
                 # from ansible_collections.ns.coll.plugins.module_utils.MODULE import IDENTIFIER [as aname]
-                self.submodules.add(tuple(node.module.split('.')))
+                # FIXME: Unhandled cornercase (needs to be ignored):
+                # from ansible_collections.ns.coll.plugins.[!module_utils].[FOO].plugins.module_utils import IDENTIFIER
+                py_mod = tuple(node_module.split('.'))
+            else:
+                # Not from module_utils so ignore.  for instance:
+                # from ansible_collections.ns.coll.plugins.lookup import IDENTIFIER
+                pass
+
+        if py_mod:
+            for alias in node.names:
+                self.submodules.add(py_mod + (alias.name,))
 
         self.generic_visit(node)
 
@@ -601,73 +633,126 @@ class ModuleInfo:
                 self._info[0].close()
         return _slurp(self.path)
 
+    def __repr__(self):
+        return 'ModuleInfo: py_src=%s, pkg_dir=%s, path=%s' % (self.py_src, self.pkg_dir, self.path)
 
-def recursive_finder(name, data, py_module_names, py_module_cache, zf):
+
+class CollectionModuleInfo(ModuleInfo):
+    def __init__(self, name, paths):
+        self._mod_name = name
+        self.py_src = True
+        # FIXME: Implement pkg_dir so that we can place __init__.py files
+        self.pkg_dir = False
+
+        for path in paths:
+            self._package_name = '.'.join(path.split('/'))
+            try:
+                self.get_source()
+            except FileNotFoundError:
+                pass
+            else:
+                self.path = os.path.join(path, self._mod_name) + '.py'
+                break
+        else:
+            # FIXME (nitz): implement package fallback code
+            raise ImportError('unable to load collection-hosted module_util'
+                              ' {0}.{1}'.format(to_native(self._package_name),
+                                                to_native(name)))
+
+    def get_source(self):
+        # FIXME (nitz): need this in py2 for some reason TBD, but we shouldn't (get_data delegates
+        # to wrong loader without it)
+        pkg = import_module(self._package_name)
+        data = pkgutil.get_data(to_native(self._package_name), to_native(self._mod_name + '.py'))
+        return data
+
+
+def recursive_finder(name, module_fqn, data, py_module_names, py_module_cache, zf):
     """
     Using ModuleDepFinder, make sure we have all of the module_utils files that
-    the module its module_utils files needs.
+    the module and its module_utils files needs.
+    :arg name: Name of the python module we're examining
+    :arg module_fqn: Fully qualified name of the python module we're scanning
+    :arg py_module_names: set of the fully qualified module names represented as a tuple of their
+        FQN with __init__ appended if the module is also a python package).  Presence of a FQN in
+        this set means that we've already examined it for module_util deps.
+    :arg py_module_cache: map python module names (represented as a tuple of their FQN with __init__
+        appended if the module is also a python package) to a tuple of the code in the module and
+        the pathname the module would have inside of a Python toplevel (like site-packages)
+    :arg zf: An open :python:class:`zipfile.ZipFile` object that holds the Ansible module payload
+        which we're assembling
     """
     # Parse the module and find the imports of ansible.module_utils
     try:
         tree = ast.parse(data)
     except (SyntaxError, IndentationError) as e:
         raise AnsibleError("Unable to import %s due to %s" % (name, e.msg))
-    finder = ModuleDepFinder()
+
+    finder = ModuleDepFinder(module_fqn)
     finder.visit(tree)
 
     #
     # Determine what imports that we've found are modules (vs class, function.
     # variable names) for packages
     #
+    module_utils_paths = [p for p in module_utils_loader._get_paths(subdirs=False) if os.path.isdir(p)]
+    # FIXME: Do we still need this?  It feels like module-utils_loader should include
+    # _MODULE_UTILS_PATH
+    module_utils_paths.append(_MODULE_UTILS_PATH)
 
     normalized_modules = set()
     # Loop through the imports that we've found to normalize them
     # Exclude paths that match with paths we've already processed
     # (Have to exclude them a second time once the paths are processed)
 
-    module_utils_paths = [p for p in module_utils_loader._get_paths(subdirs=False) if os.path.isdir(p)]
-    module_utils_paths.append(_MODULE_UTILS_PATH)
     for py_module_name in finder.submodules.difference(py_module_names):
         module_info = None
 
-        if py_module_name[0] == 'six':
-            # Special case the python six library because it messes up the
+        if py_module_name[0:3] == ('ansible', 'module_utils', 'six'):
+            # Special case the python six library because it messes with the
             # import process in an incompatible way
             module_info = ModuleInfo('six', module_utils_paths)
-            py_module_name = ('six',)
+            py_module_name = ('ansible', 'module_utils', 'six')
             idx = 0
-        elif py_module_name[0] == '_six':
-            # Special case the python six library because it messes up the
+        elif py_module_name[0:3] == ('ansible', 'module_utils', '_six'):
+            # Special case the python six library because it messes with the
             # import process in an incompatible way
             module_info = ModuleInfo('_six', [os.path.join(p, 'six') for p in module_utils_paths])
-            py_module_name = ('six', '_six')
+            py_module_name = ('ansible', 'module_utils', 'six', '_six')
             idx = 0
         elif py_module_name[0] == 'ansible_collections':
-            # FIXME: replicate module name resolution like below for granular imports
-            # this is a collection-hosted MU; look it up with get_data
-            package_name = '.'.join(py_module_name[:-1])
-            resource_name = py_module_name[-1] + '.py'
-            try:
-                # FIXME: need this in py2 for some reason TBD, but we shouldn't (get_data delegates to wrong loader without it)
-                pkg = import_module(package_name)
-                module_info = pkgutil.get_data(package_name, resource_name)
-            except FileNotFoundError:
-                # FIXME: implement package fallback code
-                raise AnsibleError('unable to load collection-hosted module_util {0}.{1}'.format(to_native(package_name),
-                                                                                                 to_native(resource_name)))
-            idx = 0
-        else:
-            # Check whether either the last or the second to last identifier is
-            # a module name
+            # FIXME (nitz): replicate module name resolution like below for granular imports
             for idx in (1, 2):
                 if len(py_module_name) < idx:
                     break
                 try:
-                    module_info = ModuleInfo(py_module_name[-idx],
-                                             [os.path.join(p, *py_module_name[:-idx]) for p in module_utils_paths])
+                    # this is a collection-hosted MU; look it up with pkgutil.get_data()
+                    module_info = CollectionModuleInfo(py_module_name[-idx],
+                                                       [os.path.join(*py_module_name[:-idx])])
                     break
                 except ImportError:
                     continue
+        elif py_module_name[0:2] == ('ansible', 'module_utils'):
+            # Need to remove ansible.module_utils because PluginLoader may find different paths
+            # for us to look in
+            relative_module_utils_dir = py_module_name[2:]
+            # Check whether either the last or the second to last identifier is
+            # a module name
+            for idx in (1, 2):
+                if len(relative_module_utils_dir) < idx:
+                    break
+                try:
+                    module_info = ModuleInfo(py_module_name[-idx],
+                                             [os.path.join(p, *relative_module_utils_dir[:-idx]) for p in module_utils_paths])
+                    break
+                except ImportError:
+                    continue
+        else:
+            # If we get here, it's because of a bug in ModuleDepFinder.  If we get a reproducer we
+            # should then fix ModuleDepFinder
+            display.warning('ModuleDepFinder improperly found a non-module_utils import %s'
+                            % [py_module_name])
+            continue
 
         # Could not find the module.  Construct a helpful error message.
         if module_info is None:
@@ -678,10 +763,15 @@ def recursive_finder(name, data, py_module_names, py_module_cache, zf):
                 msg.append(py_module_name[-1])
             raise AnsibleError(' '.join(msg))
 
-        if isinstance(module_info, bytes):  # collection-hosted, just the code
+        if isinstance(module_info, CollectionModuleInfo):
+            if idx == 2:
+                # We've determined that the last portion was an identifier and
+                # thus, not part of the module name
+                py_module_name = py_module_name[:-1]
+
             # HACK: maybe surface collection dirs in here and use existing find_module code?
             normalized_name = py_module_name
-            normalized_data = module_info
+            normalized_data = module_info.get_source()
             normalized_path = os.path.join(*py_module_name)
             py_module_cache[normalized_name] = (normalized_data, normalized_path)
             normalized_modules.add(normalized_name)
@@ -738,13 +828,18 @@ def recursive_finder(name, data, py_module_names, py_module_cache, zf):
                         py_module_cache[normalized_name] = (normalized_data, module_info.path)
                         normalized_modules.add(normalized_name)
 
+                #
                 # Make sure that all the packages that this module is a part of
                 # are also added
+                #
                 for i in range(1, len(py_module_name)):
                     py_pkg_name = py_module_name[:-i] + ('__init__',)
                     if py_pkg_name not in py_module_names:
-                        pkg_dir_info = ModuleInfo(py_pkg_name[-1],
-                                                  [os.path.join(p, *py_pkg_name[:-1]) for p in module_utils_paths])
+                        # Need to remove ansible.module_utils because PluginLoader may find
+                        # different paths for us to look in
+                        relative_module_utils = py_pkg_name[2:]
+                        pkg_dir_info = ModuleInfo(relative_module_utils[-1],
+                                                  [os.path.join(p, *relative_module_utils[:-1]) for p in module_utils_paths])
                         normalized_modules.add(py_pkg_name)
                         py_module_cache[py_pkg_name] = (pkg_dir_info.get_source(), pkg_dir_info.path)
 
@@ -758,10 +853,10 @@ def recursive_finder(name, data, py_module_names, py_module_cache, zf):
     # want basic.py can import that instead.  AnsibleModule will need to change to import the vars
     # from the separate python module and mirror the args into its global variable for backwards
     # compatibility.
-    if ('basic',) not in py_module_names:
+    if ('ansible', 'module_utils', 'basic',) not in py_module_names:
         pkg_dir_info = ModuleInfo('basic', module_utils_paths)
-        normalized_modules.add(('basic',))
-        py_module_cache[('basic',)] = (pkg_dir_info.get_source(), pkg_dir_info.path)
+        normalized_modules.add(('ansible', 'module_utils', 'basic',))
+        py_module_cache[('ansible', 'module_utils', 'basic',)] = (pkg_dir_info.get_source(), pkg_dir_info.path)
     # End of AnsiballZ hack
 
     #
@@ -773,17 +868,11 @@ def recursive_finder(name, data, py_module_names, py_module_cache, zf):
     unprocessed_py_module_names = normalized_modules.difference(py_module_names)
 
     for py_module_name in unprocessed_py_module_names:
-        # HACK: this seems to work as a way to identify a collections-based import, but a stronger identifier would be better
-        if not py_module_cache[py_module_name][1].startswith('/'):
-            dir_prefix = ''
-        else:
-            dir_prefix = 'ansible/module_utils'
 
         py_module_path = os.path.join(*py_module_name)
         py_module_file_name = '%s.py' % py_module_path
 
-        zf.writestr(os.path.join(dir_prefix,
-                    py_module_file_name), py_module_cache[py_module_name][0])
+        zf.writestr(py_module_file_name, py_module_cache[py_module_name][0])
         display.vvvvv("Using module_utils file %s" % py_module_cache[py_module_name][1])
 
     # Add the names of the files we're scheduling to examine in the loop to
@@ -792,7 +881,9 @@ def recursive_finder(name, data, py_module_names, py_module_cache, zf):
     py_module_names.update(unprocessed_py_module_names)
 
     for py_module_file in unprocessed_py_module_names:
-        recursive_finder(py_module_file, py_module_cache[py_module_file][0], py_module_names, py_module_cache, zf)
+        next_fqn = '.'.join(py_module_file)
+        recursive_finder(py_module_file[-1], next_fqn, py_module_cache[py_module_file][0],
+                         py_module_names, py_module_cache, zf)
         # Save memory; the file won't have to be read again for this ansible module.
         del py_module_cache[py_module_file]
 
@@ -803,6 +894,67 @@ def _is_binary(b_module_data):
     return bool(start.translate(None, textchars))
 
 
+def _get_ansible_module_fqn(module_path):
+    """
+    Get the fully qualified name for an ansible module based on its pathname
+
+    remote_module_fqn is the fully qualified name.  Like ansible.modules.system.ping
+    Or ansible_collections.Namespace.Collection_name.plugins.modules.ping
+    .. warning:: This function is for ansible modules only.  It won't work for other things
+        (non-module plugins, etc)
+    """
+    remote_module_fqn = None
+
+    # Is this a core module?
+    match = CORE_LIBRARY_PATH_RE.search(module_path)
+    if not match:
+        # Is this a module in a collection?
+        match = COLLECTION_PATH_RE.search(module_path)
+
+    # We can tell the FQN for core modules and collection modules
+    if match:
+        path = match.group('path')
+        if '.' in path:
+            # FQNs must be valid as python identifiers.  This sanity check has failed.
+            # we could check other things as well
+            raise ValueError('Module name (or path) was not a valid python identifier')
+
+        remote_module_fqn = '.'.join(path.split('/'))
+    else:
+        # Currently we do not handle modules in roles so we can end up here for that reason
+        raise ValueError("Unable to determine module's fully qualified name")
+
+    return remote_module_fqn
+
+
+def _add_module_to_zip(zf, remote_module_fqn, b_module_data):
+    """Add a module from ansible or from an ansible collection into the module zip"""
+    module_path_parts = remote_module_fqn.split('.')
+
+    # Write the module
+    module_path = '/'.join(module_path_parts) + '.py'
+    zf.writestr(module_path, b_module_data)
+
+    # Write the __init__.py's necessary to get there
+    if module_path_parts[0] == 'ansible':
+        # The ansible namespace is setup as part of the module_utils setup...
+        start = 2
+        existing_paths = frozenset()
+    else:
+        # ... but ansible_collections and other toplevels are not
+        start = 1
+        existing_paths = frozenset(zf.namelist())
+
+    for idx in range(start, len(module_path_parts)):
+        package_path = '/'.join(module_path_parts[:idx]) + '/__init__.py'
+        # If a collections module uses module_utils from a collection then most packages will have already been added by recursive_finder.
+        if package_path in existing_paths:
+            continue
+        # Note: We don't want to include more than one ansible module in a payload at this time
+        # so no need to fill the __init__.py with namespace code
+        zf.writestr(package_path, b'')
+
+
 def _find_module_utils(module_name, b_module_data, module_path, module_args, task_vars, templar, module_compression, async_timeout, become,
                        become_method, become_user, become_password, become_flags, environment):
     """
@@ -825,9 +977,7 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas
         module_style = 'new'
         module_substyle = 'python'
         b_module_data = b_module_data.replace(REPLACER, b'from ansible.module_utils.basic import *')
-    # FUTURE: combined regex for this stuff, or a "looks like Python, let's inspect further" mechanism
-    elif b'from ansible.module_utils.' in b_module_data or b'from ansible_collections.' in b_module_data\
-            or b'import ansible_collections.' in b_module_data:
+    elif NEW_STYLE_PYTHON_MODULE_RE.search(b_module_data):
         module_style = 'new'
         module_substyle = 'python'
     elif REPLACER_WINDOWS in b_module_data:
@@ -869,6 +1019,17 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas
             display.warning(u'Bad module compression string specified: %s.  Using ZIP_STORED (no compression)' % module_compression)
             compression_method = zipfile.ZIP_STORED
 
+        try:
+            remote_module_fqn = _get_ansible_module_fqn(module_path)
+        except ValueError:
+            # Modules in roles currently are not found by the fqn heuristic so we
+            # fallback to this.  This means that relative imports inside a module from
+            # a role may fail.  Absolute imports should be used for future-proofness.
+            # People should start writing collections instead of modules in roles so we
+            # may never fix this
+            display.debug('ANSIBALLZ: Could not determine module FQN')
+            remote_module_fqn = 'ansible.modules.%s' % module_name
+
         lookup_path = os.path.join(C.DEFAULT_LOCAL_TMP, 'ansiballz_cache')
         cached_module_filename = os.path.join(lookup_path, "%s-%s" % (module_name, module_compression))
 
@@ -900,18 +1061,42 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas
                     # Create the module zip data
                     zipoutput = BytesIO()
                     zf = zipfile.ZipFile(zipoutput, mode='w', compression=compression_method)
-                    # Note: If we need to import from release.py first,
-                    # remember to catch all exceptions: https://github.com/ansible/ansible/issues/16523
-                    zf.writestr('ansible/__init__.py',
-                                b'from pkgutil import extend_path\n__path__=extend_path(__path__,__name__)\n__version__="' +
-                                to_bytes(__version__) + b'"\n__author__="' +
-                                to_bytes(__author__) + b'"\n')
-                    zf.writestr('ansible/module_utils/__init__.py', b'from pkgutil import extend_path\n__path__=extend_path(__path__,__name__)\n')
 
-                    zf.writestr('__main__.py', b_module_data)
+                    # py_module_cache maps python module names to a tuple of the code in the module
+                    # and the pathname to the module.  See the recursive_finder() documentation for
+                    # more info.
+                    # Here we pre-load it with modules which we create without bothering to
+                    # read from actual files (In some cases, these need to differ from what ansible
+                    # ships because they're namespace packages in the module)
+                    py_module_cache = {
+                        ('ansible', '__init__',): (
+                            b'from pkgutil import extend_path\n'
+                            b'__path__=extend_path(__path__,__name__)\n'
+                            b'__version__="' + to_bytes(__version__) +
+                            b'"\n__author__="' + to_bytes(__author__) + b'"\n',
+                            'ansible/__init__.py'),
+                        ('ansible', 'module_utils', '__init__',): (
+                            b'from pkgutil import extend_path\n'
+                            b'__path__=extend_path(__path__,__name__)\n',
+                            'ansible/module_utils/__init__.py')}
+
+                    for (py_module_name, (file_data, filename)) in py_module_cache.items():
+                        zf.writestr(filename, file_data)
+                        # py_module_names keeps track of which modules we've already scanned for
+                        # module_util dependencies
+                        py_module_names.add(py_module_name)
+
+                    # Returning the ast tree is a temporary hack.  We need to know if the module has
+                    # a main() function or not as we are deprecating new-style modules without
+                    # main().  Because parsing the ast is expensive, return it from recursive_finder
+                    # instead of reparsing.  Once the deprecation is over and we remove that code,
+                    # also remove returning of the ast tree.
+                    recursive_finder(module_name, remote_module_fqn, b_module_data, py_module_names,
+                                     py_module_cache, zf)
+
+                    display.debug('ANSIBALLZ: Writing module into payload')
+                    _add_module_to_zip(zf, remote_module_fqn, b_module_data)
 
-                    py_module_cache = {('__init__',): (b'', '[builtin]')}
-                    recursive_finder(module_name, b_module_data, py_module_names, py_module_cache, zf)
                     zf.close()
                     zipdata = base64.b64encode(zipoutput.getvalue())
 
@@ -950,11 +1135,6 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas
         if shebang is None:
             shebang = u'#!/usr/bin/python'
 
-        # Enclose the parts of the interpreter in quotes because we're
-        # substituting it into the template as a Python string
-        interpreter_parts = interpreter.split(u' ')
-        interpreter = u"'{0}'".format(u"', '".join(interpreter_parts))
-
         # FUTURE: the module cache entry should be invalidated if we got this value from a host-dependent source
         rlimit_nofile = C.config.get_config_value('PYTHON_MODULE_RLIMIT_NOFILE', variables=task_vars)
 
@@ -991,9 +1171,9 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas
         output.write(to_bytes(ACTIVE_ANSIBALLZ_TEMPLATE % dict(
             zipdata=zipdata,
             ansible_module=module_name,
+            module_fqn=remote_module_fqn,
             params=python_repred_params,
             shebang=shebang,
-            interpreter=interpreter,
             coding=ENCODING_STRING,
             year=now.year,
             month=now.month,

lib/ansible/module_utils/basic.py

@@ -79,7 +79,7 @@ except ImportError:
 # Python2 & 3 way to get NoneType
 NoneType = type(None)
 
-from ansible.module_utils._text import to_native, to_bytes, to_text
+from ._text import to_native, to_bytes, to_text
 from ansible.module_utils.common.text.converters import (
     jsonify,
     container_to_bytes as json_dict_unicode_to_bytes,

lib/ansible/modules/cloud/amazon/lambda_event.py

@@ -393,7 +393,6 @@ def lambda_event_stream(module, aws):
 
 def main():
     """Produce a list of function suffixes which handle lambda events."""
-    this_module = sys.modules[__name__]
     source_choices = ["stream", "sqs"]
 
     argument_spec = ec2_argument_spec()

lib/ansible/modules/system/setup.py

@@ -131,7 +131,7 @@ EXAMPLES = """
 """
 
 # import module snippets
-from ansible.module_utils.basic import AnsibleModule
+from ...module_utils.basic import AnsibleModule
 
 from ansible.module_utils.facts.namespace import PrefixFactNamespace
 from ansible.module_utils.facts import ansible_collector

lib/ansible/modules/utilities/logic/async_wrapper.py

@@ -199,8 +199,7 @@ def _run_module(wrapped_cmd, jid, job_path):
     os.rename(tmp_job_path, job_path)
 
 
-if __name__ == '__main__':
-
+def main():
     if len(sys.argv) < 5:
         print(json.dumps({
             "failed": True,
@@ -334,3 +333,7 @@ if __name__ == '__main__':
             "msg": "FATAL ERROR: %s" % e
         }))
         sys.exit(1)
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/ansiballz_python/library/custom_module.py

@@ -0,0 +1,19 @@
+#!/usr/bin/python
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+from ..module_utils.basic import AnsibleModule  # pylint: disable=relative-beyond-top-level
+from ..module_utils.custom_util import forty_two  # pylint: disable=relative-beyond-top-level
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=dict()
+    )
+
+    module.exit_json(answer=forty_two())
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/ansiballz_python/module_utils/custom_util.py

@@ -0,0 +1,6 @@
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+
+def forty_two():
+    return 42

test/integration/targets/ansiballz_python/tasks/main.yml

@@ -32,6 +32,10 @@
     ansible_python_module_rlimit_nofile: '{{ rlimit_original_return.rlimit_nofile[1] + 1 }}'
   register: rlimit_above_hard_return
 
+- name: run a role module which uses a role module_util using relative imports
+  custom_module:
+  register: custom_module_return
+
 - assert:
     that:
       # make sure ansible-test was able to set the limit unless it exceeds the hard limit or the value is lower on macOS
@@ -54,3 +58,6 @@
 
       # setting the limit higher than the existing hard limit should use the hard limit (except on macOS)
       - rlimit_above_hard_return.rlimit_nofile[0] == rlimit_original_return.rlimit_nofile[1] or ansible_distribution == 'MacOSX'
+
+      # custom module returned the correct answer
+      - custom_module_return.answer == 42

test/integration/targets/collections_relative_imports/aliases

@@ -0,0 +1,2 @@
+shippable/posix/group1
+skip/python2.6

test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/my_util1.py

@@ -0,0 +1,6 @@
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+
+def one():
+    return 1

test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/my_util2.py

@@ -0,0 +1,8 @@
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+from .my_util1 import one
+
+
+def two():
+    return one() * 2

test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/my_util3.py

@@ -0,0 +1,8 @@
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+from . import my_util2
+
+
+def three():
+    return my_util2.two() + 1

test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/modules/my_module.py

@@ -0,0 +1,24 @@
+#!/usr/bin/python
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+from ansible.module_utils.basic import AnsibleModule
+from ..module_utils.my_util2 import two
+from ..module_utils import my_util3
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=dict(),
+        supports_check_mode=True
+    )
+
+    result = dict(
+        two=two(),
+        three=my_util3.three(),
+    )
+    module.exit_json(**result)
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/roles/test/tasks/main.yml

@@ -0,0 +1,4 @@
+- name: fully qualified module usage with relative imports
+  my_ns.my_col.my_module:
+- name: collection relative module usage with relative imports
+  my_module:

test/integration/targets/collections_relative_imports/runme.sh

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+set -eux
+
+ANSIBLE_COLLECTIONS_PATHS="${PWD}/collection_root" ansible-playbook test.yml -i ../../inventory "$@"

test/integration/targets/collections_relative_imports/test.yml

@@ -0,0 +1,3 @@
+- hosts: testhost
+  roles:
+    - my_ns.my_col.test

test/lib/ansible_test/_data/sanity/import/importer.py

@@ -38,6 +38,15 @@ def main():
         # noinspection PyPep8Naming
         AnsibleCollectionLoader = None
 
+    # These are the public attribute sof a doc-only module
+    doc_keys = ('ANSIBLE_METADATA',
+                'DOCUMENTATION',
+                'EXAMPLES',
+                'RETURN',
+                'absolute_import',
+                'division',
+                'print_function')
+
     class ImporterAnsibleModuleException(Exception):
         """Exception thrown during initialization of ImporterAnsibleModule."""
 
@@ -80,17 +89,21 @@ def main():
             if not path.startswith('lib/ansible/modules/'):
                 return
 
+            # __init__ in module directories is empty (enforced by a different test)
+            if path.endswith('__init__.py'):
+                return
+
             # async_wrapper is not an Ansible module
             if path == 'lib/ansible/modules/utilities/logic/async_wrapper.py':
                 return
 
-            # run code protected by __name__ conditional
-            name = '__main__'
+            name = calculate_python_module_name(path)
             # show the Ansible module responsible for the exception, even if it was thrown in module_utils
             filter_dir = os.path.join(base_dir, 'lib/ansible/modules')
         else:
-            # do not run code protected by __name__ conditional
-            name = 'module_import_test'
+            # Calculate module name
+            name = calculate_python_module_name(path)
+
             # show the Ansible file responsible for the exception, even if it was thrown in 3rd party code
             filter_dir = base_dir
 
@@ -98,15 +111,58 @@ def main():
 
         try:
             if imp:
-                with open(path, 'r') as module_fd:
-                    with capture_output(capture):
-                        imp.load_module(name, module_fd, os.path.abspath(path), ('.py', 'r', imp.PY_SOURCE))
+                with capture_output(capture):
+                    # On Python2 without absolute_import we have to import parent modules all
+                    # the way up the tree
+                    full_path = os.path.abspath(path)
+                    parent_mod = None
+
+                    py_packages = name.split('.')
+                    # BIG HACK: reimporting module_utils breaks the monkeypatching of basic we did
+                    # above and also breaks modules which import names directly from module_utils
+                    # modules (you'll get errors like ERROR:
+                    # lib/ansible/modules/storage/netapp/na_ontap_vserver_cifs_security.py:151:0:
+                    # AttributeError: 'module' object has no attribute 'netapp').
+                    # So when we import a module_util here, use a munged name.
+                    if 'module_utils' in py_packages:
+                        # Avoid accidental double underscores by using _1 as a prefix
+                        py_packages[-1] = '_1%s' % py_packages[-1]
+                        name = '.'.join(py_packages)
+
+                    for idx in range(1, len(py_packages)):
+                        parent_name = '.'.join(py_packages[:idx])
+                        if parent_mod is None:
+                            toplevel_end = full_path.find('ansible/module')
+                            toplevel = full_path[:toplevel_end]
+                            parent_mod_info = imp.find_module(parent_name, [toplevel])
+                        else:
+                            parent_mod_info = imp.find_module(py_packages[idx - 1], parent_mod.__path__)
+
+                        parent_mod = imp.load_module(parent_name, *parent_mod_info)
+                        # skip distro due to an apparent bug or bad interaction in
+                        # imp.load_module() with our distro/__init__.py.
+                        # distro/__init__.py sets sys.modules['ansible.module_utils.distro']
+                        # = _distro.pyc
+                        # but after running imp.load_module(),
+                        # sys.modules['ansible.module_utils.distro._distro'] = __init__.pyc
+                        # (The opposite of what we set)
+                        # This does not affect runtime so regular import seems to work.  It's
+                        # just imp.load_module()
+                        if name == 'ansible.module_utils.distro._1__init__':
+                            return
+
+                    with open(path, 'r') as module_fd:
+                        module = imp.load_module(name, module_fd, full_path, ('.py', 'r', imp.PY_SOURCE))
+                        if ansible_module:
+                            run_if_really_module(module)
             else:
                 spec = importlib.util.spec_from_file_location(name, os.path.abspath(path))
                 module = importlib.util.module_from_spec(spec)
 
                 with capture_output(capture):
                     spec.loader.exec_module(module)
+                    if ansible_module:
+                        run_if_really_module(module)
 
             capture_report(path, capture, messages)
         except ImporterAnsibleModuleException:
@@ -153,6 +209,34 @@ def main():
 
             report_message(error, messages)
 
+    def run_if_really_module(module):
+        # Module was removed
+        if ('removed' not in module.ANSIBLE_METADATA['status'] and
+                # Documentation only module
+                [attr for attr in
+                 (frozenset(module.__dict__.keys()).difference(doc_keys))
+                 if not (attr.startswith('__') and attr.endswith('__'))]):
+            # Run main() code for ansible_modules
+            module.main()
+
+    def calculate_python_module_name(path):
+        name = None
+        try:
+            idx = path.index('ansible/modules')
+        except ValueError:
+            try:
+                idx = path.index('ansible/module_utils')
+            except ValueError:
+                try:
+                    idx = path.index('ansible_collections')
+                except ValueError:
+                    # Default
+                    name = 'module_import_test'
+        if name is None:
+            name = path[idx:-len('.py')].replace('/', '.')
+
+        return name
+
     class Capture:
         """Captured output and/or exception."""
         def __init__(self):

test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py

@@ -105,13 +105,25 @@ def get_ps_argument_spec(filename):
 
 
 def get_py_argument_spec(filename):
-    with setup_env(filename) as fake:
+    # Calculate the module's name so that relative imports work correctly
+    name = None
+    try:
+        idx = filename.index('ansible/modules')
+    except ValueError:
         try:
-            # We use ``module`` here instead of ``__main__``
+            idx = filename.index('ansible_collections/')
+        except ValueError:
+            # We default to ``module`` here instead of ``__main__``
             # which helps with some import issues in this tool
             # where modules may import things that conflict
+            name = 'module'
+    if name is None:
+        name = filename[idx:-len('.py')].replace('/', '.')
+
+    with setup_env(filename) as fake:
+        try:
             with CaptureStd():
-                mod = imp.load_source('module', filename)
+                mod = imp.load_source(name, filename)
                 if not fake.called:
                     mod.main()
         except AnsibleModuleCallError:

test/sanity/ignore.txt

@@ -6027,6 +6027,9 @@ test/integration/targets/async_fail/library/async_test.py future-import-boilerpl
 test/integration/targets/async_fail/library/async_test.py metaclass-boilerplate
 test/integration/targets/aws_lambda/files/mini_lambda.py future-import-boilerplate
 test/integration/targets/aws_lambda/files/mini_lambda.py metaclass-boilerplate
+test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/my_util2.py pylint:relative-beyond-top-level
+test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/my_util3.py pylint:relative-beyond-top-level
+test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/modules/my_module.py pylint:relative-beyond-top-level
 test/integration/targets/expect/files/test_command.py future-import-boilerplate
 test/integration/targets/expect/files/test_command.py metaclass-boilerplate
 test/integration/targets/get_url/files/testserver.py future-import-boilerplate

test/units/executor/module_common/test_module_common.py

@@ -19,6 +19,8 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import os.path
+
 import pytest
 
 import ansible.errors
@@ -28,7 +30,7 @@ from ansible.executor.interpreter_discovery import InterpreterDiscoveryRequiredE
 from ansible.module_utils.six import PY2
 
 
-class TestStripComments(object):
+class TestStripComments:
     def test_no_changes(self):
         no_comments = u"""def some_code():
     return False"""
@@ -69,7 +71,7 @@ def test(arg):
         assert amc._strip_comments(mixed) == mixed_results
 
 
-class TestSlurp(object):
+class TestSlurp:
     def test_slurp_nonexistent(self, mocker):
         mocker.patch('os.path.exists', side_effect=lambda x: False)
         with pytest.raises(ansible.errors.AnsibleError):
@@ -96,14 +98,14 @@ class TestSlurp(object):
 
 @pytest.fixture
 def templar():
-    class FakeTemplar(object):
+    class FakeTemplar:
         def template(self, template_string, *args, **kwargs):
             return template_string
 
     return FakeTemplar()
 
 
-class TestGetShebang(object):
+class TestGetShebang:
     """Note: We may want to change the API of this function in the future.  It isn't a great API"""
     def test_no_interpreter_set(self, templar):
         # normally this would return /usr/bin/python, but so long as we're defaulting to auto python discovery, we'll get
@@ -129,3 +131,67 @@ class TestGetShebang(object):
     def test_python_via_env(self, templar):
         assert amc._get_shebang(u'/usr/bin/python', {u'ansible_python_interpreter': u'/usr/bin/env python'}, templar) == \
             (u'#!/usr/bin/env python', u'/usr/bin/env python')
+
+
+class TestDetectionRegexes:
+    ANSIBLE_MODULE_UTIL_STRINGS = (
+        # Absolute collection imports
+        b'import ansible_collections.my_ns.my_col.plugins.module_utils.my_util',
+        b'from ansible_collections.my_ns.my_col.plugins.module_utils import my_util',
+        b'from ansible_collections.my_ns.my_col.plugins.module_utils.my_util import my_func',
+        # Absolute core imports
+        b'import ansible.module_utils.basic',
+        b'from ansible.module_utils import basic',
+        b'from ansible.module_utils.basic import AnsibleModule',
+        # Relative imports
+        b'from ..module_utils import basic',
+        b'from .. module_utils import basic',
+        b'from ....module_utils import basic',
+        b'from ..module_utils.basic import AnsibleModule',
+    )
+    NOT_ANSIBLE_MODULE_UTIL_STRINGS = (
+        b'from ansible import release',
+        b'from ..release import __version__',
+        b'from .. import release',
+        b'from ansible.modules.system import ping',
+        b'from ansible_collecitons.my_ns.my_col.plugins.modules import function',
+    )
+
+    OFFSET = os.path.dirname(os.path.dirname(amc.__file__))
+    CORE_PATHS = (
+        ('%s/modules/from_role.py' % OFFSET, 'ansible/modules/from_role'),
+        ('%s/modules/system/ping.py' % OFFSET, 'ansible/modules/system/ping'),
+        ('%s/modules/cloud/amazon/s3.py' % OFFSET, 'ansible/modules/cloud/amazon/s3'),
+    )
+
+    COLLECTION_PATHS = (
+        ('/root/ansible_collections/ns/col/plugins/modules/ping.py',
+         'ansible_collections/ns/col/plugins/modules/ping'),
+        ('/root/ansible_collections/ns/col/plugins/modules/subdir/ping.py',
+         'ansible_collections/ns/col/plugins/modules/subdir/ping'),
+    )
+
+    @pytest.mark.parametrize('testcase', ANSIBLE_MODULE_UTIL_STRINGS)
+    def test_detect_new_style_python_module_re(self, testcase):
+        assert amc.NEW_STYLE_PYTHON_MODULE_RE.search(testcase)
+
+    @pytest.mark.parametrize('testcase', NOT_ANSIBLE_MODULE_UTIL_STRINGS)
+    def test_no_detect_new_style_python_module_re(self, testcase):
+        assert not amc.NEW_STYLE_PYTHON_MODULE_RE.search(testcase)
+
+    # pylint bug: https://github.com/PyCQA/pylint/issues/511
+    @pytest.mark.parametrize('testcase, result', CORE_PATHS)  # pylint: disable=undefined-variable
+    def test_detect_core_library_path_re(self, testcase, result):
+        assert amc.CORE_LIBRARY_PATH_RE.search(testcase).group('path') == result
+
+    @pytest.mark.parametrize('testcase', (p[0] for p in COLLECTION_PATHS))  # pylint: disable=undefined-variable
+    def test_no_detect_core_library_path_re(self, testcase):
+        assert not amc.CORE_LIBRARY_PATH_RE.search(testcase)
+
+    @pytest.mark.parametrize('testcase, result', COLLECTION_PATHS)  # pylint: disable=undefined-variable
+    def test_detect_collection_path_re(self, testcase, result):
+        assert amc.COLLECTION_PATH_RE.search(testcase).group('path') == result
+
+    @pytest.mark.parametrize('testcase', (p[0] for p in CORE_PATHS))  # pylint: disable=undefined-variable
+    def test_no_detect_collection_path_re(self, testcase):
+        assert not amc.COLLECTION_PATH_RE.search(testcase)

test/units/executor/module_common/test_recursive_finder.py

@@ -19,6 +19,7 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import os
 import pytest
 import zipfile
 
@@ -35,27 +36,29 @@ from ansible.module_utils.six import PY2
 # when basic.py gains new imports
 # We will remove these when we modify AnsiBallZ to store its args in a separate file instead of in
 # basic.py
-MODULE_UTILS_BASIC_IMPORTS = frozenset((('_text',),
-                                        ('basic',),
-                                        ('common', '__init__'),
-                                        ('common', '_collections_compat'),
-                                        ('common', '_json_compat'),
-                                        ('common', 'collections'),
-                                        ('common', 'file'),
-                                        ('common', 'parameters'),
-                                        ('common', 'process'),
-                                        ('common', 'sys_info'),
-                                        ('common', 'text', '__init__'),
-                                        ('common', 'text', 'converters'),
-                                        ('common', 'text', 'formatters'),
-                                        ('common', 'validation'),
-                                        ('common', '_utils'),
-                                        ('distro', '__init__'),
-                                        ('distro', '_distro'),
-                                        ('parsing', '__init__'),
-                                        ('parsing', 'convert_bool'),
-                                        ('pycompat24',),
-                                        ('six', '__init__'),
+MODULE_UTILS_BASIC_IMPORTS = frozenset((('ansible', '__init__'),
+                                        ('ansible', 'module_utils', '__init__'),
+                                        ('ansible', 'module_utils', '_text'),
+                                        ('ansible', 'module_utils', 'basic'),
+                                        ('ansible', 'module_utils', 'common', '__init__'),
+                                        ('ansible', 'module_utils', 'common', '_collections_compat'),
+                                        ('ansible', 'module_utils', 'common', '_json_compat'),
+                                        ('ansible', 'module_utils', 'common', 'collections'),
+                                        ('ansible', 'module_utils', 'common', 'file'),
+                                        ('ansible', 'module_utils', 'common', 'parameters'),
+                                        ('ansible', 'module_utils', 'common', 'process'),
+                                        ('ansible', 'module_utils', 'common', 'sys_info'),
+                                        ('ansible', 'module_utils', 'common', 'text', '__init__'),
+                                        ('ansible', 'module_utils', 'common', 'text', 'converters'),
+                                        ('ansible', 'module_utils', 'common', 'text', 'formatters'),
+                                        ('ansible', 'module_utils', 'common', 'validation'),
+                                        ('ansible', 'module_utils', 'common', '_utils'),
+                                        ('ansible', 'module_utils', 'distro', '__init__'),
+                                        ('ansible', 'module_utils', 'distro', '_distro'),
+                                        ('ansible', 'module_utils', 'parsing', '__init__'),
+                                        ('ansible', 'module_utils', 'parsing', 'convert_bool'),
+                                        ('ansible', 'module_utils', 'pycompat24',),
+                                        ('ansible', 'module_utils', 'six', '__init__'),
                                         ))
 
 MODULE_UTILS_BASIC_FILES = frozenset(('ansible/module_utils/_text.py',
@@ -84,15 +87,19 @@ MODULE_UTILS_BASIC_FILES = frozenset(('ansible/module_utils/_text.py',
                                       'ansible/module_utils/six/__init__.py',
                                       ))
 
-ONLY_BASIC_IMPORT = frozenset((('basic',),))
+ONLY_BASIC_IMPORT = frozenset((('ansible', '__init__'),
+                               ('ansible', 'module_utils', '__init__'),
+                               ('ansible', 'module_utils', 'basic',),))
 ONLY_BASIC_FILE = frozenset(('ansible/module_utils/basic.py',))
 
+ANSIBLE_LIB = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__)))), 'lib', 'ansible')
+
 
 @pytest.fixture
 def finder_containers():
     FinderContainers = namedtuple('FinderContainers', ['py_module_names', 'py_module_cache', 'zf'])
 
-    py_module_names = set()
+    py_module_names = set((('ansible', '__init__'), ('ansible', 'module_utils', '__init__')))
     # py_module_cache = {('__init__',): b''}
     py_module_cache = {}
 
@@ -107,7 +114,7 @@ class TestRecursiveFinder(object):
     def test_no_module_utils(self, finder_containers):
         name = 'ping'
         data = b'#!/usr/bin/python\nreturn \'{\"changed\": false}\''
-        recursive_finder(name, data, *finder_containers)
+        recursive_finder(name, os.path.join(ANSIBLE_LIB, 'modules', 'system', 'ping.py'), data, *finder_containers)
         assert finder_containers.py_module_names == set(()).union(MODULE_UTILS_BASIC_IMPORTS)
         assert finder_containers.py_module_cache == {}
         assert frozenset(finder_containers.zf.namelist()) == MODULE_UTILS_BASIC_FILES
@@ -116,14 +123,14 @@ class TestRecursiveFinder(object):
         name = 'fake_module'
         data = b'#!/usr/bin/python\ndef something(:\n   pass\n'
         with pytest.raises(ansible.errors.AnsibleError) as exec_info:
-            recursive_finder(name, data, *finder_containers)
+            recursive_finder(name, os.path.join(ANSIBLE_LIB, 'modules', 'system', 'fake_module.py'), data, *finder_containers)
         assert 'Unable to import fake_module due to invalid syntax' in str(exec_info.value)
 
     def test_module_utils_with_identation_error(self, finder_containers):
         name = 'fake_module'
         data = b'#!/usr/bin/python\n    def something():\n    pass\n'
         with pytest.raises(ansible.errors.AnsibleError) as exec_info:
-            recursive_finder(name, data, *finder_containers)
+            recursive_finder(name, os.path.join(ANSIBLE_LIB, 'modules', 'system', 'fake_module.py'), data, *finder_containers)
         assert 'Unable to import fake_module due to unexpected indent' in str(exec_info.value)
 
     def test_from_import_toplevel_package(self, finder_containers, mocker):
@@ -140,10 +147,10 @@ class TestRecursiveFinder(object):
 
         name = 'ping'
         data = b'#!/usr/bin/python\nfrom ansible.module_utils import foo'
-        recursive_finder(name, data, *finder_containers)
+        recursive_finder(name, os.path.join(ANSIBLE_LIB, 'modules', 'system', 'ping.py'), data, *finder_containers)
         mocker.stopall()
 
-        assert finder_containers.py_module_names == set((('foo', '__init__'),)).union(ONLY_BASIC_IMPORT)
+        assert finder_containers.py_module_names == set((('ansible', 'module_utils', 'foo', '__init__'),)).union(ONLY_BASIC_IMPORT)
         assert finder_containers.py_module_cache == {}
         assert frozenset(finder_containers.zf.namelist()) == frozenset(('ansible/module_utils/foo/__init__.py',)).union(ONLY_BASIC_FILE)
 
@@ -158,10 +165,10 @@ class TestRecursiveFinder(object):
 
         name = 'ping'
         data = b'#!/usr/bin/python\nfrom ansible.module_utils import foo'
-        recursive_finder(name, data, *finder_containers)
+        recursive_finder(name, os.path.join(ANSIBLE_LIB, 'modules', 'system', 'ping.py'), data, *finder_containers)
         mocker.stopall()
 
-        assert finder_containers.py_module_names == set((('foo',),)).union(ONLY_BASIC_IMPORT)
+        assert finder_containers.py_module_names == set((('ansible', 'module_utils', 'foo',),)).union(ONLY_BASIC_IMPORT)
         assert finder_containers.py_module_cache == {}
         assert frozenset(finder_containers.zf.namelist()) == frozenset(('ansible/module_utils/foo.py',)).union(ONLY_BASIC_FILE)
 
@@ -171,23 +178,23 @@ class TestRecursiveFinder(object):
     def test_from_import_six(self, finder_containers):
         name = 'ping'
         data = b'#!/usr/bin/python\nfrom ansible.module_utils import six'
-        recursive_finder(name, data, *finder_containers)
-        assert finder_containers.py_module_names == set((('six', '__init__'),)).union(MODULE_UTILS_BASIC_IMPORTS)
+        recursive_finder(name, os.path.join(ANSIBLE_LIB, 'modules', 'system', 'ping.py'), data, *finder_containers)
+        assert finder_containers.py_module_names == set((('ansible', 'module_utils', 'six', '__init__'),)).union(MODULE_UTILS_BASIC_IMPORTS)
         assert finder_containers.py_module_cache == {}
         assert frozenset(finder_containers.zf.namelist()) == frozenset(('ansible/module_utils/six/__init__.py', )).union(MODULE_UTILS_BASIC_FILES)
 
     def test_import_six(self, finder_containers):
         name = 'ping'
         data = b'#!/usr/bin/python\nimport ansible.module_utils.six'
-        recursive_finder(name, data, *finder_containers)
-        assert finder_containers.py_module_names == set((('six', '__init__'),)).union(MODULE_UTILS_BASIC_IMPORTS)
+        recursive_finder(name, os.path.join(ANSIBLE_LIB, 'modules', 'system', 'ping.py'), data, *finder_containers)
+        assert finder_containers.py_module_names == set((('ansible', 'module_utils', 'six', '__init__'),)).union(MODULE_UTILS_BASIC_IMPORTS)
         assert finder_containers.py_module_cache == {}
         assert frozenset(finder_containers.zf.namelist()) == frozenset(('ansible/module_utils/six/__init__.py', )).union(MODULE_UTILS_BASIC_FILES)
 
     def test_import_six_from_many_submodules(self, finder_containers):
         name = 'ping'
         data = b'#!/usr/bin/python\nfrom ansible.module_utils.six.moves.urllib.parse import urlparse'
-        recursive_finder(name, data, *finder_containers)
-        assert finder_containers.py_module_names == set((('six', '__init__'),)).union(MODULE_UTILS_BASIC_IMPORTS)
+        recursive_finder(name, os.path.join(ANSIBLE_LIB, 'modules', 'system', 'ping.py'), data, *finder_containers)
+        assert finder_containers.py_module_names == set((('ansible', 'module_utils', 'six', '__init__'),)).union(MODULE_UTILS_BASIC_IMPORTS)
         assert finder_containers.py_module_cache == {}
         assert frozenset(finder_containers.zf.namelist()) == frozenset(('ansible/module_utils/six/__init__.py',)).union(MODULE_UTILS_BASIC_FILES)