From 10e2a9c37617fdce08ebb267d00d8c127759fd65 Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Wed, 2 Sep 2026 15:43:20 -0400 Subject: [PATCH] Align deltaproxy proxy setup with the single proxy (#70223) deltaproxy merged the control proxy's pillar into its opts unconditionally. salt/metaproxy/proxy.py guards that behind proxy_merge_pillar_in_opts and treats proxy_mines_pillar as the elif, so the documented option (default False) had no effect here and the control proxy's pillar always won over its opts. Sub-proxy opts start as a copy of the control proxy's, so whatever the pillar injected was inherited fleet-wide. Restore the guard and the elif. A proxymodule's module_executors declaration was also dropped for sub-proxies. post_master_init reads it for the control proxy, but subproxy_post_master_init never set it, so thread_return's getattr(minion_instance, "module_executors", []) found nothing and fell back to the opts default. Set it from the sub-proxy's own proxymodule. --- changelog/70223.fixed.md | 1 + salt/metaproxy/deltaproxy.py | 36 +++++++++---- .../pytests/unit/metaproxy/test_deltaproxy.py | 51 +++++++++++++++++++ 3 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 changelog/70223.fixed.md diff --git a/changelog/70223.fixed.md b/changelog/70223.fixed.md new file mode 100644 index 000000000000..dfdfdb4465b1 --- /dev/null +++ b/changelog/70223.fixed.md @@ -0,0 +1 @@ +Made deltaproxy honour ``proxy_merge_pillar_in_opts`` instead of always merging the control proxy's pillar into its opts. Because sub-proxy opts start as a copy of the control proxy's, that merge was inherited by every sub-proxy. Also made a sub-proxy pick up its proxymodule's ``module_executors`` declaration, which was read for the control proxy but never set for sub-proxies, so ``thread_return`` silently fell back to the opts default. diff --git a/salt/metaproxy/deltaproxy.py b/salt/metaproxy/deltaproxy.py index cb2b8e2c9a09..46fa66fca0b0 100644 --- a/salt/metaproxy/deltaproxy.py +++ b/salt/metaproxy/deltaproxy.py @@ -97,16 +97,23 @@ async def post_master_init(self, master): if "proxy" not in self.opts: self.opts["proxy"] = self.opts["pillar"]["proxy"] - pillar = copy.deepcopy(self.opts["pillar"]) - pillar.pop("master", None) - self.opts = salt.utils.dictupdate.merge( - self.opts, - pillar, - strategy=self.opts.get("proxy_merge_pillar_in_opts_strategy"), - merge_lists=self.opts.get("proxy_deep_merge_pillar_in_opts", False), - ) - - if self.opts.get("proxy_mines_pillar"): + if self.opts.get("proxy_merge_pillar_in_opts"): + # Override proxy opts with pillar data when the user required. But do + # not override master in opts. + # + # This is guarded in salt/metaproxy/proxy.py and was not here, so the + # documented option (default False) was ignored and the control proxy's + # pillar always won over its opts. Sub-proxy opts start life as a copy + # of the control proxy's, so that leaked fleet-wide. + pillar = copy.deepcopy(self.opts["pillar"]) + pillar.pop("master", None) + self.opts = salt.utils.dictupdate.merge( + self.opts, + pillar, + strategy=self.opts.get("proxy_merge_pillar_in_opts_strategy"), + merge_lists=self.opts.get("proxy_deep_merge_pillar_in_opts", False), + ) + elif self.opts.get("proxy_mines_pillar"): # Even when not required, some details such as mine configuration # should be merged anyway whenever possible. if "mine_interval" in self.opts["pillar"]: @@ -521,6 +528,15 @@ async def subproxy_post_master_init(minion_id, uid, opts, main_proxy, main_utils _fq_proxyname = proxyopts["proxy"]["proxytype"] + # A proxymodule may declare the executors its functions must run through. + # ``post_master_init`` reads this for the control proxy, but sub-proxies + # never had it set, so ``thread_return`` fell through to the opts default + # and the proxymodule's declaration was silently ignored for every + # sub-proxy. The single-proxy metaproxy honours it. + _proxy_minion.module_executors = _proxy_minion.proxy.get( + f"{_fq_proxyname}.module_executors", lambda: [] + )() + proxy_init_fn = _proxy_minion.proxy[_fq_proxyname + ".init"] try: proxy_init_fn(proxyopts) diff --git a/tests/pytests/unit/metaproxy/test_deltaproxy.py b/tests/pytests/unit/metaproxy/test_deltaproxy.py index ded184c23daf..a9f76ac3f0c5 100644 --- a/tests/pytests/unit/metaproxy/test_deltaproxy.py +++ b/tests/pytests/unit/metaproxy/test_deltaproxy.py @@ -113,6 +113,9 @@ def _fake_proxy_loader(opts, utils=None, context=None, **kwargs): items={ f"{proxytype}.init": MagicMock(return_value=True), f"{proxytype}.shutdown": MagicMock(return_value=True), + f"{proxytype}.module_executors": MagicMock( + return_value=["direct_call"] + ), } ) @@ -229,3 +232,51 @@ def test_subproxy_post_master_init_packs_per_minion_grains( # control proxy stores the right grains in ``self.deltaproxy_opts``. assert result1["proxy_opts"]["grains"]["serial_number"] == "SN-AAA-001" assert result2["proxy_opts"]["grains"]["serial_number"] == "SN-BBB-002" + + +def test_subproxy_post_master_init_sets_module_executors( + proxy_opts, fake_main_proxy, fake_main_utils +): + """ + A proxymodule may declare the executors its functions must run through. + ``post_master_init`` reads that for the control proxy, but sub-proxies never + had it set, so ``thread_return`` fell through to the opts default and the + proxymodule's declaration was silently ignored for every sub-proxy. + """ + per_minion_grains = { + "minion1": {"serial_number": "SN-AAA-001", "id": "minion1"}, + "minion2": {"serial_number": "SN-BBB-002", "id": "minion2"}, + } + p = _make_subproxy_patches(per_minion_grains) + + loop = tornado.ioloop.IOLoop() + with patch.object( + deltaproxy.salt.config, "proxy_config", p["proxy_config"] + ), patch.object( + deltaproxy.salt.pillar, "get_async_pillar", p["get_pillar"] + ), patch.object( + deltaproxy.salt.loader, "grains", p["grains"] + ), patch.object( + deltaproxy.salt.loader, "proxy", p["proxy_loader"] + ), patch.object( + deltaproxy.salt.loader, "utils", p["utils_loader"] + ), patch.object( + deltaproxy, "ProxyMinion", p["proxy_minion_cls"] + ), patch.object( + deltaproxy.salt.minion, "get_proc_dir", p["get_proc_dir"] + ), patch.object( + deltaproxy.salt.utils.schedule, "Schedule", p["schedule"] + ): + try: + result = loop.run_sync( + lambda: deltaproxy.subproxy_post_master_init( + "minion1", 0, proxy_opts, fake_main_proxy, fake_main_utils + ) + ) + finally: + loop.close() + + sub = result["proxy_minion"] + assert sub is not None + # thread_return reads this off the sub-proxy via getattr; it must be there. + assert getattr(sub, "module_executors", None) == ["direct_call"]