(Backport 53167) loader: support extra modules in sys.path

salt/loader.py

@@ -16,7 +16,6 @@
 import tempfile
 import threading
 import functools
-import threading
 import traceback
 import types
 from zipimport import zipimporter
@@ -257,6 +256,7 @@ def minion_mods(
         whitelist=whitelist,
         loaded_base_name=loaded_base_name,
         static_modules=static_modules,
+        extra_module_dirs=utils.module_dirs if utils else None,
     )
 
     ret.pack['__salt__'] = ret
@@ -351,6 +351,7 @@ def engines(opts, functions, runners, utils, proxy=None):
         opts,
         tag='engines',
         pack=pack,
+        extra_module_dirs=utils.module_dirs if utils else None,
     )
 
 
@@ -363,6 +364,7 @@ def proxy(opts, functions=None, returners=None, whitelist=None, utils=None):
         opts,
         tag='proxy',
         pack={'__salt__': functions, '__ret__': returners, '__utils__': utils},
+        extra_module_dirs=utils.module_dirs if utils else None,
     )
 
     ret.pack['__proxy__'] = ret
@@ -400,12 +402,14 @@ def pillars(opts, functions, context=None):
     '''
     Returns the pillars modules
     '''
+    _utils = utils(opts)
     ret = LazyLoader(_module_dirs(opts, 'pillar'),
                      opts,
                      tag='pillar',
                      pack={'__salt__': functions,
                            '__context__': context,
-                           '__utils__': utils(opts)})
+                           '__utils__': _utils},
+                     extra_module_dirs=_utils.module_dirs)
     ret.pack['__ext_pillar__'] = ret
     return FilterDictWrapper(ret, '.ext_pillar')
 
@@ -505,11 +509,13 @@ def fileserver(opts, backends):
     '''
     Returns the file server modules
     '''
+    _utils = utils(opts)
     return LazyLoader(_module_dirs(opts, 'fileserver'),
                       opts,
                       tag='fileserver',
                       whitelist=backends,
-                      pack={'__utils__': utils(opts)})
+                      pack={'__utils__': _utils},
+                      extra_module_dirs=_utils.module_dirs)
 
 
 def roster(opts, runner=None, utils=None, whitelist=None):
@@ -525,6 +531,7 @@ def roster(opts, runner=None, utils=None, whitelist=None):
             '__runner__': runner,
             '__utils__': utils,
         },
+        extra_module_dirs=utils.module_dirs if utils else None,
     )
 
 
@@ -566,6 +573,7 @@ def states(opts, functions, utils, serializers, whitelist=None, proxy=None, cont
         tag='states',
         pack={'__salt__': functions, '__proxy__': proxy or {}},
         whitelist=whitelist,
+        extra_module_dirs=utils.module_dirs if utils else None,
     )
     ret.pack['__states__'] = ret
     ret.pack['__utils__'] = utils
@@ -678,6 +686,7 @@ def grain_funcs(opts, proxy=None):
           __opts__ = salt.config.minion_config('/etc/salt/minion')
           grainfuncs = salt.loader.grain_funcs(__opts__)
     '''
+    _utils = utils(opts, proxy=proxy)
     ret = LazyLoader(
         _module_dirs(
             opts,
@@ -687,8 +696,9 @@ def grain_funcs(opts, proxy=None):
         ),
         opts,
         tag='grains',
+        extra_module_dirs=_utils.module_dirs,
     )
-    ret.pack['__utils__'] = utils(opts, proxy=proxy)
+    ret.pack['__utils__'] = _utils
     return ret
 
 
@@ -940,6 +950,7 @@ def runner(opts, utils=None, context=None, whitelist=None):
         tag='runners',
         pack={'__utils__': utils, '__context__': context},
         whitelist=whitelist,
+        extra_module_dirs=utils.module_dirs if utils else None,
     )
     # TODO: change from __salt__ to something else, we overload __salt__ too much
     ret.pack['__salt__'] = ret
@@ -975,6 +986,7 @@ def sdb(opts, functions=None, whitelist=None, utils=None):
             '__salt__': minion_mods(opts, utils),
         },
         whitelist=whitelist,
+        extra_module_dirs=utils.module_dirs if utils else None,
     )
 
 
@@ -1016,6 +1028,7 @@ def clouds(opts):
     '''
     Return the cloud functions
     '''
+    _utils = salt.loader.utils(opts)
     # Let's bring __active_provider_name__, defaulting to None, to all cloud
     # drivers. This will get temporarily updated/overridden with a context
     # manager when needed.
@@ -1027,8 +1040,9 @@ def clouds(opts):
                      int_type='clouds'),
         opts,
         tag='clouds',
-        pack={'__utils__': salt.loader.utils(opts),
+        pack={'__utils__': _utils,
               '__active_provider_name__': None},
+        extra_module_dirs=_utils.module_dirs,
     )
     for funcname in LIBCLOUD_FUNCS_NOT_SUPPORTED:
         log.trace(
@@ -1211,6 +1225,7 @@ class LazyLoader(salt.utils.lazy.LazyDict):
     :param bool virtual_enable: Whether or not to respect the __virtual__ function when loading modules.
     :param str virtual_funcs: The name of additional functions in the module to call to verify its functionality.
                                 If not true, the module will not load.
+    :param list extra_module_dirs: A list of directories that will be able to import from
     :returns: A LazyLoader object which functions as a dictionary. Keys are 'module.function' and values
     are function references themselves which are loaded on-demand.
     # TODO:
@@ -1232,6 +1247,7 @@ def __init__(self,
                  static_modules=None,
                  proxy=None,
                  virtual_funcs=None,
+                 extra_module_dirs=None,
                  ):  # pylint: disable=W0231
         '''
         In pack, if any of the values are None they will be replaced with an
@@ -1278,6 +1294,9 @@ def __init__(self,
             virtual_funcs = []
         self.virtual_funcs = virtual_funcs
 
+        self.extra_module_dirs = extra_module_dirs if extra_module_dirs else []
+        self._clean_module_dirs = []
+
         self.disabled = set(
             self.opts.get(
                 'disable_{0}{1}'.format(
@@ -1594,12 +1613,30 @@ def _reload_submodules(self, mod):
                 reload_module(submodule)
                 self._reload_submodules(submodule)
 
+    def __populate_sys_path(self):
+        for directory in self.extra_module_dirs:
+            if directory not in sys.path:
+                sys.path.append(directory)
+                self._clean_module_dirs.append(directory)
+
+    def __clean_sys_path(self):
+        for directory in self._clean_module_dirs:
+            if directory in sys.path:
+                sys.path.remove(directory)
+        self._clean_module_dirs = []
+
+        # Be sure that sys.path_importer_cache do not contains any
+        # invalid FileFinder references
+        if USE_IMPORTLIB:
+            importlib.invalidate_caches()
+
     def _load_module(self, name):
         mod = None
         fpath, suffix = self.file_mapping[name][:2]
         self.loaded_files.add(name)
         fpath_dirname = os.path.dirname(fpath)
         try:
+            self.__populate_sys_path()
             sys.path.append(fpath_dirname)
             if suffix == '.pyx':
                 mod = pyximport.load_module(name, fpath, tempfile.gettempdir())
@@ -1722,6 +1759,7 @@ def _load_module(self, name):
             return False
         finally:
             sys.path.remove(fpath_dirname)
+            self.__clean_sys_path()
 
         if hasattr(mod, '__opts__'):
             mod.__opts__.update(self.opts)

salt/states/file.py

@@ -291,7 +291,11 @@ def run():
 import sys
 import time
 import traceback
-from collections import Iterable, Mapping, defaultdict
+try:
+    from collections.abc import Iterable, Mapping
+except ImportError:
+    from collections import Iterable, Mapping
+from collections import defaultdict
 from datetime import datetime, date   # python3 problem in the making?
 
 # Import salt libs

salt/utils/extmods.py

@@ -8,7 +8,6 @@
 import logging
 import os
 import shutil
-import sys
 
 # Import salt libs
 import salt.fileclient
@@ -132,12 +131,6 @@ def sync(opts,
                         shutil.copyfile(fn_, dest)
                         ret.append('{0}.{1}'.format(form, relname))
 
-                    # If the synchronized module is an utils
-                    # directory, we add it to sys.path
-                    for util_dir in opts['utils_dirs']:
-                        if mod_dir.endswith(util_dir) and mod_dir not in sys.path:
-                            sys.path.append(mod_dir)
-
             touched = bool(ret)
             if opts['clean_dynamic_modules'] is True:
                 current = set(_listdir_recursively(mod_dir))

salt/utils/oset.py

@@ -22,7 +22,10 @@
     - added __getitem__
 '''
 from __future__ import absolute_import, unicode_literals, print_function
-import collections
+try:
+    from collections.abc import MutableSet
+except ImportError:
+    from collections import MutableSet
 
 SLICE_ALL = slice(None)
 __version__ = '2.0.1'
@@ -44,7 +47,7 @@ def is_iterable(obj):
     return hasattr(obj, '__iter__') and not isinstance(obj, str) and not isinstance(obj, tuple)
 
 
-class OrderedSet(collections.MutableSet):
+class OrderedSet(MutableSet):
     """
     An OrderedSet is a custom MutableSet that remembers its order, so that
     every entry has an index that can be looked up.

tests/unit/test_loader.py

@@ -119,6 +119,97 @@ def test_depends(self):
         self.assertTrue(self.module_name + '.not_loaded' not in self.loader)
 
 
+loader_template_module = '''
+import my_utils
+
+def run():
+    return my_utils.run()
+'''
+
+loader_template_utils = '''
+def run():
+    return True
+'''
+
+
+class LazyLoaderUtilsTest(TestCase):
+    '''
+    Test the loader
+    '''
+    module_name = 'lazyloaderutilstest'
+    utils_name = 'my_utils'
+
+    @classmethod
+    def setUpClass(cls):
+        cls.opts = salt.config.minion_config(None)
+        cls.opts['grains'] = salt.loader.grains(cls.opts)
+        if not os.path.isdir(RUNTIME_VARS.TMP):
+            os.makedirs(RUNTIME_VARS.TMP)
+
+    def setUp(self):
+        # Setup the module
+        self.module_dir = tempfile.mkdtemp(dir=RUNTIME_VARS.TMP)
+        self.module_file = os.path.join(self.module_dir,
+                                        '{}.py'.format(self.module_name))
+        with salt.utils.files.fopen(self.module_file, 'w') as fh:
+            fh.write(salt.utils.stringutils.to_str(loader_template_module))
+            fh.flush()
+            os.fsync(fh.fileno())
+
+        self.utils_dir = tempfile.mkdtemp(dir=RUNTIME_VARS.TMP)
+        self.utils_file = os.path.join(self.utils_dir,
+                                       '{}.py'.format(self.utils_name))
+        with salt.utils.files.fopen(self.utils_file, 'w') as fh:
+            fh.write(salt.utils.stringutils.to_str(loader_template_utils))
+            fh.flush()
+            os.fsync(fh.fileno())
+
+    def tearDown(self):
+        shutil.rmtree(self.module_dir)
+        if os.path.isdir(self.module_dir):
+            shutil.rmtree(self.module_dir)
+        shutil.rmtree(self.utils_dir)
+        if os.path.isdir(self.utils_dir):
+            shutil.rmtree(self.utils_dir)
+        del self.module_dir
+        del self.module_file
+        del self.utils_dir
+        del self.utils_file
+
+        if self.module_name in sys.modules:
+            del sys.modules[self.module_name]
+        if self.utils_name in sys.modules:
+            del sys.modules[self.utils_name]
+
+    @classmethod
+    def tearDownClass(cls):
+        del cls.opts
+
+    def test_utils_found(self):
+        '''
+        Test that the extra module directory is available for imports
+        '''
+        loader = salt.loader.LazyLoader(
+            [self.module_dir],
+            copy.deepcopy(self.opts),
+            tag='module',
+            extra_module_dirs=[self.utils_dir])
+        self.assertTrue(
+            inspect.isfunction(
+                loader[self.module_name + '.run']))
+        self.assertTrue(loader[self.module_name + '.run']())
+
+    def test_utils_not_found(self):
+        '''
+        Test that the extra module directory is not available for imports
+        '''
+        loader = salt.loader.LazyLoader(
+            [self.module_dir],
+            copy.deepcopy(self.opts),
+            tag='module')
+        self.assertTrue(self.module_name + '.run' not in loader)
+
+
 class LazyLoaderVirtualEnabledTest(TestCase):
     '''
     Test the base loader of salt.
@@ -1021,8 +1112,9 @@ def _verify_globals(self, mod_dict):
 
         # Now, test each module!
         for item in six.itervalues(global_vars):
-            for name in names:
-                self.assertIn(name, list(item.keys()))
+            if item['__name__'].startswith('salt.loaded'):
+                for name in names:
+                    self.assertIn(name, list(item.keys()))
 
     def test_auth(self):
         '''