1. Cleanup the user creation so that the distro class is

responsible only for creating users and groups and
normalizing a input configuration into a normalized
format that splits up the user list, the group list
and the default user listsand let the add user/group config 
module handle calling those methods to add its own users/groups 
and the default user (if any).

2. Also add in tests for this normalization process to ensure
that it is pretty bug free and works with the different types
of formats that users/groups/defaults + options can take.

3 files changed, 176 insertions, 101 deletions

diff --git a/cloudinit/config/cc_users_groups.py b/cloudinit/config/cc_users_groups.py
index 418f3330..273c5068 100644
--- a/cloudinit/config/cc_users_groups.py
+++ b/cloudinit/config/cc_users_groups.py
@@ -16,63 +16,34 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from cloudinit import util
+
 from cloudinit.settings import PER_INSTANCE
 
 frequency = PER_INSTANCE
 
 
 def handle(name, cfg, cloud, log, _args):
-    user_zero = None
-
-    if 'groups' in cfg:
-        for group in cfg['groups']:
-            if isinstance(group, dict):
-                for name, values in group.iteritems():
-                    if isinstance(values, list):
-                        cloud.distro.create_group(name, values)
-                    elif isinstance(values, str):
-                        cloud.distro.create_group(name, values.split(','))
-            else:
-                cloud.distro.create_group(group, [])
-
-    if 'users' in cfg:
-        user_zero = None
-
-        for user_config in cfg['users']:
-
-            # Handle the default user creation
-            if 'default' in user_config:
-                log.info("Creating default user")
-
-                # Create the default user if so defined
-                try:
-                    cloud.distro.add_default_user()
-
-                    if not user_zero:
-                        user_zero = cloud.distro.get_default_user()
-
-                except NotImplementedError:
-
-                    if user_zero == name:
-                        user_zero = None
-
-                    log.warn("Distro has not implemented default user "
-                             "creation. No default user will be created")
-
-            elif isinstance(user_config, dict) and 'name' in user_config:
-
-                name = user_config['name']
-                if not user_zero:
-                    user_zero = name
-
-                # Make options friendly for distro.create_user
-                new_opts = {}
-                if isinstance(user_config, dict):
-                    for opt in user_config:
-                        new_opts[opt.replace('-', '_')] = user_config[opt]
-
-                cloud.distro.create_user(**new_opts)
 
-            else:
-                # create user with no configuration
-                cloud.distro.create_user(user_config)
+    distro = cloud.distro
+    ((users, default_user), groups) = distro.normalize_users_groups(cfg)
+    for (name, members) in groups.items():
+        distro.create_group(name, members)
+
+    if default_user:
+        user = default_user['name']
+        config = default_user['config']
+        def_base_config = {
+            'name': user,
+            'plain_text_passwd': user,
+            'home': "/home/%s" % user,
+            'shell': "/bin/bash",
+            'lock_passwd': True,
+            'gecos': "%s%s" % (user.title()),
+            'sudo': "ALL=(ALL) NOPASSWD:ALL",
+        }
+        u_config = util.mergemanydict([def_base_config, config])
+        distro.create_user(**u_config)
+
+    for (user, config) in users.items():
+        distro.create_user(user, **config)
diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 3e9d934d..4fb1d8c2 100644
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -24,9 +24,7 @@
 from StringIO import StringIO
 
 import abc
-import grp
 import os
-import pwd
 import re
 
 from cloudinit import importer
@@ -54,34 +52,6 @@ class Distro(object):
         self._cfg = cfg
         self.name = name
 
-    def add_default_user(self):
-        # Adds the distro user using the rules:
-        #  - Password is same as username but is locked
-        #  - nopasswd sudo access
-
-        user = self.get_default_user()
-        groups = self.get_default_user_groups()
-
-        if not user:
-            raise NotImplementedError("No Default user")
-
-        user_dict = {
-                    'name': user,
-                    'plain_text_passwd': user,
-                    'home': "/home/%s" % user,
-                    'shell': "/bin/bash",
-                    'lock_passwd': True,
-                    'gecos': "%s%s" % (user[0:1].upper(), user[1:]),
-                    'sudo': "ALL=(ALL) NOPASSWD:ALL",
-                    }
-
-        if groups:
-            user_dict['groups'] = groups
-
-        self.create_user(**user_dict)
-
-        LOG.info("Added default '%s' user with passwordless sudo", user)
-
     @abc.abstractmethod
     def install_packages(self, pkglist):
         raise NotImplementedError()
@@ -204,18 +174,19 @@ class Distro(object):
             util.logexc(LOG, "Running interface command %s failed", cmd)
             return False
 
-    def isuser(self, name):
-        try:
-            if pwd.getpwnam(name):
-                return True
-        except KeyError:
-            return False
-
     def get_default_user(self):
         return self.default_user
 
     def get_default_user_groups(self):
-        return self.default_user_groups
+        if not self.default_user_groups:
+            return []
+        def_groups = []
+        if isinstance(self.default_user_groups, (str, basestring)):
+            def_groups = self.default_user_groups.split(",")
+        else:
+            def_groups = list(self.default_user_groups)
+        def_groups = list(sorted(set(def_groups)))
+        return def_groups
 
     def create_user(self, name, **kwargs):
         """
@@ -272,7 +243,7 @@ class Distro(object):
             adduser_cmd.append('-m')
 
         # Create the user
-        if self.isuser(name):
+        if util.is_user(name):
             LOG.warn("User %s already exists, skipping." % name)
         else:
             LOG.debug("Creating name %s" % name)
@@ -323,6 +294,130 @@ class Distro(object):
 
         return True
 
+    def _normalize_groups(self, grp_cfg):
+        groups = {}
+        if isinstance(grp_cfg, (str, basestring)):
+            grp_cfg = grp_cfg.strip().split(",")
+
+        if isinstance(grp_cfg, (list)):
+            for g in grp_cfg:
+                g = g.strip()
+                if g:
+                    groups[g] = []
+        elif isinstance(grp_cfg, (dict)):
+            for grp_name, grp_members in grp_cfg.items():
+                if isinstance(grp_members, (str, basestring)):
+                    r_grp_members = []
+                    for gc in grp_members.strip().split(','):
+                        gc = gc.strip()
+                        if gc and gc not in r_grp_members:
+                            r_grp_members.append(gc)
+                    grp_members = r_grp_members
+                elif not isinstance(grp_members, (list)):
+                    raise TypeError(("Group member config must be list "
+                                     " or string types only and not %s") %
+                                    util.obj_name(grp_members))
+                groups[grp_name] = grp_members
+        else:
+            raise TypeError(("Group config must be list, dict "
+                             " or string types only and not %s") %
+                            util.obj_name(grp_cfg))
+        return groups
+
+    def _normalize_users(self, u_cfg):
+        if isinstance(u_cfg, (dict)):
+            ad_ucfg = []
+            for (k, v) in u_cfg.items():
+                if isinstance(v, (bool, int, basestring, str)):
+                    if util.is_true(v):
+                        ad_ucfg.append(str(k))
+                elif isinstance(v, (dict)):
+                    v['name'] = k
+                    ad_ucfg.append(v)
+                else:
+                    raise TypeError(("Unmappable user value type %s"
+                                     " for key %s") % (util.obj_name(v), k))
+            u_cfg = ad_ucfg
+
+        users = {}
+        for user_config in u_cfg:
+            if isinstance(user_config, (str, basestring)):
+                for u in user_config.strip().split(","):
+                    u = u.strip()
+                    if u and u not in users:
+                        users[u] = {}
+            elif isinstance(user_config, (dict)):
+                if 'name' in user_config:
+                    n = user_config.pop('name')
+                    prev_config = users.get(n) or {}
+                    users[n] = util.mergemanydict([prev_config,
+                                                   user_config])
+                else:
+                    # Assume the default user then
+                    prev_config = users.get('default') or {}
+                    users['default'] = util.mergemanydict([prev_config,
+                                                           user_config])
+            elif isinstance(user_config, (bool, int)):
+                pass
+            else:
+                raise TypeError(("User config must be dictionary "
+                                 " or string types only and not %s") %
+                                util.obj_name(user_config))
+
+        # Ensure user options are in the right python friendly format
+        if users:
+            c_users = {}
+            for (uname, uconfig) in users.items():
+                c_uconfig = {}
+                for (k, v) in uconfig.items():
+                    k = k.replace('-', '_').strip()
+                    if k:
+                        c_uconfig[k] = v
+                c_users[uname] = c_uconfig
+            users = c_users
+
+        # Fixup the default user into the real
+        # default user name and extract it
+        default_user = {}
+        if users and 'default' in users:
+            try:
+                def_config = users.pop('default')
+                def_user = self.get_default_user()
+                def_groups = self.get_default_user_groups()
+                if def_user:
+                    u_config = users.pop(def_user, None) or {}
+                    u_groups = u_config.get('groups') or []
+                    if isinstance(u_groups, (str, basestring)):
+                        u_groups = u_groups.strip().split(",")
+                    u_groups.extend(def_groups)
+                    u_groups = set([x.strip() for x in u_groups if x.strip()])
+                    u_config['groups'] = ",".join(sorted(u_groups))
+                    default_user = {
+                        'name': def_user,
+                        'config': util.mergemanydict([def_config, u_config]),
+                    }
+                else:
+                    LOG.warn(("Distro has not provided a default user "
+                             "creation. No default user will be normalized."))
+                    users.pop('default', None)
+            except NotImplementedError:
+                LOG.warn(("Distro has not implemented default user "
+                         "creation. No default user will be normalized."))
+                users.pop('default', None)
+
+        return (default_user, users)
+
+    def normalize_users_groups(self, ug_cfg):
+        users = {}
+        groups = {}
+        default_user = {}
+        if 'groups' in ug_cfg:
+            groups = self._normalize_groups(ug_cfg['groups'])
+
+        if 'users' in ug_cfg:
+            default_user, users = self._normalize_users(ug_cfg['users'])
+        return ((users, default_user), groups)
+
     def write_sudo_rules(self,
         user,
         rules,
@@ -349,18 +444,11 @@ class Distro(object):
                 util.logexc(LOG, "Failed to write %s" % sudo_file, e)
                 raise e
 
-    def isgroup(self, name):
-        try:
-            if grp.getgrnam(name):
-                return True
-        except:
-            return False
-
     def create_group(self, name, members):
         group_add_cmd = ['groupadd', name]
 
         # Check if group exists, and then add it doesn't
-        if self.isgroup(name):
+        if util.is_group(name):
             LOG.warn("Skipping creation of existing group '%s'" % name)
         else:
             try:
@@ -372,7 +460,7 @@ class Distro(object):
         # Add members to the group, if so defined
         if len(members) > 0:
             for member in members:
-                if not self.isuser(member):
+                if not util.is_user(member):
                     LOG.warn("Unable to add group member '%s' to group '%s'"
                             "; user does not exist." % (member, name))
                     continue
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 33da73eb..94b17dfa 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1104,6 +1104,22 @@ def hash_blob(blob, routine, mlen=None):
         return digest
 
 
+def is_user(name):
+    try:
+        if pwd.getpwnam(name):
+            return True
+    except KeyError:
+        return False
+
+
+def is_group(name):
+    try:
+        if grp.getgrnam(name):
+            return True
+    except KeyError:
+        return False
+
+
 def rename(src, dest):
     LOG.debug("Renaming %s to %s", src, dest)
     # TODO(harlowja) use a se guard here??