← Back to team overview

cloud-init-dev team mailing list archive

[Merge] lp:~harlowja/cloud-init/ug-cleanup into lp:cloud-init

 

Joshua Harlow has proposed merging lp:~harlowja/cloud-init/ug-cleanup into lp:cloud-init.

Requested reviews:
  cloud init development team (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~harlowja/cloud-init/ug-cleanup/+merge/125817

Big cleanup of user/group creation with a cleaner approach for group/user config extraction. Rest is untouched.
-- 
https://code.launchpad.net/~harlowja/cloud-init/ug-cleanup/+merge/125817
Your team cloud init development team is requested to review the proposed merge of lp:~harlowja/cloud-init/ug-cleanup into lp:cloud-init.
=== modified file 'cloudinit/config/cc_users_groups.py'
--- cloudinit/config/cc_users_groups.py	2012-08-31 18:45:40 +0000
+++ cloudinit/config/cc_users_groups.py	2012-09-21 21:25:22 +0000
@@ -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)

=== modified file 'cloudinit/distros/__init__.py'
--- cloudinit/distros/__init__.py	2012-09-18 17:27:41 +0000
+++ cloudinit/distros/__init__.py	2012-09-21 21:25:22 +0000
@@ -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 @@
         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 @@
             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 @@
             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 @@
 
         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 @@
                 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 @@
         # 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

=== modified file 'cloudinit/util.py'
--- cloudinit/util.py	2012-08-28 03:51:00 +0000
+++ cloudinit/util.py	2012-09-21 21:25:22 +0000
@@ -1104,6 +1104,22 @@
         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??

=== modified file 'doc/examples/cloud-config-user-groups.txt'
--- doc/examples/cloud-config-user-groups.txt	2012-09-18 17:27:41 +0000
+++ doc/examples/cloud-config-user-groups.txt	2012-09-21 21:25:22 +0000
@@ -81,14 +81,16 @@
 #               directive.
 #   system: Create the user as a system user. This means no home directory.
 #
-# Default user creation: Ubuntu Only
-# Unless you define users, you will get a Ubuntu user on Ubuntu systems with the
+# Default user creation:
+#
+# Unless you define users, you will get a 'ubuntu' user on buntu systems with the
 # legacy permission (no password sudo, locked user, etc). If however, you want
 # to have the ubuntu user in addition to other users, you need to instruct
 # cloud-init that you also want the default user. To do this use the following
 # syntax:
 #    users:
-#      default: True
+#      - default
+#      - bob
 #  foobar: ...
 #
 # users[0] (the first user in users) overrides the user directive.

=== added file 'tests/unittests/test_distros/test_user_data_normalize.py'
--- tests/unittests/test_distros/test_user_data_normalize.py	1970-01-01 00:00:00 +0000
+++ tests/unittests/test_distros/test_user_data_normalize.py	2012-09-21 21:25:22 +0000
@@ -0,0 +1,187 @@
+from mocker import MockerTestCase
+
+from cloudinit import distros
+from cloudinit import helpers
+from cloudinit import settings
+
+
+class TestUGNormalize(MockerTestCase):
+
+    def _make_distro(self, dtype, def_user=None, def_groups=None):
+        cfg = dict(settings.CFG_BUILTIN)
+        cfg['system_info']['distro'] = dtype
+        paths = helpers.Paths(cfg['system_info']['paths'])
+        distro_cls = distros.fetch(dtype)
+        distro = distro_cls(dtype, cfg['system_info'], paths)
+        if def_user:
+            distro.default_user = def_user
+        if def_groups:
+            distro.default_user_groups = def_groups
+        return distro
+
+    def test_basic_groups(self):
+        distro = self._make_distro('ubuntu')
+        ug_cfg = {
+            'groups': ['bob'],
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('bob', groups)
+        self.assertEquals({}, users)
+        self.assertEquals({}, def_user)
+
+    def test_csv_groups(self):
+        distro = self._make_distro('ubuntu')
+        ug_cfg = {
+            'groups': 'bob,joe,steve',
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('bob', groups)
+        self.assertIn('joe', groups)
+        self.assertIn('steve', groups)
+        self.assertEquals({}, users)
+        self.assertEquals({}, def_user)
+
+    def test_more_groups(self):
+        distro = self._make_distro('ubuntu')
+        ug_cfg = {
+            'groups': ['bob', 'joe', 'steve',]
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('bob', groups)
+        self.assertIn('joe', groups)
+        self.assertIn('steve', groups)
+        self.assertEquals({}, users)
+        self.assertEquals({}, def_user)
+
+    def test_member_groups(self):
+        distro = self._make_distro('ubuntu')
+        ug_cfg = {
+            'groups': {
+                'bob': ['s'],
+                'joe': [],
+                'steve': [],
+            }
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('bob', groups)
+        self.assertEquals(['s'], groups['bob'])
+        self.assertEquals([], groups['joe'])
+        self.assertIn('joe', groups)
+        self.assertIn('steve', groups)
+        self.assertEquals({}, users)
+        self.assertEquals({}, def_user)
+
+    def test_users_simple_dict(self):
+        distro = self._make_distro('ubuntu', 'bob')
+        ug_cfg = {
+            'users': {
+                'default': True,
+            }
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertEquals('bob', def_user['name'])
+        ug_cfg = {
+            'users': {
+                'default': 'yes',
+            }
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertEquals('bob', def_user['name'])
+        ug_cfg = {
+            'users': {
+                'default': '1',
+            }
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertEquals('bob', def_user['name'])
+
+    def test_users_simple_dict_no(self):
+        distro = self._make_distro('ubuntu', 'bob')
+        ug_cfg = {
+            'users': {
+                'default': False,
+            }
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertEquals({}, def_user)
+        ug_cfg = {
+            'users': {
+                'default': 'no',
+            }
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertEquals({}, def_user)
+
+    def test_users_simple(self):
+        distro = self._make_distro('ubuntu')
+        ug_cfg = {
+            'users': [
+                'joe',
+                'bob'
+            ],
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('joe', users)
+        self.assertIn('bob', users)
+        self.assertEquals({}, users['joe'])
+        self.assertEquals({}, users['bob'])
+
+    def test_users_dict_default_additional(self):
+        distro = self._make_distro('ubuntu', 'bob')
+        ug_cfg = {
+            'users': [
+                {'name': 'default', 'blah': True}
+            ],
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('bob', def_user['name'])
+        self.assertEquals(",".join(distro.get_default_user_groups()),
+                          def_user['config']['groups'])
+        self.assertEquals(True,
+                          def_user['config']['blah'])
+        self.assertNotIn('bob', users)
+
+    def test_users_dict_default(self):
+        distro = self._make_distro('ubuntu', 'bob')
+        ug_cfg = {
+            'users': [
+                'default',
+            ],
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('bob', def_user['name'])
+        self.assertEquals(",".join(distro.get_default_user_groups()),
+                          def_user['config']['groups'])
+        self.assertNotIn('bob', users)
+
+    def test_users_dict_trans(self):
+        distro = self._make_distro('ubuntu')
+        ug_cfg = {
+            'users': [
+                {'name': 'joe',
+                 'tr-me': True},
+                {'name': 'bob'},
+            ],
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('joe', users)
+        self.assertIn('bob', users)
+        self.assertEquals({'tr_me': True}, users['joe'])
+        self.assertEquals({}, users['bob'])
+
+    def test_users_dict(self):
+        distro = self._make_distro('ubuntu')
+        ug_cfg = {
+            'users': [
+                {'name': 'joe'},
+                {'name': 'bob'},
+            ],
+        }
+        ((users, def_user), groups) = distro.normalize_users_groups(ug_cfg)
+        self.assertIn('joe', users)
+        self.assertIn('bob', users)
+        self.assertEquals({}, users['joe'])
+        self.assertEquals({}, users['bob'])
+
+
+


Follow ups