Merge lp:~marcoceppi/pyjuju/ssh-key-opt into lp:pyjuju

[Marco Ceppi]

Allow for public and private key setting (depending on the nature of the subcommand) to be set via the command line and the -i --identitiy-file opt.

This also adds a generalized "create_parser" method to allow for buried, deep down methods to access command line options.

[Kapil Thangavelu (hazmat)]

making the provider code depend on sys.argv is quite odd. what's the goal/value add?

[Clint Byrum (clint-fewbar)]

I think its more than odd, I don't think its appropriate to access sys.argv from that deep within juju. It should not be a challenge to have the argument parsed in juju.control.* where it makes sense, and then pushed down into the providers the same way other parameters are.

review: Needs Fixing

Unmerged revisions

585. By Marco Ceppi on 2012-09-22

Remove ~ from 'args' for common provider test

584. By Marco Ceppi on 2012-09-21

Now with unit tests and code coverage

583. By Marco Ceppi on 2012-09-21

sshclient doesn't need argparse anymore

582. By Marco Ceppi on 2012-09-21

Allow uses to specify which 'public-key' to bootstrap with, -i overrides config options

581. By Marco Ceppi on 2012-09-21

Make create_parser method and use that instead of defining it each time

580. By Marco Ceppi on 2012-09-21

Allow for passing ssh private keys via -i flag

579. By Marco Ceppi on 2012-09-20

Added -i flag

=== modified file 'juju/control/__init__.py'
--- juju/control/__init__.py	2012-03-30 19:12:05 +0000
+++ juju/control/__init__.py	2012-09-22 03:13:42 +0000
@@ -96,10 +96,9 @@
                 return (result, ) * tuple_size
         return format
 
-
-def setup_parser(subcommands, **kw):
+def create_parser(formatter, **kw):
     """Setup a command line argument/option parser."""
-    parser = JujuParser(formatter_class=JujuFormatter, **kw)
+    parser = JujuParser(formatter_class=formatter, **kw)
     parser.add_argument(
         "--verbose", "-v", default=False,
         action="store_true",
@@ -109,8 +108,16 @@
         "--log-file", "-l", default=sys.stderr, type=argparse.FileType('a'),
         help="Log output to file")
 
+    parser.add_argument(
+        "--identity-file", "-i", help="SSH key to authenticate with")
+
+    """This should probably take care of subparsers too, but I can't get
+    it to work properly, so it's just doing the "main" ones for now"""
+    return parser
+
+def setup_parser(subcommands, **kw):
+    parser = create_parser(JujuFormatter, **kw)
     subparsers = parser.add_subparsers()
-
     for module in subcommands:
         configure_subparser = getattr(module, "configure_subparser", None)
         passthrough = getattr(module, "passthrough", None)

=== modified file 'juju/providers/common/tests/test_utils.py'
--- juju/providers/common/tests/test_utils.py	2012-09-10 03:20:20 +0000
+++ juju/providers/common/tests/test_utils.py	2012-09-22 03:13:42 +0000
@@ -29,6 +29,35 @@
         key_data = get_user_authorized_keys(config)
         self.assertEqual(data, key_data)
 
+    def test_key_public_identity_file_retrieval(self):
+        """
+        The F{juju.providers.common.get_user_authorized_keys} will retrieve
+        a key's contents from disk OR the commandline and return the key data.
+        """
+        data = "through the looking glass"
+        self.makeFile(
+            data,
+            path=os.path.join(self.tmp_home, ".ssh", "identity_test.pub"))
+        key_data = get_user_authorized_keys({},
+            args=["juju", "-i", "%s/.ssh/identity_test.pub" % self.tmp_home])
+        self.assertEqual(data, key_data)
+
+    def test_key_private_identity_file_retrieval(self):
+        """
+        The F{juju.providers.common.get_user_authorized_keys} will retrieve
+        a key's contents from disk OR the commandline and return the key data.
+        """
+        data = "wabbits"
+        self.makeFile(
+            "wong %s" % data,
+            path=os.path.join(self.tmp_home, ".ssh", "identity_crisis"))
+        self.makeFile(
+            data,
+            path=os.path.join(self.tmp_home, ".ssh", "identity_crisis.pub"))
+        key_data = get_user_authorized_keys({},
+            args=["juju", "-i", "%s/.ssh/identity_crisis" % self.tmp_home])
+        self.assertEqual(data, key_data)
+
     def test_content_utilized(self):
         """
         The entire key content can be specified via configuration.

=== modified file 'juju/providers/common/utils.py'
--- juju/providers/common/utils.py	2012-09-10 03:20:20 +0000
+++ juju/providers/common/utils.py	2012-09-22 03:13:42 +0000
@@ -1,11 +1,12 @@
 import logging
 import os
-
+import sys
 
 from twisted.python.failure import Failure
 
 from juju.lib import serializer
 from juju.errors import JujuError, ProviderInteractionError
+from juju.control import create_parser, JujuFormatter
 
 log = logging.getLogger("juju.common")
 
@@ -50,7 +51,7 @@
 #     lines in the data obtained, and then the call site can add each
 #     individual key to CloudInit.
 #
-def get_user_authorized_keys(config):
+def get_user_authorized_keys(config, args=sys.argv):
     """Locate a public key for the user.
 
     If neither "authorized-keys" nor "authorized-keys-path" is
@@ -67,6 +68,20 @@
     :raises: :exc:`LookupError` if an SSH public key is not found.
     """
     key_names = ["id_dsa.pub", "id_rsa.pub", "identity.pub"]
+
+    parser = create_parser(JujuFormatter)
+    options, extra = parser.parse_known_args(args[1:])
+
+    if options.identity_file:
+        # This could be a private key, or a weirdly named public key
+        # so check if there is a .pub for this file first.
+        identity_file = os.path.expanduser(options.identity_file)
+        if os.path.exists(identity_file):
+            if os.path.exists("%s.pub" % identity_file):
+                identity_file = "%s.pub" % identity_file
+
+            return open(identity_file).read()
+
     if config.get("authorized-keys"):
         return config["authorized-keys"]
 

=== modified file 'juju/state/sshclient.py'
--- juju/state/sshclient.py	2011-12-07 21:13:52 +0000
+++ juju/state/sshclient.py	2012-09-22 03:13:42 +0000
@@ -1,4 +1,5 @@
 import re
+import sys
 import socket
 import time
 
@@ -9,11 +10,11 @@
 from juju.state.security import SecurityPolicyConnection
 from juju.state.sshforward import forward_port, ClientTunnelProtocol
 
+from juju.control import create_parser, JujuFormatter
 from .utils import PortWatcher, get_open_port
 
 SERVER_RE = re.compile("^(\S+):(\d+)$")
 
-
 class SSHClient(SecurityPolicyConnection):
     """
     A ZookeeperClient which will internally handle an SSH tunnel
@@ -40,6 +41,7 @@
         """
         hostname, port = self._parse_servers(server or self._servers)
         start_time = time.time()
+        identity_file = self._parse_identity(sys.argv[1:])
 
         # Determine which port we'll be using.
         local_port = get_open_port()
@@ -55,7 +57,8 @@
         protocol = ClientTunnelProtocol(self, tunnel_error)
         self._process = forward_port(
             self.remote_user, local_port, hostname, int(port),
-            process_protocol=protocol, share=share)
+            private_key_path=identity_file, process_protocol=protocol,
+            share=share)
 
         # Wait for the tunneled port to open.
         try:
@@ -91,6 +94,11 @@
         hostname, port = match.groups()
         return hostname, port
 
+    def _parse_identity(self, args):
+        parser = create_parser(JujuFormatter)
+        options, extra = parser.parse_known_args(args)
+        return options.identity_file
+
     @inlineCallbacks
     def connect(self, server=None, timeout=60, share=False):
         """Probe ZK is accessible via ssh tunnel, return client on success."""

=== modified file 'juju/state/tests/test_sshclient.py'
--- juju/state/tests/test_sshclient.py	2011-12-07 04:55:29 +0000
+++ juju/state/tests/test_sshclient.py	2012-09-22 03:13:42 +0000
@@ -82,7 +82,8 @@
             # First, get the forwarding going, targetting the remote
             # address provided.
             forward("ubuntu", MATCH_PORT, "remote", 2181,
-                    process_protocol=MATCH_PROTOCOL, share=False)
+                    private_key_path=None, process_protocol=MATCH_PROTOCOL,
+                    share=False)
             self.mocker.result(process)
 
             # Next ensure the port check succeeds
@@ -127,7 +128,9 @@
 
         protocol = self.mocker.mock()
         forward("ubuntu", MATCH_PORT, "remote", 2181,
-                process_protocol=MATCH_PROTOCOL, share=False)
+                private_key_path=None, process_protocol=MATCH_PROTOCOL,
+                share=False)
+
         self.mocker.result(protocol)
 
         thread(MATCH_FUNC)
@@ -151,7 +154,8 @@
 
         protocol = self.mocker.mock()
         forward("ubuntu", MATCH_PORT, "remote", 2181,
-                process_protocol=MATCH_PROTOCOL, share=False)
+                private_key_path=None, process_protocol=MATCH_PROTOCOL,
+                share=False)
         self.mocker.result(protocol)
 
         thread(MATCH_FUNC)
@@ -175,7 +179,8 @@
 
         protocol = self.mocker.mock()
         forward("ubuntu", MATCH_PORT, "remote", 2181,
-                process_protocol=MATCH_PROTOCOL, share=False)
+                private_key_path=None, process_protocol=MATCH_PROTOCOL,
+                share=False)
         self.mocker.result(protocol)
 
         thread(MATCH_FUNC)