Additional fixes for windows root_dir logic

changelog/64728.fixed.md

@@ -1 +1 @@
-Cloud honors root_dir config settin when determining log file location
+salt-cloud honors root_dir config setting for log_file location and fixes for root_dir locations on windows.

salt/config/__init__.py

@@ -2218,6 +2218,7 @@ def include_config(include, orig_path, verbose, exit_on_config_errors=False):
 def should_prepend_root_dir(key, opts):
     return (
         key in opts
+        and opts[key] is not None
         and urllib.parse.urlparse(os.path.splitdrive(opts[key])[1]).scheme == ""
     )
 
@@ -2519,7 +2520,7 @@ def syndic_config(
     ]
     for config_key in ("log_file", "key_logfile", "syndic_log_file"):
         # If this is not a URI and instead a local path
-        if urllib.parse.urlparse(opts.get(config_key, "")).scheme == "":
+        if should_prepend_root_dir(config_key, opts):
             prepend_root_dirs.append(config_key)
     prepend_root_dir(opts, prepend_root_dirs)
     salt.features.setup_features(opts)
@@ -3842,7 +3843,7 @@ def apply_minion_config(
 
     # These can be set to syslog, so, not actual paths on the system
     for config_key in ("log_file", "key_logfile"):
-        if urllib.parse.urlparse(opts.get(config_key, "")).scheme == "":
+        if should_prepend_root_dir(config_key, opts):
             prepend_root_dirs.append(config_key)
 
     prepend_root_dir(opts, prepend_root_dirs)
@@ -4078,11 +4079,7 @@ def apply_master_config(overrides=None, defaults=None):
 
     # These can be set to syslog, so, not actual paths on the system
     for config_key in ("log_file", "key_logfile", "ssh_log_file"):
-        log_setting = opts.get(config_key, "")
+        if should_prepend_root_dir(config_key, opts):
-        if log_setting is None:
-            continue
-
-        if urllib.parse.urlparse(log_setting).scheme == "":
             prepend_root_dirs.append(config_key)
 
     prepend_root_dir(opts, prepend_root_dirs)
@@ -4289,11 +4286,7 @@ def apply_spm_config(overrides, defaults):
 
     # These can be set to syslog, so, not actual paths on the system
     for config_key in ("spm_logfile",):
-        log_setting = opts.get(config_key, "")
+        if should_prepend_root_dir(config_key, opts):
-        if log_setting is None:
-            continue
-
-        if urllib.parse.urlparse(log_setting).scheme == "":
             prepend_root_dirs.append(config_key)
 
     prepend_root_dir(opts, prepend_root_dirs)

tests/pytests/functional/test_config.py

@@ -27,27 +27,117 @@ def test_minion_config_type_check(caplog):
         os.remove(path)
 
 
-def test_cloud_config_relative_log_file(tmp_path):
+def test_cloud_config_relative_to_root_dir(tmp_path):
     root_path = tmp_path
     config_path = tmp_path / "conf"
     config_path.mkdir()
     cloud_config = config_path / "cloud"
     cloud_config.write_text("")
     master_config = config_path / "master"
-    master_config = config_path / "master"
     master_config.write_text(f"root_dir: {root_path}")
     opts = salt.config.cloud_config(cloud_config)
     assert opts["log_file"] == str(root_path / "var" / "log" / "salt" / "cloud")
+    assert opts["cachedir"] == str(root_path / "var" / "cache" / "salt" / "cloud")
 
 
-def test_cloud_config_relative_cachedir(tmp_path):
+def test_master_config_relative_to_root_dir(tmp_path):
     root_path = tmp_path
     config_path = tmp_path / "conf"
     config_path.mkdir()
-    cloud_config = config_path / "cloud"
-    cloud_config.write_text("")
-    master_config = config_path / "master"
     master_config = config_path / "master"
     master_config.write_text(f"root_dir: {root_path}")
-    opts = salt.config.cloud_config(cloud_config)
+    opts = salt.config.master_config(master_config)
-    assert opts["cachedir"] == str(root_path / "var" / "cache" / "salt" / "cloud")
+    assert opts["pki_dir"] == str(root_path / "etc" / "salt" / "pki" / "master")
+    assert opts["cachedir"] == str(root_path / "var" / "cache" / "salt" / "master")
+    assert opts["pidfile"] == str(root_path / "var" / "run" / "salt-master.pid")
+    assert opts["sock_dir"] == str(root_path / "var" / "run" / "salt" / "master")
+    assert opts["extension_modules"] == str(
+        root_path / "var" / "cache" / "salt" / "master" / "extmods"
+    )
+    assert opts["token_dir"] == str(
+        root_path / "var" / "cache" / "salt" / "master" / "tokens"
+    )
+    assert opts["syndic_dir"] == str(
+        root_path / "var" / "cache" / "salt" / "master" / "syndics"
+    )
+    assert opts["sqlite_queue_dir"] == str(
+        root_path / "var" / "cache" / "salt" / "master" / "queues"
+    )
+    assert opts["log_file"] == str(root_path / "var" / "log" / "salt" / "master")
+    assert opts["key_logfile"] == str(root_path / "var" / "log" / "salt" / "key")
+    assert opts["ssh_log_file"] == str(root_path / "var" / "log" / "salt" / "ssh")
+
+    # These are not tested because we didn't define them in the master config.
+    # assert opts["autosign_file"] == str(root_path / "var" / "run" / "salt"/ "master")
+    # assert opts["autoreject_file"] == str(root_path / "var" / "run" / "salt"/ "master")
+    # assert opts["autosign_grains_dir"] == str(root_path / "var" / "run" / "salt"/ "master")
+
+
+def test_minion_config_relative_to_root_dir(tmp_path):
+    root_path = tmp_path
+    config_path = tmp_path / "conf"
+    config_path.mkdir()
+    minion_config = config_path / "minion"
+    minion_config.write_text(f"root_dir: {root_path}")
+    opts = salt.config.minion_config(minion_config)
+    assert opts["pki_dir"] == str(root_path / "etc" / "salt" / "pki" / "minion")
+    assert opts["cachedir"] == str(root_path / "var" / "cache" / "salt" / "minion")
+    assert opts["pidfile"] == str(root_path / "var" / "run" / "salt-minion.pid")
+    assert opts["sock_dir"] == str(root_path / "var" / "run" / "salt" / "minion")
+    assert opts["extension_modules"] == str(
+        root_path / "var" / "cache" / "salt" / "minion" / "extmods"
+    )
+    assert opts["log_file"] == str(root_path / "var" / "log" / "salt" / "minion")
+
+
+def test_api_config_relative_to_root_dir(tmp_path):
+    root_path = tmp_path
+    config_path = tmp_path / "conf"
+    config_path.mkdir()
+    master_config = config_path / "master"
+    master_config.write_text(f"root_dir: {root_path}")
+    opts = salt.config.api_config(master_config)
+    assert opts["pidfile"] == str(root_path / "var" / "run" / "salt-api.pid")
+    assert opts["log_file"] == str(root_path / "var" / "log" / "salt" / "api")
+    assert opts["api_pidfile"] == str(root_path / "var" / "run" / "salt-api.pid")
+    assert opts["api_logfile"] == str(root_path / "var" / "log" / "salt" / "api")
+
+
+def test_spm_config_relative_to_root_dir(tmp_path):
+    root_path = tmp_path
+    config_path = tmp_path / "conf"
+    config_path.mkdir()
+    spm_config = config_path / "spm"
+    spm_config.write_text(f"root_dir: {root_path}")
+    opts = salt.config.spm_config(spm_config)
+
+    assert opts["formula_path"] == str(root_path / "srv" / "spm" / "salt")
+    assert opts["pillar_path"] == str(root_path / "srv" / "spm" / "pillar")
+    assert opts["reactor_path"] == str(root_path / "srv" / "spm" / "reactor")
+    assert opts["spm_cache_dir"] == str(root_path / "var" / "cache" / "salt" / "spm")
+    assert opts["spm_build_dir"] == str(root_path / "srv" / "spm_build")
+    assert opts["spm_logfile"] == str(root_path / "var" / "log" / "salt" / "spm")
+
+
+def test_syndic_config_relative_to_root_dir(tmp_path):
+    root_path = tmp_path
+    config_path = tmp_path / "conf"
+    config_path.mkdir()
+    master_config = config_path / "master"
+    master_config.write_text(f"root_dir: {root_path}")
+    minion_config = config_path / "master"
+    minion_config.write_text(f"root_dir: {root_path}")
+    opts = salt.config.syndic_config(master_config, minion_config)
+    assert opts["pki_dir"] == str(root_path / "etc" / "salt" / "pki" / "minion")
+    assert opts["cachedir"] == str(root_path / "var" / "cache" / "salt" / "master")
+    assert opts["pidfile"] == str(root_path / "var" / "run" / "salt-syndic.pid")
+    assert opts["sock_dir"] == str(root_path / "var" / "run" / "salt" / "minion")
+    assert opts["extension_modules"] == str(
+        root_path / "var" / "cache" / "salt" / "minion" / "extmods"
+    )
+    assert opts["token_dir"] == str(
+        root_path / "var" / "cache" / "salt" / "master" / "tokens"
+    )
+    assert opts["log_file"] == str(root_path / "var" / "log" / "salt" / "syndic")
+    assert opts["key_logfile"] == str(root_path / "var" / "log" / "salt" / "key")
+    assert opts["syndic_log_file"] == str(root_path / "var" / "log" / "salt" / "syndic")