If check mode enabled and file missing set changed to true 32676 (#33967)

* basic.py: add mock to os.path.exists

* set_*_if_different: if check_mode enabled & file missing: set changed to True

Fixes #32676
Thanks to mscherer and Spredzy for the distributed triplet programming
session!
This commit is contained in:
Pilou 2018-01-05 05:33:14 +01:00 committed by Toshio Kuratomi
parent 9f969a2176
commit e9df2083a3
2 changed files with 41 additions and 9 deletions

View file

@ -1081,6 +1081,10 @@ class AnsibleModule(object):
if not HAVE_SELINUX or not self.selinux_enabled(): if not HAVE_SELINUX or not self.selinux_enabled():
return changed return changed
if self.check_file_absent_if_check_mode(path):
return True
cur_context = self.selinux_context(path) cur_context = self.selinux_context(path)
new_context = list(cur_context) new_context = list(cur_context)
# Iterate over the current context instead of the # Iterate over the current context instead of the
@ -1119,11 +1123,17 @@ class AnsibleModule(object):
return changed return changed
def set_owner_if_different(self, path, owner, changed, diff=None, expand=True): def set_owner_if_different(self, path, owner, changed, diff=None, expand=True):
if owner is None:
return changed
b_path = to_bytes(path, errors='surrogate_or_strict') b_path = to_bytes(path, errors='surrogate_or_strict')
if expand: if expand:
b_path = os.path.expanduser(os.path.expandvars(b_path)) b_path = os.path.expanduser(os.path.expandvars(b_path))
if owner is None:
return changed if self.check_file_absent_if_check_mode(b_path):
return True
orig_uid, orig_gid = self.user_and_group(b_path, expand) orig_uid, orig_gid = self.user_and_group(b_path, expand)
try: try:
uid = int(owner) uid = int(owner)
@ -1154,11 +1164,17 @@ class AnsibleModule(object):
return changed return changed
def set_group_if_different(self, path, group, changed, diff=None, expand=True): def set_group_if_different(self, path, group, changed, diff=None, expand=True):
if group is None:
return changed
b_path = to_bytes(path, errors='surrogate_or_strict') b_path = to_bytes(path, errors='surrogate_or_strict')
if expand: if expand:
b_path = os.path.expanduser(os.path.expandvars(b_path)) b_path = os.path.expanduser(os.path.expandvars(b_path))
if group is None:
return changed if self.check_file_absent_if_check_mode(b_path):
return True
orig_uid, orig_gid = self.user_and_group(b_path, expand) orig_uid, orig_gid = self.user_and_group(b_path, expand)
try: try:
gid = int(group) gid = int(group)
@ -1189,13 +1205,17 @@ class AnsibleModule(object):
return changed return changed
def set_mode_if_different(self, path, mode, changed, diff=None, expand=True): def set_mode_if_different(self, path, mode, changed, diff=None, expand=True):
if mode is None:
return changed
b_path = to_bytes(path, errors='surrogate_or_strict') b_path = to_bytes(path, errors='surrogate_or_strict')
if expand: if expand:
b_path = os.path.expanduser(os.path.expandvars(b_path)) b_path = os.path.expanduser(os.path.expandvars(b_path))
path_stat = os.lstat(b_path) path_stat = os.lstat(b_path)
if mode is None: if self.check_file_absent_if_check_mode(b_path):
return changed return True
if not isinstance(mode, int): if not isinstance(mode, int):
try: try:
@ -1273,6 +1293,9 @@ class AnsibleModule(object):
if expand: if expand:
b_path = os.path.expanduser(os.path.expandvars(b_path)) b_path = os.path.expanduser(os.path.expandvars(b_path))
if self.check_file_absent_if_check_mode(b_path):
return True
existing = self.get_file_attributes(b_path) existing = self.get_file_attributes(b_path)
if existing.get('attr_flags', '') != attributes: if existing.get('attr_flags', '') != attributes:
@ -1467,6 +1490,9 @@ class AnsibleModule(object):
) )
return changed return changed
def check_file_absent_if_check_mode(self, file_path):
return self.check_mode and not os.path.exists(file_path)
def set_directory_attributes_if_different(self, file_args, changed, diff=None, expand=True): def set_directory_attributes_if_different(self, file_args, changed, diff=None, expand=True):
return self.set_fs_attributes_if_different(file_args, changed, diff, expand) return self.set_fs_attributes_if_different(file_args, changed, diff, expand)

View file

@ -49,16 +49,18 @@ def mock_lchmod(mocker):
yield m_lchmod yield m_lchmod
@pytest.mark.parametrize('previous_changes, check_mode, stdin', @pytest.mark.parametrize('previous_changes, check_mode, exists, stdin',
product((True, False), (True, False), ({},)), product((True, False), (True, False), (True, False), ({},)),
indirect=['stdin']) indirect=['stdin'])
def test_no_mode_given_returns_previous_changes(am, mock_stats, mock_lchmod, mocker, previous_changes, check_mode): def test_no_mode_given_returns_previous_changes(am, mock_stats, mock_lchmod, mocker, previous_changes, check_mode, exists):
am.check_mode = check_mode am.check_mode = check_mode
mocker.patch('os.lstat', side_effect=[mock_stats['before']]) mocker.patch('os.lstat', side_effect=[mock_stats['before']])
m_lchmod = mocker.patch('os.lchmod', return_value=None, create=True) m_lchmod = mocker.patch('os.lchmod', return_value=None, create=True)
m_path_exists = mocker.patch('os.path.exists', return_value=exists)
assert am.set_mode_if_different('/path/to/file', None, previous_changes) == previous_changes assert am.set_mode_if_different('/path/to/file', None, previous_changes) == previous_changes
assert not m_lchmod.called assert not m_lchmod.called
assert not m_path_exists.called
@pytest.mark.parametrize('mode, check_mode, stdin', @pytest.mark.parametrize('mode, check_mode, stdin',
@ -71,6 +73,7 @@ def test_mode_changed_to_0660(am, mock_stats, mocker, mode, check_mode):
am.check_mode = check_mode am.check_mode = check_mode
mocker.patch('os.lstat', side_effect=[mock_stats['before'], mock_stats['after'], mock_stats['after']]) mocker.patch('os.lstat', side_effect=[mock_stats['before'], mock_stats['after'], mock_stats['after']])
m_lchmod = mocker.patch('os.lchmod', return_value=None, create=True) m_lchmod = mocker.patch('os.lchmod', return_value=None, create=True)
mocker.patch('os.path.exists', return_value=True)
assert am.set_mode_if_different('/path/to/file', mode, False) assert am.set_mode_if_different('/path/to/file', mode, False)
if check_mode: if check_mode:
@ -89,6 +92,7 @@ def test_mode_unchanged_when_already_0660(am, mock_stats, mocker, mode, check_mo
am.check_mode = check_mode am.check_mode = check_mode
mocker.patch('os.lstat', side_effect=[mock_stats['after'], mock_stats['after'], mock_stats['after']]) mocker.patch('os.lstat', side_effect=[mock_stats['after'], mock_stats['after'], mock_stats['after']])
m_lchmod = mocker.patch('os.lchmod', return_value=None, create=True) m_lchmod = mocker.patch('os.lchmod', return_value=None, create=True)
mocker.patch('os.path.exists', return_value=True)
assert not am.set_mode_if_different('/path/to/file', mode, False) assert not am.set_mode_if_different('/path/to/file', mode, False)
assert not m_lchmod.called assert not m_lchmod.called
@ -111,6 +115,7 @@ def test_missing_lchmod_is_not_link(am, mock_stats, mocker, check_mode):
mocker.patch('os.lstat', side_effect=[mock_stats['before'], mock_stats['after']]) mocker.patch('os.lstat', side_effect=[mock_stats['before'], mock_stats['after']])
mocker.patch.object(builtins, 'hasattr', side_effect=_hasattr) mocker.patch.object(builtins, 'hasattr', side_effect=_hasattr)
mocker.patch('os.path.islink', return_value=False) mocker.patch('os.path.islink', return_value=False)
mocker.patch('os.path.exists', return_value=True)
m_chmod = mocker.patch('os.chmod', return_value=None) m_chmod = mocker.patch('os.chmod', return_value=None)
assert am.set_mode_if_different('/path/to/file/no_lchmod', 0o660, False) assert am.set_mode_if_different('/path/to/file/no_lchmod', 0o660, False)
@ -137,6 +142,7 @@ def test_missing_lchmod_is_link(am, mock_stats, mocker, check_mode):
mocker.patch('os.lstat', side_effect=[mock_stats['before'], mock_stats['after']]) mocker.patch('os.lstat', side_effect=[mock_stats['before'], mock_stats['after']])
mocker.patch.object(builtins, 'hasattr', side_effect=_hasattr) mocker.patch.object(builtins, 'hasattr', side_effect=_hasattr)
mocker.patch('os.path.islink', return_value=True) mocker.patch('os.path.islink', return_value=True)
mocker.patch('os.path.exists', return_value=True)
m_chmod = mocker.patch('os.chmod', return_value=None) m_chmod = mocker.patch('os.chmod', return_value=None)
mocker.patch('os.stat', return_value=mock_stats['after']) mocker.patch('os.stat', return_value=mock_stats['after'])