From 1f1fb865a708f960cb102993c1aba2c886b74228 Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sat, 11 Jul 2026 14:18:18 -0400 Subject: [PATCH] Resolve master_uri in minion engines to fix pillar.data KeyError A minion forks its engines early in startup, before it has connected and resolved master_uri into its opts, so the engine runs against a snapshot that lacks it. Any __salt__ call needing the master transport (e.g. pillar.data) then raises KeyError: 'master_uri' in salt.crypt. Engine.run() now completes the engine's opts by resolving master_uri in the engine's own process before the execution modules are loaded. It is best-effort and cannot block or break engine startup: master engines are skipped by role, masterless minions need no transport, and DNS is resolved once without the minion connect loop's retry so a transport-less engine still starts when the master is unresolvable. Fixes #57952 --- changelog/57952.fixed.md | 1 + salt/engines/__init__.py | 41 +++++++++++++++++ tests/pytests/unit/engines/test_engines.py | 52 ++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 changelog/57952.fixed.md diff --git a/changelog/57952.fixed.md b/changelog/57952.fixed.md new file mode 100644 index 000000000000..83954c354787 --- /dev/null +++ b/changelog/57952.fixed.md @@ -0,0 +1 @@ +Fixed ``KeyError: 'master_uri'`` when a minion engine calls a salt function that needs the master transport (for example ``pillar.data``). A minion forks its engines before it has connected and resolved ``master_uri`` into its opts, so the engine ran against a snapshot that lacked it. The engine now resolves ``master_uri`` in its own process before loading its execution modules, best-effort so it never blocks or breaks engine startup. diff --git a/salt/engines/__init__.py b/salt/engines/__init__.py index 11baafb2b86a..5a5cc2d35072 100644 --- a/salt/engines/__init__.py +++ b/salt/engines/__init__.py @@ -80,10 +80,51 @@ def __init__(self, opts, fun, config, funcs, runners, proxy, **kwargs): self.runners = runners self.proxy = proxy + def _ensure_master_uri(self): + """ + Complete the transport opts for a minion engine. + + A minion forks its engines early in startup -- before it has connected + and resolved ``master_uri`` into its opts -- so the engine runs against + a snapshot that lacks it. Any ``__salt__`` call that needs the master + transport (e.g. ``pillar.data``) then raises ``KeyError: 'master_uri'`` + (#57952). Resolve it here, in the engine's own process, before the + execution modules are loaded/used. + + This is deliberately best-effort so it can never block or break engine + startup: master engines have a different role (and no master), + masterless minions need no transport, and DNS is resolved once without + the minion connect loop's retry so a transport-less engine still starts + even when the master is not resolvable. + """ + if ( + self.opts.get("__role") != "minion" + or "master_uri" in self.opts + or ( + self.opts.get("file_client") == "local" + and not self.opts.get("use_master_when_local") + ) + ): + return + try: + import salt.minion + + self.opts.update( + salt.minion.resolve_dns(dict(self.opts, retry_dns=0), fallback=False) + ) + except Exception as exc: # pylint: disable=broad-except + log.warning( + "%s: could not resolve master_uri; salt functions that need " + "the master transport may not work in this engine: %s", + self.name, + exc, + ) + def run(self): """ Run the master service! """ + self._ensure_master_uri() self.utils = salt.loader.utils(self.opts, proxy=self.proxy) if salt.utils.platform.spawning_platform(): # Calculate function references since they can't be pickled. diff --git a/tests/pytests/unit/engines/test_engines.py b/tests/pytests/unit/engines/test_engines.py index f83ced785b00..a5b600406393 100644 --- a/tests/pytests/unit/engines/test_engines.py +++ b/tests/pytests/unit/engines/test_engines.py @@ -31,3 +31,55 @@ def test_engine_title_set(kwargs): with patch("salt.utils.process.appendproctitle", MagicMock()) as mm: engine.run() mm.assert_called_with(kwargs["name"]) + + +def test_ensure_master_uri_resolves_for_minion(kwargs): + # #57952: a minion engine's opts snapshot is taken before the minion has + # resolved master_uri, so the engine resolves it in its own process before + # __salt__ (e.g. pillar.data) touches the transport. + engine = salt.engines.Engine(**kwargs) + engine.opts = {"__role": "minion", "file_client": "remote"} + with patch( + "salt.minion.resolve_dns", + return_value={"master_uri": "tcp://1.2.3.4:4506", "master_ip": "1.2.3.4"}, + ) as resolve: + engine._ensure_master_uri() + resolve.assert_called_once() + assert engine.opts["master_uri"] == "tcp://1.2.3.4:4506" + + +def test_ensure_master_uri_skips_master_role(kwargs): + engine = salt.engines.Engine(**kwargs) + engine.opts = {"__role": "master"} + with patch("salt.minion.resolve_dns") as resolve: + engine._ensure_master_uri() + resolve.assert_not_called() + assert "master_uri" not in engine.opts + + +def test_ensure_master_uri_skips_when_already_present(kwargs): + engine = salt.engines.Engine(**kwargs) + engine.opts = {"__role": "minion", "master_uri": "tcp://existing:4506"} + with patch("salt.minion.resolve_dns") as resolve: + engine._ensure_master_uri() + resolve.assert_not_called() + assert engine.opts["master_uri"] == "tcp://existing:4506" + + +def test_ensure_master_uri_skips_masterless(kwargs): + engine = salt.engines.Engine(**kwargs) + engine.opts = {"__role": "minion", "file_client": "local"} + with patch("salt.minion.resolve_dns") as resolve: + engine._ensure_master_uri() + resolve.assert_not_called() + assert "master_uri" not in engine.opts + + +def test_ensure_master_uri_is_non_fatal(kwargs): + # A resolution failure (failover/list master, or an unresolvable master at + # boot for a transport-less engine) must not prevent the engine starting. + engine = salt.engines.Engine(**kwargs) + engine.opts = {"__role": "minion", "file_client": "remote"} + with patch("salt.minion.resolve_dns", side_effect=Exception("boom")): + engine._ensure_master_uri() # must not raise + assert "master_uri" not in engine.opts