Fix deltaproxy sub-proxies returning identical grains data (#68248)#69426
Open
dwoz wants to merge 2 commits into
Open
Fix deltaproxy sub-proxies returning identical grains data (#68248)#69426dwoz wants to merge 2 commits into
dwoz wants to merge 2 commits into
Conversation
subproxy_post_master_init computes the sub-proxy grains twice: first through the parent control proxy's LazyLoader (because the sub-proxy's own proxymodule has not been initialised yet), then again after the sub-proxy's init() has been run so the grains can talk to the actual device. Only the first grains dict was packed into the sub-proxy's execution-module/returner/executor/proxy LazyLoaders; the post-init refresh updated proxyopts["grains"] but left the loaders' pack pointing at the original placeholder dict. The result is that every sub-proxy under a deltaproxy ends up serving its modules a __grains__ that was populated by the same shared control proxy connection, so e.g. grains.item serial_number returns the same value for every controlled minion. Re-pack the post-init grains into all four loaders so __grains__ inside loaded modules reflects each sub-proxy's own device. Fixes saltstack#68248
Drive ``subproxy_post_master_init`` end-to-end through real ``salt.loader.proxy`` / ``salt.loader.grains`` loaders against an on-disk extension_modules tree containing a purpose-built proxy module that returns ``serial_number = <sub-proxy id>``. Two distinct sub-proxy ids are exercised; the test asserts each sub-proxy's execution-module / returner / executor / proxy loaders expose an ``__grains__`` dict whose ``serial_number`` reflects that sub-proxy's own device (not the placeholder shared with its siblings) and that ``grains.items()`` invoked via the loader sees the per-device values. Before the fix, both sub-proxies' loaders point at the same first-pass grains dict and the test fails with the same serial_number for every controlled minion. Complements the unit test in tests/pytests/unit/metaproxy/test_deltaproxy.py which mocks the loader stack; this functional test uses real LazyLoaders. Refs saltstack#68248
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
In a deltaproxy controller, each sub-proxy is meant to talk to a distinct
device. With this PR, the grains visible inside the modules loaded for a
sub-proxy (e.g.
grains.item serial_number) are the ones produced by thatsub-proxy's own proxymodule, not whatever was loaded on the first pass
through the shared control proxy.
What issues does this PR fix or reference?
Fixes #68248
Previous Behavior
subproxy_post_master_initcomputed each sub-proxy's grains twice:using the parent control proxy's LazyLoader (
main_proxy). This dictwas packed into the sub-proxy's execution-module, returner, executor and
proxy LazyLoaders via
pack["__grains__"].proxytype.init(proxyopts), using the sub-proxy'sown freshly-created proxymodule. The result was assigned to
proxyopts["grains"]but not re-packed into any LazyLoader, so__grains__inside loaded modules continued to point at the first-passdict.
Because the first-pass grains all flow through the same
main_proxyLazyLoader, every sub-proxy ends up serving its modules identical
__grains__data — so runninggrains.item serial_numberagainst fourcontrolled minions returns the same value from each.
New Behavior
After the post-init grains refresh, the fresh per-sub-proxy grains dict is
re-packed into
_proxy_minion.functions.pack["__grains__"],_proxy_minion.returners.pack["__grains__"],_proxy_minion.executors.pack["__grains__"]and_proxy_minion.proxy.pack["__grains__"]. Each sub-proxy's modules nowsee grain values that come from that sub-proxy's own device.
A regression test in
tests/pytests/unit/metaproxy/test_deltaproxy.pydrives
subproxy_post_master_initfor two distinct minion IDs with amocked loader stack and asserts that each sub-proxy's
functions.pack["__grains__"]andproxy.pack["__grains__"]reflect thepost-init per-id grains rather than the shared placeholder values.
Merge requirements satisfied?
Commits signed with GPG?
Yes