launchpad-reviewers team mailing list archive

[Martin Packman <martin.packman@xxxxxxxxxxxxx>]

Martin Packman has proposed merging lp:~gz/lp-dev-utils/creds_from_environment into lp:lp-dev-utils.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~gz/lp-dev-utils/creds_from_environment/+merge/112289

Takes credentials for EC2 from the environment before checking in a file in a ~/.ec2 directory. This is what I use currently with euca2ools and I prefer not to have the same details in two places.

Added some tests for loading credentials, using testtools as I really wanted addCleanup based functionality and I'm not sure everyone is on 2.7 locally yet, whereas testtools should be available as bzr depends on it and is required.
-- 
https://code.launchpad.net/~gz/lp-dev-utils/creds_from_environment/+merge/112289
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gz/lp-dev-utils/creds_from_environment into lp:lp-dev-utils.

=== modified file 'ec2test/builtins.py'
--- ec2test/builtins.py	2012-05-01 16:54:06 +0000
+++ ec2test/builtins.py	2012-06-27 08:26:30 +0000
@@ -679,7 +679,7 @@
 
     def run(self, region=None):
         session_name = EC2SessionName.make(EC2TestRunner.name)
-        credentials = EC2Credentials.load_from_file(region_name=region)
+        credentials = EC2Credentials.load(region_name=region)
         account = credentials.connect(session_name)
         format = "%5s  %-12s  %-12s  %-12s %s\n"
         self.outf.write(
@@ -704,7 +704,7 @@
     takes_args = ['instance_id*']
 
     def run(self, instance_id_list, region=None):
-        credentials = EC2Credentials.load_from_file(region_name=region)
+        credentials = EC2Credentials.load(region_name=region)
         account = credentials.connect('ec2 kill')
         self.outf.write("killing %d instances: " % len(instance_id_list,))
         account.conn.terminate_instances(instance_id_list)
@@ -806,7 +806,7 @@
 
     def run(self, show_urls=False, all=False, region=None):
         session_name = EC2SessionName.make(EC2TestRunner.name)
-        credentials = EC2Credentials.load_from_file(region_name=region)
+        credentials = EC2Credentials.load(region_name=region)
         account = credentials.connect(session_name)
         instances = list(self.iter_instances(account))
         if len(instances) == 0:

=== modified file 'ec2test/credentials.py'
--- ec2test/credentials.py	2012-02-24 20:10:13 +0000
+++ ec2test/credentials.py	2012-06-27 08:26:30 +0000
@@ -41,6 +41,25 @@
         self.region_name = region_name or instance.DEFAULT_REGION
 
     @classmethod
+    def load(cls, region_name=None):
+        """Load credentials from either environment or default file path."""
+        try:
+            return cls(*cls._keys_from_environment(), region_name=region_name)
+        except KeyError:
+            return cls.load_from_file(region_name=region_name)
+
+    @staticmethod
+    def _keys_from_environment():
+        """Get the EC2 api keys from environment variables.
+
+        The EC2_ forms euca2ools uses, and the AWS_ forms txaws uses."""
+        try:
+            return os.environ["EC2_ACCESS_KEY"], os.environ["EC2_SECRET_KEY"]
+        except KeyError:
+            return (os.environ["AWS_ACCESS_KEY_ID"],
+                os.environ["AWS_SECRET_ACCESS_KEY"])
+
+    @classmethod
     def load_from_file(cls, filename=None, region_name=None):
         """Load the EC2 credentials from 'filename'."""
         if filename is None:

=== modified file 'ec2test/instance.py'
--- ec2test/instance.py	2012-05-04 10:14:56 +0000
+++ ec2test/instance.py	2012-06-27 08:26:30 +0000
@@ -231,7 +231,7 @@
         user_key = get_user_key()
 
         if credentials is None:
-            credentials = EC2Credentials.load_from_file(region_name=region)
+            credentials = EC2Credentials.load(region_name=region)
 
         # Make the EC2 connection.
         account = credentials.connect(name)

=== added file 'ec2test/tests/test_credentials.py'
--- ec2test/tests/test_credentials.py	1970-01-01 00:00:00 +0000
+++ ec2test/tests/test_credentials.py	2012-06-27 08:26:30 +0000
@@ -0,0 +1,74 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for EC2 credential handling."""
+
+import os
+import tempfile
+
+import testtools
+
+from ec2test import (
+    credentials,
+    instance,
+    )
+
+
+class TestEC2Credentials(testtools.TestCase):
+
+    def _patch_creds(self, environ, default_filename=os.devnull):
+        self.patch(os, "environ", environ)
+        self.patch(credentials.EC2Credentials, "DEFAULT_CREDENTIALS_FILE",
+            default_filename)
+
+    def test_load_environ_ec2_vars(self):
+        """Envvars as used with euca2ools will be loaded if present"""
+        self._patch_creds({
+            "EC2_ACCESS_KEY": "ec2-identifier",
+            "EC2_SECRET_KEY": "ec2-secret",
+            })
+        creds = credentials.EC2Credentials.load()
+        self.assertEqual("ec2-identifier", creds.identifier)
+        self.assertEqual("ec2-secret", creds.secret)
+        self.assertEqual(instance.DEFAULT_REGION, creds.region_name)
+
+    def test_load_environ_aws_vars(self):
+        """Envvars as used with txaws will be loaded if present"""
+        self._patch_creds({
+            "AWS_ACCESS_KEY_ID": "aws-identifier",
+            "AWS_SECRET_ACCESS_KEY": "aws-secret",
+            })
+        creds = credentials.EC2Credentials.load("a-region")
+        self.assertEqual("aws-identifier", creds.identifier)
+        self.assertEqual("aws-secret", creds.secret)
+        self.assertEqual("a-region", creds.region_name)
+
+    def test_load_default_file(self):
+        """Credentials loaded from default file if there are no envvars"""
+        with tempfile.NamedTemporaryFile() as f:
+            f.write("an-identifier\na-secret\n")
+            f.flush()
+            self._patch_creds({}, f.name)
+            creds = credentials.EC2Credentials.load()
+        self.assertEqual("an-identifier", creds.identifier)
+        self.assertEqual("a-secret", creds.secret)
+        self.assertEqual(instance.DEFAULT_REGION, creds.region_name)
+
+    def test_load_default_file_with_region(self):
+        """Passing custom region works when loading from default file"""
+        with tempfile.NamedTemporaryFile() as f:
+            f.write("an-identifier\na-secret\n")
+            f.flush()
+            self._patch_creds({}, f.name)
+            creds = credentials.EC2Credentials.load(region_name="a-region")
+        self.assertEqual("an-identifier", creds.identifier)
+        self.assertEqual("a-secret", creds.secret)
+        self.assertEqual("a-region", creds.region_name)
+
+    def test_load_missing_file(self):
+        """No envvars and a missing credentials file raises a useful error"""
+        bad_filename = "/non-existant-file"
+        self._patch_creds({}, bad_filename)
+        e = self.assertRaises(credentials.CredentialsError,
+            credentials.EC2Credentials.load)
+        self.assertEqual(bad_filename, e.filename)