yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/launchpad/setuplxc-ssh-keys into lp:launchpad with lp:~frankban/launchpad/setuplxc-language-pack as a prerequisite.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~frankban/launchpad/setuplxc-ssh-keys/+merge/94939

== Changes ==

- Added the ability to change name of the ssh key used to connect to LXC.
The new -s (or --ssh-key-name) argument can be used to change the ssh key name (default is "id_rsa").

- ssh helper now accepts the ssh key to be used during connection.

- the ssh keys are no longer created for root: root can easily reuse the user's one.
-- 
https://code.launchpad.net/~frankban/launchpad/setuplxc-ssh-keys/+merge/94939
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/launchpad/setuplxc-ssh-keys into lp:launchpad.

=== modified file 'utilities/setuplxc.py'
--- utilities/setuplxc.py	2012-02-28 11:05:37 +0000
+++ utilities/setuplxc.py	2012-02-28 11:05:38 +0000
@@ -88,6 +88,7 @@
 """
 LXC_PATH = '/var/lib/lxc/'
 RESOLV_FILE = '/etc/resolv.conf'
+SSH_KEY_NAME = 'id_rsa'
 
 
 Env = namedtuple('Env', 'uid gid home')
@@ -210,6 +211,22 @@
             f.writelines(lines)
 
 
+def generate_ssh_keys(path):
+    """Generate ssh key pair saving them inside the given `directory`.
+
+        >>> generate_ssh_keys('/tmp/id_rsa')
+        0
+        >>> open('/tmp/id_rsa').readlines()[0].strip()
+        '-----BEGIN RSA PRIVATE KEY-----'
+        >>> open('/tmp/id_rsa.pub').read().startswith('ssh-rsa')
+        True
+        >>> os.remove('/tmp/id_rsa')
+        >>> os.remove('/tmp/id_rsa.pub')
+    """
+    return subprocess.call([
+        'ssh-keygen', '-q', '-t', 'rsa', '-N', '', '-f', path])
+
+
 def get_container_path(lxcname, path='', base_path=LXC_PATH):
     """Return the path of LXC container called `lxcname`.
 
@@ -257,7 +274,7 @@
     return pwd.getpwnam(user).pw_dir
 
 
-def ssh(location, user=None, caller=subprocess.call):
+def ssh(location, user=None, key=None, caller=subprocess.call):
     """Return a callable that can be used to run ssh shell commands.
 
     The ssh `location` and, optionally, `user` must be given.
@@ -266,7 +283,7 @@
     The callable internally uses the given `caller`::
 
         >>> def caller(cmd):
-        ...     print cmd
+        ...     print tuple(cmd)
         >>> sshcall = ssh('example.com', 'myuser', caller=caller)
         >>> root_sshcall = ssh('example.com', caller=caller)
         >>> sshcall('ls -l') # doctest: +ELLIPSIS
@@ -274,6 +291,13 @@
         >>> root_sshcall('ls -l') # doctest: +ELLIPSIS
         ('ssh', '-t', ..., 'example.com', '--', 'ls -l')
 
+    The ssh key path can be optionally provided::
+
+        >>> root_sshcall = ssh('example.com', key='/tmp/foo', caller=caller)
+        >>> root_sshcall('ls -l') # doctest: +ELLIPSIS
+        ('ssh', '-t', ..., '-i', '/tmp/foo', 'example.com', '--', 'ls -l')
+
+
     If the ssh command exits with an error code, an `SSHError` is raised::
 
         >>> ssh('loc', caller=lambda cmd: 1)('ls -l') # doctest: +ELLIPSIS
@@ -286,21 +310,23 @@
         >>> sshcall = ssh('loc', caller=lambda cmd: 1)
         >>> sshcall('ls -l', ignore_errors=True)
     """
+    sshcmd = [
+        'ssh',
+        '-t',
+        '-t',  # Yes, this second -t is deliberate. See `man ssh`.
+        '-o', 'StrictHostKeyChecking=no',
+        '-o', 'UserKnownHostsFile=/dev/null',
+        ]
+    if key is not None:
+        sshcmd.extend(['-i', key])
     if user is not None:
         location = '{}@{}'.format(user, location)
+    sshcmd.extend([location, '--'])
 
     def _sshcall(cmd, ignore_errors=False):
-        sshcmd = (
-            'ssh',
-            '-t',
-            '-t',  # Yes, this second -t is deliberate. See `man ssh`.
-            '-o', 'StrictHostKeyChecking=no',
-            '-o', 'UserKnownHostsFile=/dev/null',
-            location,
-            '--', cmd,
-            )
-        if caller(sshcmd) and not ignore_errors:
-            raise SSHError('Error running command: ' + ' '.join(sshcmd))
+        command = sshcmd + [cmd]
+        if caller(command) and not ignore_errors:
+            raise SSHError('Error running command: ' + ' '.join(command))
 
     return _sshcall
 
@@ -503,18 +529,25 @@
         >>> private = r'PRIVATE\nKEY'
         >>> public = r'PUBLIC\nKEY'
         >>> namespace = argparse.Namespace(
-        ...     private_key=private, public_key=public)
+        ...     private_key=private, public_key=public,
+        ...     ssh_key_name='id_rsa', home_dir='/tmp/')
         >>> handle_ssh_keys(namespace)
         >>> namespace.private_key == private.decode('string-escape')
         True
         >>> namespace.public_key == public.decode('string-escape')
         True
 
+    After this handler is called, the ssh key path is present as an attribute
+    of the namespace::
+
+        >>> namespace.ssh_key_path
+        '/tmp/.ssh/id_rsa'
+
     Keys are None if they are not provided and can not be found in the
     current home directory::
 
         >>> namespace = argparse.Namespace(
-        ...     private_key=None, public_key=None,
+        ...     private_key=None, public_key=None, ssh_key_name='id_rsa',
         ...     home_dir='/tmp/__does_not_exists__')
         >>> handle_ssh_keys(namespace) # doctest: +ELLIPSIS
         >>> print namespace.private_key
@@ -526,20 +559,21 @@
     ValidationError will be raised.
 
         >>> namespace = argparse.Namespace(
-        ...     private_key=private, public_key=None,
+        ...     private_key=private, public_key=None, ssh_key_name='id_rsa',
         ...     home_dir='/tmp/__does_not_exists__')
         >>> handle_ssh_keys(namespace) # doctest: +ELLIPSIS
         Traceback (most recent call last):
         ValidationError: arguments private-key...
     """
-    for attr, filename in (
-        ('private_key', 'id_rsa'),
-        ('public_key', 'id_rsa.pub')):
+    namespace.ssh_key_path = os.path.join(
+        namespace.home_dir, '.ssh', namespace.ssh_key_name)
+    for attr, path in (
+        ('private_key', namespace.ssh_key_path),
+        ('public_key', namespace.ssh_key_path + '.pub')):
         value = getattr(namespace, attr)
         if value:
             setattr(namespace, attr, value.decode('string-escape'))
         else:
-            path = os.path.join(namespace.home_dir, '.ssh', filename)
             try:
                 value = open(path).read()
             except IOError:
@@ -635,6 +669,10 @@
     metavar='LXC_NAME (default={})'.format(LXC_NAME),
     help='The LXC container name.')
 parser.add_argument(
+    '-s', '--ssh-key-name', default=SSH_KEY_NAME,
+    metavar='SSH_KEY_NAME (default={})'.format(SSH_KEY_NAME),
+    help='The ssh key name used to connect to the LXC container.')
+parser.add_argument(
     '-d', '--dependencies-dir', default=DEPENDENCIES_DIR,
     metavar='DEPENDENCIES_DIR (default={})'.format(DEPENDENCIES_DIR),
     help='The directory of the Launchpad dependencies to be created. '
@@ -654,7 +692,7 @@
 
 
 def initialize_host(
-    user, fullname, email, lpuser, private_key, public_key,
+    user, fullname, email, lpuser, private_key, public_key, ssh_key_path,
     dependencies_dir, directory):
     """Initialize host machine."""
     # Install necessary deb packages.  This requires Oneiric or later.
@@ -663,11 +701,6 @@
     # Create the user (if he does not exist).
     if not user_exists(user):
         subprocess.call(['useradd', '-m', '-s', '/bin/bash', '-U', user])
-    # Generate root ssh keys if they do not exist.
-    if not os.path.exists('/root/.ssh/id_rsa.pub'):
-        subprocess.call([
-            'ssh-keygen', '-q', '-t', 'rsa', '-N', '',
-            '-f', '/root/.ssh/id_rsa'])
     with su(user) as env:
         # Set up the user's ssh directory.  The ssh key must be associated
         # with the lpuser's Launchpad account.
@@ -676,29 +709,26 @@
             os.makedirs(ssh_dir)
         # Generate user ssh keys if none are supplied.
         valid_ssh_keys = True
+        pub_key_path = ssh_key_path + '.pub'
         if private_key is None:
-            subprocess.call([
-                'ssh-keygen', '-q', '-t', 'rsa', '-N', '',
-                '-f', os.path.join(ssh_dir, 'id_rsa')])
-            private_key = open(os.path.join(ssh_dir, 'id_rsa')).read()
-            public_key = open(os.path.join(ssh_dir, 'id_rsa.pub')).read()
+            generate_ssh_keys(ssh_key_path)
+            private_key = open(ssh_key_path).read()
+            public_key = open(pub_key_path).read()
             valid_ssh_keys = False
-        priv_file = os.path.join(ssh_dir, 'id_rsa')
-        pub_file = os.path.join(ssh_dir, 'id_rsa.pub')
         auth_file = os.path.join(ssh_dir, 'authorized_keys')
         known_hosts = os.path.join(ssh_dir, 'known_hosts')
         known_host_content = subprocess.check_output([
             'ssh-keyscan', '-t', 'rsa', 'bazaar.launchpad.net'])
         for filename, contents, mode in [
-            (priv_file, private_key, 'w'),
-            (pub_file, public_key, 'w'),
+            (ssh_key_path, private_key, 'w'),
+            (pub_key_path, public_key, 'w'),
             (auth_file, public_key, 'a'),
             (known_hosts, known_host_content, 'a'),
             ]:
             with open(filename, mode) as f:
                 f.write('{}\n'.format(contents))
             os.chmod(filename, 0644)
-        os.chmod(priv_file, 0600)
+        os.chmod(ssh_key_path, 0600)
         # Set up bzr and Launchpad authentication.
         subprocess.call(['bzr', 'whoami', formataddr((fullname, email))])
         if valid_ssh_keys:
@@ -742,7 +772,7 @@
                 LP_SOURCE_DEPS, 'download-cache'])
 
 
-def create_lxc(user, lxcname):
+def create_lxc(user, lxcname, ssh_key_path):
     """Create the LXC container that will be used for ephemeral instances."""
     # XXX 2012-02-02 gmb bug=925024:
     #     These calls need to be removed once the lxc vs. apparmor bug
@@ -782,14 +812,12 @@
     # Set up root ssh key.
     user_authorized_keys = os.path.expanduser(
         '~' + user + '/.ssh/authorized_keys')
-    with open(user_authorized_keys, 'a') as f:
-        f.write(open('/root/.ssh/id_rsa.pub').read())
     dst = get_container_path(lxcname, '/root/.ssh/')
     if not os.path.exists(dst):
         os.makedirs(dst)
     shutil.copy(user_authorized_keys, dst)
     # SSH into the container.
-    sshcall = ssh(lxcname, user)
+    sshcall = ssh(lxcname, user, key=ssh_key_path)
     trials = 60
     while True:
         trials -= 1
@@ -803,11 +831,11 @@
             break
 
 
-def initialize_lxc(user, dependencies_dir, directory, lxcname):
+def initialize_lxc(user, dependencies_dir, directory, lxcname, ssh_key_path):
     """Set up the Launchpad development environment inside the LXC container.
     """
-    root_sshcall = ssh(lxcname)
-    sshcall = ssh(lxcname, user)
+    root_sshcall = ssh(lxcname, key=ssh_key_path)
+    sshcall = ssh(lxcname, user, key=ssh_key_path)
     # APT repository update.
     for apt_repository in APT_REPOSITORIES:
         repository = apt_repository.format(distro=LXC_GUEST_OS)
@@ -861,9 +889,9 @@
     root_sshcall('cd {} && make install'.format(checkout_dir))
 
 
-def stop_lxc(lxcname):
+def stop_lxc(lxcname, ssh_key_path):
     """Stop the lxc instance named `lxcname`."""
-    ssh(lxcname)('poweroff')
+    ssh(lxcname, key=ssh_key_path)('poweroff')
     timeout = 30
     while timeout:
         try:
@@ -882,13 +910,15 @@
 
 def main(
     user, fullname, email, lpuser, private_key, public_key, actions,
-    lxc_name, dependencies_dir, directory):
+    lxc_name, ssh_key_path, dependencies_dir, directory):
     function_args_map = OrderedDict((
-        ('initialize_host', (user, fullname, email, lpuser, private_key,
-                             public_key, dependencies_dir, directory)),
-        ('create_lxc', (user, lxc_name)),
-        ('initialize_lxc', (user, dependencies_dir, directory, lxc_name)),
-        ('stop_lxc', (lxc_name,)),
+        ('initialize_host', (
+            user, fullname, email, lpuser, private_key, public_key,
+            ssh_key_path, dependencies_dir, directory)),
+        ('create_lxc', (user, lxc_name, ssh_key_path)),
+        ('initialize_lxc', (
+            user, dependencies_dir, directory, lxc_name, ssh_key_path)),
+        ('stop_lxc', (lxc_name, ssh_key_path)),
         ))
     if actions is None:
         actions = function_args_map.keys()
@@ -912,6 +942,7 @@
             args.public_key,
             args.actions,
             args.lxc_name,
+            args.ssh_key_path,
             args.dependencies_dir,
             args.directory,
             )