Skip to content

Commit

Permalink
fixes saltstack#62953 file.symlink backupname operation can copy remo…
Browse files Browse the repository at this point in the history
…te contents to local disk
  • Loading branch information
nicholasmhughes committed Oct 25, 2022
1 parent c20115c commit a74dde4
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog/62953.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix file.symlink backupname operation can copy remote contents to local disk
15 changes: 14 additions & 1 deletion salt/modules/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
16 changes: 15 additions & 1 deletion salt/states/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,7 @@ def symlink(
win_deny_perms=None,
win_inheritance=None,
atomic=False,
disallow_copy_and_unlink=False,
**kwargs
):
"""
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
19 changes: 19 additions & 0 deletions tests/pytests/unit/modules/file/test_file_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit a74dde4

Please sign in to comment.