Fix __pillar__ access in modules in pillar render

Historically the __pillar__ dunder has never been available or worked
in extension modules during pillar render, due to what I can only assume
is a bug/oversight in the overly-complex mess of branches and logic in
the pillar rendering code. Functionally there is very little reason this
shouldn't work, and it even appears like it may have worked at some
point, or at least was intended to work by the fact that the ext_pillar
data was stuffed right back into opts before attempting to render the
remainder of the pillar.

The missing piece to the puzzle here was that the partially-rendered
ext_pillar data wasn't available in the opts dict packed into the
salt.loader.minion_mods() call when it was generated, and the fact that
the functions weren't reloaded after the pillar dict was updated. This
commit fixes both of those things.

Ensure the minion_mods are *always* reloaded with the updated opts dict
containing the partial pillar data when rendering pillar with
ext_pillar_first enabled.

Signed-off-by: Joe Groocock <jgroocock@cloudflare.com>

Author: frebib

changelog/63398.fixed

@@ -0,0 +1 @@
+Fix __pillar__ access in execution modules during pillar render with ext_pillar_first

salt/pillar/__init__.py

@@ -537,6 +537,7 @@ def __init__(
             if opts.get("pillarenv_from_saltenv", False):
                 opts["pillarenv"] = saltenv
         # use the local file client
+        self.raw_opts = opts
         self.opts = self.__gen_opts(opts, grains, saltenv=saltenv, pillarenv=pillarenv)
         self.saltenv = saltenv
         self.client = salt.fileclient.get_file_client(self.opts, True)
@@ -547,16 +548,8 @@ def __init__(
         ):
             opts["grains"] = grains
 
-        # if we didn't pass in functions, lets load them
-        if functions is None:
-            utils = salt.loader.utils(opts)
-            if opts.get("file_client", "") == "local":
-                self.functions = salt.loader.minion_mods(opts, utils=utils)
-            else:
-                self.functions = salt.loader.minion_mods(self.opts, utils=utils)
-        else:
-            self.functions = functions
-
+        self.utils = None
+        self.functions = self.__gen_functions(functions)
         self.opts["minion_id"] = minion_id
         self.matchers = salt.loader.matchers(self.opts)
         self.rend = salt.loader.render(self.opts, self.functions)
@@ -620,6 +613,29 @@ def __gather_avail(self):
             avail[saltenv] = self.client.list_states(saltenv)
         return avail
 
+    def __gen_functions(self, context=None, functions=None, force=False):
+        """
+        Generates the minion_mods loader object using the correct opts, context
+        and pillar values in the correct scenarios
+        """
+        if not force and functions is not None:
+            return functions
+
+        # This is some historical cruft that I don't want to pretend to try to
+        # understand. Keeping compatible for the sake of compatibility.
+        # Origin: 340aed8049b885c0736681e925cf7210ede0f961
+        opts = self.opts
+        if self.raw_opts.get("file_client", "") == "local":
+            opts = self.raw_opts
+            # Ensure we keep the partially-rendered pillar regardless of which
+            # opts we're using. Copy pillar into the alternative opts.
+            if "pillar" in self.opts:
+                opts["pillar"] = self.opts["pillar"]
+
+        if self.utils is None:
+            self.utils = salt.loader.utils(self.raw_opts, context=context)
+        return salt.loader.minion_mods(opts, utils=self.utils, context=context)
+
     def __gen_opts(self, opts_in, grains, saltenv=None, ext=None, pillarenv=None):
         """
         The options need to be altered to conform to the file client
@@ -1217,6 +1233,8 @@ def compile_pillar(self, ext=True):
         if ext:
             if self.opts.get("ext_pillar_first", False):
                 self.opts["pillar"], errors = self.ext_pillar(self.pillar_override)
+                # Reload minion_mods with ext_pillar as pillar dict
+                self.functions = self.__gen_functions(force=True)
                 self.rend = salt.loader.render(self.opts, self.functions)
                 matches = self.top_matches(top, reload=True)
                 pillar, errors = self.render_pillar(matches, errors=errors)

tests/pytests/unit/pillar/test_pillar.py

@@ -3,6 +3,7 @@
 import salt.loader
 import salt.pillar
 from salt.utils.odict import OrderedDict
+from tests.support.mock import patch
 
 
 @pytest.mark.parametrize(
@@ -123,3 +124,27 @@ def test_pillar_envs_path_substitution(env, temp_salt_minion, tmp_path):
 
     # The __env__ string in the path has been substituted for the actual env
     assert pillar.opts["pillar_roots"] == expected
+
+
+def test_ext_pillar_dunder_in_modules_in_pillar(temp_salt_minion):
+    """
+    Test that ext_pillar is available in __pillar__ inside execution modules
+    during pillar render when using ext_pillar_first=true
+    """
+    opts = temp_salt_minion.config.copy()
+    opts["ext_pillar_first"] = True
+
+    grains = salt.loader.grains(opts)
+    pillar = salt.pillar.Pillar(opts, grains, temp_salt_minion.id, "base")
+    # Pillar should be empty to start with
+    assert pillar.functions.pack["__pillar__"] == {}
+
+    ext_value = {"ext": "some ext value"}
+    pil_value = {"pillar": "some pillar value"}
+    with patch.object(pillar, "ext_pillar", return_value=(ext_value, [])):
+        with patch.object(pillar, "render_pillar", return_value=(pil_value, [])):
+            compiled = pillar.compile_pillar()
+            assert compiled == dict(**ext_value, **pil_value)
+
+    # Loader should pack the opts pillar dict from the ext_pillar() call
+    assert pillar.functions.pack["__pillar__"] == ext_value