groupadd: normalize the use of root parameter

(cherry picked from commit 79e621c)

salt/modules/groupadd.py

@@ -12,6 +12,7 @@
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
 import logging
+import functools
 import os
 
 from salt.ext import six
@@ -43,6 +44,18 @@ def add(name, gid=None, system=False, root=None):
     '''
     Add the specified group
 
+    name
+        Name of the new group
+
+    gid
+        Use GID for the new group
+
+    system
+        Create a system account
+
+    root
+        Directory to chroot into
+
     CLI Example:
 
     .. code-block:: bash
@@ -54,11 +67,12 @@ def add(name, gid=None, system=False, root=None):
         cmd.append('-g {0}'.format(gid))
     if system and __grains__['kernel'] != 'OpenBSD':
         cmd.append('-r')
-    cmd.append(name)
 
     if root is not None:
         cmd.extend(('-R', root))
 
+    cmd.append(name)
+
     ret = __salt__['cmd.run_all'](cmd, python_shell=False)
 
     return not ret['retcode']
@@ -68,34 +82,53 @@ def delete(name, root=None):
     '''
     Remove the named group
 
+    name
+        Name group to delete
+
+    root
+        Directory to chroot into
+
     CLI Example:
 
     .. code-block:: bash
 
         salt '*' group.delete foo
     '''
-    cmd = ['groupdel', name]
+    cmd = ['groupdel']
 
     if root is not None:
         cmd.extend(('-R', root))
 
+    cmd.append(name)
+
     ret = __salt__['cmd.run_all'](cmd, python_shell=False)
 
     return not ret['retcode']
 
 
-def info(name):
+def info(name, root=None):
     '''
     Return information about a group
 
+    name
+        Name of the group
+
+    root
+        Directory to chroot into
+
     CLI Example:
 
     .. code-block:: bash
 
         salt '*' group.info foo
     '''
+    if root is not None:
+        getgrnam = functools.partial(_getgrnam, root=root)
+    else:
+        getgrnam = functools.partial(grp.getgrnam)
+
     try:
-        grinfo = grp.getgrnam(name)
+        grinfo = getgrnam(name)
     except KeyError:
         return {}
     else:
@@ -112,10 +145,16 @@ def _format_info(data):
             'members': data.gr_mem}
 
 
-def getent(refresh=False):
+def getent(refresh=False, root=None):
     '''
     Return info on all groups
 
+    refresh
+        Force a refresh of group information
+
+    root
+        Directory to chroot into
+
     CLI Example:
 
     .. code-block:: bash
@@ -126,41 +165,74 @@ def getent(refresh=False):
         return __context__['group.getent']
 
     ret = []
-    for grinfo in grp.getgrall():
+    if root is not None:
+        getgrall = functools.partial(_getgrall, root=root)
+    else:
+        getgrall = functools.partial(grp.getgrall)
+
+    for grinfo in getgrall():
         ret.append(_format_info(grinfo))
     __context__['group.getent'] = ret
     return ret
 
 
+def _chattrib(name, key, value, param, root=None):
+    '''
+    Change an attribute for a named user
+    '''
+    pre_info = info(name, root=root)
+    if not pre_info:
+        return False
+
+    if value == pre_info[key]:
+        return True
+
+    cmd = ['groupmod']
+
+    if root is not None:
+        cmd.extend(('-R', root))
+
+    cmd.extend((param, value, name))
+
+    __salt__['cmd.run'](cmd, python_shell=False)
+    return info(name, root=root).get(key) == value
+
+
 def chgid(name, gid, root=None):
     '''
     Change the gid for a named group
 
+    name
+        Name of the group to modify
+
+    gid
+        Change the group ID to GID
+
+    root
+        Directory to chroot into
+
     CLI Example:
 
     .. code-block:: bash
 
         salt '*' group.chgid foo 4376
     '''
-    pre_gid = __salt__['file.group_to_gid'](name)
-    if gid == pre_gid:
-        return True
-    cmd = ['groupmod', '-g', gid, name]
-
-    if root is not None:
-        cmd.extend(('-R', root))
-
-    __salt__['cmd.run'](cmd, python_shell=False)
-    post_gid = __salt__['file.group_to_gid'](name)
-    if post_gid != pre_gid:
-        return post_gid == gid
-    return False
+    return _chattrib(name, 'gid', gid, '-g', root=root)
 
 
 def adduser(name, username, root=None):
     '''
     Add a user in the group.
 
+    name
+        Name of the group to modify
+
+    username
+        Username to add to the group
+
+    root
+        Directory to chroot into
+
     CLI Example:
 
     .. code-block:: bash
@@ -196,6 +268,15 @@ def deluser(name, username, root=None):
     '''
     Remove a user from the group.
 
+    name
+        Name of the group to modify
+
+    username
+        Username to delete from the group
+
+    root
+        Directory to chroot into
+
     CLI Example:
 
     .. code-block:: bash
@@ -242,6 +323,15 @@ def members(name, members_list, root=None):
     '''
     Replaces members of the group with a provided list.
 
+    name
+        Name of the group to modify
+
+    members_list
+        Username list to set into the group
+
+    root
+        Directory to chroot into
+
     CLI Example:
 
         salt '*' group.members foo 'user1,user2,user3,...'
@@ -293,30 +383,36 @@ def _getgrnam(name, root=None):
     '''
     Alternative implementation for getgrnam, that use only /etc/group
     '''
-    root = '/' if not root else root
+    root = root or '/'
     passwd = os.path.join(root, 'etc/group')
     with salt.utils.files.fopen(passwd) as fp_:
         for line in fp_:
             line = salt.utils.stringutils.to_unicode(line)
             comps = line.strip().split(':')
+            if len(comps) < 4:
+                log.debug('Ignoring group line: %s', line)
+                continue
             if comps[0] == name:
                 # Generate a getpwnam compatible output
                 comps[2] = int(comps[2])
                 comps[3] = comps[3].split(',') if comps[3] else []
                 return grp.struct_group(comps)
-    raise KeyError
+    raise KeyError('getgrnam(): name not found: {}'.format(name))
 
 
 def _getgrall(root=None):
     '''
     Alternative implemetantion for getgrall, that use only /etc/group
     '''
-    root = '/' if not root else root
+    root = root or '/'
     passwd = os.path.join(root, 'etc/group')
     with salt.utils.files.fopen(passwd) as fp_:
         for line in fp_:
             line = salt.utils.stringutils.to_unicode(line)
             comps = line.strip().split(':')
+            if len(comps) < 4:
+                log.debug('Ignoring group line: %s', line)
+                continue
             # Generate a getgrall compatible output
             comps[2] = int(comps[2])
             comps[3] = comps[3].split(',') if comps[3] else []

salt/modules/shadow.py

@@ -25,6 +25,7 @@
 import salt.utils.stringutils
 from salt.exceptions import CommandExecutionError
 from salt.ext import six
+from salt.ext.six.moves import range
 try:
     import salt.utils.pycrypto
     HAS_CRYPT = True

tests/unit/modules/test_groupadd.py

@@ -84,21 +84,19 @@ def test_chgid_gid_same(self):
         '''
         Tests if the group id is the same as argument
         '''
-        mock_pre_gid = MagicMock(return_value=10)
-        with patch.dict(groupadd.__salt__,
-                        {'file.group_to_gid': mock_pre_gid}):
+        mock = MagicMock(return_value={'gid': 10})
+        with patch.object(groupadd, 'info', mock):
             self.assertTrue(groupadd.chgid('test', 10))
 
     def test_chgid(self):
         '''
         Tests the gid for a named group was changed
         '''
-        mock_pre_gid = MagicMock(return_value=0)
-        mock_cmdrun = MagicMock(return_value=0)
-        with patch.dict(groupadd.__salt__,
-                        {'file.group_to_gid': mock_pre_gid}):
-            with patch.dict(groupadd.__salt__, {'cmd.run': mock_cmdrun}):
-                self.assertFalse(groupadd.chgid('test', 500))
+        mock = MagicMock(return_value=None)
+        with patch.dict(groupadd.__salt__, {'cmd.run': mock}):
+            mock = MagicMock(side_effect=[{'gid': 10}, {'gid': 500}])
+            with patch.object(groupadd, 'info', mock):
+                self.assertTrue(groupadd.chgid('test', 500))
 
     # 'delete' function tests: 1