Fix non-deterministic pillar rendering across pillar_roots envs (#44937)

changelog/44937.fixed.md

@@ -0,0 +1 @@
+Fixed non-deterministic pillar rendering when multiple ``pillar_roots`` environments matched the same minion. ``Pillar.get_tops`` collected saltenvs into a ``set`` and iterated them in hash order, so top-file processing order depended on ``PYTHONHASHSEED`` and varied per ``salt-call`` invocation. An earlier change made ``_get_envs`` return an ordered list, but the caller wrapped the result back into a ``set``. ``get_tops`` now uses an insertion-ordered dict so iteration follows ``pillar_roots`` config order.

salt/pillar/__init__.py

@@ -720,7 +720,12 @@ def get_tops(self):
         errors = []
         # Gather initial top files
         try:
-            saltenvs = set()
+            # Use a dict (insertion-ordered) instead of a set so the
+            # iteration order matches the order returned by ``_get_envs``,
+            # which itself reflects the order of ``pillar_roots`` in the
+            # config. ``set`` iteration order depends on PYTHONHASHSEED
+            # and produced non-deterministic top-file processing (#44937).
+            saltenvs = {}
             if self.opts["pillarenv"]:
                 # If the specified pillarenv is not present in the available
                 # pillar environments, do not cache the pillar top file.
@@ -732,11 +737,13 @@ def get_tops(self):
                         ", ".join(self.opts["pillar_roots"]),
                     )
                 else:
-                    saltenvs.add(self.opts["pillarenv"])
+                    saltenvs[self.opts["pillarenv"]] = None
             else:
-                saltenvs.update(self._get_envs())
+                for env in self._get_envs():
+                    saltenvs[env] = None
                 if self.opts.get("pillar_source_merging_strategy", None) == "none":
-                    saltenvs &= {self.saltenv or "base"}
+                    only = self.saltenv or "base"
+                    saltenvs = {only: None} if only in saltenvs else {}
 
             for saltenv in saltenvs:
                 top = self.client.cache_file(self.opts["state_top"], saltenv)

tests/pytests/unit/pillar/test_pillar.py

@@ -35,6 +35,58 @@ def test_pillar_envs_order(envs, temp_salt_minion, tmp_path):
     assert pillar._get_envs() == ["base"] + envs
 
 
+def test_pillar_get_tops_iterates_envs_in_pillar_roots_order(
+    temp_salt_minion, tmp_path
+):
+    """
+    Regression test for issue #44937.
+
+    ``Pillar.get_tops`` used to collect saltenvs into a ``set`` which is
+    iterated in hash order. With a randomized ``PYTHONHASHSEED`` this made
+    top-file processing non-deterministic. The fix uses an insertion-ordered
+    dict so iteration follows the order returned by ``_get_envs``, which
+    itself follows ``pillar_roots`` config order.
+
+    This test exercises ``get_tops`` directly with multiple envs whose
+    top.sls all match the same minion and asserts the resulting ``tops``
+    keys appear in ``_get_envs`` order.
+    """
+    envs = ["base", "foo", "bar", "baz"]
+    for envname in envs:
+        env_dir = tmp_path / envname
+        env_dir.mkdir()
+        (env_dir / "top.sls").write_text(
+            f"{envname}:\n  '{temp_salt_minion.id}':\n    - data\n",
+            encoding="utf-8",
+        )
+        (env_dir / "data.sls").write_text(f"key: {envname}\n", encoding="utf-8")
+
+    opts = temp_salt_minion.config.copy()
+    opts["pillarenv"] = None
+    opts["pillar_roots"] = OrderedDict(
+        (envname, [str(tmp_path / envname)]) for envname in envs
+    )
+    grains = salt.loader.grains(opts)
+    pillar = salt.pillar.Pillar(
+        opts=opts,
+        grains=grains,
+        minion_id=temp_salt_minion.id,
+        saltenv="base",
+    )
+    tops, errors = pillar.get_tops()
+    assert not errors, errors
+    # ``_get_envs`` prepends "base" then appends the remaining roots in
+    # config order. Any env whose top.sls rendered should appear in
+    # ``tops`` in that same order.
+    expected_order = pillar._get_envs()
+    actual_order = list(tops.keys())
+    assert actual_order == expected_order, (
+        f"get_tops iteration order {actual_order!r} does not match "
+        f"_get_envs order {expected_order!r}; pillar top-file processing "
+        f"is non-deterministic (issue #44937)."
+    )
+
+
 def test_pillar_get_tops_should_not_error_when_merging_strategy_is_none_and_no_pillarenv(
     temp_salt_minion,
 ):