Merge lp:~jaypipes/glance/bug771849 into lp:~hudson-openstack/glance/trunk

[Jay Pipes]

This patch:

    Overhaul the way that the store URI works. We can now support
    specifying the authurls for Swift and S3 with either an
    http://, an https:// or no prefix at all.

    The following specifies an HTTP Swift auth URL:

    swift_store_auth_address = http://localhost/v1

    The following specifies an HTTPS Swift auth URL:

    swift_store_auth_address = https://localhost/v1
    swift_store_autth_address = localhost/v1

    The default scheme for Swift auth URL is HTTPS. For S3,
    the default for the s3_store_service_address is HTTP,
    since boto uses private key signing over HTTP by default
    on the s3.amazonaws.com domain. Note that the new S3 stuff
    isn't merged yet...

Future refactoring:

* Instead of storing the store_uri in the database in images.location, split the store_uri out into component pieces in an ImageLocation table, reducing duplicate records in the registry database.

* Never return the store_uri in the metadata. This contains security credentials (see LP Bug #755916)

[Brian Waldon (bcwaldon)]

Seeing some test failures, Jay: http://paste.openstack.org/show/1887/

review: Needs Fixing

[Brian Waldon (bcwaldon)]

Awesome work, Jay. Great tests, btw.

review: Approve

[Brian Lamar (blamar)]

Yeah, these imports need some serious un-twisting. Logic seems sound and tests abound, but tests seem to be failing for me:

http://paste.openstack.org/show/1920/

Also, is there a blueprint for the refactoring of backends/locations so as to remove the delayed imports?

review: Needs Fixing

[Jay Pipes (jaypipes)]

> Yeah, these imports need some serious un-twisting. Logic seems sound and tests
> abound, but tests seem to be failing for me:
>
> http://paste.openstack.org/show/1920/

Grrr, fix for MacOSX broke everything else... gah, back to work I go.

> Also, is there a blueprint for the refactoring of backends/locations so as to
> remove the delayed imports?

Yup: https://blueprints.launchpad.net/glance/+spec/refactor-stores was the one I was going to do that in.
-jay

[Jay Pipes (jaypipes)]

Alright, tests pass on Ubuntu 10.10, 11.04, and MacOSX. Please re-try. Thanks.
-jay

[Brian Waldon (bcwaldon)]

Yep, still working for me.

[lp:~jaypipes/glance/bug771849]

136. By Jay Pipes on 2011-07-21

PEP8 nit

[Brian Lamar (blamar)]

review: Approve

=== modified file 'glance/common/exception.py'
--- glance/common/exception.py	2011-06-26 20:43:41 +0000
+++ glance/common/exception.py	2011-07-21 13:59:26 +0000
@@ -54,6 +54,24 @@
     pass
 
 
+class UnknownScheme(Error):
+
+    msg = "Unknown scheme '%s' found in URI"
+
+    def __init__(self, scheme):
+        msg = self.__class__.msg % scheme
+        super(UnknownScheme, self).__init__(msg)
+
+
+class BadStoreUri(Error):
+
+    msg = "The Store URI %s was malformed. Reason: %s"
+
+    def __init__(self, uri, reason):
+        msg = self.__class__.msg % (uri, reason)
+        super(BadStoreUri, self).__init__(msg)
+
+
 class Duplicate(Error):
     pass
 
 
=== modified file 'glance/store/__init__.py'
--- glance/store/__init__.py	2011-06-28 14:37:31 +0000
+++ glance/store/__init__.py	2011-07-21 13:59:26 +0000
@@ -20,6 +20,7 @@
 import urlparse
 
 from glance.common import exception
+from glance.store import location
 
 
 # TODO(sirp): should this be moved out to common/utils.py ?
@@ -74,69 +75,29 @@
 def get_from_backend(uri, **kwargs):
     """Yields chunks of data from backend specified by uri"""
 
-    parsed_uri = urlparse.urlparse(uri)
-    scheme = parsed_uri.scheme
-
-    backend_class = get_backend_class(scheme)
-
-    return backend_class.get(parsed_uri, **kwargs)
+    loc = location.get_location_from_uri(uri)
+    backend_class = get_backend_class(loc.store_name)
+
+    return backend_class.get(loc, **kwargs)
 
 
 def delete_from_backend(uri, **kwargs):
     """Removes chunks of data from backend specified by uri"""
 
-    parsed_uri = urlparse.urlparse(uri)
-    scheme = parsed_uri.scheme
-
-    backend_class = get_backend_class(scheme)
+    loc = location.get_location_from_uri(uri)
+    backend_class = get_backend_class(loc.store_name)
 
     if hasattr(backend_class, 'delete'):
-        return backend_class.delete(parsed_uri, **kwargs)
-
-
-def get_store_from_location(location):
+        return backend_class.delete(loc, **kwargs)
+
+
+def get_store_from_location(uri):
     """
     Given a location (assumed to be a URL), attempt to determine
     the store from the location.  We use here a simple guess that
     the scheme of the parsed URL is the store...
 
-    :param location: Location to check for the store
-    """
-    loc_pieces = urlparse.urlparse(location)
-    return loc_pieces.scheme
-
-
-def parse_uri_tokens(parsed_uri, example_url):
-    """
-    Given a URI and an example_url, attempt to parse the uri to assemble an
-    authurl. This method returns the user, key, authurl, referenced container,
-    and the object we're looking for in that container.
-
-    Parsing the uri is three phases:
-        1) urlparse to split the tokens
-        2) use RE to split on @ and /
-        3) reassemble authurl
-    """
-    path = parsed_uri.path.lstrip('//')
-    netloc = parsed_uri.netloc
-
-    try:
-        try:
-            creds, netloc = netloc.split('@')
-        except ValueError:
-            # Python 2.6.1 compat
-            # see lp659445 and Python issue7904
-            creds, path = path.split('@')
-        user, key = creds.split(':')
-        path_parts = path.split('/')
-        obj = path_parts.pop()
-        container = path_parts.pop()
-    except (ValueError, IndexError):
-        raise BackendException(
-             "Expected four values to unpack in: %s:%s. "
-             "Should have received something like: %s."
-             % (parsed_uri.scheme, parsed_uri.path, example_url))
-
-    authurl = "https://%s" % '/'.join(path_parts)
-
-    return user, key, authurl, container, obj
+    :param uri: Location to check for the store
+    """
+    loc = location.get_location_from_uri(uri)
+    return loc.store_name
 
=== modified file 'glance/store/filesystem.py'
--- glance/store/filesystem.py	2011-06-28 14:37:31 +0000
+++ glance/store/filesystem.py	2011-07-21 13:59:26 +0000
@@ -26,9 +26,39 @@
 
 from glance.common import exception
 import glance.store
+import glance.store.location
 
 logger = logging.getLogger('glance.store.filesystem')
 
+glance.store.location.add_scheme_map({'file': 'filesystem'})
+
+
+class StoreLocation(glance.store.location.StoreLocation):
+
+    """Class describing a Filesystem URI"""
+
+    def process_specs(self):
+        self.scheme = self.specs.get('scheme', 'file')
+        self.path = self.specs.get('path')
+
+    def get_uri(self):
+        return "file://%s" % self.path
+
+    def parse_uri(self, uri):
+        """
+        Parse URLs. This method fixes an issue where credentials specified
+        in the URL are interpreted differently in Python 2.6.1+ than prior
+        versions of Python.
+        """
+        pieces = urlparse.urlparse(uri)
+        assert pieces.scheme == 'file'
+        self.scheme = pieces.scheme
+        path = (pieces.netloc + pieces.path).strip()
+        if path == '':
+            reason = "No path specified"
+            raise exception.BadStoreUri(uri, reason)
+        self.path = path
+
 
 class ChunkedFile(object):
 
@@ -64,13 +94,19 @@
 
 class FilesystemBackend(glance.store.Backend):
     @classmethod
-    def get(cls, parsed_uri, expected_size=None, options=None):
-        """
-        Filesystem-based backend
-
-        file:///path/to/file.tar.gz.0
-        """
-        filepath = parsed_uri.path
+    def get(cls, location, expected_size=None, options=None):
+        """
+        Takes a `glance.store.location.Location` object that indicates
+        where to find the image file, and returns a generator to use in
+        reading the image file.
+
+        :location `glance.store.location.Location` object, supplied
+                  from glance.store.location.get_location_from_uri()
+
+        :raises NotFound if file does not exist
+        """
+        loc = location.store_location
+        filepath = loc.path
         if not os.path.exists(filepath):
             raise exception.NotFound("Image file %s not found" % filepath)
         else:
@@ -79,17 +115,19 @@
             return ChunkedFile(filepath)
 
     @classmethod
-    def delete(cls, parsed_uri):
+    def delete(cls, location):
         """
-        Removes a file from the filesystem backend.
+        Takes a `glance.store.location.Location` object that indicates
+        where to find the image file to delete
 
-        :param parsed_uri: Parsed pieces of URI in form of::
-            file:///path/to/filename.ext
+        :location `glance.store.location.Location` object, supplied
+                  from glance.store.location.get_location_from_uri()
 
         :raises NotFound if file does not exist
         :raises NotAuthorized if cannot delete because of permissions
         """
-        fn = parsed_uri.path
+        loc = location.store_location
+        fn = loc.path
         if os.path.exists(fn):
             try:
                 logger.debug("Deleting image at %s", fn)
 
=== modified file 'glance/store/http.py'
--- glance/store/http.py	2011-06-28 14:37:31 +0000
+++ glance/store/http.py	2011-07-21 13:59:26 +0000
@@ -16,31 +16,104 @@
 #    under the License.
 
 import httplib
+import urlparse
 
+from glance.common import exception
 import glance.store
+import glance.store.location
+
+glance.store.location.add_scheme_map({'http': 'http',
+                                      'https': 'http'})
+
+
+class StoreLocation(glance.store.location.StoreLocation):
+
+    """Class describing an HTTP(S) URI"""
+
+    def process_specs(self):
+        self.scheme = self.specs.get('scheme', 'http')
+        self.netloc = self.specs['netloc']
+        self.user = self.specs.get('user')
+        self.password = self.specs.get('password')
+        self.path = self.specs.get('path')
+
+    def _get_credstring(self):
+        if self.user:
+            return '%s:%s@' % (self.user, self.password)
+        return ''
+
+    def get_uri(self):
+        return "%s://%s%s%s" % (
+            self.scheme,
+            self._get_credstring(),
+            self.netloc,
+            self.path)
+
+    def parse_uri(self, uri):
+        """
+        Parse URLs. This method fixes an issue where credentials specified
+        in the URL are interpreted differently in Python 2.6.1+ than prior
+        versions of Python.
+        """
+        pieces = urlparse.urlparse(uri)
+        assert pieces.scheme in ('https', 'http')
+        self.scheme = pieces.scheme
+        netloc = pieces.netloc
+        path = pieces.path
+        try:
+            if '@' in netloc:
+                creds, netloc = netloc.split('@')
+            else:
+                creds = None
+        except ValueError:
+            # Python 2.6.1 compat
+            # see lp659445 and Python issue7904
+            if '@' in path:
+                creds, path = path.split('@')
+            else:
+                creds = None
+        if creds:
+            try:
+                self.user, self.password = creds.split(':')
+            except ValueError:
+                reason = ("Credentials '%s' not well-formatted."
+                          % "".join(creds))
+                raise exception.BadStoreUri(uri, reason)
+        else:
+            self.user = None
+        if netloc == '':
+            reason = "No address specified in HTTP URL"
+            raise exception.BadStoreUri(uri, reason)
+        self.netloc = netloc
+        self.path = path
 
 
 class HTTPBackend(glance.store.Backend):
     """ An implementation of the HTTP Backend Adapter """
 
     @classmethod
-    def get(cls, parsed_uri, expected_size, options=None, conn_class=None):
-        """
-        Takes a parsed uri for an HTTP resource, fetches it, and
-        yields the data.
-        """
+    def get(cls, location, expected_size, options=None, conn_class=None):
+        """
+        Takes a `glance.store.location.Location` object that indicates
+        where to find the image file, and returns a generator from Swift
+        provided by Swift client's get_object() method.
+
+        :location `glance.store.location.Location` object, supplied
+                  from glance.store.location.get_location_from_uri()
+        """
+        loc = location.store_location
         if conn_class:
             pass  # use the conn_class passed in
-        elif parsed_uri.scheme == "http":
+        elif loc.scheme == "http":
             conn_class = httplib.HTTPConnection
-        elif parsed_uri.scheme == "https":
+        elif loc.scheme == "https":
             conn_class = httplib.HTTPSConnection
         else:
             raise glance.store.BackendException(
                 "scheme '%s' not supported for HTTPBackend")
 
-        conn = conn_class(parsed_uri.netloc)
-        conn.request("GET", parsed_uri.path, "", {})
+        conn = conn_class(loc.netloc)
+        conn.request("GET", loc.path, "", {})
 
         try:
             return glance.store._file_iter(conn.getresponse(), cls.CHUNKSIZE)
 
=== added file 'glance/store/location.py'
--- glance/store/location.py	1970-01-01 00:00:00 +0000
+++ glance/store/location.py	2011-07-21 13:59:26 +0000
@@ -0,0 +1,182 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack, LLC
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+A class that describes the location of an image in Glance.
+
+In Glance, an image can either be **stored** in Glance, or it can be
+**registered** in Glance but actually be stored somewhere else.
+
+We needed a class that could support the various ways that Glance
+describes where exactly an image is stored.
+
+An image in Glance has two location properties: the image URI
+and the image storage URI.
+
+The image URI is essentially the permalink identifier for the image.
+It is displayed in the output of various Glance API calls and,
+while read-only, is entirely user-facing. It shall **not** contain any
+security credential information at all. The Glance image URI shall
+be the host:port of that Glance API server along with /images/<IMAGE_ID>.
+
+The Glance storage URI is an internal URI structure that Glance
+uses to maintain critical information about how to access the images
+that it stores in its storage backends. It **does contain** security
+credentials and is **not** user-facing.
+"""
+
+import logging
+import urlparse
+
+from glance.common import exception
+from glance.common import utils
+
+logger = logging.getLogger('glance.store.location')
+
+SCHEME_TO_STORE_MAP = {}
+
+
+def get_location_from_uri(uri):
+    """
+    Given a URI, return a Location object that has had an appropriate
+    store parse the URI.
+
+    :param uri: A URI that could come from the end-user in the Location
+                attribute/header
+
+    Example URIs:
+        https://user:[e-mail]:80/images/some-id
+        http://images.oracle.com/123456
+        swift://user:account:[e-mail]/container/obj-id
+        swift+http://user:account:[e-mail]/container/obj-id
+        s3://accesskey:[e-mail]/bucket/key-id
+        s3+https://accesskey:[e-mail]/bucket/key-id
+        file:///var/lib/glance/images/1
+    """
+    # Add known stores to mapping... this gets past circular import
+    # issues. There's a better way to do this, but that's for another
+    # patch...
+    # TODO(jaypipes) Clear up these imports in refactor-stores blueprint
+    import glance.store.filesystem
+    import glance.store.http
+    import glance.store.s3
+    import glance.store.swift
+    pieces = urlparse.urlparse(uri)
+    if pieces.scheme not in SCHEME_TO_STORE_MAP.keys():
+        raise exception.UnknownScheme(pieces.scheme)
+    loc = Location(pieces.scheme, uri=uri)
+    return loc
+
+
+def add_scheme_map(scheme_map):
+    """
+    Given a mapping of 'scheme' to store_name, adds the mapping to the
+    known list of schemes.
+
+    Each store should call this method and let Glance know about which
+    schemes to map to a store name.
+    """
+    SCHEME_TO_STORE_MAP.update(scheme_map)
+
+
+class Location(object):
+
+    """
+    Class describing the location of an image that Glance knows about
+    """
+
+    def __init__(self, store_name, uri=None, image_id=None, store_specs=None):
+        """
+        Create a new Location object.
+
+        :param store_name: The string identifier of the storage backend
+        :param image_id: The identifier of the image in whatever storage
+                         backend is used.
+        :param uri: Optional URI to construct location from
+        :param store_specs: Dictionary of information about the location
+                            of the image that is dependent on the backend
+                            store
+        """
+        self.store_name = store_name
+        self.image_id = image_id
+        self.store_specs = store_specs or {}
+        self.store_location = self._get_store_location()
+        if uri:
+            self.store_location.parse_uri(uri)
+
+    def _get_store_location(self):
+        """
+        We find the store module and then grab an instance of the store's
+        StoreLocation class which handles store-specific location information
+        """
+        try:
+            cls = utils.import_class('glance.store.%s.StoreLocation'
+                                     % SCHEME_TO_STORE_MAP[self.store_name])
+            return cls(self.store_specs)
+        except exception.NotFound:
+            logger.error("Unable to find StoreLocation class in store %s",
+                         self.store_name)
+            return None
+
+    def get_store_uri(self):
+        """
+        Returns the Glance image URI, which is the host:port of the API server
+        along with /images/<IMAGE_ID>
+        """
+        return self.store_location.get_uri()
+
+    def get_uri(self):
+        return None
+
+
+class StoreLocation(object):
+
+    """
+    Base class that must be implemented by each store
+    """
+
+    def __init__(self, store_specs):
+        self.specs = store_specs
+        if self.specs:
+            self.process_specs()
+
+    def process_specs(self):
+        """
+        Subclasses should implement any processing of the self.specs collection
+        such as storing credentials and possibly establishing connections.
+        """
+        pass
+
+    def get_uri(self):
+        """
+        Subclasses should implement a method that returns an internal URI that,
+        when supplied to the StoreLocation instance, can be interpreted by the
+        StoreLocation's parse_uri() method. The URI returned from this method
+        shall never be public and only used internally within Glance, so it is
+        fine to encode credentials in this URI.
+        """
+        raise NotImplementedError("StoreLocation subclass must implement "
+                                  "get_uri()")
+
+    def parse_uri(self, uri):
+        """
+        Subclasses should implement a method that accepts a string URI and
+        sets appropriate internal fields such that a call to get_uri() will
+        return a proper internal URI
+        """
+        raise NotImplementedError("StoreLocation subclass must implement "
+                                  "parse_uri()")
 
=== modified file 'glance/store/s3.py'
--- glance/store/s3.py	2011-01-27 04:19:13 +0000
+++ glance/store/s3.py	2011-07-21 13:59:26 +0000
@@ -17,7 +17,101 @@
 
 """The s3 backend adapter"""
 
+import urlparse
+
+from glance.common import exception
 import glance.store
+import glance.store.location
+
+glance.store.location.add_scheme_map({'s3': 's3',
+                                      's3+http': 's3',
+                                      's3+https': 's3'})
+
+
+class StoreLocation(glance.store.location.StoreLocation):
+
+    """
+    Class describing an S3 URI. An S3 URI can look like any of
+    the following:
+
+        s3://accesskey:[e-mail]/bucket/key-id
+        s3+http://accesskey:[e-mail]/bucket/key-id
+        s3+https://accesskey:[e-mail]/bucket/key-id
+
+    The s3+https:// URIs indicate there is an HTTPS s3service URL
+    """
+
+    def process_specs(self):
+        self.scheme = self.specs.get('scheme', 's3')
+        self.accesskey = self.specs.get('accesskey')
+        self.secretkey = self.specs.get('secretkey')
+        self.s3serviceurl = self.specs.get('s3serviceurl')
+        self.bucket = self.specs.get('bucket')
+        self.key = self.specs.get('key')
+
+    def _get_credstring(self):
+        if self.accesskey:
+            return '%s:%s@' % (self.accesskey, self.secretkey)
+        return ''
+
+    def get_uri(self):
+        return "%s://%s%s/%s/%s" % (
+            self.scheme,
+            self._get_credstring(),
+            self.s3serviceurl,
+            self.bucket,
+            self.key)
+
+    def parse_uri(self, uri):
+        """
+        Parse URLs. This method fixes an issue where credentials specified
+        in the URL are interpreted differently in Python 2.6.1+ than prior
+        versions of Python.
+
+        Note that an Amazon AWS secret key can contain the forward slash,
+        which is entirely retarded, and breaks urlparse miserably.
+        This function works around that issue.
+        """
+        pieces = urlparse.urlparse(uri)
+        assert pieces.scheme in ('s3', 's3+http', 's3+https')
+        self.scheme = pieces.scheme
+        path = pieces.path.strip('/')
+        netloc = pieces.netloc.strip('/')
+        entire_path = (netloc + '/' + path).strip('/')
+
+        if '@' in uri:
+            creds, path = entire_path.split('@')
+            cred_parts = creds.split(':')
+
+            try:
+                access_key = cred_parts[0]
+                secret_key = cred_parts[1]
+                # NOTE(jaypipes): Need to encode to UTF-8 here because of a
+                # bug in the HMAC library that boto uses.
+                # See: http://bugs.python.org/issue5285
+                # See: http://trac.edgewall.org/ticket/8083
+                access_key = access_key.encode('utf-8')
+                secret_key = secret_key.encode('utf-8')
+                self.accesskey = access_key
+                self.secretkey = secret_key
+            except IndexError:
+                reason = "Badly formed S3 credentials %s" % creds
+                raise exception.BadStoreUri(uri, reason)
+        else:
+            self.accesskey = None
+            path = entire_path
+        try:
+            path_parts = path.split('/')
+            self.key = path_parts.pop()
+            self.bucket = path_parts.pop()
+            if len(path_parts) > 0:
+                self.s3serviceurl = '/'.join(path_parts)
+            else:
+                reason = "Badly formed S3 URI. Missing s3 service URL."
+                raise exception.BadStoreUri(uri, reason)
+        except IndexError:
+            reason = "Badly formed S3 URI"
+            raise exception.BadStoreUri(uri, reason)
 
 
 class S3Backend(glance.store.Backend):
@@ -26,29 +120,30 @@
     EXAMPLE_URL = "s3://ACCESS_KEY:SECRET_KEY@s3_url/bucket/file.gz.0"
 
     @classmethod
-    def get(cls, parsed_uri, expected_size, conn_class=None):
-        """
-        Takes a parsed_uri in the format of:
-        s3://access_key:[e-mail]/bucket/file.gz.0, connects
-        to s3 and downloads the file. Returns the generator resp_body provided
-        by get_object.
-        """
+    def get(cls, location, expected_size, conn_class=None):
+        """
+        Takes a `glance.store.location.Location` object that indicates
+        where to find the image file, and returns a generator from S3
+        provided by S3's key object
 
+        :location `glance.store.location.Location` object, supplied
+                  from glance.store.location.get_location_from_uri()
+        """
         if conn_class:
             pass
         else:
             import boto.s3.connection
             conn_class = boto.s3.connection.S3Connection
 
-        (access_key, secret_key, host, bucket, obj) = \
-            cls._parse_s3_tokens(parsed_uri)
+        loc = location.store_location
 
         # Close the connection when we're through.
-        with conn_class(access_key, secret_key, host=host) as s3_conn:
-            bucket = cls._get_bucket(s3_conn, bucket)
+        with conn_class(loc.accesskey, loc.secretkey,
+                        host=loc.s3serviceurl) as s3_conn:
+            bucket = cls._get_bucket(s3_conn, loc.bucket)
 
             # Close the key when we're through.
-            with cls._get_key(bucket, obj) as key:
+            with cls._get_key(bucket, loc.obj) as key:
                 if not key.size == expected_size:
                     raise glance.store.BackendException(
                         "Expected %s bytes, got %s" %
@@ -59,28 +154,28 @@
                     yield chunk
 
     @classmethod
-    def delete(cls, parsed_uri, conn_class=None):
-        """
-        Takes a parsed_uri in the format of:
-        s3://access_key:[e-mail]/bucket/file.gz.0, connects
-        to s3 and deletes the file. Returns whatever boto.s3.key.Key.delete()
-        returns.
-        """
+    def delete(cls, location, conn_class=None):
+        """
+        Takes a `glance.store.location.Location` object that indicates
+        where to find the image file to delete
 
+        :location `glance.store.location.Location` object, supplied
+                  from glance.store.location.get_location_from_uri()
+        """
         if conn_class:
             pass
         else:
             conn_class = boto.s3.connection.S3Connection
 
-        (access_key, secret_key, host, bucket, obj) = \
-            cls._parse_s3_tokens(parsed_uri)
+        loc = location.store_location
 
         # Close the connection when we're through.
-        with conn_class(access_key, secret_key, host=host) as s3_conn:
-            bucket = cls._get_bucket(s3_conn, bucket)
+        with conn_class(loc.accesskey, loc.secretkey,
+                        host=loc.s3serviceurl) as s3_conn:
+            bucket = cls._get_bucket(s3_conn, loc.bucket)
 
             # Close the key when we're through.
-            with cls._get_key(bucket, obj) as key:
+            with cls._get_key(bucket, loc.obj) as key:
                 return key.delete()
 
     @classmethod
@@ -102,8 +197,3 @@
         if not key:
             raise glance.store.BackendException("Could not get key: %s" % key)
         return key
-
-    @classmethod
-    def _parse_s3_tokens(cls, parsed_uri):
-        """Parse tokens from the parsed_uri"""
-        return glance.store.parse_uri_tokens(parsed_uri, cls.EXAMPLE_URL)
 
=== modified file 'glance/store/swift.py'
--- glance/store/swift.py	2011-06-28 14:37:31 +0000
+++ glance/store/swift.py	2011-07-21 13:59:26 +0000
@@ -21,15 +21,114 @@
 
 import httplib
 import logging
+import urlparse
 
 from glance.common import config
 from glance.common import exception
 import glance.store
+import glance.store.location
 
 DEFAULT_SWIFT_CONTAINER = 'glance'
 
 logger = logging.getLogger('glance.store.swift')
 
+glance.store.location.add_scheme_map({'swift': 'swift',
+                                      'swift+http': 'swift',
+                                      'swift+https': 'swift'})
+
+
+class StoreLocation(glance.store.location.StoreLocation):
+
+    """
+    Class describing a Swift URI. A Swift URI can look like any of
+    the following:
+
+        swift://user:[e-mail]/container/obj-id
+        swift+http://user:[e-mail]/container/obj-id
+        swift+https://user:[e-mail]/container/obj-id
+
+    The swift+https:// URIs indicate there is an HTTPS authentication URL
+    """
+
+    def process_specs(self):
+        self.scheme = self.specs.get('scheme', 'swift+https')
+        self.user = self.specs.get('user')
+        self.key = self.specs.get('key')
+        self.authurl = self.specs.get('authurl')
+        self.container = self.specs.get('container')
+        self.obj = self.specs.get('obj')
+
+    def _get_credstring(self):
+        if self.user:
+            return '%s:%s@' % (self.user, self.key)
+        return ''
+
+    def get_uri(self):
+        return "%s://%s%s/%s/%s" % (
+            self.scheme,
+            self._get_credstring(),
+            self.authurl,
+            self.container,
+            self.obj)
+
+    def parse_uri(self, uri):
+        """
+        Parse URLs. This method fixes an issue where credentials specified
+        in the URL are interpreted differently in Python 2.6.1+ than prior
+        versions of Python. It also deals with the peculiarity that new-style
+        Swift URIs have where a username can contain a ':', like so:
+
+            swift://account:user:[e-mail]/container/obj
+        """
+        pieces = urlparse.urlparse(uri)
+        assert pieces.scheme in ('swift', 'swift+http', 'swift+https')
+        self.scheme = pieces.scheme
+        netloc = pieces.netloc
+        path = pieces.path.lstrip('/')
+        if netloc != '':
+            # > Python 2.6.1
+            if '@' in netloc:
+                creds, netloc = netloc.split('@')
+            else:
+                creds = None
+        else:
+            # Python 2.6.1 compat
+            # see lp659445 and Python issue7904
+            if '@' in path:
+                creds, path = path.split('@')
+            else:
+                creds = None
+            netloc = path[0:path.find('/')].strip('/')
+            path = path[path.find('/'):].strip('/')
+        if creds:
+            cred_parts = creds.split(':')
+
+            # User can be account:user, in which case cred_parts[0:2] will be
+            # the account and user. Combine them into a single username of
+            # account:user
+            if len(cred_parts) == 1:
+                reason = "Badly formed credentials '%s' in Swift URI" % creds
+                raise exception.BadStoreUri(uri, reason)
+            elif len(cred_parts) == 3:
+                user = ':'.join(cred_parts[0:2])
+            else:
+                user = cred_parts[0]
+            key = cred_parts[-1]
+            self.user = user
+            self.key = key
+        else:
+            self.user = None
+        path_parts = path.split('/')
+        try:
+            self.obj = path_parts.pop()
+            self.container = path_parts.pop()
+            self.authurl = netloc
+            if len(path_parts) > 0:
+                self.authurl = netloc + '/' + '/'.join(path_parts).strip('/')
+        except IndexError:
+            reason = "Badly formed Swift URI"
+            raise exception.BadStoreUri(uri, reason)
+
 
 class SwiftBackend(glance.store.Backend):
     """An implementation of the swift backend adapter."""
@@ -39,31 +138,33 @@
     CHUNKSIZE = 65536
 
     @classmethod
-    def get(cls, parsed_uri, expected_size=None, options=None):
+    def get(cls, location, expected_size=None, options=None):
         """
-        Takes a parsed_uri in the format of:
-        swift://user:password@auth_url/container/file.gz.0, connects to the
-        swift instance at auth_url and downloads the file. Returns the
-        generator resp_body provided by get_object.
+        Takes a `glance.store.location.Location` object that indicates
+        where to find the image file, and returns a generator from Swift
+        provided by Swift client's get_object() method.
+
+        :location `glance.store.location.Location` object, supplied
+                  from glance.store.location.get_location_from_uri()
         """
         from swift.common import client as swift_client
-        (user, key, authurl, container, obj) = parse_swift_tokens(parsed_uri)
 
         # TODO(sirp): snet=False for now, however, if the instance of
         # swift we're talking to is within our same region, we should set
         # snet=True
+        loc = location.store_location
         swift_conn = swift_client.Connection(
-            authurl=authurl, user=user, key=key, snet=False)
+            authurl=loc.authurl, user=loc.user, key=loc.key, snet=False)
 
         try:
             (resp_headers, resp_body) = swift_conn.get_object(
-                container=container, obj=obj, resp_chunk_size=cls.CHUNKSIZE)
+                container=loc.container, obj=loc.obj,
+                resp_chunk_size=cls.CHUNKSIZE)
         except swift_client.ClientException, e:
             if e.http_status == httplib.NOT_FOUND:
-                location = format_swift_location(user, key, authurl,
-                                                 container, obj)
+                uri = location.get_store_uri()
                 raise exception.NotFound("Swift could not find image at "
-                                         "location %(location)s" % locals())
+                                         "uri %(uri)s" % locals())
 
         if expected_size:
             obj_size = int(resp_headers['content-length'])
@@ -98,6 +199,10 @@
             <CONTAINER> = ``swift_store_container``
             <ID> = The id of the image being added
 
+        :note Swift auth URLs by default use HTTPS. To specify an HTTP
+              auth URL, you can specify http://someurl.com for the
+              swift_store_auth_address config option
+
         :param id: The opaque image identifier
         :param data: The image data to write, as a file-like object
         :param options: Conf mapping
@@ -119,9 +224,14 @@
         user = cls._option_get(options, 'swift_store_user')
         key = cls._option_get(options, 'swift_store_key')
 
-        full_auth_address = auth_address
-        if not full_auth_address.startswith('http'):
-            full_auth_address = 'https://' + full_auth_address
+        scheme = 'swift+https'
+        if auth_address.startswith('http://'):
+            scheme = 'swift+http'
+            full_auth_address = auth_address
+        elif auth_address.startswith('https://'):
+            full_auth_address = auth_address
+        else:
+            full_auth_address = 'https://' + auth_address  # Defaults https
 
         swift_conn = swift_client.Connection(
             authurl=full_auth_address, user=user, key=key, snet=False)
@@ -133,8 +243,13 @@
         create_container_if_missing(container, swift_conn, options)
 
         obj_name = str(id)
-        location = format_swift_location(user, key, auth_address,
-                                         container, obj_name)
+        location = StoreLocation({'scheme': scheme,
+                                  'container': container,
+                                  'obj': obj_name,
+                                  'authurl': auth_address,
+                                  'user': user,
+                                  'key': key})
+
         try:
             obj_etag = swift_conn.put_object(container, obj_name, data)
 
@@ -152,7 +267,7 @@
             # header keys are lowercased by Swift
             if 'content-length' in resp_headers:
                 size = int(resp_headers['content-length'])
-            return (location, size, obj_etag)
+            return (location.get_uri(), size, obj_etag)
         except swift_client.ClientException, e:
             if e.http_status == httplib.CONFLICT:
                 raise exception.Duplicate("Swift already has an image at "
@@ -162,89 +277,34 @@
             raise glance.store.BackendException(msg)
 
     @classmethod
-    def delete(cls, parsed_uri):
+    def delete(cls, location):
         """
-        Deletes the swift object(s) at the parsed_uri location
+        Takes a `glance.store.location.Location` object that indicates
+        where to find the image file to delete
+
+        :location `glance.store.location.Location` object, supplied
+                  from glance.store.location.get_location_from_uri()
         """
         from swift.common import client as swift_client
-        (user, key, authurl, container, obj) = parse_swift_tokens(parsed_uri)
 
         # TODO(sirp): snet=False for now, however, if the instance of
         # swift we're talking to is within our same region, we should set
         # snet=True
+        loc = location.store_location
         swift_conn = swift_client.Connection(
-            authurl=authurl, user=user, key=key, snet=False)
+            authurl=loc.authurl, user=loc.user, key=loc.key, snet=False)
 
         try:
-            swift_conn.delete_object(container, obj)
+            swift_conn.delete_object(loc.container, loc.obj)
         except swift_client.ClientException, e:
             if e.http_status == httplib.NOT_FOUND:
-                location = format_swift_location(user, key, authurl,
-                                                 container, obj)
+                uri = location.get_store_uri()
                 raise exception.NotFound("Swift could not find image at "
-                                         "location %(location)s" % locals())
+                                         "uri %(uri)s" % locals())
             else:
                 raise
 
 
-def parse_swift_tokens(parsed_uri):
-    """
-    Return the various tokens used by Swift.
-
-    :param parsed_uri: The pieces of a URI returned by urlparse
-    :retval A tuple of (user, key, auth_address, container, obj_name)
-    """
-    path = parsed_uri.path.lstrip('//')
-    netloc = parsed_uri.netloc
-
-    try:
-        try:
-            creds, netloc = netloc.split('@')
-            path = '/'.join([netloc, path])
-        except ValueError:
-            # Python 2.6.1 compat
-            # see lp659445 and Python issue7904
-            creds, path = path.split('@')
-
-        cred_parts = creds.split(':')
-
-        # User can be account:user, in which case cred_parts[0:2] will be
-        # the account and user. Combine them into a single username of
-        # account:user
-        if len(cred_parts) == 3:
-            user = ':'.join(cred_parts[0:2])
-        else:
-            user = cred_parts[0]
-        key = cred_parts[-1]
-        path_parts = path.split('/')
-        obj = path_parts.pop()
-        container = path_parts.pop()
-    except (ValueError, IndexError):
-        raise glance.store.BackendException(
-             "Expected four values to unpack in: swift:%s. "
-             "Should have received something like: %s."
-             % (parsed_uri.path, SwiftBackend.EXAMPLE_URL))
-
-    authurl = "https://%s" % '/'.join(path_parts)
-
-    return user, key, authurl, container, obj
-
-
-def format_swift_location(user, key, auth_address, container, obj_name):
-    """
-    Returns the swift URI in the format:
-        swift://<USER>:<KEY>@<AUTH_ADDRESS>/<CONTAINER>/<OBJNAME>
-
-    :param user: The swift user to authenticate with
-    :param key: The auth key for the authenticating user
-    :param auth_address: The base URL for the authentication service
-    :param container: The name of the container
-    :param obj_name: The name of the object
-    """
-    return "swift://%(user)s:%(key)s@%(auth_address)s/"\
-           "%(container)s/%(obj_name)s" % locals()
-
-
 def create_container_if_missing(container, swift_conn, options):
     """
     Creates a missing container in Swift if the
 
=== modified file 'tests/stubs.py'
--- tests/stubs.py	2011-07-18 18:22:41 +0000
+++ tests/stubs.py	2011-07-21 13:59:26 +0000
@@ -128,13 +128,9 @@
         DATA = 'I am a teapot, short and stout\n'
 
         @classmethod
-        def get(cls, parsed_uri, expected_size, conn_class=None):
+        def get(cls, location, expected_size, conn_class=None):
             S3Backend = glance.store.s3.S3Backend
 
-            # raise BackendException if URI is bad.
-            (user, key, authurl, container, obj) = \
-                S3Backend._parse_s3_tokens(parsed_uri)
-
             def chunk_it():
                 for i in xrange(0, len(cls.DATA), cls.CHUNK_SIZE):
                     yield cls.DATA[i:i + cls.CHUNK_SIZE]
 
=== modified file 'tests/unit/test_filesystem_store.py'
--- tests/unit/test_filesystem_store.py	2011-03-08 15:22:44 +0000
+++ tests/unit/test_filesystem_store.py	2011-07-21 13:59:26 +0000
@@ -20,11 +20,11 @@
 import StringIO
 import hashlib
 import unittest
-import urlparse
 
 import stubout
 
 from glance.common import exception
+from glance.store.location import get_location_from_uri
 from glance.store.filesystem import FilesystemBackend, ChunkedFile
 from tests import stubs
 
@@ -51,8 +51,8 @@
 
     def test_get(self):
         """Test a "normal" retrieval of an image in chunks"""
-        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/2")
-        image_file = FilesystemBackend.get(url_pieces)
+        loc = get_location_from_uri("file:///tmp/glance-tests/2")
+        image_file = FilesystemBackend.get(loc)
 
         expected_data = "chunk00000remainder"
         expected_num_chunks = 2
@@ -70,10 +70,10 @@
         Test that trying to retrieve a file that doesn't exist
         raises an error
         """
-        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/non-existing")
+        loc = get_location_from_uri("file:///tmp/glance-tests/non-existing")
         self.assertRaises(exception.NotFound,
                           FilesystemBackend.get,
-                          url_pieces)
+                          loc)
 
     def test_add(self):
         """Test that we can add an image via the filesystem backend"""
@@ -93,8 +93,8 @@
         self.assertEquals(expected_file_size, size)
         self.assertEquals(expected_checksum, checksum)
 
-        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/42")
-        new_image_file = FilesystemBackend.get(url_pieces)
+        loc = get_location_from_uri("file:///tmp/glance-tests/42")
+        new_image_file = FilesystemBackend.get(loc)
         new_image_contents = ""
         new_image_file_size = 0
 
@@ -122,20 +122,19 @@
         """
         Test we can delete an existing image in the filesystem store
         """
-        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/2")
-
-        FilesystemBackend.delete(url_pieces)
+        loc = get_location_from_uri("file:///tmp/glance-tests/2")
+        FilesystemBackend.delete(loc)
 
         self.assertRaises(exception.NotFound,
                           FilesystemBackend.get,
-                          url_pieces)
+                          loc)
 
     def test_delete_non_existing(self):
         """
         Test that trying to delete a file that doesn't exist
         raises an error
         """
-        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/non-existing")
+        loc = get_location_from_uri("file:///tmp/glance-tests/non-existing")
         self.assertRaises(exception.NotFound,
                           FilesystemBackend.delete,
-                          url_pieces)
+                          loc)
 
=== added file 'tests/unit/test_store_location.py'
--- tests/unit/test_store_location.py	1970-01-01 00:00:00 +0000
+++ tests/unit/test_store_location.py	2011-07-21 13:59:26 +0000
@@ -0,0 +1,243 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack, LLC
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import unittest
+
+from glance.common import exception
+import glance.store.location as location
+import glance.store.http
+import glance.store.filesystem
+import glance.store.swift
+import glance.store.s3
+
+
+class TestStoreLocation(unittest.TestCase):
+
+    def test_get_location_from_uri_back_to_uri(self):
+        """
+        Test that for various URIs, the correct Location
+        object can be contructed and then the original URI
+        returned via the get_store_uri() method.
+        """
+        good_store_uris = [
+            'https://user:[e-mail]:80/images/some-id',
+            'http://images.oracle.com/123456',
+            'swift://account:user:[e-mail]/container/obj-id',
+            'swift+https://account:user:[e-mail]/container/obj-id',
+            's3://accesskey:[e-mail]/bucket/key-id',
+            's3://accesskey:secretwith/[e-mail]/bucket/key-id',
+            's3+http://accesskey:[e-mail]/bucket/key-id',
+            's3+https://accesskey:[e-mail]/bucket/key-id',
+            'file:///var/lib/glance/images/1']
+
+        for uri in good_store_uris:
+            loc = location.get_location_from_uri(uri)
+            # The get_store_uri() method *should* return an identical URI
+            # to the URI that is passed to get_location_from_uri()
+            self.assertEqual(loc.get_store_uri(), uri)
+
+    def test_bad_store_scheme(self):
+        """
+        Test that a URI with a non-existing scheme triggers exception
+        """
+        bad_uri = 'unknown://user:[e-mail]:80/images/some-id'
+
+        self.assertRaises(exception.UnknownScheme,
+                          location.get_location_from_uri,
+                          bad_uri)
+
+    def test_filesystem_store_location(self):
+        """
+        Test the specific StoreLocation for the Filesystem store
+        """
+        uri = 'file:///var/lib/glance/images/1'
+        loc = glance.store.filesystem.StoreLocation({})
+        loc.parse_uri(uri)
+
+        self.assertEqual("file", loc.scheme)
+        self.assertEqual("/var/lib/glance/images/1", loc.path)
+        self.assertEqual(uri, loc.get_uri())
+
+        bad_uri = 'fil://'
+        self.assertRaises(Exception, loc.parse_uri, bad_uri)
+
+        bad_uri = 'file://'
+        self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri)
+
+    def test_http_store_location(self):
+        """
+        Test the specific StoreLocation for the HTTP store
+        """
+        uri = 'http://example.com/images/1'
+        loc = glance.store.http.StoreLocation({})
+        loc.parse_uri(uri)
+
+        self.assertEqual("http", loc.scheme)
+        self.assertEqual("example.com", loc.netloc)
+        self.assertEqual("/images/1", loc.path)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 'https://example.com:8080/images/container/1'
+        loc.parse_uri(uri)
+
+        self.assertEqual("https", loc.scheme)
+        self.assertEqual("example.com:8080", loc.netloc)
+        self.assertEqual("/images/container/1", loc.path)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 'https://user:[e-mail]:8080/images/container/1'
+        loc.parse_uri(uri)
+
+        self.assertEqual("https", loc.scheme)
+        self.assertEqual("example.com:8080", loc.netloc)
+        self.assertEqual("user", loc.user)
+        self.assertEqual("password", loc.password)
+        self.assertEqual("/images/container/1", loc.path)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 'https://user:@example.com:8080/images/1'
+        loc.parse_uri(uri)
+
+        self.assertEqual("https", loc.scheme)
+        self.assertEqual("example.com:8080", loc.netloc)
+        self.assertEqual("user", loc.user)
+        self.assertEqual("", loc.password)
+        self.assertEqual("/images/1", loc.path)
+        self.assertEqual(uri, loc.get_uri())
+
+        bad_uri = 'htt://'
+        self.assertRaises(Exception, loc.parse_uri, bad_uri)
+
+        bad_uri = 'http://'
+        self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri)
+
+        bad_uri = 'http://[e-mail]:8080/images/1'
+        self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri)
+
+    def test_swift_store_location(self):
+        """
+        Test the specific StoreLocation for the Swift store
+        """
+        uri = 'swift://example.com/images/1'
+        loc = glance.store.swift.StoreLocation({})
+        loc.parse_uri(uri)
+
+        self.assertEqual("swift", loc.scheme)
+        self.assertEqual("example.com", loc.authurl)
+        self.assertEqual("images", loc.container)
+        self.assertEqual("1", loc.obj)
+        self.assertEqual(None, loc.user)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 'swift+https://user:[e-mail]/images/1'
+        loc.parse_uri(uri)
+
+        self.assertEqual("swift+https", loc.scheme)
+        self.assertEqual("authurl.com", loc.authurl)
+        self.assertEqual("images", loc.container)
+        self.assertEqual("1", loc.obj)
+        self.assertEqual("user", loc.user)
+        self.assertEqual("pass", loc.key)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 'swift+https://user:[e-mail]/v1/container/12345'
+        loc.parse_uri(uri)
+
+        self.assertEqual("swift+https", loc.scheme)
+        self.assertEqual("authurl.com/v1", loc.authurl)
+        self.assertEqual("container", loc.container)
+        self.assertEqual("12345", loc.obj)
+        self.assertEqual("user", loc.user)
+        self.assertEqual("pass", loc.key)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 'swift://account:user:[e-mail]/v1/container/12345'
+        loc.parse_uri(uri)
+
+        self.assertEqual("swift", loc.scheme)
+        self.assertEqual("authurl.com/v1", loc.authurl)
+        self.assertEqual("container", loc.container)
+        self.assertEqual("12345", loc.obj)
+        self.assertEqual("account:user", loc.user)
+        self.assertEqual("pass", loc.key)
+        self.assertEqual(uri, loc.get_uri())
+
+        bad_uri = 'swif://'
+        self.assertRaises(Exception, loc.parse_uri, bad_uri)
+
+        bad_uri = 'swift://'
+        self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri)
+
+        bad_uri = 'swift://[e-mail]:8080/images/1'
+        self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri)
+
+    def test_s3_store_location(self):
+        """
+        Test the specific StoreLocation for the S3 store
+        """
+        uri = 's3://example.com/images/1'
+        loc = glance.store.s3.StoreLocation({})
+        loc.parse_uri(uri)
+
+        self.assertEqual("s3", loc.scheme)
+        self.assertEqual("example.com", loc.s3serviceurl)
+        self.assertEqual("images", loc.bucket)
+        self.assertEqual("1", loc.key)
+        self.assertEqual(None, loc.accesskey)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 's3+https://accesskey:[e-mail]/images/1'
+        loc.parse_uri(uri)
+
+        self.assertEqual("s3+https", loc.scheme)
+        self.assertEqual("s3serviceurl.com", loc.s3serviceurl)
+        self.assertEqual("images", loc.bucket)
+        self.assertEqual("1", loc.key)
+        self.assertEqual("accesskey", loc.accesskey)
+        self.assertEqual("pass", loc.secretkey)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 's3+https://accesskey:[e-mail]/v1/bucket/12345'
+        loc.parse_uri(uri)
+
+        self.assertEqual("s3+https", loc.scheme)
+        self.assertEqual("s3serviceurl.com/v1", loc.s3serviceurl)
+        self.assertEqual("bucket", loc.bucket)
+        self.assertEqual("12345", loc.key)
+        self.assertEqual("accesskey", loc.accesskey)
+        self.assertEqual("pass", loc.secretkey)
+        self.assertEqual(uri, loc.get_uri())
+
+        uri = 's3://accesskey:pass/[e-mail]/v1/bucket/12345'
+        loc.parse_uri(uri)
+
+        self.assertEqual("s3", loc.scheme)
+        self.assertEqual("s3serviceurl.com/v1", loc.s3serviceurl)
+        self.assertEqual("bucket", loc.bucket)
+        self.assertEqual("12345", loc.key)
+        self.assertEqual("accesskey", loc.accesskey)
+        self.assertEqual("pass/withslash", loc.secretkey)
+        self.assertEqual(uri, loc.get_uri())
+
+        bad_uri = 'swif://'
+        self.assertRaises(Exception, loc.parse_uri, bad_uri)
+
+        bad_uri = 's3://'
+        self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri)
+
+        bad_uri = 's3://[e-mail]:8080/images/1'
+        self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri)
 
=== modified file 'tests/unit/test_stores.py'
--- tests/unit/test_stores.py	2011-03-05 00:02:26 +0000
+++ tests/unit/test_stores.py	2011-07-21 13:59:26 +0000
@@ -19,7 +19,6 @@
 
 import stubout
 import unittest
-import urlparse
 
 from glance.store.s3 import S3Backend
 from glance.store import Backend, BackendException, get_from_backend
 
=== modified file 'tests/unit/test_swift_store.py'
--- tests/unit/test_swift_store.py	2011-04-13 23:18:26 +0000
+++ tests/unit/test_swift_store.py	2011-07-21 13:59:26 +0000
@@ -29,9 +29,8 @@
 
 from glance.common import exception
 from glance.store import BackendException
-from glance.store.swift import (SwiftBackend,
-                                format_swift_location,
-                                parse_swift_tokens)
+from glance.store.swift import SwiftBackend
+from glance.store.location import get_location_from_uri
 
 FIVE_KB = (5 * 1024)
 SWIFT_OPTIONS = {'verbose': True,
@@ -146,6 +145,18 @@
               'http_connection', fake_http_connection)
 
 
+def format_swift_location(user, key, authurl, container, obj):
+    """
+    Helper method that returns a Swift store URI given
+    the component pieces.
+    """
+    scheme = 'swift+https'
+    if authurl.startswith('http://'):
+        scheme = 'swift+http'
+    return "%s://%s:%s@%s/%s/%s" % (scheme, user, key, authurl,
+                                    container, obj)
+
+
 class TestSwiftBackend(unittest.TestCase):
 
     def setUp(self):
@@ -157,46 +168,27 @@
         """Clear the test environment"""
         self.stubs.UnsetAll()
 
-    def test_parse_swift_tokens(self):
-        """
-        Test that the parse_swift_tokens function returns
-        user, key, authurl, container, and objname properly
-        """
-        uri = "swift://user:key@localhost/v1.0/container/objname"
-        url_pieces = urlparse.urlparse(uri)
-        user, key, authurl, container, objname =\
-                parse_swift_tokens(url_pieces)
-        self.assertEqual("user", user)
-        self.assertEqual("key", key)
-        self.assertEqual("https://localhost/v1.0", authurl)
-        self.assertEqual("container", container)
-        self.assertEqual("objname", objname)
-
-        uri = "swift://user:key@localhost:9090/v1.0/container/objname"
-        url_pieces = urlparse.urlparse(uri)
-        user, key, authurl, container, objname =\
-                parse_swift_tokens(url_pieces)
-        self.assertEqual("user", user)
-        self.assertEqual("key", key)
-        self.assertEqual("https://localhost:9090/v1.0", authurl)
-        self.assertEqual("container", container)
-        self.assertEqual("objname", objname)
-
-        uri = "swift://account:user:key@localhost:9090/v1.0/container/objname"
-        url_pieces = urlparse.urlparse(uri)
-        user, key, authurl, container, objname =\
-                parse_swift_tokens(url_pieces)
-        self.assertEqual("account:user", user)
-        self.assertEqual("key", key)
-        self.assertEqual("https://localhost:9090/v1.0", authurl)
-        self.assertEqual("container", container)
-        self.assertEqual("objname", objname)
-
     def test_get(self):
         """Test a "normal" retrieval of an image in chunks"""
-        url_pieces = urlparse.urlparse(
-            "swift://user:key@auth_address/glance/2")
-        image_swift = SwiftBackend.get(url_pieces)
+        loc = get_location_from_uri("swift://user:key@auth_address/glance/2")
+        image_swift = SwiftBackend.get(loc)
+
+        expected_data = "*" * FIVE_KB
+        data = ""
+
+        for chunk in image_swift:
+            data += chunk
+        self.assertEqual(expected_data, data)
+
+    def test_get_with_http_auth(self):
+        """
+        Test a retrieval from Swift with an HTTP authurl. This is
+        specified either via a Location header with swift+http:// or using
+        http:// in the swift_store_auth_address config value
+        """
+        loc = get_location_from_uri("swift+http://user:key@auth_address/"
+                                    "glance/2")
+        image_swift = SwiftBackend.get(loc)
 
         expected_data = "*" * FIVE_KB
         data = ""
@@ -210,11 +202,10 @@
         Test retrieval of an image with wrong expected_size param
         raises an exception
         """
-        url_pieces = urlparse.urlparse(
-            "swift://user:key@auth_address/glance/2")
+        loc = get_location_from_uri("swift://user:key@auth_address/glance/2")
         self.assertRaises(BackendException,
                           SwiftBackend.get,
-                          url_pieces,
+                          loc,
                           {'expected_size': 42})
 
     def test_get_non_existing(self):
@@ -222,11 +213,10 @@
         Test that trying to retrieve a swift that doesn't exist
         raises an error
         """
-        url_pieces = urlparse.urlparse(
-            "swift://user:key@auth_address/noexist")
+        loc = get_location_from_uri("swift://user:key@authurl/glance/noexist")
         self.assertRaises(exception.NotFound,
                           SwiftBackend.get,
-                          url_pieces)
+                          loc)
 
     def test_add(self):
         """Test that we can add an image via the swift backend"""
@@ -249,14 +239,62 @@
         self.assertEquals(expected_swift_size, size)
         self.assertEquals(expected_checksum, checksum)
 
-        url_pieces = urlparse.urlparse(expected_location)
-        new_image_swift = SwiftBackend.get(url_pieces)
+        loc = get_location_from_uri(expected_location)
+        new_image_swift = SwiftBackend.get(loc)
         new_image_contents = new_image_swift.getvalue()
         new_image_swift_size = new_image_swift.len
 
         self.assertEquals(expected_swift_contents, new_image_contents)
         self.assertEquals(expected_swift_size, new_image_swift_size)
 
+    def test_add_auth_url_variations(self):
+        """
+        Test that we can add an image via the swift backend with
+        a variety of different auth_address values
+        """
+        variations = ['http://localhost:80',
+                      'http://localhost',
+                      'http://localhost/v1',
+                      'http://localhost/v1/',
+                      'https://localhost',
+                      'https://localhost:8080',
+                      'https://localhost/v1',
+                      'https://localhost/v1/',
+                      'localhost',
+                      'localhost:8080/v1']
+        i = 42
+        for variation in variations:
+            expected_image_id = i
+            expected_swift_size = FIVE_KB
+            expected_swift_contents = "*" * expected_swift_size
+            expected_checksum = \
+                    hashlib.md5(expected_swift_contents).hexdigest()
+            new_options = SWIFT_OPTIONS.copy()
+            new_options['swift_store_auth_address'] = variation
+            expected_location = format_swift_location(
+                new_options['swift_store_user'],
+                new_options['swift_store_key'],
+                new_options['swift_store_auth_address'],
+                new_options['swift_store_container'],
+                expected_image_id)
+            image_swift = StringIO.StringIO(expected_swift_contents)
+
+            location, size, checksum = SwiftBackend.add(i, image_swift,
+                                                        new_options)
+
+            self.assertEquals(expected_location, location)
+            self.assertEquals(expected_swift_size, size)
+            self.assertEquals(expected_checksum, checksum)
+
+            loc = get_location_from_uri(expected_location)
+            new_image_swift = SwiftBackend.get(loc)
+            new_image_contents = new_image_swift.getvalue()
+            new_image_swift_size = new_image_swift.len
+
+            self.assertEquals(expected_swift_contents, new_image_contents)
+            self.assertEquals(expected_swift_size, new_image_swift_size)
+            i = i + 1
+
     def test_add_no_container_no_create(self):
         """
         Tests that adding an image with a non-existing container
@@ -306,8 +344,8 @@
         self.assertEquals(expected_swift_size, size)
         self.assertEquals(expected_checksum, checksum)
 
-        url_pieces = urlparse.urlparse(expected_location)
-        new_image_swift = SwiftBackend.get(url_pieces)
+        loc = get_location_from_uri(expected_location)
+        new_image_swift = SwiftBackend.get(loc)
         new_image_contents = new_image_swift.getvalue()
         new_image_swift_size = new_image_swift.len
 
@@ -356,22 +394,20 @@
         """
         Test we can delete an existing image in the swift store
         """
-        url_pieces = urlparse.urlparse(
-            "swift://user:key@auth_address/glance/2")
+        loc = get_location_from_uri("swift://user:key@authurl/glance/2")
 
-        SwiftBackend.delete(url_pieces)
+        SwiftBackend.delete(loc)
 
         self.assertRaises(exception.NotFound,
                           SwiftBackend.get,
-                          url_pieces)
+                          loc)
 
     def test_delete_non_existing(self):
         """
         Test that trying to delete a swift that doesn't exist
         raises an error
         """
-        url_pieces = urlparse.urlparse(
-            "swift://user:key@auth_address/noexist")
+        loc = get_location_from_uri("swift://user:key@authurl/glance/noexist")
         self.assertRaises(exception.NotFound,
                           SwiftBackend.delete,
-                          url_pieces)
+                          loc)