From e9df2083a3ef91ce7a092a65368bb4a7eb92e0fe Mon Sep 17 00:00:00 2001 From: Pilou Date: Fri, 5 Jan 2018 05:33:14 +0100 Subject: [PATCH] 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! --- lib/ansible/module_utils/basic.py | 38 ++++++++++++++++--- .../basic/test_set_mode_if_different.py | 12 ++++-- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index a7eef260aab..47540ee5437 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1081,6 +1081,10 @@ class AnsibleModule(object): if not HAVE_SELINUX or not self.selinux_enabled(): return changed + + if self.check_file_absent_if_check_mode(path): + return True + cur_context = self.selinux_context(path) new_context = list(cur_context) # Iterate over the current context instead of the @@ -1119,11 +1123,17 @@ class AnsibleModule(object): return changed 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') if expand: 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) try: uid = int(owner) @@ -1154,11 +1164,17 @@ class AnsibleModule(object): return changed 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') if expand: 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) try: gid = int(group) @@ -1189,13 +1205,17 @@ class AnsibleModule(object): return changed 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') if expand: b_path = os.path.expanduser(os.path.expandvars(b_path)) path_stat = os.lstat(b_path) - if mode is None: - return changed + if self.check_file_absent_if_check_mode(b_path): + return True if not isinstance(mode, int): try: @@ -1273,6 +1293,9 @@ class AnsibleModule(object): if expand: 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) if existing.get('attr_flags', '') != attributes: @@ -1467,6 +1490,9 @@ class AnsibleModule(object): ) 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): return self.set_fs_attributes_if_different(file_args, changed, diff, expand) diff --git a/test/units/module_utils/basic/test_set_mode_if_different.py b/test/units/module_utils/basic/test_set_mode_if_different.py index a184e3a6447..7035f4c6879 100644 --- a/test/units/module_utils/basic/test_set_mode_if_different.py +++ b/test/units/module_utils/basic/test_set_mode_if_different.py @@ -49,16 +49,18 @@ def mock_lchmod(mocker): yield m_lchmod -@pytest.mark.parametrize('previous_changes, check_mode, stdin', - product((True, False), (True, False), ({},)), +@pytest.mark.parametrize('previous_changes, check_mode, exists, stdin', + product((True, False), (True, False), (True, False), ({},)), 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 mocker.patch('os.lstat', side_effect=[mock_stats['before']]) 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 not m_lchmod.called + assert not m_path_exists.called @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 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) + mocker.patch('os.path.exists', return_value=True) assert am.set_mode_if_different('/path/to/file', mode, False) 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 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) + mocker.patch('os.path.exists', return_value=True) assert not am.set_mode_if_different('/path/to/file', mode, False) 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.object(builtins, 'hasattr', side_effect=_hasattr) 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) 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.object(builtins, 'hasattr', side_effect=_hasattr) 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) mocker.patch('os.stat', return_value=mock_stats['after'])