Stop loading utils modules into __utils__ if they don't define `__v…

…irtual__`

Refs saltstack#62191

Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>

salt/loader/__init__.py

@@ -146,7 +146,7 @@ def _module_dirs(
     ext_type_types = []
     if ext_dirs:
         if ext_type_dirs is None:
-            ext_type_dirs = "{}_dirs".format(tag)
+            ext_type_dirs = f"{tag}_dirs"
         if ext_type_dirs in opts:
             ext_type_types.extend(opts[ext_type_dirs])
         if ext_type_dirs and load_extensions is True:
@@ -246,7 +246,7 @@ def _module_dirs(
             cli_module_dirs.insert(0, maybe_dir)
             continue
 
-        maybe_dir = os.path.join(_dir, "_{}".format(ext_type))
+        maybe_dir = os.path.join(_dir, f"_{ext_type}")
         if os.path.isdir(maybe_dir):
             cli_module_dirs.insert(0, maybe_dir)
 
@@ -539,6 +539,7 @@ def utils(
         pack={"__context__": context, "__proxy__": proxy or {}},
         pack_self=pack_self,
         loaded_base_name=loaded_base_name,
+        _ast_dunder_virtual_inspect=True,
         _only_pack_properly_namespaced_functions=False,
     )
 
@@ -1208,7 +1209,7 @@ def grains(opts, force_refresh=False, proxy=None, context=None, loaded_base_name
                     import salt.modules.cmdmod
 
                     # Make sure cache file isn't read-only
-                    salt.modules.cmdmod._run_quiet('attrib -R "{}"'.format(cfn))
+                    salt.modules.cmdmod._run_quiet(f'attrib -R "{cfn}"')
                 with salt.utils.files.fopen(cfn, "w+b") as fp_:
                     try:
                         salt.payload.dump(grains_data, fp_)

salt/loader/lazy.py

@@ -1,3 +1,4 @@
+import ast
 import copy
 import functools
 import importlib
@@ -231,7 +232,8 @@ def __init__(
         virtual_funcs=None,
         extra_module_dirs=None,
         pack_self=None,
-        # Once we get rid of __utils__, the keyword argument bellow should be removed
+        # Once we get rid of __utils__, the keyword arguments bellow should be removed
+        _ast_dunder_virtual_inspect=False,
         _only_pack_properly_namespaced_functions=True,
     ):  # pylint: disable=W0231
         """
@@ -262,6 +264,7 @@ def __init__(
         self._gc_finalizer = None
         self.loaded_base_name = loaded_base_name or LOADED_BASE_NAME
         self.mod_type_check = mod_type_check or _mod_type
+        self._ast_dunder_virtual_inspect = _ast_dunder_virtual_inspect
         self._only_pack_properly_namespaced_functions = (
             _only_pack_properly_namespaced_functions
         )
@@ -652,6 +655,24 @@ def __clean_sys_path(self):
     def _load_module(self, name):
         mod = None
         fpath, suffix = self.file_mapping[name][:2]
+        if suffix == ".py" and self._ast_dunder_virtual_inspect:
+            with salt.utils.files.fopen(fpath) as rfh:
+                tree = ast.parse(rfh.read())
+                for node in tree.body:
+                    if not isinstance(node, ast.FunctionDef):
+                        continue
+                    if node.name == "__virtual__":
+                        # The module defines a __virtual__ function.
+                        # Continue regular loading.
+                        break
+                else:
+                    # The module did not define a __virtual__ function, stop
+                    # processing the module
+                    log.debug(
+                        "Not loading %r because it does not define a __virtual__ function",
+                        name,
+                    )
+                    return False
         # if the fpath has `.cpython-3x` in it, but the running Py version
         # is 3.y, the following will cause us to return immediately and we won't try to import this .pyc.
         # This is for the unusual case where several Python versions share a single

tests/pytests/functional/loader/test_utils.py

@@ -0,0 +1,48 @@
+import warnings
+
+import pytest
+from saltfactories.utils import random_string
+
+import salt.config
+import salt.loader
+from salt.loader.lazy import LazyLoader
+
+
+@pytest.fixture(scope="module")
+def loaded_base_name():
+    return random_string(f"{__name__}.", digits=False, uppercase=False)
+
+
+@pytest.fixture
+def loader(minion_opts, loaded_base_name):
+    loader = salt.loader.utils(minion_opts, loaded_base_name=loaded_base_name)
+    try:
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore")
+            # Force loading all functions
+            list(loader)
+            yield loader
+    finally:
+        if not isinstance(loader, LazyLoader):
+            for loaded_func in loader.values():
+                loader = loaded_func.loader
+                break
+        if isinstance(loader, LazyLoader):
+            loader.clean_modules()
+
+
+def test_ast_inspect_loading(loader):
+    loaded_functions = list(loader)
+    # Check that utils modules defining __virtual__ are loaded by the loader
+    # Of course, we can only check modules which load in most/all circumstances.
+    assert "ansible.targets" in loaded_functions
+    # However, modules which do not define a __virtual__ function should not load,
+    # at all!
+    modules_which_do_not_define_dunder_virtual = (
+        "args.",
+        "environment.",
+        "entrypoints.",
+        "process.",
+    )
+    for loaded_func in loaded_functions:
+        assert not loaded_func.startswith(modules_which_do_not_define_dunder_virtual)