saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-17 10:10:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Merge pull request #43379 from twangboy/win_fix_file.managed

Browse source 
Fix file.managed on Windows with test=True
This commit is contained in:
Nicole Thomas 2017-12-07 16:10:42 -05:00 • committed by GitHub
commit abe089ad54
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 228 additions and 136 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
salt/modules/file.py
View file

@ -3368,7 +3368,11 @@ def stats(path, hash_type=None, follow_symlinks=True):
pstat = os.lstat(path)
except OSError:
# Not a broken symlink, just a nonexistent path
return ret
# NOTE: The file.directory state checks the content of the error
# message in this exception. Any changes made to the message for this
# exception will reflect the file.directory state as well, and will
# likely require changes there.
raise CommandExecutionError('Path not found: {0}'.format(path))
else:
if follow_symlinks:
pstat = os.stat(path)
@ -4193,12 +4197,6 @@ def check_perms(name, ret, user, group, mode, follow_symlinks=False):
# Check permissions
perms = {}
cur = stats(name, follow_symlinks=follow_symlinks)
if not cur:
# NOTE: The file.directory state checks the content of the error
# message in this exception. Any changes made to the message for this
# exception will reflect the file.directory state as well, and will
# likely require changes there.
raise CommandExecutionError('{0} does not exist'.format(name))
perms['luser'] = cur['user']
perms['lgroup'] = cur['group']
perms['lmode'] = salt.utils.normalize_mode(cur['mode'])
@ -4498,11 +4496,18 @@ def check_file_meta(
'''
changes = {}
if not source_sum:
source_sum = {}
lstats = stats(name, hash_type=source_sum.get('hash_type', None), follow_symlinks=False)
source_sum = dict()
try:
lstats = stats(name, hash_type=source_sum.get('hash_type', None),
follow_symlinks=False)
except CommandExecutionError:
lstats = {}
if not lstats:
changes['newfile'] = name
return changes
if 'hsum' in source_sum:
if source_sum['hsum'] != lstats['sum']:
if not sfn and source:

190
salt/modules/win_file.py
View file

@ -791,7 +791,7 @@ def chgrp(path, group):
def stats(path, hash_type='sha256', follow_symlinks=True):
'''
Return a dict containing the stats for a given file
Return a dict containing the stats about a given file
Under Windows, `gid` will equal `uid` and `group` will equal `user`.
@ -820,6 +820,8 @@ def stats(path, hash_type='sha256', follow_symlinks=True):
salt '*' file.stats /etc/passwd
'''
# This is to mirror the behavior of file.py. `check_file_meta` expects an
# empty dictionary when the file does not exist
if not os.path.exists(path):
raise CommandExecutionError('Path not found: {0}'.format(path))
@ -1227,33 +1229,37 @@ def mkdir(path,
path (str): The full path to the directory.
owner (str): The owner of the directory. If not passed, it will be the
account that created the directory, likely SYSTEM
owner (str):
The owner of the directory. If not passed, it will be the account
that created the directory, likely SYSTEM
grant_perms (dict): A dictionary containing the user/group and the basic
permissions to grant, ie: ``{'user': {'perms': 'basic_permission'}}``.
You can also set the ``applies_to`` setting here. The default is
``this_folder_subfolders_files``. Specify another ``applies_to`` setting
like this:
grant_perms (dict):
A dictionary containing the user/group and the basic permissions to
grant, ie: ``{'user': {'perms': 'basic_permission'}}``. You can also
set the ``applies_to`` setting here. The default is
``this_folder_subfolders_files``. Specify another ``applies_to``
setting like this:
.. code-block:: yaml
.. code-block:: yaml
{'user': {'perms': 'full_control', 'applies_to': 'this_folder'}}
{'user': {'perms': 'full_control', 'applies_to': 'this_folder'}}
To set advanced permissions use a list for the ``perms`` parameter, ie:
To set advanced permissions use a list for the ``perms`` parameter, ie:
.. code-block:: yaml
.. code-block:: yaml
{'user': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'this_folder'}}
{'user': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'this_folder'}}
deny_perms (dict): A dictionary containing the user/group and
permissions to deny along with the ``applies_to`` setting. Use the same
format used for the ``grant_perms`` parameter. Remember, deny
permissions supersede grant permissions.
deny_perms (dict):
A dictionary containing the user/group and permissions to deny along
with the ``applies_to`` setting. Use the same format used for the
``grant_perms`` parameter. Remember, deny permissions supersede
grant permissions.
inheritance (bool): If True the object will inherit permissions from the
parent, if False, inheritance will be disabled. Inheritance setting will
not apply to parent directories if they must be created
inheritance (bool):
If True the object will inherit permissions from the parent, if
False, inheritance will be disabled. Inheritance setting will not
apply to parent directories if they must be created
Returns:
bool: True if successful
@ -1312,33 +1318,37 @@ def makedirs_(path,
path (str): The full path to the directory.
owner (str): The owner of the directory. If not passed, it will be the
account that created the directly, likely SYSTEM
owner (str):
The owner of the directory. If not passed, it will be the account
that created the directly, likely SYSTEM
grant_perms (dict): A dictionary containing the user/group and the basic
permissions to grant, ie: ``{'user': {'perms': 'basic_permission'}}``.
You can also set the ``applies_to`` setting here. The default is
``this_folder_subfolders_files``. Specify another ``applies_to`` setting
like this:
grant_perms (dict):
A dictionary containing the user/group and the basic permissions to
grant, ie: ``{'user': {'perms': 'basic_permission'}}``. You can also
set the ``applies_to`` setting here. The default is
``this_folder_subfolders_files``. Specify another ``applies_to``
setting like this:
.. code-block:: yaml
.. code-block:: yaml
{'user': {'perms': 'full_control', 'applies_to': 'this_folder'}}
{'user': {'perms': 'full_control', 'applies_to': 'this_folder'}}
To set advanced permissions use a list for the ``perms`` parameter, ie:
To set advanced permissions use a list for the ``perms`` parameter, ie:
.. code-block:: yaml
.. code-block:: yaml
{'user': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'this_folder'}}
{'user': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'this_folder'}}
deny_perms (dict): A dictionary containing the user/group and
permissions to deny along with the ``applies_to`` setting. Use the same
format used for the ``grant_perms`` parameter. Remember, deny
permissions supersede grant permissions.
deny_perms (dict):
A dictionary containing the user/group and permissions to deny along
with the ``applies_to`` setting. Use the same format used for the
``grant_perms`` parameter. Remember, deny permissions supersede
grant permissions.
inheritance (bool): If True the object will inherit permissions from the
parent, if False, inheritance will be disabled. Inheritance setting will
not apply to parent directories if they must be created
inheritance (bool):
If True the object will inherit permissions from the parent, if
False, inheritance will be disabled. Inheritance setting will not
apply to parent directories if they must be created
.. note::
@ -1423,36 +1433,40 @@ def makedirs_perms(path,
path (str): The full path to the directory.
owner (str): The owner of the directory. If not passed, it will be the
account that created the directory, likely SYSTEM
owner (str):
The owner of the directory. If not passed, it will be the account
that created the directory, likely SYSTEM
grant_perms (dict): A dictionary containing the user/group and the basic
permissions to grant, ie: ``{'user': {'perms': 'basic_permission'}}``.
You can also set the ``applies_to`` setting here. The default is
``this_folder_subfolders_files``. Specify another ``applies_to`` setting
like this:
grant_perms (dict):
A dictionary containing the user/group and the basic permissions to
grant, ie: ``{'user': {'perms': 'basic_permission'}}``. You can also
set the ``applies_to`` setting here. The default is
``this_folder_subfolders_files``. Specify another ``applies_to``
setting like this:
.. code-block:: yaml
.. code-block:: yaml
{'user': {'perms': 'full_control', 'applies_to': 'this_folder'}}
{'user': {'perms': 'full_control', 'applies_to': 'this_folder'}}
To set advanced permissions use a list for the ``perms`` parameter, ie:
To set advanced permissions use a list for the ``perms`` parameter, ie:
.. code-block:: yaml
.. code-block:: yaml
{'user': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'this_folder'}}
{'user': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'this_folder'}}
deny_perms (dict): A dictionary containing the user/group and
permissions to deny along with the ``applies_to`` setting. Use the same
format used for the ``grant_perms`` parameter. Remember, deny
permissions supersede grant permissions.
deny_perms (dict):
A dictionary containing the user/group and permissions to deny along
with the ``applies_to`` setting. Use the same format used for the
``grant_perms`` parameter. Remember, deny permissions supersede
grant permissions.
inheritance (bool): If True the object will inherit permissions from the
parent, if False, inheritance will be disabled. Inheritance setting will
not apply to parent directories if they must be created
inheritance (bool):
If True the object will inherit permissions from the parent, if
False, inheritance will be disabled. Inheritance setting will not
apply to parent directories if they must be created
Returns:
bool: True if successful, otherwise raise an error
bool: True if successful, otherwise raises an error
CLI Example:
@ -1505,45 +1519,54 @@ def check_perms(path,
deny_perms=None,
inheritance=True):
'''
Set owner and permissions for each directory created.
Set owner and permissions for each directory created. Used mostly by the
state system.
Args:
path (str): The full path to the directory.
ret (dict): A dictionary to append changes to and return. If not passed,
will create a new dictionary to return.
ret (dict):
A dictionary to append changes to and return. If not passed, will
create a new dictionary to return.
owner (str): The owner of the directory. If not passed, it will be the
account that created the directory, likely SYSTEM
owner (str):
The owner of the directory. If not passed, it will be the account
that created the directory, likely SYSTEM
grant_perms (dict): A dictionary containing the user/group and the basic
permissions to grant, ie: ``{'user': {'perms': 'basic_permission'}}``.
You can also set the ``applies_to`` setting here. The default is
``this_folder_subfolders_files``. Specify another ``applies_to`` setting
like this:
grant_perms (dict):
A dictionary containing the user/group and the basic permissions to
grant, ie: ``{'user': {'perms': 'basic_permission'}}``. You can also
set the ``applies_to`` setting here. The default is
``this_folder_subfolders_files``. Specify another ``applies_to``
setting like this:
.. code-block:: yaml
.. code-block:: yaml
{'user': {'perms': 'full_control', 'applies_to': 'this_folder'}}
{'user': {'perms': 'full_control', 'applies_to': 'this_folder'}}
To set advanced permissions use a list for the ``perms`` parameter, ie:
To set advanced permissions use a list for the ``perms`` parameter, ie:
.. code-block:: yaml
.. code-block:: yaml
{'user': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'this_folder'}}
{'user': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'this_folder'}}
deny_perms (dict): A dictionary containing the user/group and
permissions to deny along with the ``applies_to`` setting. Use the same
format used for the ``grant_perms`` parameter. Remember, deny
permissions supersede grant permissions.
deny_perms (dict):
A dictionary containing the user/group and permissions to deny along
with the ``applies_to`` setting. Use the same format used for the
``grant_perms`` parameter. Remember, deny permissions supersede
grant permissions.
inheritance (bool): If True the object will inherit permissions from the
parent, if False, inheritance will be disabled. Inheritance setting will
not apply to parent directories if they must be created
inheritance (bool):
If True the object will inherit permissions from the parent, if
False, inheritance will be disabled. Inheritance setting will not
apply to parent directories if they must be created
Returns:
bool: True if successful, otherwise raise an error
dict: A dictionary of changes made to the object
Raises:
CommandExecutionError: If the object does not exist
CLI Example:
@ -1558,6 +1581,9 @@ def check_perms(path,
# Specify advanced attributes with a list
salt '*' file.check_perms C:\\Temp\\ Administrators "{'jsnuffy': {'perms': ['read_attributes', 'read_ea'], 'applies_to': 'files_only'}}"
'''
if not os.path.exists(path):
raise CommandExecutionError('Path not found: {0}'.format(path))
path = os.path.expanduser(path)
if not ret:

10
salt/states/file.py
View file

@ -758,7 +758,7 @@ def _check_directory_win(name,
changes = {}
if not os.path.isdir(name):
changes = {'directory': 'new'}
changes = {name: {'directory': 'new'}}
else:
# Check owner
owner = salt.utils.win_dacl.get_owner(name)
@ -883,7 +883,11 @@ def _check_dir_meta(name,
'''
Check the changes in directory metadata
'''
stats = __salt__['file.stats'](name, None, follow_symlinks)
try:
stats = __salt__['file.stats'](name, None, follow_symlinks)
except CommandExecutionError:
stats = {}
changes = {}
if not stats:
changes['directory'] = 'new'
@ -2988,7 +2992,7 @@ def directory(name,
ret, _ = __salt__['file.check_perms'](
full, ret, user, group, dir_mode, follow_symlinks)
except CommandExecutionError as exc:
if not exc.strerror.endswith('does not exist'):
if not exc.strerror.startswith('Path not found'):
errors.append(exc.strerror)
if clean:

83
tests/integration/states/test_file.py
View file

@ -67,14 +67,13 @@ def _test_managed_file_mode_keep_helper(testcase, local=False):
'''
DRY helper function to run the same test with a local or remote path
'''
rel_path = 'grail/scene33'
name = os.path.join(TMP, os.path.basename(rel_path))
grail_fs_path = os.path.join(FILES, 'file', 'base', rel_path)
grail = 'salt://' + rel_path if not local else grail_fs_path
name = os.path.join(TMP, 'scene33')
grail_fs_path = os.path.join(FILES, 'file', 'base', 'grail', 'scene33')
grail = 'salt://grail/scene33' if not local else grail_fs_path
# Get the current mode so that we can put the file back the way we
# found it when we're done.
grail_fs_mode = os.stat(grail_fs_path).st_mode
grail_fs_mode = int(testcase.run_function('file.get_mode', [grail_fs_path]), 8)
initial_mode = 504 # 0770 octal
new_mode_1 = 384 # 0600 octal
new_mode_2 = 420 # 0644 octal
@ -585,19 +584,29 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin):
try:
tmp_dir = os.path.join(TMP, 'pgdata')
sym_dir = os.path.join(TMP, 'pg_data')
os.mkdir(tmp_dir, 0o700)
os.symlink(tmp_dir, sym_dir)
ret = self.run_state(
'file.directory', test=True, name=sym_dir, follow_symlinks=True,
mode=700
)
if IS_WINDOWS:
self.run_function('file.mkdir', [tmp_dir, 'Administrators'])
else:
os.mkdir(tmp_dir, 0o700)
self.run_function('file.symlink', [tmp_dir, sym_dir])
if IS_WINDOWS:
ret = self.run_state(
'file.directory', test=True, name=sym_dir,
follow_symlinks=True, win_owner='Administrators')
else:
ret = self.run_state(
'file.directory', test=True, name=sym_dir,
follow_symlinks=True, mode=700)
self.assertSaltTrueReturn(ret)
finally:
if os.path.isdir(tmp_dir):
shutil.rmtree(tmp_dir)
self.run_function('file.remove', [tmp_dir])
if os.path.islink(sym_dir):
os.unlink(sym_dir)
self.run_function('file.remove', [sym_dir])
@skip_if_not_root
@skipIf(IS_WINDOWS, 'Mode not available in Windows')
@ -1592,25 +1601,24 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin):
'''
fname = 'append_issue_1864_makedirs'
name = os.path.join(TMP, fname)
try:
self.assertFalse(os.path.exists(name))
except AssertionError:
os.remove(name)
# Make sure the file is not there to begin with
if os.path.isfile(name):
self.run_function('file.remove', [name])
try:
# Non existing file get's touched
if os.path.isfile(name):
# left over
os.remove(name)
ret = self.run_state(
'file.append', name=name, text='cheese', makedirs=True
)
self.assertSaltTrueReturn(ret)
finally:
if os.path.isfile(name):
os.remove(name)
self.run_function('file.remove', [name])
# Nested directory and file get's touched
name = os.path.join(TMP, 'issue_1864', fname)
try:
ret = self.run_state(
'file.append', name=name, text='cheese', makedirs=True
@ -1618,20 +1626,17 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin):
self.assertSaltTrueReturn(ret)
finally:
if os.path.isfile(name):
os.remove(name)
self.run_function('file.remove', [name])
# Parent directory exists but file does not and makedirs is False
try:
# Parent directory exists but file does not and makedirs is False
ret = self.run_state(
'file.append', name=name, text='cheese'
)
self.assertSaltTrueReturn(ret)
self.assertTrue(os.path.isfile(name))
finally:
shutil.rmtree(
os.path.join(TMP, 'issue_1864'),
ignore_errors=True
)
self.run_function('file.remove', [os.path.join(TMP, 'issue_1864')])
def test_prepend_issue_27401_makedirs(self):
'''
@ -1966,19 +1971,21 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin):
ret = self.run_function('state.sls', mods='issue-8343')
for name, step in six.iteritems(ret):
self.assertSaltTrueReturn({name: step})
with salt.utils.fopen(testcase_filedest) as fp_:
contents = fp_.read().split(os.linesep)
self.assertEqual(
['#-- start salt managed zonestart -- PLEASE, DO NOT EDIT',
'foo',
'#-- end salt managed zonestart --',
'#',
'#-- start salt managed zoneend -- PLEASE, DO NOT EDIT',
'bar',
'#-- end salt managed zoneend --',
''],
contents
)
expected = [
'#-- start salt managed zonestart -- PLEASE, DO NOT EDIT',
'foo',
'#-- end salt managed zonestart --',
'#',
'#-- start salt managed zoneend -- PLEASE, DO NOT EDIT',
'bar',
'#-- end salt managed zoneend --',
'']
self.assertEqual(expected, contents)
finally:
if os.path.isdir(testcase_filedest):
os.unlink(testcase_filedest)

51
tests/unit/modules/test_win_file.py Normal file
View file

@ -0,0 +1,51 @@
# -*- coding: utf-8 -*-
'''
:codeauthor: :email:`Shane Lee <slee@saltstack.com>`
'''
# Import Python Libs
from __future__ import absolute_import
import os
# Import Salt Testing Libs
from tests.support.unit import TestCase, skipIf
from tests.support.mock import (
patch,
NO_MOCK,
NO_MOCK_REASON
)
# Import Salt Libs
import salt.modules.win_file as win_file
from salt.exceptions import CommandExecutionError
import salt.utils
@skipIf(NO_MOCK, NO_MOCK_REASON)
class WinFileTestCase(TestCase):
'''
Test cases for salt.modules.win_file
'''
FAKE_RET = {'fake': 'ret data'}
if salt.utils.is_windows():
FAKE_PATH = os.sep.join(['C:', 'path', 'does', 'not', 'exist'])
else:
FAKE_PATH = os.sep.join(['path', 'does', 'not', 'exist'])
def test_issue_43328_stats(self):
'''
Make sure that a CommandExecutionError is raised if the file does NOT
exist
'''
with patch('os.path.exists', return_value=False):
self.assertRaises(CommandExecutionError,
win_file.stats,
self.FAKE_PATH)
def test_issue_43328_check_perms_no_ret(self):
'''
Make sure that a CommandExecutionError is raised if the file does NOT
exist
'''
with patch('os.path.exists', return_value=False):
self.assertRaises(
CommandExecutionError, win_file.check_perms, self.FAKE_PATH)

7
tests/unit/states/test_file.py
View file

@ -743,7 +743,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
mock_check = MagicMock(return_value=(
None,
'The directory "{0}" will be changed'.format(name),
{'directory': 'new'}))
{name: {'directory': 'new'}}))
mock_error = CommandExecutionError
with patch.dict(filestate.__salt__, {'config.manage_mode': mock_t,
'file.user_to_uid': mock_uid,
@ -801,16 +801,15 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
group=group),
ret)
with patch.object(os.path, 'isfile', mock_f):
with patch.object(os.path, 'isdir', mock_f):
with patch.dict(filestate.__opts__, {'test': True}):
if salt.utils.is_windows():
comt = 'The directory "{0}" will be changed' \
''.format(name)
p_chg = {'directory': 'new'}
else:
comt = ('The following files will be changed:\n{0}:'
' directory - new\n'.format(name))
p_chg = {'/etc/grub.conf': {'directory': 'new'}}
p_chg = {'/etc/grub.conf': {'directory': 'new'}}
ret.update({
'comment': comt,
'result': None,