Warn when salt-call --file-root/--pillar-root/--states-dir ignored (#68137)

changelog/68137.fixed.md

@@ -0,0 +1 @@
+Fixed ``salt-call`` silently ignoring ``--file-root``, ``--pillar-root``, and ``--states-dir`` when ``--local`` was not passed. These overrides only affect the local minion config and are clobbered by the master's values via the remote file client, so ``salt-call`` now emits a warning explaining that ``--local`` is required for the override to take effect.

salt/cli/call.py

@@ -1,4 +1,5 @@
 import os
+import sys
 
 import salt.cli.caller
 import salt.defaults.exitcodes
@@ -33,6 +34,29 @@ def run(self):
             states_dir = os.path.abspath(self.options.states_dir)
             self.config["states_dirs"] = [states_dir]
 
+        # Warn when the user passed local-roots overrides without --local.
+        # Without --local the remote file client retrieves state/pillar data
+        # from the master, which silently overwrites the local file_roots /
+        # pillar_roots / states_dirs configured above (see #68137).
+        if not self.options.local:
+            ignored = []
+            if self.options.file_root:
+                ignored.append("--file-root")
+            if self.options.pillar_root:
+                ignored.append("--pillar-root")
+            if self.options.states_dir:
+                ignored.append("--states-dir")
+            if ignored:
+                sys.stderr.write(
+                    "Warning: {} {} ignored because --local was not "
+                    "specified; the remote file client retrieves these "
+                    "from the master. Re-run with --local to use the "
+                    "local override.\n".format(
+                        ", ".join(ignored),
+                        "is" if len(ignored) == 1 else "are",
+                    )
+                )
+
         if self.options.local:
             self.config["file_client"] = "local"
         if self.options.master:

tests/pytests/unit/cli/test_call.py

@@ -153,3 +153,162 @@ def test_check_user_called_with_cli_override():
         assert salt_call.config["user"] == "custom_user"
         # check_user called with override value
         mock_check_user.assert_called_with("custom_user")
+
+
+def _make_salt_call_for_roots_check(file_root, pillar_root, states_dir, local):
+    """
+    Build a SaltCall instance wired up to skip everything except the
+    --file-root / --pillar-root / --states-dir handling in ``run()``.
+    """
+    salt_call = salt.cli.call.SaltCall()
+    salt_call.config = {
+        "verify_env": False,
+        "pki_dir": "/etc/salt/pki",
+        "cachedir": "/var/cache/salt",
+        "extension_modules": "/var/cache/salt/extmods",
+        "user": "root",
+        "sudo_user": None,
+        "permissive_pki_access": False,
+    }
+    salt_call.options = MagicMock()
+    salt_call.options.user = None
+    salt_call.options.master = None
+    salt_call.options.doc = False
+    salt_call.options.grains_run = False
+    salt_call.options.local = local
+    salt_call.options.file_root = file_root
+    salt_call.options.pillar_root = pillar_root
+    salt_call.options.states_dir = states_dir
+    salt_call.options.cachedir = None
+    return salt_call
+
+
+def test_file_root_without_local_warns(capsys):
+    """
+    Regression test for #68137: using --file-root without --local should not
+    silently do nothing — the user must be told that the remote file client
+    will ignore the override.
+    """
+    with patch("salt.utils.parsers.SaltCallOptionParser.parse_args"), patch(
+        "salt.cli.caller.Caller"
+    ), patch("salt.utils.verify.verify_env"), patch(
+        "salt.utils.verify.check_user", return_value=True
+    ), patch(
+        "salt.utils.user.get_user", return_value="root"
+    ):
+        salt_call = _make_salt_call_for_roots_check(
+            file_root="/tmp/myroot",
+            pillar_root=None,
+            states_dir=None,
+            local=False,
+        )
+        salt_call.run()
+
+    captured = capsys.readouterr()
+    combined = captured.err + captured.out
+    assert "--file-root" in combined
+    assert "--local" in combined
+
+
+def test_pillar_root_without_local_warns(capsys):
+    """
+    Regression test for #68137: --pillar-root without --local must warn.
+    """
+    with patch("salt.utils.parsers.SaltCallOptionParser.parse_args"), patch(
+        "salt.cli.caller.Caller"
+    ), patch("salt.utils.verify.verify_env"), patch(
+        "salt.utils.verify.check_user", return_value=True
+    ), patch(
+        "salt.utils.user.get_user", return_value="root"
+    ):
+        salt_call = _make_salt_call_for_roots_check(
+            file_root=None,
+            pillar_root="/tmp/mypillar",
+            states_dir=None,
+            local=False,
+        )
+        salt_call.run()
+
+    captured = capsys.readouterr()
+    combined = captured.err + captured.out
+    assert "--pillar-root" in combined
+    assert "--local" in combined
+
+
+def test_states_dir_without_local_warns(capsys):
+    """
+    Regression test for #68137: --states-dir without --local must warn.
+    """
+    with patch("salt.utils.parsers.SaltCallOptionParser.parse_args"), patch(
+        "salt.cli.caller.Caller"
+    ), patch("salt.utils.verify.verify_env"), patch(
+        "salt.utils.verify.check_user", return_value=True
+    ), patch(
+        "salt.utils.user.get_user", return_value="root"
+    ):
+        salt_call = _make_salt_call_for_roots_check(
+            file_root=None,
+            pillar_root=None,
+            states_dir="/tmp/mystates",
+            local=False,
+        )
+        salt_call.run()
+
+    captured = capsys.readouterr()
+    combined = captured.err + captured.out
+    assert "--states-dir" in combined
+    assert "--local" in combined
+
+
+def test_file_root_with_local_does_not_warn(capsys):
+    """
+    Regression test for #68137: when --local is set, --file-root works as
+    intended and no warning should be emitted.
+    """
+    with patch("salt.utils.parsers.SaltCallOptionParser.parse_args"), patch(
+        "salt.cli.caller.Caller"
+    ), patch("salt.utils.verify.verify_env"), patch(
+        "salt.utils.verify.check_user", return_value=True
+    ), patch(
+        "salt.utils.user.get_user", return_value="root"
+    ):
+        salt_call = _make_salt_call_for_roots_check(
+            file_root="/tmp/myroot",
+            pillar_root="/tmp/mypillar",
+            states_dir="/tmp/mystates",
+            local=True,
+        )
+        salt_call.run()
+
+    captured = capsys.readouterr()
+    combined = captured.err + captured.out
+    assert "--local" not in combined
+    assert "ignored" not in combined.lower()
+
+
+def test_no_root_options_does_not_warn(capsys):
+    """
+    Regression test for #68137: when none of --file-root, --pillar-root,
+    --states-dir are passed, no warning should be emitted regardless of
+    --local.
+    """
+    with patch("salt.utils.parsers.SaltCallOptionParser.parse_args"), patch(
+        "salt.cli.caller.Caller"
+    ), patch("salt.utils.verify.verify_env"), patch(
+        "salt.utils.verify.check_user", return_value=True
+    ), patch(
+        "salt.utils.user.get_user", return_value="root"
+    ):
+        salt_call = _make_salt_call_for_roots_check(
+            file_root=None,
+            pillar_root=None,
+            states_dir=None,
+            local=False,
+        )
+        salt_call.run()
+
+    captured = capsys.readouterr()
+    combined = captured.err + captured.out
+    assert "--file-root" not in combined
+    assert "--pillar-root" not in combined
+    assert "--states-dir" not in combined