[Bug]: deltaproxy leaks one sub-proxy's return configuration onto its siblings via the shared publish load
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description
On a deltaproxy, one sub-proxy's return configuration leaks onto its siblings for the same job, so a sub-proxy with no returner configured sends its job return to another sub-proxy's returner.
handle_payload (salt/metaproxy/deltaproxy.py) hands the same publish-load dict to the control proxy and to every sub-proxy the job matched:
for _id in sub_ids:
if _id in self.deltaproxy_objs:
instance = self.deltaproxy_objs[_id]
if instance._target_load(payload["load"]):
await instance._handle_decoded_payload(payload["load"])
thread_return then merges the sub-proxy's own returner setting back into that shared dict:
if isinstance(opts.get("return"), str):
if data["ret"]:
data["ret"] = ",".join((data["ret"], opts["return"]))
else:
data["ret"] = opts["return"]
With multiprocessing: False every sub-proxy's job runs as a thread in the one salt-proxy process, so they all share that dict and whichever sub-proxy runs first stamps its configuration onto the ones that run after it. With the default multiprocessing: True each job is forked, so the mutation stays process-local and nothing leaks.
multiprocessing: False is the normal configuration for a proxy driving a real device, since a live NETCONF/SSH session cannot be forked.
Setup
salt 3008.2, one control proxy with three sub-proxies (minion1, minion2, minion3) using the dummy proxytype, multiprocessing: False.
Give only minion1 a returner, in its opts. Note this has to go in the per-sub-proxy config, not pillar: sub-proxy opts come from salt.config.proxy_config(opts["conf_file"], defaults=proxyopts, minion_id=minion_id), so /etc/salt/proxy.d/minion1/ret.conf works, while a pillar key is not visible to opts.get("return").
# /etc/salt/proxy.d/minion1/ret.conf
return: some_returner
Confirm the others have nothing:
salt minion1 config.get return omit_pillar=True # some_returner
salt minion2 config.get return omit_pillar=True # (empty)
salt minion3 config.get return omit_pillar=True # (empty)
Steps to reproduce
Run a job that matches more than one sub-proxy:
salt 'minion*' test.ping
Observed: minion2 and minion3 both attempt some_returner, because data["ret"] was set by minion1 earlier in the same job. Instrumenting thread_return shows the three sub-proxies sharing one load object:
id=minion1 opts_return='some_returner' data_ret='' data_id=130622793244416
id=minion3 opts_return=None data_ret='some_returner' data_id=130622793244416
id=minion2 opts_return=None data_ret='some_returner' data_id=130622793244416
A single-target job shows data_ret='' throughout, so the leak is specific to a job fanning out to two or more sub-proxies.
Expected: each sub-proxy uses only its own return configuration.
Versions Report
Salt Version:
Salt: 3008.2
Python Version:
Python: 3.10
Salt Package Information:
Package Type: onedir
Reproduces against the current 3008.x branch head.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in salt/metaproxy/deltaproxy.py with handle_payload and thread_return, tracing how the publish-load dict is passed to multiple sub-proxies and how data["ret"] is merged. Reproduce with three dummy sub-proxies, multiprocessing: False, and only minion1 configured with a returner. Done means each sub-proxy uses only its own return configuration and minion2/minion3 do not attempt some_returner.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100