From b7bb74092ec9542a6494658716c26980c6247dfe Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 15 Sep 2026 15:59:23 -0400 Subject: [PATCH] Rebuild the Salt-SSH thin archive when its configured contents change gen_thin only compared the Salt version and the Python major version, so an archive cached in the master cachedir was reused even after a Salt extension was installed or thin_extra_mods changed, and neither reached the target. Record an identity of what the archive was generated from, and compare against it. thin_sum calls gen_thin again, so it now forwards the same arguments: otherwise the regeneration this makes possible would drop the caller's configuration. Also fix thin_extra_mods for a module inside a namespace package, such as saltext.mysql: __import__ returns the namespace package, whose __file__ is None, which raised a TypeError instead of packing the extension. Document that thin_extra_mods takes a dotted path, along with the thin_exclude_saltexts, thin_saltext_allowlist and thin_saltext_blocklist options added in 3008.0. --- changelog/70287.fixed.md | 1 + doc/ref/configuration/master.rst | 61 +++++++++++++++++-- salt/client/ssh/__init__.py | 10 ++- salt/utils/thin.py | 80 +++++++++++++++++++++--- tests/pytests/unit/utils/test_thin.py | 88 +++++++++++++++++++++++++-- 5 files changed, 222 insertions(+), 18 deletions(-) create mode 100644 changelog/70287.fixed.md diff --git a/changelog/70287.fixed.md b/changelog/70287.fixed.md new file mode 100644 index 000000000000..ba83ae69c5c8 --- /dev/null +++ b/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. diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index 40b3165fa7c2..656d55c081a0 100644 --- a/doc/ref/configuration/master.rst +++ b/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: diff --git a/salt/client/ssh/__init__.py b/salt/client/ssh/__init__.py index 2afa611ee7cf..69a9dd62913a 100644 --- a/salt/client/ssh/__init__.py +++ b/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 = \ """ diff --git a/salt/utils/thin.py b/salt/utils/thin.py index fb1ba1dc80eb..d052493ed45b 100644 --- a/salt/utils/thin.py +++ b/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: diff --git a/tests/pytests/unit/utils/test_thin.py b/tests/pytests/unit/utils/test_thin.py index 3feb2dad37bb..9cc461b2a525 100644 --- a/tests/pytests/unit/utils/test_thin.py +++ b/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" + ]