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:

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,8 @@ 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 +675,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 +685,7 @@ def test_includes_override_sls():
         "pillar_roots": {},
         "file_roots": {},
         "extension_modules": "",
-        "fileserver_backend": "roots",
+        "fileserver_backend": ["roots"],
         "cachedir": "",
     }
     grains = {
@@ -723,16 +739,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 +915,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 +925,10 @@ def test_relative_include(tmp_path):
         "pillar_roots": {"base": [str(tmp_path)]},
         "extension_modules": "",
         "saltenv": "base",
-        "file_roots": [],
+        "file_roots": {"base": [str(tmp_path)]},
         "file_ignore_regex": None,
         "file_ignore_glob": None,
-        "fileserver_backend": "roots",
+        "fileserver_backend": ["roots"],
         "cachedir": "",
     }
     grains = {
@@ -928,6 +948,7 @@ def test_relative_include(tmp_path):
 
     # Assert
     assert compiled_pillar["this"] == "is all good"
+
     assert compiled_pillar["that"] == "is also all good"
     assert compiled_pillar["simple"] == "simon"
     assert compiled_pillar["super simple"] == "a caveman"
@@ -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": {"base": [str(tmp_path)]},
         "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 = {