From 7e04157a2fc7256a840a48af2df26de7dbea07df Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 11 Jun 2026 17:47:19 -0700 Subject: [PATCH] Warn when salt-call --file-root/--pillar-root/--states-dir ignored salt-call's --file-root, --pillar-root, and --states-dir options silently had no observable effect when --local was not also passed. These flags only update the local minion's file_roots, pillar_roots, and states_dirs config; the remote file client then retrieves state and pillar data from the master and HighState.__gen_opts() in salt/state.py overwrites the local file_roots with master_opts' file_roots, so the user-supplied directories were ignored without any indication. Emit a single stderr warning naming the ignored option(s) and pointing the user at --local. No control-flow changes; the underlying config keys are still populated so behaviour with --local is unchanged. Fixes #68137 --- changelog/68137.fixed.md | 1 + salt/cli/call.py | 24 +++++ tests/pytests/unit/cli/test_call.py | 159 ++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+) create mode 100644 changelog/68137.fixed.md diff --git a/changelog/68137.fixed.md b/changelog/68137.fixed.md new file mode 100644 index 000000000000..d8d3d808af49 --- /dev/null +++ b/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. diff --git a/salt/cli/call.py b/salt/cli/call.py index 81a7113f0dff..71c8de4bac97 100644 --- a/salt/cli/call.py +++ b/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: diff --git a/tests/pytests/unit/cli/test_call.py b/tests/pytests/unit/cli/test_call.py index 9e8891ee17e1..9b358c3d4602 100644 --- a/tests/pytests/unit/cli/test_call.py +++ b/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