Address PR #64043 review comments: optimize pillar updates and fix UnboundLocalError

Author: dwoz

salt/matchers/pillar_match.py

@@ -21,6 +21,7 @@ def match(tgt, delimiter=DEFAULT_TARGET_DELIM, opts=None, minion_id=None):
         log.error("Got insufficient arguments for pillar match statement from master")
         return False
 
+    pillar = {}
     if "pillar" in opts:
         pillar = opts["pillar"]
     elif "ext_pillar" in opts:

salt/pillar/__init__.py

@@ -1246,27 +1246,29 @@ def compile_pillar(self, ext=True):
         """
         Render the pillar data and return
         """
-        top, top_errors = self.get_top()
-        if ext:
-            if self.opts.get("ext_pillar_first", False):
-                self.opts["pillar"], errors = self.ext_pillar(self.pillar_override)
-                self.rend = salt.loader.render(self.opts, self.functions)
-                matches = self.top_matches(top, reload=True)
-                pillar, errors = self.render_pillar(matches, errors=errors)
-                pillar = merge(
-                    self.opts["pillar"],
-                    pillar,
-                    self.merge_strategy,
-                    self.opts.get("renderer", "yaml"),
-                    self.opts.get("pillar_merge_lists", False),
-                )
-            else:
+        if ext and self.opts.get("ext_pillar_first", False):
+            self.opts["pillar"], errors = self.ext_pillar(self.pillar_override)
+            self.functions.pack["__pillar__"] = self.opts["pillar"]
+            self.rend = salt.loader.render(self.opts, self.functions)
+            top, top_errors = self.get_top()
+            matches = self.top_matches(top, reload=True)
+            pillar, errors = self.render_pillar(matches, errors=errors)
+            pillar = merge(
+                self.opts["pillar"],
+                pillar,
+                self.merge_strategy,
+                self.opts.get("renderer", "yaml"),
+                self.opts.get("pillar_merge_lists", False),
+            )
+        else:
+            top, top_errors = self.get_top()
+            if ext:
                 matches = self.top_matches(top)
                 pillar, errors = self.render_pillar(matches)
                 pillar, errors = self.ext_pillar(pillar, errors=errors)
-        else:
-            matches = self.top_matches(top)
-            pillar, errors = self.render_pillar(matches)
+            else:
+                matches = self.top_matches(top)
+                pillar, errors = self.render_pillar(matches)
         errors.extend(top_errors)
         if self.opts.get("pillar_opts", False):
             mopts = dict(self.opts)

tests/pytests/unit/test_pillar.py

@@ -257,6 +257,8 @@ def _setup_test_include_sls(tempdir):
 
 def test_ext_pillar_no_extra_minion_data_val_dict():
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "json",
         "renderer_blacklist": [],
@@ -298,6 +300,8 @@ def test_ext_pillar_no_extra_minion_data_val_dict():
 
 def test_ext_pillar_no_extra_minion_data_val_list():
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "json",
         "renderer_blacklist": [],
@@ -335,6 +339,8 @@ def test_ext_pillar_no_extra_minion_data_val_list():
 
 def test_ext_pillar_no_extra_minion_data_val_elem():
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "json",
         "renderer_blacklist": [],
@@ -376,6 +382,8 @@ def test_ext_pillar_no_extra_minion_data_val_elem():
 
 def test_ext_pillar_with_extra_minion_data_val_dict():
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "json",
         "renderer_blacklist": [],
@@ -422,6 +430,8 @@ def test_ext_pillar_with_extra_minion_data_val_dict():
 
 def test_ext_pillar_with_extra_minion_data_val_list():
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "json",
         "renderer_blacklist": [],
@@ -463,6 +473,8 @@ def test_ext_pillar_with_extra_minion_data_val_list():
 
 def test_ext_pillar_with_extra_minion_data_val_elem():
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "json",
         "renderer_blacklist": [],
@@ -507,15 +519,17 @@ def test_ext_pillar_first(tmp_path):
     test when using ext_pillar and ext_pillar_first
     """
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "yaml",
         "renderer_blacklist": [],
         "renderer_whitelist": [],
         "state_top": "",
-        "pillar_roots": [],
+        "pillar_roots": {},
         "extension_modules": "",
         "saltenv": "base",
-        "file_roots": [],
+        "file_roots": {},
         "ext_pillar_first": True,
         "fileserver_backend": "",
         "cachedir": "",
@@ -561,8 +575,9 @@ def test_malformed_pillar_sls(mock_list_states):
             "renderer_blacklist": [],
             "renderer_whitelist": [],
             "state_top": "",
-            "pillar_roots": [],
-            "file_roots": [],
+            "pillar_roots": {},
+            "file_roots": {},
+
             "extension_modules": "",
             "fileserver_backend": "",
             "cachedir": "",
@@ -661,6 +676,8 @@ def test_malformed_pillar_sls(mock_list_states):
 
 def test_includes_override_sls():
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "json",
         "renderer_blacklist": [],
@@ -669,7 +686,7 @@ def test_includes_override_sls():
         "pillar_roots": {},
         "file_roots": {},
         "extension_modules": "",
-        "fileserver_backend": "roots",
+        "fileserver_backend": ["roots"],
         "cachedir": "",
     }
     grains = {
@@ -723,16 +740,18 @@ def test_includes_override_sls():
 
 def test_topfile_order():
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "yaml",
         "renderer_blacklist": [],
         "renderer_whitelist": [],
         "state_top": "",
-        "pillar_roots": [],
+        "pillar_roots": {},
         "extension_modules": "",
         "saltenv": "base",
-        "file_roots": [],
-        "fileserver_backend": "roots",
+        "file_roots": {},
+        "fileserver_backend": ["roots"],
         "cachedir": "",
     }
     grains = {
@@ -897,6 +916,8 @@ def test_relative_include(tmp_path):
             file=f,
         )
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "yaml",
         "renderer_blacklist": [],
@@ -905,10 +926,10 @@ def test_relative_include(tmp_path):
         "pillar_roots": {"base": [str(tmp_path)]},
         "extension_modules": "",
         "saltenv": "base",
-        "file_roots": [],
+        "file_roots": {},
         "file_ignore_regex": None,
         "file_ignore_glob": None,
-        "fileserver_backend": "roots",
+        "fileserver_backend": ["roots"],
         "cachedir": "",
     }
     grains = {
@@ -938,6 +959,8 @@ def test_relative_include(tmp_path):
 
 def test_missing_include(tmp_path):
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "yaml",
         "renderer_blacklist": [],
@@ -946,10 +969,10 @@ def test_missing_include(tmp_path):
         "pillar_roots": {"base": [str(tmp_path)]},
         "extension_modules": "",
         "saltenv": "base",
-        "file_roots": [],
+        "file_roots": {},
         "file_ignore_regex": None,
         "file_ignore_glob": None,
-        "fileserver_backend": "roots",
+        "fileserver_backend": ["roots"],
         "cachedir": "",
     }
     grains = {
@@ -1126,16 +1149,18 @@ def test_pillar_send_extra_minion_data_from_config(tmp_pki, grains):
 
 def test_include(tmp_path):
     opts = {
+        "file_client": "local",
+        "pillar": {},
         "optimization_order": [0, 1, 2],
         "renderer": "yaml",
         "renderer_blacklist": [],
         "renderer_whitelist": [],
         "state_top": "",
-        "pillar_roots": [],
+        "pillar_roots": {},
         "extension_modules": "",
         "saltenv": "base",
-        "file_roots": [],
-        "fileserver_backend": "roots",
+        "file_roots": {},
+        "fileserver_backend": ["roots"],
         "cachedir": "",
     }
     grains = {