Merge lp:~harlowja/cloud-init/legacy-user-make-default into lp:~cloud-init-dev/cloud-init/trunk

[Joshua Harlow]

[Clint Byrum (clint-fewbar)]

Tested on a stock Ubuntu 12.10 cloud image with just cloud-init replaced. The test case of 'user: ec2-user' results in ec2-user being treated as the default user and getting the host's ssh keypair added.

review: Approve

[Clint Byrum (clint-fewbar)]

So, while this does make sure the SSH keys get installed, it doesn't configure ec2-user with the 'admin' group or setup passwordless sudo.. so the ec2-user is still not equivalent to the normal distro default user.

review: Needs Fixing

[Joshua Harlow (harlowja)]

Can you upload your cloud.cfg, that might help me see why the user should also show up in the admin group and such.

[Joshua Harlow (harlowja)]

If its just the default, let me know. I think i have a reason why! :-P

[lp:~harlowja/cloud-init/legacy-user-make-default]

760. By harlowja <email address hidden> on 2013-01-20

Merge the old user style with the distro provided config.

When the old user: style entry is found, don't forget that
we need to use the distro settings that are provided but
override the name with the new name, this is now accomplished
by merging them together in the correct order (using the standard
cloud-init merging algo).

[Joshua Harlow (harlowja)]

Can you try again, this should now be merging with the distro config (and overriding the name) so that the 'user:' 'becomes' the distro user.

[Clint Byrum (clint-fewbar)]

Just now got around to testing r760. It works! As I discussed with Joshua in IRC, it probably won't be backportable to quantal because of the missing default_user section in cloud.cfg.

review: Approve

[Joshua Harlow (harlowja)]

review: Approve

=== modified file 'cloudinit/distros/__init__.py'
--- cloudinit/distros/__init__.py	2013-01-07 16:36:10 +0000
+++ cloudinit/distros/__init__.py	2013-01-20 01:54:22 +0000
@@ -705,41 +705,68 @@
 def normalize_users_groups(cfg, distro):
     if not cfg:
         cfg = {}
+
     users = {}
     groups = {}
     if 'groups' in cfg:
         groups = _normalize_groups(cfg['groups'])
 
-    # Handle the previous style of doing this...
-    old_user = None
+    # Handle the previous style of doing this where the first user
+    # overrides the concept of the default user if provided in the user: XYZ
+    # format.
+    old_user = {}
     if 'user' in cfg and cfg['user']:
-        old_user = str(cfg['user'])
-        if not 'users' in cfg:
-            cfg['users'] = old_user
-            old_user = None
-    if 'users' in cfg:
-        default_user_config = None
-        try:
-            default_user_config = distro.get_default_user()
-        except NotImplementedError:
-            LOG.warn(("Distro has not implemented default user "
-                      "access. No default user will be normalized."))
-        base_users = cfg['users']
-        if old_user:
-            if isinstance(base_users, (list)):
-                if len(base_users):
-                    # The old user replaces user[0]
-                    base_users[0] = {'name': old_user}
-                else:
-                    # Just add it on at the end...
-                    base_users.append({'name': old_user})
-            elif isinstance(base_users, (dict)):
-                if old_user not in base_users:
-                    base_users[old_user] = True
-            elif isinstance(base_users, (str, basestring)):
-                # Just append it on to be re-parsed later
-                base_users += ",%s" % (old_user)
-        users = _normalize_users(base_users, default_user_config)
+        old_user = cfg['user']
+        # Translate it into the format that is more useful
+        # going forward
+        if isinstance(old_user, (basestring, str)):
+            old_user = {
+                'name': old_user,
+            }
+        if not isinstance(old_user, (dict)):
+            LOG.warn(("Format for 'user' key must be a string or "
+                      "dictionary and not %s"), util.obj_name(old_user))
+            old_user = {}
+
+    # If no old user format, then assume the distro
+    # provides what the 'default' user maps to, but notice
+    # that if this is provided, we won't automatically inject
+    # a 'default' user into the users list, while if a old user
+    # format is provided we will.
+    distro_user_config = {}
+    try:
+        distro_user_config = distro.get_default_user()
+    except NotImplementedError:
+        LOG.warn(("Distro has not implemented default user "
+                  "access. No distribution provided default user"
+                  " will be normalized."))
+
+    # Merge the old user (which may just be an empty dict when not
+    # present with the distro provided default user configuration so
+    # that the old user style picks up all the distribution specific
+    # attributes (if any)
+    default_user_config = util.mergemanydict([old_user, distro_user_config])
+
+    base_users = cfg.get('users', [])
+    if not isinstance(base_users, (list, dict, str, basestring)):
+        LOG.warn(("Format for 'users' key must be a comma separated string"
+                  " or a dictionary or a list and not %s"),
+                 util.obj_name(base_users))
+        base_users = []
+
+    if old_user:
+        # Ensure that when user: is provided that this user
+        # always gets added (as the default user)
+        if isinstance(base_users, (list)):
+            # Just add it on at the end...
+            base_users.append({'name': 'default'})
+        elif isinstance(base_users, (dict)):
+            base_users['default'] = base_users.get('default', True)
+        elif isinstance(base_users, (str, basestring)):
+            # Just append it on to be re-parsed later
+            base_users += ",default"
+
+    users = _normalize_users(base_users, default_user_config)
     return (users, groups)
 
 

=== modified file 'tests/unittests/test_distros/test_user_data_normalize.py'
--- tests/unittests/test_distros/test_user_data_normalize.py	2012-11-09 23:35:48 +0000
+++ tests/unittests/test_distros/test_user_data_normalize.py	2013-01-20 01:54:22 +0000
@@ -173,26 +173,29 @@
             'users': 'default'
         }
         (users, _groups) = self._norm(ug_cfg, distro)
-        self.assertIn('bob', users)
+        self.assertNotIn('bob', users)  # Bob is not the default now, zetta is
         self.assertIn('zetta', users)
+        self.assertTrue(users['zetta']['default'])
         self.assertNotIn('default', users)
         ug_cfg = {
             'user': 'zetta',
             'users': 'default, joe'
         }
         (users, _groups) = self._norm(ug_cfg, distro)
-        self.assertIn('bob', users)
+        self.assertNotIn('bob', users)  # Bob is not the default now, zetta is
         self.assertIn('joe', users)
         self.assertIn('zetta', users)
+        self.assertTrue(users['zetta']['default'])
         self.assertNotIn('default', users)
         ug_cfg = {
             'user': 'zetta',
             'users': ['bob', 'joe']
         }
         (users, _groups) = self._norm(ug_cfg, distro)
-        self.assertNotIn('bob', users)
+        self.assertIn('bob', users)
         self.assertIn('joe', users)
         self.assertIn('zetta', users)
+        self.assertTrue(users['zetta']['default'])
         ug_cfg = {
             'user': 'zetta',
             'users': {
@@ -204,6 +207,7 @@
         self.assertIn('bob', users)
         self.assertIn('joe', users)
         self.assertIn('zetta', users)
+        self.assertTrue(users['zetta']['default'])
         ug_cfg = {
             'user': 'zetta',
         }