atomic_move - fix preserving extended acls (#82818)
* use copystat to copy as many attributes as possible before os.rename update unit test mocks for updated method of attribute preservation add integration test for lineinfile case remove erroneous `- meta: end_play` from lineinfile test suite * add keep_dest_attrs parameter to control whether src attributes are copied initially, and for existing destinations, whether the src is updated using the dest before being renamed consolidate with copy unsetting extended attrs ci_complete
This commit is contained in:
@ -32,6 +32,7 @@ def atomic_am(am, mocker):
|
||||
def atomic_mocks(mocker, monkeypatch):
|
||||
environ = dict()
|
||||
mocks = {
|
||||
'copystat': mocker.patch('shutil.copystat'),
|
||||
'chmod': mocker.patch('os.chmod'),
|
||||
'chown': mocker.patch('os.chown'),
|
||||
'close': mocker.patch('os.close'),
|
||||
@ -102,7 +103,7 @@ def test_existing_file(atomic_am, atomic_mocks, fake_stat, mocker, selinux):
|
||||
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
|
||||
|
||||
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
|
||||
assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
|
||||
assert atomic_mocks['copystat'].call_args_list == [mocker.call(b'/path/to/dest', b'/path/to/src')]
|
||||
|
||||
if selinux:
|
||||
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
|
||||
@ -125,7 +126,8 @@ def test_no_tty_fallback(atomic_am, atomic_mocks, fake_stat, mocker):
|
||||
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
|
||||
|
||||
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
|
||||
assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
|
||||
assert atomic_mocks['copystat'].call_args_list == [mocker.call(b'/path/to/dest', b'/path/to/src')]
|
||||
assert atomic_mocks['chmod'].call_args_list == []
|
||||
|
||||
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
|
||||
assert atomic_am.selinux_context.call_args_list == [mocker.call('/path/to/dest')]
|
||||
@ -134,19 +136,19 @@ def test_no_tty_fallback(atomic_am, atomic_mocks, fake_stat, mocker):
|
||||
@pytest.mark.parametrize('stdin', [{}], indirect=['stdin'])
|
||||
def test_existing_file_stat_failure(atomic_am, atomic_mocks, mocker):
|
||||
"""Failure to stat an existing file in order to copy permissions propogates the error (unless EPERM)"""
|
||||
atomic_mocks['stat'].side_effect = OSError()
|
||||
atomic_mocks['copystat'].side_effect = FileNotFoundError('testing os copystat with non EPERM error')
|
||||
atomic_mocks['path_exists'].return_value = True
|
||||
|
||||
with pytest.raises(OSError):
|
||||
with pytest.raises(FileNotFoundError, match='testing os copystat with non EPERM error'):
|
||||
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
|
||||
|
||||
|
||||
@pytest.mark.parametrize('stdin', [{}], indirect=['stdin'])
|
||||
def test_existing_file_stat_perms_failure(atomic_am, atomic_mocks, mocker):
|
||||
"""Failure to stat an existing file to copy the permissions due to permissions passes fine"""
|
||||
# and now have os.stat return EPERM, which should not fail
|
||||
# and now have os.copystat return EPERM, which should not fail
|
||||
mock_context = atomic_am.selinux_context.return_value
|
||||
atomic_mocks['stat'].side_effect = OSError(errno.EPERM, 'testing os stat with EPERM')
|
||||
atomic_mocks['copystat'].side_effect = OSError(errno.EPERM, 'testing os copystat with EPERM')
|
||||
atomic_mocks['path_exists'].return_value = True
|
||||
atomic_am.selinux_enabled.return_value = True
|
||||
|
||||
@ -155,7 +157,7 @@ def test_existing_file_stat_perms_failure(atomic_am, atomic_mocks, mocker):
|
||||
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
|
||||
# FIXME: Should atomic_move() set a default permission value when it cannot retrieve the
|
||||
# existing file's permissions? (Right now it's up to the calling code.
|
||||
# assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
|
||||
assert atomic_mocks['copystat'].call_args_list == [mocker.call(b'/path/to/dest', b'/path/to/src')]
|
||||
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
|
||||
assert atomic_am.selinux_context.call_args_list == [mocker.call('/path/to/dest')]
|
||||
|
||||
@ -203,8 +205,6 @@ def test_rename_perms_fail_temp_succeeds(atomic_am, atomic_mocks, fake_stat, moc
|
||||
mock_context = atomic_am.selinux_default_context.return_value
|
||||
atomic_mocks['path_exists'].return_value = False
|
||||
atomic_mocks['rename'].side_effect = [OSError(errno.EPERM, 'failing with EPERM'), None]
|
||||
atomic_mocks['stat'].return_value = fake_stat
|
||||
atomic_mocks['stat'].side_effect = None
|
||||
atomic_mocks['mkstemp'].return_value = (None, '/path/to/tempfile')
|
||||
atomic_mocks['mkstemp'].side_effect = None
|
||||
atomic_am.selinux_enabled.return_value = selinux
|
||||
|
||||
Reference in New Issue
Block a user