Merge lp:~jml/lp-dev-utils/public-ip into lp:lp-dev-utils

[Jonathan Lange]

When using the 'ec2' tool behind a transparent proxy, such as is running at Canonical's Millbank offices, it will fail with an odd error:

  SSHException: Error reading SSH protocol banner

See http://pastebin.ubuntu.com/960374/ for the full version.

This is because Amazon's checkip service is honouring the X-Forwarded-For HTTP header. Since the IP being forwarded for is internal, checkip returns the internal IP, and then ec2test grants that internal IP ssh access, which means we don't get SSH access and so can't complete setting up the instance.

This branch solves the problem in two ways:
 1. Use whatismyip.akamai.com instead of checkip.amazonaws.com, which behaves properly
 2. Allow users to specify a --public-ip option to override that behaviour

Particular thanks to cjwatson who diagnosed this bug and told me how to fix it.

[Aaron Bentley (abentley)]

review: Approve

=== modified file 'ec2test/account.py'
--- ec2test/account.py	2012-02-24 20:10:13 +0000
+++ ec2test/account.py	2012-05-02 13:43:22 +0000
@@ -49,8 +49,12 @@
 Please double-check before reporting a problem.
 """
 
-
-def get_ip():
+# An external website to consult to determine the public IP address of this
+# machine.  Must return an IP address (and nothing else) on GET.
+IP_GUESS_URL = 'http://whatismyip.akamai.com/'
+
+
+def guess_public_ip():
     """Uses AWS checkip to obtain this machine's IP address.
 
     Consults an external website to determine the public IP address of this
@@ -58,7 +62,7 @@
 
     :return: This machine's net-visible IP address as a string.
     """
-    return urllib.urlopen('http://checkip.amazonaws.com').read().strip()
+    return urllib.urlopen(IP_GUESS_URL).read().strip()
 
 
 class EC2Account:
@@ -99,7 +103,7 @@
                     session_name.expires < now)):
                 yield artifact
 
-    def acquire_security_group(self, demo_networks=None):
+    def acquire_security_group(self, demo_networks=None, public_ip=None):
         """Get a security group with the appropriate configuration.
 
         "Appropriate" means configured to allow this machine to connect via
@@ -115,10 +119,11 @@
         security_group = self.conn.create_security_group(
             self.name, 'Authorization to access the test runner instance.')
         # Authorize SSH and HTTP.
-        ip = get_ip()
-        security_group.authorize('tcp', 22, 22, '%s/32' % ip)
-        security_group.authorize('tcp', 80, 80, '%s/32' % ip)
-        security_group.authorize('tcp', 443, 443, '%s/32' % ip)
+        if public_ip is None:
+            public_ip = guess_public_ip()
+        security_group.authorize('tcp', 22, 22, '%s/32' % public_ip)
+        security_group.authorize('tcp', 80, 80, '%s/32' % public_ip)
+        security_group.authorize('tcp', 443, 443, '%s/32' % public_ip)
         for network in demo_networks:
             # Add missing netmask info for single ips.
             if '/' not in network:

=== modified file 'ec2test/builtins.py'
--- ec2test/builtins.py	2012-02-28 16:59:33 +0000
+++ ec2test/builtins.py	2012-05-02 13:43:22 +0000
@@ -93,6 +93,14 @@
           (AVAILABLE_INSTANCE_TYPES, DEFAULT_INSTANCE_TYPE)))
 
 
+public_ip_option = Option(
+    'public-ip', type=str, param_name='public_ip',
+    help=('The public IP address to use.  If not provided, '
+          'we try to guess it.  If we guess wrong (say, you '
+          'are behind a transparent proxy) then you should use '
+          'this to correct the guess.'))
+
+
 debug_option = Option(
     'debug', short_name='d',
     help=('Drop to pdb trace as soon as possible.'))
@@ -226,6 +234,7 @@
         machine_id_option,
         instance_type_option,
         region_option,
+        public_ip_option,
         Option(
             'file', short_name='f', type=filename_type,
             help=('Store abridged test results in FILE.')),
@@ -293,7 +302,7 @@
             pqm_submit_location=None, pqm_email=None, postmortem=False,
             attached=False, debug=False, open_browser=False,
             region=None,
-            include_download_cache_changes=False):
+            include_download_cache_changes=False, public_ip=None):
         set_trace_if(debug)
         if branch is None:
             branch = []
@@ -322,7 +331,7 @@
 
         session_name = EC2SessionName.make(EC2TestRunner.name)
         instance = EC2Instance.make(session_name, instance_type, machine,
-            region=region)
+            region=region, public_ip=public_ip)
 
         runner = EC2TestRunner(
             test_branch, email=email, file=file,
@@ -346,6 +355,7 @@
         instance_type_option,
         region_option,
         machine_id_option,
+        public_ip_option,
         Option('dry-run', help="Just print the equivalent ec2 test command."),
         Option('print-commit', help="Print the full commit message."),
         Option(
@@ -396,7 +406,7 @@
             debug=False, commit_text=None, dry_run=False, testfix=False,
             no_qa=False, incremental=False, rollback=None, print_commit=False,
             force=False, attached=False,
-            region=DEFAULT_REGION,
+            region=DEFAULT_REGION, public_ip=None,
             ):
         from bzrlib.plugins.pqm.lpland import (
             LaunchpadBranchLander, MissingReviewError, MissingBugsError,
@@ -464,7 +474,8 @@
 
         session_name = EC2SessionName.make(EC2TestRunner.name)
         instance = EC2Instance.make(
-            session_name, instance_type, machine, region=region)
+            session_name, instance_type, machine, region=region,
+            public_ip=public_ip)
 
         runner = EC2TestRunner(
             mp.source_branch, email=emails,
@@ -493,6 +504,7 @@
         debug_option,
         include_download_cache_changes_option,
         region_option,
+        public_ip_option,
         ListOption(
             'demo', type=str,
             help="Allow this netmask to connect to the instance."),
@@ -502,7 +514,7 @@
 
     def run(self, test_branch=None, branch=None, trunk=False, machine=None,
             instance_type=DEFAULT_INSTANCE_TYPE, debug=False,
-            include_download_cache_changes=False, demo=None):
+            include_download_cache_changes=False, demo=None, public_ip=None):
         set_trace_if(debug)
         if branch is None:
             branch = []
@@ -511,7 +523,7 @@
 
         session_name = EC2SessionName.make(EC2TestRunner.name)
         instance = EC2Instance.make(
-            session_name, instance_type, machine, demo)
+            session_name, instance_type, machine, demo, public_ip=public_ip)
 
         runner = EC2TestRunner(
             test_branch, branches=branches,
@@ -562,6 +574,7 @@
         postmortem_option,
         debug_option,
         region_option,
+        public_ip_option,
         ListOption(
             'extra-update-image-command', type=str,
             help=('Run this command (with an ssh agent) on the image before '
@@ -579,7 +592,7 @@
     def run(self, ami_name, machine=None, instance_type='m1.large',
             debug=False, postmortem=False, extra_update_image_command=None,
             region=None,
-            public=False):
+            public=False, public_ip=None):
         set_trace_if(debug)
 
         if extra_update_image_command is None:
@@ -595,7 +608,7 @@
         session_name = EC2SessionName.make(EC2TestRunner.name)
         instance = EC2Instance.make(
             session_name, instance_type, machine,
-            region=region)
+            region=region, public_ip=public_ip)
         instance.check_bundling_prerequisites(ami_name)
         instance.set_up_and_run(
             postmortem, True, self.update_image, instance,

=== modified file 'ec2test/instance.py'
--- ec2test/instance.py	2012-03-19 11:51:18 +0000
+++ ec2test/instance.py	2012-05-02 13:43:22 +0000
@@ -201,7 +201,7 @@
 
     @classmethod
     def make(cls, name, instance_type, machine_id, demo_networks=None,
-             credentials=None, region=None):
+             credentials=None, region=None, public_ip=None):
         """Construct an `EC2Instance`.
 
         :param name: The name to use for the key pair and security group for
@@ -266,12 +266,13 @@
 
         instance = EC2Instance(
             name, image, instance_type, demo_networks, account,
-            from_scratch, user_key, login, region)
+            from_scratch, user_key, login, region, public_ip=public_ip)
         instance._credentials = credentials
         return instance
 
     def __init__(self, name, image, instance_type, demo_networks, account,
-                 from_scratch, user_key, launchpad_login, region):
+                 from_scratch, user_key, launchpad_login, region,
+                 public_ip=None):
         self._name = name
         self._image = image
         self._account = account
@@ -282,6 +283,7 @@
         self._user_key = user_key
         self._launchpad_login = launchpad_login
         self._region = region
+        self._public_ip = public_ip
 
     def log(self, msg):
         """Log a message on stdout, flushing afterwards."""
@@ -298,7 +300,7 @@
         start = time.time()
         self.private_key = self._account.acquire_private_key()
         self.security_group = self._account.acquire_security_group(
-            demo_networks=self._demo_networks)
+            demo_networks=self._demo_networks, public_ip=self._public_ip)
         reservation = self._image.run(
             key_name=self._name, security_groups=[self._name],
             instance_type=self._instance_type)

=== modified file 'ec2test/tests/test_ec2instance.py'
--- ec2test/tests/test_ec2instance.py	2012-04-13 14:35:59 +0000
+++ ec2test/tests/test_ec2instance.py	2012-05-02 13:43:22 +0000
@@ -16,7 +16,7 @@
     def acquire_private_key(self):
         pass
 
-    def acquire_security_group(self, demo_networks=None):
+    def acquire_security_group(self, demo_networks=None, public_ip=None):
         pass