Merge lp:~harlowja/cloud-init/patch-ssh-key-users into lp:~cloud-init-dev/cloud-init/trunk

Proposed by Joshua Harlow
Status: Rejected
Rejected by: Scott Moser
Proposed branch: lp:~harlowja/cloud-init/patch-ssh-key-users
Merge into: lp:~cloud-init-dev/cloud-init/trunk
Diff against target: 79 lines (+45/-8)
1 file modified
cloudinit/config/cc_ssh_authkey_fingerprints.py (+45/-8)
To merge this branch: bzr merge lp:~harlowja/cloud-init/patch-ssh-key-users
Reviewer Review Type Date Requested Status
Joshua Harlow (community) Disapprove
Garrett Holmstrom (community) Approve
Review via email: mp+125606@code.launchpad.net
To post a comment you must log in.
662. By Joshua Harlow

Add a catch of the value error when the hashlib
type isn't actually a real known hash routine name.

Revision history for this message
Garrett Holmstrom (gholms) wrote :

I just tested this on Fedora; it seems to fix things when the config uses the new-style "users" list. Or at least when the only thing in the list is the "default" user, anyway.

review: Approve
Revision history for this message
Joshua Harlow (harlowja) wrote :

This will also be aided/affected (not initially) by https://code.launchpad.net/~harlowja/cloud-init/ug-cleanup/+merge/125817 which allows the distro class to normalize the user config, while letting others do whatever they want with it (thus not duplicating that normalization code everywhere else when someone wants to do something with the new user list).

Revision history for this message
Joshua Harlow (harlowja) wrote :

https://code.launchpad.net/~harlowja/cloud-init/ug-cleanup/+merge/125817 is also handling this in the correct way now. Perhaps we should just approve that one instead since its probably the right way to do this...

review: Disapprove
Revision history for this message
Scott Moser (smoser) wrote :

Garret, do you think trunk is sane now?
If so, maybe just mark this rejected or merged.

Revision history for this message
Garrett Holmstrom (gholms) wrote :

Now that the other branch has merged, trunk looks reasonable to me.

Revision history for this message
Scott Moser (smoser) wrote :

per comment above from Garret, the solution in trunk is acceptable.
marking this rejected.

Unmerged revisions

662. By Joshua Harlow

Add a catch of the value error when the hashlib
type isn't actually a real known hash routine name.

661. By Joshua Harlow

Fix pylint line lengths.

660. By Joshua Harlow

Instead of using the old 'user' parse the
'users' list and extract the user names
to print from that list instead.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cloudinit/config/cc_ssh_authkey_fingerprints.py'
2--- cloudinit/config/cc_ssh_authkey_fingerprints.py 2012-08-22 18:12:32 +0000
3+++ cloudinit/config/cc_ssh_authkey_fingerprints.py 2012-09-20 23:46:21 +0000
4@@ -21,7 +21,8 @@
5
6 from prettytable import PrettyTable
7
8-from cloudinit import ssh_util
9+from cloudinit.ssh_util import extract_authorized_keys as eak
10+
11 from cloudinit import util
12
13
14@@ -40,8 +41,9 @@
15 hasher = hashlib.new(hash_meth)
16 hasher.update(base64.b64decode(b64_text))
17 return ":".join(_split_hash(hasher.hexdigest()))
18- except TypeError:
19- # Raised when b64 not really b64...
20+ except (TypeError, ValueError):
21+ # Raised when b64 not really b64... or
22+ # when the hash type isn't valid
23 return '?'
24
25
26@@ -84,13 +86,48 @@
27 stderr=False, console=True)
28
29
30+def translate_user_name(uname, distro, log):
31+ if not uname:
32+ uname = ''
33+ uname = uname.strip()
34+ real_name = None
35+ if uname.lower() == 'default':
36+ try:
37+ real_name = distro.get_default_user()
38+ except NotImplementedError:
39+ log.warn("Distro has not implemented default user "
40+ "creation. No default user will be translated.")
41+ else:
42+ real_name = uname
43+ return real_name
44+
45+
46 def handle(name, cfg, cloud, log, _args):
47 if 'no_ssh_fingerprints' in cfg:
48 log.debug(("Skipping module named %s, "
49 "logging of ssh fingerprints disabled"), name)
50-
51- user_name = util.get_cfg_option_str(cfg, "user", "ubuntu")
52+ return
53+
54+ if not 'users' in cfg:
55+ log.debug(("Skipping module named %s, "
56+ "logging of ssh fingerprints disabled "
57+ "since no user/s provided"), name)
58+ return
59+
60+ users_to_hash = []
61+ for user_config in cfg['users']:
62+ user_name = None
63+ if isinstance(user_config, (basestring, str)):
64+ user_name = translate_user_name(user_config, cloud.distro, log)
65+ elif isinstance(user_config, (dict)):
66+ if 'name' in user_config:
67+ user_name = translate_user_name(user_config['name'],
68+ cloud.distro, log)
69+ if user_name:
70+ users_to_hash.append(user_name)
71+
72 hash_meth = util.get_cfg_option_str(cfg, "authkey_hash", "md5")
73- extract = ssh_util.extract_authorized_keys
74- (auth_key_fn, auth_key_entries) = extract(user_name, cloud.paths)
75- _pprint_key_entries(user_name, auth_key_fn, auth_key_entries, hash_meth)
76+ for user_name in users_to_hash:
77+ (auth_key_fn, auth_key_entries) = eak(user_name, cloud.paths)
78+ _pprint_key_entries(user_name, auth_key_fn,
79+ auth_key_entries, hash_meth)