Skip to content

Commit

Permalink
Merge pull request #29708 from lagesag/fix-file-directory-test-mode
Browse files Browse the repository at this point in the history
Fix test=True for file.directory with recurse ignore_files/ignore_dirs.
  • Loading branch information
Mike Place committed Dec 15, 2015
2 parents 7c38dec + a872b5e commit aba82ab
Showing 1 changed file with 120 additions and 103 deletions.
223 changes: 120 additions & 103 deletions salt/states/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,37 +468,42 @@ def _check_directory(name,
walk_d[i[0]] = (i[1], i[2])

if recurse:
if not set(['user', 'group', 'mode']) >= set(recurse):
return False, 'Types for "recurse" limited to "user", ' \
'"group" and "mode"'
if 'user' not in recurse:
try:
recurse_set = _get_recurse_set(recurse)
except (TypeError, ValueError) as exc:
return False, '{0}'.format(exc)
if 'user' not in recurse_set:
user = None
if 'group' not in recurse:
if 'group' not in recurse_set:
group = None
if 'mode' not in recurse:
if 'mode' not in recurse_set:
mode = None
check_files = 'ignore_files' not in recurse_set
check_dirs = 'ignore_dirs' not in recurse_set
for root, dirs, files in walk_l:
for fname in files:
fchange = {}
path = os.path.join(root, fname)
stats = __salt__['file.stats'](
path, None, follow_symlinks=False
)
if user is not None and user != stats.get('user'):
fchange['user'] = user
if group is not None and group != stats.get('group'):
fchange['group'] = group
if fchange:
changes[path] = fchange
for name_ in dirs:
path = os.path.join(root, name_)
fchange = _check_dir_meta(path, user, group, mode)
if fchange:
changes[path] = fchange
else:
fchange = _check_dir_meta(name, user, group, mode)
if fchange:
changes[name] = fchange
if check_files:
for fname in files:
fchange = {}
path = os.path.join(root, fname)
stats = __salt__['file.stats'](
path, None, follow_symlinks=False
)
if user is not None and user != stats.get('user'):
fchange['user'] = user
if group is not None and group != stats.get('group'):
fchange['group'] = group
if fchange:
changes[path] = fchange
if check_dirs:
for name_ in dirs:
path = os.path.join(root, name_)
fchange = _check_dir_meta(path, user, group, mode)
if fchange:
changes[path] = fchange
# Recurse skips root (we always do dirs, not root), so always check root:
fchange = _check_dir_meta(name, user, group, mode)
if fchange:
changes[name] = fchange
if clean:
keep = _gen_keep_files(name, require, walk_d)

Expand Down Expand Up @@ -1581,6 +1586,32 @@ def managed(name,
os.remove(tmp_filename)


_RECURSE_TYPES = ['user', 'group', 'mode', 'ignore_files', 'ignore_dirs']


def _get_recurse_set(recurse):
'''
Converse *recurse* definition to a set of strings.
Raises TypeError or ValueError when *recurse* has wrong structure.
'''
if not recurse:
return set()
if not isinstance(recurse, list):
raise TypeError('"recurse" must be formed as a list of strings')
try:
recurse_set = set(recurse)
except TypeError: # non-hashable elements
recurse_set = None
if recurse_set is None or not set(_RECURSE_TYPES) >= recurse_set:
raise ValueError('Types for "recurse" limited to {0}.'.format(
', '.join('"{0}"'.format(rtype) for rtype in _RECURSE_TYPES)))
if 'ignore_files' in recurse_set and 'ignore_dirs' in recurse_set:
raise ValueError('Must not specify "recurse" options "ignore_files"'
' and "ignore_dirs" at the same time.')
return recurse_set


def directory(name,
user=None,
group=None,
Expand Down Expand Up @@ -1831,91 +1862,77 @@ def directory(name,
for i in walk_l:
walk_d[i[0]] = (i[1], i[2])

recurse_set = None
if recurse:
if not isinstance(recurse, list):
ret['result'] = False
ret['comment'] = '"recurse" must be formed as a list of strings'
elif not set(['user', 'group', 'mode', 'ignore_files',
'ignore_dirs']) >= set(recurse):
try:
recurse_set = _get_recurse_set(recurse)
except (TypeError, ValueError) as exc:
ret['result'] = False
ret['comment'] = 'Types for "recurse" limited to "user", ' \
'"group", "mode", "ignore_files, and "ignore_dirs"'
else:
if 'ignore_files' in recurse and 'ignore_dirs' in recurse:
ret['result'] = False
ret['comment'] = 'Can not specify "recurse" options "ignore_files" ' \
'and "ignore_dirs" at the same time.'
return ret

if 'user' in recurse:
if user:
uid = __salt__['file.user_to_uid'](user)
# file.user_to_uid returns '' if user does not exist. Above
# check for user is not fatal, so we need to be sure user
# exists.
if isinstance(uid, six.string_types):
ret['result'] = False
ret['comment'] = 'Failed to enforce ownership for ' \
'user {0} (user does not ' \
'exist)'.format(user)
else:
ret['comment'] = '{0}'.format(exc)
# NOTE: Should this be enough to stop the whole check altogether?
if recurse_set:
if 'user' in recurse_set:
if user:
uid = __salt__['file.user_to_uid'](user)
# file.user_to_uid returns '' if user does not exist. Above
# check for user is not fatal, so we need to be sure user
# exists.
if isinstance(uid, six.string_types):
ret['result'] = False
ret['comment'] = 'user not specified, but configured as ' \
'a target for recursive ownership ' \
'management'
ret['comment'] = 'Failed to enforce ownership for ' \
'user {0} (user does not ' \
'exist)'.format(user)
else:
user = None
if 'group' in recurse:
if group:
gid = __salt__['file.group_to_gid'](group)
# As above with user, we need to make sure group exists.
if isinstance(gid, six.string_types):
ret['result'] = False
ret['comment'] = 'Failed to enforce group ownership ' \
'for group {0}'.format(group)
else:
ret['result'] = False
ret['comment'] = 'user not specified, but configured as ' \
'a target for recursive ownership ' \
'management'
else:
user = None
if 'group' in recurse_set:
if group:
gid = __salt__['file.group_to_gid'](group)
# As above with user, we need to make sure group exists.
if isinstance(gid, six.string_types):
ret['result'] = False
ret['comment'] = 'group not specified, but configured ' \
'as a target for recursive ownership ' \
'management'
ret['comment'] = 'Failed to enforce group ownership ' \
'for group {0}'.format(group)
else:
group = None
ret['result'] = False
ret['comment'] = 'group not specified, but configured ' \
'as a target for recursive ownership ' \
'management'
else:
group = None

if 'mode' not in recurse:
file_mode = None
dir_mode = None
if 'mode' not in recurse_set:
file_mode = None
dir_mode = None

if 'ignore_files' in recurse:
ignore_files = True
else:
ignore_files = False
check_files = 'ignore_files' not in recurse_set
check_dirs = 'ignore_dirs' not in recurse_set

if 'ignore_dirs' in recurse:
ignore_dirs = True
else:
ignore_dirs = False

for root, dirs, files in walk_l:
if not ignore_files:
for fn_ in files:
full = os.path.join(root, fn_)
ret, perms = __salt__['file.check_perms'](
full,
ret,
user,
group,
file_mode,
follow_symlinks)
if not ignore_dirs:
for dir_ in dirs:
full = os.path.join(root, dir_)
ret, perms = __salt__['file.check_perms'](
full,
ret,
user,
group,
dir_mode,
follow_symlinks)
for root, dirs, files in walk_l:
if check_files:
for fn_ in files:
full = os.path.join(root, fn_)
ret, perms = __salt__['file.check_perms'](
full,
ret,
user,
group,
file_mode,
follow_symlinks)
if check_dirs:
for dir_ in dirs:
full = os.path.join(root, dir_)
ret, perms = __salt__['file.check_perms'](
full,
ret,
user,
group,
dir_mode,
follow_symlinks)

if clean:
keep = _gen_keep_files(name, require, walk_d)
Expand Down

0 comments on commit aba82ab

Please sign in to comment.