[master] Fix 64433: Add dynamic loading of file_roots, pillar_roots, and thorium_roots

changelog/64433.added.md

@@ -0,0 +1 @@
+Added dynamic loading of file_roots, pillar_roots, and thorium_roots to salt config

doc/ref/configuration/master.rst

@@ -3194,10 +3194,40 @@ is equivalent to this static configuration:
       prod:
         - /srv/prod/salt
 
+As of 3008.0, the ``file_roots`` option is dynamically expanded on each use,
+meaning directories or files may be added without restarting the Salt master.
+For instance, this configuration:
+
+.. code-block:: yaml
+
+    file_roots:
+      base:
+        - /srv/salt/*-formula
+
+may be used to dynamically add new formula directories to the ``file_roots``
+without restarting the master, as long as their name matches ``*-formula``.
+
 .. note::
     For masterless Salt, this parameter must be specified in the minion config
     file.
 
+.. conf_master:: dynamic_roots_ttl
+
+``dynamic_roots_ttl``
+**********************
+
+.. versionadded:: 3008.0
+
+Default: ``5.0``
+
+The number of seconds a dynamically-expanded (globbed) entry in
+:conf_master:`file_roots`, :conf_master:`pillar_roots`, or ``thorium_roots``
+is cached before the directory is re-scanned. Since globbed roots are
+re-evaluated on every access, environments with many saltenvs or a busy
+master accessing these options repeatedly (e.g. during a highstate) can
+incur significant, avoidable disk I/O without this cache. Set to ``0`` to
+disable caching and always re-scan on every access.
+
 .. conf_master:: roots_update_interval
 
 ``roots_update_interval``
@@ -4341,6 +4371,19 @@ is equivalent to this static configuration:
       prod:
         - /srv/prod/pillar
 
+As of 3008.0, the ``pillar_roots`` option is dynamically expanded on each use,
+meaning directories or files may be added without restarting the Salt master.
+For instance, this configuration:
+
+.. code-block:: yaml
+
+    pillar_roots:
+      base:
+        - /srv/salt/*-pillar
+
+may be used to dynamically add new pillar directories to the ``pillar_roots``
+without restarting the master, as long as their name matches ``*-pillar``.
+
 .. conf_master:: on_demand_ext_pillar
 
 ``on_demand_ext_pillar``

salt/config/__init__.py

@@ -20,6 +20,7 @@
 import salt.syspaths
 import salt.utils.data
 import salt.utils.dictupdate
+import salt.utils.dynamic_dict
 import salt.utils.files
 import salt.utils.immutabletypes as immutabletypes
 import salt.utils.network
@@ -330,6 +331,9 @@ def _gather_buffer_space():
         "file_roots": dict,
         # A map of saltenvs and fileserver backend locations
         "pillar_roots": dict,
+        # Number of seconds to cache the dynamically-expanded (globbed) entries
+        # of file_roots, pillar_roots, and thorium_roots before re-scanning disk
+        "dynamic_roots_ttl": (float, int),
         # The external pillars permitted to be used on-demand using pillar.ext
         "on_demand_ext_pillar": list,
         # A map of glob paths to be used
@@ -1234,6 +1238,7 @@ def _gather_buffer_space():
         "file_roots": {
             "base": [salt.syspaths.BASE_FILE_ROOTS_DIR, salt.syspaths.SPM_FORMULA_PATH]
         },
+        "dynamic_roots_ttl": 5.0,
         "top_file_merging_strategy": "merge",
         "env_order": [],
         "default_top": "base",
@@ -1580,6 +1585,7 @@ def _gather_buffer_space():
         "file_roots": {
             "base": [salt.syspaths.BASE_FILE_ROOTS_DIR, salt.syspaths.SPM_FORMULA_PATH]
         },
+        "dynamic_roots_ttl": 5.0,
         "master_roots": {"base": [salt.syspaths.BASE_MASTER_ROOTS_DIR]},
         "pillar_roots": {
             "base": [salt.syspaths.BASE_PILLAR_ROOTS_DIR, salt.syspaths.SPM_PILLAR_PATH]
@@ -2139,55 +2145,45 @@ def _gather_buffer_space():
 # <---- Salt Cloud Configuration Defaults ------------------------------------
 
 
-def _normalize_roots(file_roots):
+def _validate_roots(opts, prop_name):
     """
-    Normalize file or pillar roots.
+    If the roots option has a key that is None then we will log a warning and
+    use the defaults instead.
     """
-    for saltenv, dirs in file_roots.items():
-        normalized_saltenv = str(saltenv)
-        if normalized_saltenv != saltenv:
-            file_roots[normalized_saltenv] = file_roots.pop(saltenv)
-        if not isinstance(dirs, (list, tuple)):
-            file_roots[normalized_saltenv] = []
-        file_roots[normalized_saltenv] = _expand_glob_path(
-            file_roots[normalized_saltenv]
-        )
-    return file_roots
-
-
-def _validate_pillar_roots(pillar_roots):
-    """
-    If the pillar_roots option has a key that is None then we will error out,
-    just replace it with an empty list
-    """
-    if not isinstance(pillar_roots, dict):
+    roots = opts.get(prop_name)
+    if not isinstance(roots, dict):
         log.warning(
-            "The pillar_roots parameter is not properly formatted, using defaults"
+            "The %s parameter is not properly formatted, using defaults",
+            prop_name,
         )
-        return {"base": _expand_glob_path([salt.syspaths.BASE_PILLAR_ROOTS_DIR])}
-    return _normalize_roots(pillar_roots)
+        return {"base": _expand_glob_path(DEFAULT_MASTER_OPTS.get(prop_name)["base"])}
+    # Use a dynamic dict to resolve file roots dynamically
+    result = salt.utils.dynamic_dict.DynamicDict(
+        ttl=opts.get("dynamic_roots_ttl", DEFAULT_MASTER_OPTS["dynamic_roots_ttl"])
+    )
+    for saltenv, dirs in roots.items():
+        normalized_saltenv = str(saltenv)
+        if not isinstance(dirs, (list, tuple)):
+            dirs = []
+        result.add_dyn(normalized_saltenv, _expand_glob_path, dirs)
+    return result
 
 
-def _validate_file_roots(file_roots):
+def _expand_glob_path(dirs, dyn_dict=None, key=None):
     """
-    If the file_roots option has a key that is None then we will error out,
-    just replace it with an empty list
+    A dynamic dict function that applies shell globbing to a set of
+    directories and returns the expanded paths
     """
-    if not isinstance(file_roots, dict):
-        log.warning(
-            "The file_roots parameter is not properly formatted, using defaults"
-        )
-        return {"base": _expand_glob_path([salt.syspaths.BASE_FILE_ROOTS_DIR])}
-    return _normalize_roots(file_roots)
+    # Unused arguments from dynamic dict
+    _ = dyn_dict
+    _ = key
+    # At times, this method is called with non-iterable objects,
+    # so just return them directly
+    if not isinstance(dirs, (list, tuple)):
+        return dirs
 
-
-def _expand_glob_path(file_roots):
-    """
-    Applies shell globbing to a set of directories and returns
-    the expanded paths
-    """
     unglobbed_path = []
-    for path in file_roots:
+    for path in dirs:
         try:
             if glob.has_magic(path):
                 unglobbed_path.extend(glob.glob(path))
@@ -2943,12 +2939,16 @@ def apply_sdb(opts, sdb_opts=None, _visited=None):
 
         return salt.utils.sdb.sdb_get(sdb_opts, opts)
     elif isinstance(sdb_opts, dict):
+        is_dyn_dict = isinstance(sdb_opts, salt.utils.dynamic_dict.DynamicDict)
         # Create a list of items to avoid modifying dict during iteration
         # This is especially important for OptsDict which has special iteration behavior
         items = list(sdb_opts.items())
         for key, value in items:
             if value is None:
                 continue
+            if is_dyn_dict and sdb_opts.is_dyn_key(key):
+                # Nothing to do for dynamically generated elements
+                continue
             sdb_opts[key] = apply_sdb(opts, value, _visited)
     elif isinstance(sdb_opts, list):
         for key, value in enumerate(sdb_opts):
@@ -4225,8 +4225,8 @@ def apply_minion_config(
     # Enabling open mode requires that the value be set to True, and
     # nothing else!
     opts["open_mode"] = opts["open_mode"] is True
-    opts["file_roots"] = _validate_file_roots(opts["file_roots"])
-    opts["pillar_roots"] = _validate_pillar_roots(opts["pillar_roots"])
+    opts["file_roots"] = _validate_roots(opts, "file_roots")
+    opts["pillar_roots"] = _validate_roots(opts, "pillar_roots")
     # Make sure ext_mods gets set if it is an untrue value
     # (here to catch older bad configs)
     opts["extension_modules"] = opts.get("extension_modules") or os.path.join(
@@ -4540,8 +4540,8 @@ def apply_master_config(overrides=None, defaults=None):
     # nothing else!
     opts["open_mode"] = opts["open_mode"] is True
     opts["auto_accept"] = opts["auto_accept"] is True
-    opts["file_roots"] = _validate_file_roots(opts["file_roots"])
-    opts["pillar_roots"] = _validate_file_roots(opts["pillar_roots"])
+    opts["file_roots"] = _validate_roots(opts, "file_roots")
+    opts["pillar_roots"] = _validate_roots(opts, "pillar_roots")
 
     if opts["file_ignore_regex"]:
         # If file_ignore_regex was given, make sure it's wrapped in a list.

salt/thorium/__init__.py

@@ -19,6 +19,7 @@
 import salt.loader
 import salt.payload
 import salt.state
+from salt.config import _validate_roots
 from salt.exceptions import SaltRenderError
 
 log = logging.getLogger(__name__)
@@ -36,7 +37,7 @@ def __init__(
         self.grain_keys = grain_keys
         self.pillar = pillar
         self.pillar_keys = pillar_keys
-        opts["file_roots"] = opts["thorium_roots"]
+        opts["file_roots"] = _validate_roots(opts, "thorium_roots")
         opts["saltenv"] = opts["thoriumenv"]
         opts["state_top"] = opts["thorium_top"]
         opts["file_client"] = "local"

salt/utils/dynamic_dict.py

@@ -0,0 +1,111 @@
+"""
+A dictionary with optionally dynamic values, used for dynamic configuration such
+as file roots.
+"""
+
+import copy
+import time
+
+__all__ = ["DynamicDict"]
+
+#: Default number of seconds a dynamic value is cached before being
+#: re-evaluated. Callers (e.g. ``salt.config``) may override this per
+#: instance via the ``ttl`` argument.
+DEFAULT_TTL = 5.0
+
+
+class DynamicDict(dict):
+    """
+    A dictionary that can mix static and dynamic values.
+    """
+
+    def __init__(self, *args, ttl=DEFAULT_TTL, **argv):
+        self._func_dict = {}
+        self._cache = {}
+        self._ttl = ttl
+        super().__init__(*args, **argv)
+
+    def __getitem__(self, key):
+        val = super().__getitem__(key)
+        if key in self._func_dict:
+            now = time.time()
+            cached = self._cache.get(key)
+            if self._ttl and cached is not None and (now - cached[1]) < self._ttl:
+                return cached[0]
+            val = self._func_dict[key](val, dyn_dict=self, key=key)
+            self._cache[key] = (val, now)
+        return val
+
+    def __delitem__(self, key):
+        if key in self._func_dict:
+            del self._func_dict[key]
+        self._cache.pop(key, None)
+        super().__delitem__(key)
+
+    def get(self, key, default=None):
+        if key not in self:
+            return default
+        return self[key]
+
+    def pop(self, key, default=None):
+        if key in self:
+            val = self[key]
+            del self[key]
+        else:
+            val = default
+        return val
+
+    def values(self):
+        keys = super().keys()
+        for key in keys:
+            yield self[key]
+
+    def items(self):
+        keys = super().keys()
+        for key in keys:
+            yield key, self[key]
+
+    def copy(self):
+        new_dd = DynamicDict(ttl=self._ttl)
+        for key, val in super().items():
+            if key in self._func_dict:
+                func = self._func_dict[key]
+                data = super().__getitem__(key)
+                new_dd.add_dyn(key, func, data)
+            else:
+                new_dd[key] = val
+        return new_dd
+
+    def __copy__(self):
+        return self.copy()
+
+    def __deepcopy__(self, memo):
+        rdd = DynamicDict(ttl=self._ttl)
+        memo[id(self)] = rdd
+        for key in super().keys():
+            if key in self._func_dict:
+                func = self._func_dict[key]
+                data = copy.deepcopy(super().__getitem__(key), memo)
+                rdd.add_dyn(key, func, data)
+            else:
+                copied_key = copy.deepcopy(key, memo)
+                copied_value = copy.deepcopy(super().__getitem__(key), memo)
+                rdd[copied_key] = copied_value
+        return rdd
+
+    def static_dict(self):
+        new_dict = {}
+        for key in super().keys():
+            new_dict[key] = self[key]
+        return new_dict
+
+    def is_dyn_key(self, key):
+        return key in self._func_dict
+
+    def add_dyn(self, key, func, data=None):
+        if not hasattr(func, "__call__"):
+            raise ValueError(f"Value for key '{key}' is not a function")
+        self._func_dict[key] = func
+        self._cache.pop(key, None)
+        if data is not None or key not in self:
+            self[key] = data

salt/utils/yamldumper.py

@@ -15,6 +15,7 @@
 
 import salt.utils.context
 from salt.utils.datastructures import HashableOrderedDict
+from salt.utils.dynamic_dict import DynamicDict
 from salt.utils.optsdict import DictProxy, ListProxy, OptsDict
 from salt.utils.secret import MaskedDict, MaskedList
 
@@ -162,6 +163,9 @@ def represent_listproxy(dumper, data):
     "tag:yaml.org,2002:timestamp", SafeOrderedDumper.represent_scalar
 )
 
+OrderedDumper.add_representer(DynamicDict, represent_ordereddict)
+SafeOrderedDumper.add_representer(DynamicDict, represent_ordereddict)
+
 
 def get_dumper(dumper_name):
     return {

tests/filename_map.yml

@@ -177,6 +177,9 @@ salt/state.py:
 salt/utils/decorators/*:
   - integration.modules.test_decorators
 
+salt/utils/dynamic_dict.py:
+  - pytests.unit.utils.test_dynamic_dict
+
 salt/(utils|renderers)/jinja\.py:
   - pytests.functional.modules.state.test_jinja_filters
   - integration.states.test_renderers
@@ -221,6 +224,7 @@ salt/client/ssh/wrapper/*:
 salt/config/*:
   - unit.test_config
   - pytests.unit.config.test__validate_opts
+  - pytests.integration.master.test_dynamic_config
 
 salt/loader/*:
   - integration.loader.test_ext_modules

tests/pytests/functional/conftest.py

@@ -140,7 +140,10 @@ def master_opts(
 
 @pytest.fixture(scope="module")
 def loaders(minion_opts):
-    return Loaders(minion_opts, loaded_base_name=f"{__name__}.loaded")
+    result = Loaders(minion_opts, loaded_base_name=f"{__name__}.loaded")
+    # Convert the file roots to a static dict before appending any roots
+    result.opts["file_roots"] = result.opts["file_roots"].static_dict()
+    return result
 
 
 @pytest.fixture(autouse=True)