diff --git a/changelog/47707.fixed.md b/changelog/47707.fixed.md new file mode 100644 index 000000000000..1160b27c0cd2 --- /dev/null +++ b/changelog/47707.fixed.md @@ -0,0 +1 @@ +Include the offending path in the "A valid directory was not specified" error raised by file.readdir and file.rmdir diff --git a/salt/modules/file.py b/salt/modules/file.py index dfb1c66625fe..834cd8437a3e 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -4192,7 +4192,7 @@ def readdir(path): raise SaltInvocationError("Dir path must be absolute.") if not os.path.isdir(path): - raise SaltInvocationError("A valid directory was not specified.") + raise SaltInvocationError(f"A valid directory was not specified: {path}") dirents = [".", ".."] dirents.extend(os.listdir(path)) @@ -4342,7 +4342,7 @@ def rmdir(path, recurse=False, verbose=False, older_than=None): raise SaltInvocationError("File path must be absolute.") if not os.path.isdir(path): - raise SaltInvocationError("A valid directory was not specified.") + raise SaltInvocationError(f"A valid directory was not specified: {path}") if older_than: now = time.time() diff --git a/tests/pytests/unit/modules/file/test_file_rmdir.py b/tests/pytests/unit/modules/file/test_file_rmdir.py index d40a50be50e4..22160f90ecb8 100644 --- a/tests/pytests/unit/modules/file/test_file_rmdir.py +++ b/tests/pytests/unit/modules/file/test_file_rmdir.py @@ -37,6 +37,54 @@ def test_file_rmdir_not_found_exception(): filemod.rmdir("/tmp/not_there") +def test_file_rmdir_not_found_exception_includes_path(): + with pytest.raises(SaltInvocationError, match="/tmp/not_there"): + filemod.rmdir("/tmp/not_there") + + +def test_file_readdir_not_found_exception_includes_path(): + with pytest.raises(SaltInvocationError, match="/tmp/not_there"): + filemod.readdir("/tmp/not_there") + + +def test_file_rmdir_not_found_includes_path_with_state_args_47707(): + # The file.rmdir state (salt/states/file.py) calls this as + # rmdir(name, recurse=recurse, verbose=True, older_than=older_than). + # verbose=True is the decisive flag: with it, removal failures are + # normally collected into the returned dict's "errors" list instead of + # raised, but an invalid directory must still raise, and the message + # must include the offending path. + with pytest.raises(SaltInvocationError, match="/tmp/not_there"): + filemod.rmdir("/tmp/not_there", recurse=True, verbose=True, older_than=None) + + +def test_file_rmdir_relative_path_error_unchanged_47707(): + """ + Guard against overcorrection: a relative path must still fail the + absolute-path check, not the valid-directory check changed for #47707. + """ + with pytest.raises(SaltInvocationError, match="must be absolute"): + filemod.rmdir("not_absolute") + + +def test_file_readdir_relative_path_error_unchanged_47707(): + """ + Guard against overcorrection: a relative path must still fail readdir's + absolute-path check, not the valid-directory check changed for #47707. + """ + with pytest.raises(SaltInvocationError, match="must be absolute"): + filemod.readdir("not_absolute") + + +def test_file_readdir_valid_directory_47707(tmp_path): + """ + Guard against overcorrection: readdir on an existing directory must + still return the directory listing without raising. + """ + (tmp_path / "afile").write_text("data") + assert filemod.readdir(str(tmp_path)) == [".", "..", "afile"] + + def test_file_rmdir_success_return(): with patch("os.rmdir", MagicMock(return_value=True)), patch( "os.path.isdir", MagicMock(return_value=True)