Align deltaproxy proxy setup with the single proxy (#70223)

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.

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)

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"]