From 3cbcc3986ca647dbd732e4bb09758843f0a1896a Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sat, 4 Jul 2026 16:29:06 -0400 Subject: [PATCH 1/2] Fix minionfs raising when minions cache dir is missing Under the salt-ssh shim the master cachedir is a fresh temp dir with no 'minions' subdirectory, so file_list and dir_list called os.listdir(minions_cache_dir) on a non-existent path and raised FileNotFoundError, killing state.highstate. Guard the listdir calls by returning an empty list when the minions cache directory does not exist. Fixes #50351 --- changelog/50351.fixed.md | 1 + salt/fileserver/minionfs.py | 14 +++++++ .../pytests/unit/fileserver/test_minionfs.py | 39 +++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 changelog/50351.fixed.md create mode 100644 tests/pytests/unit/fileserver/test_minionfs.py diff --git a/changelog/50351.fixed.md b/changelog/50351.fixed.md new file mode 100644 index 000000000000..06c562562172 --- /dev/null +++ b/changelog/50351.fixed.md @@ -0,0 +1 @@ +salt-ssh: fix minionfs raising when minions cache dir is missing diff --git a/salt/fileserver/minionfs.py b/salt/fileserver/minionfs.py index 0cc77994fc2c..a8f30a3b4715 100644 --- a/salt/fileserver/minionfs.py +++ b/salt/fileserver/minionfs.py @@ -238,6 +238,13 @@ def file_list(load): prefix = prefix[len(mountpoint + os.path.sep) :] minions_cache_dir = os.path.join(__opts__["cachedir"], "minions") + if not os.path.isdir(minions_cache_dir): + # The minions cache dir may not exist yet (e.g. under the salt-ssh + # shim, where the cachedir is a fresh temp dir with no pushed files). + log.debug( + "minionfs: minions cache directory %s does not exist", minions_cache_dir + ) + return [] minion_dirs = os.listdir(minions_cache_dir) # If the prefix is not an empty string, then get the minion id from it. The @@ -314,6 +321,13 @@ def dir_list(load): prefix = prefix[len(mountpoint + os.path.sep) :] minions_cache_dir = os.path.join(__opts__["cachedir"], "minions") + if not os.path.isdir(minions_cache_dir): + # The minions cache dir may not exist yet (e.g. under the salt-ssh + # shim, where the cachedir is a fresh temp dir with no pushed files). + log.debug( + "minionfs: minions cache directory %s does not exist", minions_cache_dir + ) + return [] minion_dirs = os.listdir(minions_cache_dir) # If the prefix is not an empty string, then get the minion id from it. The diff --git a/tests/pytests/unit/fileserver/test_minionfs.py b/tests/pytests/unit/fileserver/test_minionfs.py new file mode 100644 index 000000000000..a169a4bdf64e --- /dev/null +++ b/tests/pytests/unit/fileserver/test_minionfs.py @@ -0,0 +1,39 @@ +import os + +import pytest + +import salt.fileserver.minionfs as minionfs + + +@pytest.fixture +def configure_loader_modules(tmp_path): + opts = { + "cachedir": str(tmp_path), + "minionfs_env": "base", + "minionfs_mountpoint": "", + "minionfs_whitelist": [], + "minionfs_blacklist": [], + "file_ignore_regex": [], + "file_ignore_glob": [], + } + return {minionfs: {"__opts__": opts}} + + +def test_file_list_missing_minions_cache_dir(): + """ + file_list should return an empty list rather than raising when the + minions cache directory does not exist (e.g. under the salt-ssh shim). + """ + minions_cache_dir = os.path.join(minionfs.__opts__["cachedir"], "minions") + assert not os.path.isdir(minions_cache_dir) + assert minionfs.file_list({"saltenv": "base"}) == [] + + +def test_dir_list_missing_minions_cache_dir(): + """ + dir_list should return an empty list rather than raising when the + minions cache directory does not exist (e.g. under the salt-ssh shim). + """ + minions_cache_dir = os.path.join(minionfs.__opts__["cachedir"], "minions") + assert not os.path.isdir(minions_cache_dir) + assert minionfs.dir_list({"saltenv": "base"}) == [] From 1a83de235524f7af1266f9986af8d2f973222960 Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sun, 5 Jul 2026 23:21:40 -0400 Subject: [PATCH 2/2] Add direct and inverse regression tests for minionfs missing minions cache dir The direct tests call minionfs.file_list and minionfs.dir_list with the exact load shape production sends (fileclient always includes a prefix and cmd key alongside saltenv), using a non-empty prefix since the prefix handling sits below the os.listdir() call that used to raise when the cache dir was absent. The inverse tests guard against overcorrection: with an existing, populated minions cache dir, both functions must still return the pushed files and directories rather than short-circuiting to an empty list. --- .../pytests/unit/fileserver/test_minionfs.py | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/pytests/unit/fileserver/test_minionfs.py b/tests/pytests/unit/fileserver/test_minionfs.py index a169a4bdf64e..3a5638aa0040 100644 --- a/tests/pytests/unit/fileserver/test_minionfs.py +++ b/tests/pytests/unit/fileserver/test_minionfs.py @@ -37,3 +37,60 @@ def test_dir_list_missing_minions_cache_dir(): minions_cache_dir = os.path.join(minionfs.__opts__["cachedir"], "minions") assert not os.path.isdir(minions_cache_dir) assert minionfs.dir_list({"saltenv": "base"}) == [] + + +def test_file_list_missing_minions_cache_dir_production_load_50351(): + """ + file_list must return an empty list rather than raising when the minions + cache directory is absent and the load carries the exact shape production + sends. + """ + # Production callers (fileclient.RemoteClient.file_list -> + # Fileserver.file_list -> backend) always include a "prefix" key (and + # "cmd") in the load. A non-empty prefix is the decisive case: the + # prefix-to-minion-ID handling sits below the os.listdir() call that + # used to raise FileNotFoundError, so it was never reached. + load = {"saltenv": "base", "prefix": "webserver/etc", "cmd": "_file_list"} + minions_cache_dir = os.path.join(minionfs.__opts__["cachedir"], "minions") + assert not os.path.isdir(minions_cache_dir) + assert minionfs.file_list(load) == [] + + +def test_dir_list_missing_minions_cache_dir_production_load_50351(): + """ + dir_list must return an empty list rather than raising when the minions + cache directory is absent and the load carries the exact shape production + sends. + """ + # Same production load shape as file_list: fileclient always sends + # "prefix" (and "cmd") alongside "saltenv". + load = {"saltenv": "base", "prefix": "webserver/etc", "cmd": "_dir_list"} + minions_cache_dir = os.path.join(minionfs.__opts__["cachedir"], "minions") + assert not os.path.isdir(minions_cache_dir) + assert minionfs.dir_list(load) == [] + + +def test_file_list_existing_minions_cache_dir_50351(tmp_path): + """ + Guard against overcorrection: when the minions cache directory exists + and holds pushed files, the missing-directory guard must not kick in. + file_list must still return the pushed files. + """ + files_dir = tmp_path / "minions" / "webserver" / "files" / "etc" + files_dir.mkdir(parents=True) + (files_dir / "some.conf").write_text("pushed") + load = {"saltenv": "base", "prefix": "", "cmd": "_file_list"} + assert minionfs.file_list(load) == [os.path.join("webserver", "etc", "some.conf")] + + +def test_dir_list_existing_minions_cache_dir_50351(tmp_path): + """ + Guard against overcorrection: when the minions cache directory exists + and holds pushed files, the missing-directory guard must not kick in. + dir_list must still return the pushed directories. + """ + files_dir = tmp_path / "minions" / "webserver" / "files" / "etc" + files_dir.mkdir(parents=True) + (files_dir / "some.conf").write_text("pushed") + load = {"saltenv": "base", "prefix": "", "cmd": "_dir_list"} + assert minionfs.dir_list(load) == [os.path.join("webserver", "etc")]