Use target minion id for pillar-render execution modules on the master#69752
Open
ggiesen wants to merge 2 commits into
Open
Use target minion id for pillar-render execution modules on the master#69752ggiesen wants to merge 2 commits into
ggiesen wants to merge 2 commits into
Conversation
Pillar compilation loads execution modules (for salt[...] calls in pillar templates) with the incoming opts when file_client is "local". On the master that branch is always taken, and the incoming opts carry the master's id, so salt["match.compound"] and friends matched against the master rather than the minion the pillar was being compiled for (saltstack#58407). Load them with self.opts, which __gen_opts stamps with the target minion_id, matching the non-local branch. Masterless is unaffected (opts["id"] already equals the minion_id there). Fixes saltstack#58407
…ted example The saltstack#58407 fix makes execution modules called during pillar rendering resolve against the target minion, so the documented match.filter_by example (which omits the minion_id argument) now correctly matches the minion and gets the db role. The parametrized expectation still asserted the pre-fix web* roles, which contradicted the fix and failed in CI.
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
3 participants
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?
Loads the execution modules used during pillar compilation with
self.opts(which carries the targetminion_id) instead of the raw incoming opts, sosalt[...]calls in pillar templates resolve against the minion the pillar is being compiled for rather than the master's own id.What issues does this PR fix or reference?
Fixes #58407
Previous Behavior
In
Pillar.__init__, whenfile_client == "local"the execution-module loader was built from the raw incomingopts. On the master that branch is always taken (the master'sfile_clientislocal), and those opts carry the master'sid. Sosalt['match.compound']used from a pillar template matched against the master's id and returned the wrong result, whilesalt-call match.compoundon the minion (whereopts['id']is the minion's own id) returned the correct one. The sibling non-local branch already usedself.opts.New Behavior
Both branches now load the modules from
self.opts, which__gen_optsstamps with the targetminion_id, so pillar-render execution modules resolve against the target minion. Masterless is unaffected (thereopts['id']already equals theminion_id, so the change is a no-op).Behavior-change note for reviewers: execution modules invoked during master-side pillar rendering now observe the target minion's
id(and the targetsaltenv/pillarenvcarried byself.opts) rather than the master's raw opts. External pillars are unchanged -- they still deliberately run with the master's id (ext_pillar_opts["id"] = opts["id"]).Merge requirements satisfied?
changelog/58407.fixed.mdtests/pytests/unit/test_pillar.py: master-side render loads modules with the target minion_id (pins the bug), and masterless keeps its own id (no-op)Commits signed with GPG?
No