Rebuild the Salt-SSH thin archive when its configured contents change

changelog/70287.fixed.md

@@ -0,0 +1 @@
+Rebuild a cached Salt-SSH thin archive when its configured contents change, so that installed Salt extensions and changes to `thin_extra_mods` take effect without `--regen-thin`. `thin_extra_mods` can now also name a module inside a namespace package, such as a Salt extension.

doc/ref/configuration/master.rst

@@ -1711,10 +1711,15 @@ exists first.
 
 Default: None
 
-List of additional modules, needed to be included into the Salt Thin.
-Pass a list of importable Python modules that are typically located in
-the `site-packages` Python directory so they will be also always included
-into the Salt Thin, once generated.
+Comma-separated list of additional modules, needed to be included into the
+Salt Thin. Pass importable Python modules that are typically located in the
+`site-packages` Python directory so they will be also always included into
+the Salt Thin, once generated. A module inside a namespace package, such as
+a Salt extension, is named by its dotted path.
+
+.. code-block:: yaml
+
+    thin_extra_mods: pymysql,saltext.mysql
 
 ``min_extra_mods``
 ------------------
@@ -1723,6 +1728,54 @@ Default: None
 
 Identical as `thin_extra_mods`, only applied to the Salt Minimal.
 
+.. conf_master:: thin_exclude_saltexts
+
+``thin_exclude_saltexts``
+-------------------------
+
+Default: ``False``
+
+Salt extensions installed on the master are included into the Salt Thin, so
+that their modules are available on salt-ssh targets without installing them
+there. Set this to ``True`` to leave them out.
+
+Only the extensions themselves are included, not the packages they depend on.
+Add the pure Python ones to :conf_master:`thin_extra_mods`, since a dependency
+missing on the target keeps its modules from loading.
+
+.. code-block:: yaml
+
+    thin_exclude_saltexts: False
+
+.. conf_master:: thin_saltext_allowlist
+
+``thin_saltext_allowlist``
+--------------------------
+
+Default: ``None``
+
+Distribution names of the only Salt extensions to include into the Salt Thin.
+When unset, every installed extension is included.
+
+.. code-block:: yaml
+
+    thin_saltext_allowlist:
+      - saltext.mysql
+
+.. conf_master:: thin_saltext_blocklist
+
+``thin_saltext_blocklist``
+--------------------------
+
+Default: ``[]``
+
+Distribution names of Salt extensions to exclude from the Salt Thin.
+
+.. code-block:: yaml
+
+    thin_saltext_blocklist:
+      - saltext.mysql
+
 
 .. _master-security-settings:
 

salt/client/ssh/__init__.py

@@ -1962,7 +1962,15 @@ def _cmd_str(self):
             )
             return shim.replace("__SALT_MINION_CONFIG__", self.minion_config)
 
-        thin_code_digest, thin_sum = salt.utils.thin.thin_sum(cachedir, "sha1")
+        thin_code_digest, thin_sum = salt.utils.thin.thin_sum(
+            cachedir,
+            "sha1",
+            extra_mods=self.opts.get("thin_extra_mods") or "",
+            extended_cfg=self.opts.get("ssh_ext_alternatives"),
+            exclude_saltexts=self.opts.get("thin_exclude_saltexts", False),
+            saltext_allowlist=self.opts.get("thin_saltext_allowlist"),
+            saltext_blocklist=self.opts.get("thin_saltext_blocklist"),
+        )
         arg_str = '''
 OPTIONS.config = \
 """

salt/utils/thin.py

@@ -482,16 +482,22 @@ def get_tops(extra_mods="", so_mods=""):
     for mod in [m for m in extra_mods.split(",") if m]:
         if mod not in locals() and mod not in globals():
             try:
-                moddir, modname = os.path.split(__import__(mod).__file__)
-                base, _ = os.path.splitext(modname)
-                if base == "__init__":
-                    tops.append((moddir, None))
-                else:
-                    tops.append((os.path.join(moddir, base + ".py"), None))
+                # __import__ would return the top-level package, which for a saltext
+                # is a namespace package, whose __file__ is None.
+                imported_mod = importlib.import_module(mod)
             except ImportError as err:
                 log.error(
                     'Unable to import extra-module "%s": %s', mod, err, exc_info=True
                 )
+                continue
+            # The directory to pack is the first parent that is not a namespace package.
+            root_mod, namespace = _get_package_root_mod(imported_mod)
+            moddir, modname = os.path.split(root_mod.__file__)
+            base, _ = os.path.splitext(modname)
+            if base == "__init__":
+                tops.append((moddir, namespace))
+            else:
+                tops.append((os.path.join(moddir, base + ".py"), None))
 
     for mod in [m for m in so_mods.split(",") if m]:
         try:
@@ -654,6 +660,44 @@ def _get_package_root_mod(mod):
     return sys.modules[parts[0]], ()
 
 
+def _saltext_dists(allowlist=None, blocklist=None):
+    """Return the Salt extensions that are packed into the thin archive, without importing them."""
+    blocklist = blocklist or []
+    dists = set()
+    for entry_point in salt.utils.entrypoints.iter_entry_points("salt.loader"):
+        dist = getattr(entry_point, "dist", None)
+        if dist is None:
+            continue
+        if allowlist is not None and dist.name not in allowlist:
+            continue
+        if dist.name in blocklist:
+            continue
+        dists.add(f"{dist.name}=={dist.version}")
+    return sorted(dists)
+
+
+def _thin_contents_id(
+    extra_mods="",
+    so_mods="",
+    exclude_saltexts=False,
+    saltext_allowlist=None,
+    saltext_blocklist=None,
+):
+    """Return an identity of the configured archive contents, to detect an outdated cached archive."""
+    saltexts = (
+        []
+        if exclude_saltexts
+        else _saltext_dists(allowlist=saltext_allowlist, blocklist=saltext_blocklist)
+    )
+    return "\n".join(
+        [
+            f"extra_mods={extra_mods}",
+            f"so_mods={so_mods}",
+            "saltexts=" + ",".join(saltexts),
+        ]
+    )
+
+
 def _discover_saltexts(allowlist=None, blocklist=None):
     mods = []
     loaded_saltexts = {}
@@ -783,6 +827,14 @@ def gen_thin(
     thintar = os.path.join(thindir, "thin." + (compress == "gzip" and "tgz" or "zip"))
     thinver = os.path.join(thindir, "version")
     pythinver = os.path.join(thindir, ".thin-gen-py-version")
+    thincfg = os.path.join(thindir, ".thin-gen-config")
+    contents_id = _thin_contents_id(
+        extra_mods=extra_mods,
+        so_mods=so_mods,
+        exclude_saltexts=exclude_saltexts,
+        saltext_allowlist=saltext_allowlist,
+        saltext_blocklist=saltext_blocklist,
+    )
     salt_call = os.path.join(thindir, "salt-call")
     pymap_cfg = os.path.join(thindir, "supported-versions")
     code_checksum = os.path.join(thindir, "code-checksum")
@@ -799,6 +851,13 @@ def gen_thin(
                 if overwrite is False and os.path.isfile(pythinver):
                     with salt.utils.files.fopen(pythinver) as fh_:
                         overwrite = fh_.read() != str(sys.version_info[0])
+                if overwrite is False:
+                    if os.path.isfile(thincfg):
+                        with salt.utils.files.fopen(thincfg) as fh_:
+                            overwrite = fh_.read() != contents_id
+                    else:
+                        # An archive without this file was generated by an older Salt.
+                        overwrite = True
             else:
                 overwrite = True
 
@@ -921,6 +980,8 @@ def gen_thin(
         fp_.write(salt.version.__version__)
     with salt.utils.files.fopen(pythinver, "w+") as fp_:
         fp_.write(str(sys.version_info.major))
+    with salt.utils.files.fopen(thincfg, "w+") as fp_:
+        fp_.write(contents_id)
     with salt.utils.files.fopen(code_checksum, "w+") as fp_:
         fp_.write(digest_collector.digest())
     os.chdir(os.path.dirname(thinver))
@@ -943,11 +1004,14 @@ def gen_thin(
     return thintar
 
 
-def thin_sum(cachedir, form="sha1"):
+def thin_sum(cachedir, form="sha1", **kwargs):
     """
     Return the checksum of the current thin tarball
+
+    Keyword arguments are passed to :func:`gen_thin`, which must receive the same
+    arguments as when the archive was generated, otherwise it is regenerated.
     """
-    thintar = gen_thin(cachedir)
+    thintar = gen_thin(cachedir, **kwargs)
     code_checksum_path = os.path.join(cachedir, "thin", "code-checksum")
     if os.path.isfile(code_checksum_path):
         with salt.utils.files.fopen(code_checksum_path, "r") as fh:

tests/pytests/unit/utils/test_thin.py

@@ -636,12 +636,14 @@ def test_get_tops_extra_mods(thin_ctx):
     if salt.utils.thin.has_immutables:
         base_tops.extend(["immutables"])
     libs = salt.utils.thin.find_site_modules("contextvars")
-    foo = {"__file__": os.sep + os.path.join("custom", "foo", "__init__.py")}
-    bar = {"__file__": os.sep + os.path.join("custom", "bar")}
+    foo = type(
+        "foo", (), {"__file__": os.sep + os.path.join("custom", "foo", "__init__.py")}
+    )
+    bar = type("bar", (), {"__file__": os.sep + os.path.join("custom", "bar")})
     with patch("salt.utils.thin.find_site_modules", MagicMock(side_effect=[libs])):
-        with patch(
-            "builtins.__import__",
-            MagicMock(side_effect=[type("foo", (), foo), type("bar", (), bar)]),
+        with patch.dict(sys.modules, {"foo": foo, "bar": bar}), patch(
+            "salt.utils.thin.importlib.import_module",
+            MagicMock(side_effect=[foo, bar]),
         ):
             tops = []
             for top, namespace in thin.get_tops(extra_mods="foo,bar"):
@@ -1521,3 +1523,79 @@ def test_thin_dir(thin_ctx):
                 check=False,
             )
         assert ret.returncode == 0, ret
+
+
+def test_get_tops_extra_mods_namespace_package():
+    """Test thin.get_tops packs the correct directory and namespace for a module in a namespace package."""
+    saltext = type("saltext", (), {"__file__": None})
+    saltext_foo = type(
+        "saltext.foo",
+        (),
+        {"__file__": os.sep + os.path.join("custom", "saltext", "foo", "__init__.py")},
+    )
+    with patch.dict(
+        sys.modules, {"saltext": saltext, "saltext.foo": saltext_foo}
+    ), patch(
+        "salt.utils.thin.importlib.import_module", MagicMock(return_value=saltext_foo)
+    ):
+        tops = salt.utils.thin.get_tops(extra_mods="saltext.foo")
+    assert (os.sep + os.path.join("custom", "saltext", "foo"), ("saltext",)) in tops
+
+
+def test_gen_thin_regenerates_when_saltexts_change(tmp_path):
+    """Test thin.gen_thin rebuilds a cached archive when the installed saltexts changed."""
+    cachedir = str(tmp_path)
+    thincfg = tmp_path / "thin" / ".thin-gen-config"
+
+    with patch("salt.utils.thin._saltext_dists", MagicMock(return_value=[])):
+        thintar = salt.utils.thin.gen_thin(cachedir)
+        assert thincfg.read_text().endswith("saltexts=")
+        mtime = os.stat(thintar).st_mtime_ns
+        # Nothing changed, the cached archive is kept.
+        salt.utils.thin.gen_thin(cachedir)
+        assert os.stat(thintar).st_mtime_ns == mtime
+
+    with patch(
+        "salt.utils.thin._saltext_dists",
+        MagicMock(return_value=["saltext.foo==1.0"]),
+    ):
+        salt.utils.thin.gen_thin(cachedir)
+
+    assert os.stat(thintar).st_mtime_ns != mtime
+    assert thincfg.read_text().endswith("saltexts=saltext.foo==1.0")
+
+
+def test_thin_contents_id_covers_configuration():
+    """Test thin._thin_contents_id changes with every configured part of the archive contents."""
+    with patch(
+        "salt.utils.thin._saltext_dists", MagicMock(return_value=["saltext.foo==1.0"])
+    ):
+        contents_id = salt.utils.thin._thin_contents_id()
+        assert salt.utils.thin._thin_contents_id(extra_mods="pymysql") != contents_id
+        assert salt.utils.thin._thin_contents_id(so_mods="zmq") != contents_id
+        assert salt.utils.thin._thin_contents_id(exclude_saltexts=True) != contents_id
+
+
+def test_saltext_dists_filters():
+    """Test thin._saltext_dists honours the allowlist and the blocklist."""
+    entry_points = [
+        types.SimpleNamespace(
+            dist=types.SimpleNamespace(name="saltext.foo", version="1.0")
+        ),
+        types.SimpleNamespace(
+            dist=types.SimpleNamespace(name="saltext.bar", version="2.0")
+        ),
+    ]
+    with patch(
+        "salt.utils.entrypoints.iter_entry_points", MagicMock(return_value=entry_points)
+    ):
+        assert salt.utils.thin._saltext_dists() == [
+            "saltext.bar==2.0",
+            "saltext.foo==1.0",
+        ]
+        assert salt.utils.thin._saltext_dists(allowlist=["saltext.foo"]) == [
+            "saltext.foo==1.0"
+        ]
+        assert salt.utils.thin._saltext_dists(blocklist=["saltext.foo"]) == [
+            "saltext.bar==2.0"
+        ]