diff --git a/changelog/62953.fixed b/changelog/62953.fixed new file mode 100644 index 00000000000..df38b5c4ab0 --- /dev/null +++ b/changelog/62953.fixed @@ -0,0 +1 @@ +Fix file.symlink backupname operation can copy remote contents to local disk diff --git a/salt/modules/file.py b/salt/modules/file.py index 5b4d2b6ec7b..518b5af5ae3 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -7416,16 +7416,29 @@ def join(*args): return os.path.join(*args) -def move(src, dst): +def move(src, dst, disallow_copy_and_unlink=False): """ Move a file or directory + disallow_copy_and_unlink + If ``True``, the operation is offloaded to the ``file.rename`` execution + module function. This will use ``os.rename`` underneath, which will fail + in the event that ``src`` and ``dst`` are on different filesystems. If + ``False`` (the default), ``shutil.move`` will be used in order to fall + back on a "copy then unlink" approach, which is required for moving + across filesystems. + + .. versionadded:: 3006.0 + CLI Example: .. code-block:: bash salt '*' file.move /path/to/src /path/to/dst """ + if disallow_copy_and_unlink: + return rename(src, dst) + src = os.path.expanduser(src) dst = os.path.expanduser(dst) diff --git a/salt/states/file.py b/salt/states/file.py index e3f53b984d1..b3a8cec3412 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -1536,6 +1536,7 @@ def symlink( win_deny_perms=None, win_inheritance=None, atomic=False, + disallow_copy_and_unlink=False, **kwargs ): """ @@ -1618,6 +1619,17 @@ def symlink( atomic Use atomic file operation to create the symlink. + .. versionadded:: 3006.0 + + disallow_copy_and_unlink + Only used if ``backupname`` is used and the name of the symlink exists + and is not a symlink. If set to ``True``, the operation is offloaded to + the ``file.rename`` execution module function. This will use + ``os.rename`` underneath, which will fail in the event that ``src`` and + ``dst`` are on different filesystems. If ``False`` (the default), + ``shutil.move`` will be used in order to fall back on a "copy then + unlink" approach, which is required for moving across filesystems. + .. versionadded:: 3006.0 """ name = os.path.expanduser(name) @@ -1820,7 +1832,9 @@ def symlink( else: __salt__["file.remove"](backupname) try: - __salt__["file.move"](name, backupname) + __salt__["file.move"]( + name, backupname, disallow_copy_and_unlink=disallow_copy_and_unlink + ) except Exception as exc: # pylint: disable=broad-except ret["changes"] = {} log.debug( diff --git a/tests/pytests/unit/modules/file/test_file_module.py b/tests/pytests/unit/modules/file/test_file_module.py index b9f6d3cbfe6..435b1e98ccd 100644 --- a/tests/pytests/unit/modules/file/test_file_module.py +++ b/tests/pytests/unit/modules/file/test_file_module.py @@ -589,3 +589,22 @@ def test_stats(): ret = filemod.stats("dummy", None, True) assert ret["mode"] == "0644" assert ret["type"] == "file" + + +def test_file_move_disallow_copy_and_unlink(): + mock_shutil_move = MagicMock() + mock_os_rename = MagicMock() + with patch("os.path.expanduser", MagicMock(side_effect=lambda path: path)), patch( + "os.path.isabs", MagicMock(return_value=True) + ), patch("shutil.move", mock_shutil_move), patch("os.rename", mock_os_rename): + ret = filemod.move("source", "dest", disallow_copy_and_unlink=False) + mock_shutil_move.assert_called_once() + mock_os_rename.assert_not_called() + assert ret["result"] is True + + mock_shutil_move.reset_mock() + + ret = filemod.move("source", "dest", disallow_copy_and_unlink=True) + mock_os_rename.assert_called_once() + mock_shutil_move.assert_not_called() + assert ret is True