Merge pull request #82 from dolph/master

Global fixes for issues #33 and #65

14 files changed, 176 insertions, 151 deletions

diff --git a/bin/keystone b/bin/keystone
index 4664f80d..1725ed25 100755
--- a/bin/keystone
+++ b/bin/keystone
@@ -69,20 +69,20 @@ if __name__ == '__main__':
             config_file = config.find_config_file(options, args)
             print "Using config file:", config_file
         
-        # Load API server
+        # Load Public API server
         server = wsgi.Server()
-        server.start(app, int(conf['server_bind_port']),
-                     conf['server_bind_host'])
-        print "Service API listening on %s:%s" % (conf['server_bind_host'],
-                                              conf['server_bind_port'])
-
+        server.start(app, int(conf['public_port']), conf['public_host'])
+        
+        print "Service API listening on %s:%s" % (
+            conf['public_host'], conf['public_port'])
+        
         # Load Admin API server
         admin_server = wsgi.Server()
-        admin_bind = options.get('admin_port') or admin_conf.get('bind_port')
-        admin_server.start(admin_app, int(admin_bind),
-                           admin_conf['bind_host'])
-        print "Admin API listening on %s:%s" % (admin_conf['bind_host'],
-                                              admin_bind)
+        admin_server.start(admin_app,
+            int(conf['admin_port']), conf['admin_host'])
+        
+        print "Admin API listening on %s:%s" % (
+            conf['admin_host'], conf['admin_port'])
         
         # Wait until done
         server.wait()
diff --git a/bin/keystone-admin b/bin/keystone-admin
index bf40c9fd..388e22fe 100755
--- a/bin/keystone-admin
+++ b/bin/keystone-admin
@@ -63,9 +63,10 @@ if __name__ == '__main__':
             print "Using config file:", config_file
 
         server = wsgi.Server()
-        server.start(app, int(conf['bind_port']), conf['bind_host'])
-        print "Admin API listening on %s:%s" % (conf['bind_host'],
-                                              conf['bind_port'])
+        server.start(app, int(conf['admin_port']), conf['admin_host'])
+        
+        print "Admin API listening on %s:%s" % (
+            conf['admin_host'], conf['admin_port'])
 
         server.wait()
     except RuntimeError, e:
diff --git a/bin/keystone-auth b/bin/keystone-auth
index 4ac26b33..368872b4 100755
--- a/bin/keystone-auth
+++ b/bin/keystone-auth
@@ -54,6 +54,7 @@ if __name__ == '__main__':
     try:
         # Load Service API server
         conf, app = config.load_paste_app('keystone-legacy-auth', options, args)
+        
         debug = options.get('debug') or conf.get('debug', False)
         debug = debug in [True, "True", "1"]
         verbose = options.get('verbose') or conf.get('verbose', False)
@@ -63,10 +64,11 @@ if __name__ == '__main__':
             print "Using config file:", config_file
         
         server = wsgi.Server()
-        server.start(app, int(conf['server_bind_port']),
-                     conf['server_bind_host'])
-        print "Service API listening on %s:%s" % (conf['server_bind_host'],
-                                              conf['server_bind_port'])
+        server.start(app, int(conf['public_port']), conf['public_host'])
+        
+        print "Service API listening on %s:%s" % (
+            conf['public_host'], conf['public_port'])
+        
         server.wait()
     except RuntimeError, e:
         sys.exit("ERROR: %s" % e)
diff --git a/etc/keystone.conf b/etc/keystone.conf
index 6a983f30..a404435e 100755
--- a/etc/keystone.conf
+++ b/etc/keystone.conf
@@ -14,21 +14,31 @@ default_store = sqlite
 # file for both the API and registry servers!
 #log_file = /var/log/keystone.log
 log_file = keystone.log
+
 #List of backends to be configured
 backends = keystone.backends.sqlalchemy,keystone.backends.alterdb
 
 #Dictionary Maps every service to a header.Missing services would get header X_(SERVICE_NAME) Key => Service Name, Value => Header Name
-service-header-mappings = {'nova' : 'X-Server-Management-Url' , 'swift' : 'X-Storage-Url', 'cdn' : 'X-CDN-Management-Url'}
+service-header-mappings = {
+	'nova' : 'X-Server-Management-Url',
+	'swift' : 'X-Storage-Url',
+	'cdn' : 'X-CDN-Management-Url'}
 
 # Address to bind the API server
-#TODO Properties defined within app not available via pipeline.Till then server props stay outside.
-server_bind_host = 0.0.0.0
+# TODO Properties defined within app not available via pipeline.
+public_host = 0.0.0.0
 
 # Port the bind the API server to
-server_bind_port = 5000
+public_port = 5000
+
+# Address to bind the Admin API server
+admin_host = 0.0.0.0
+
+# Port the bind the Admin API server to
+admin_port = 5001
 
 #Role that allows to perform admin operations.
-keystone-admin-role=Admin
+keystone-admin-role = Admin
 
 [keystone.backends.sqlalchemy]
 # SQLAlchemy connection string for the reference implementation
@@ -36,6 +46,7 @@ keystone-admin-role=Admin
 # See: http://www.sqlalchemy.org/docs/05/reference/sqlalchemy/connections.html#sqlalchemy.create_engine
 sql_connection = sqlite:///keystone.db
 backend_entities = ['UserGroupAssociation', 'UserRoleAssociation', 'Endpoints', 'Role', 'Tenant', 'User', 'Credentials', 'Group', 'EndpointTemplates']
+
 # Period in seconds after which SQLAlchemy should reestablish its connection
 # to the database.
 sql_idle_timeout = 30
@@ -46,26 +57,30 @@ sql_idle_timeout = 30
 # See: http://www.sqlalchemy.org/docs/05/reference/sqlalchemy/connections.html#sqlalchemy.create_engine
 sql_connection = sqlite:///keystone.token.db
 backend_entities = ['Token']
+
 # Period in seconds after which SQLAlchemy should reestablish its connection
 # to the database.
 sql_idle_timeout = 30
 
-[app:admin]
-paste.app_factory = keystone.server:admin_app_factory
-
-# Address to bind the Admin API server
-bind_host = 0.0.0.0
-
-# Port the bind the Admin API server to
-bind_port = 5001
-
-[app:server]
-paste.app_factory = keystone.server:app_factory
+[pipeline:admin]
+pipeline =
+	urlrewritefilter
+	admin_service
 
 [pipeline:keystone-legacy-auth]
 pipeline =
+	urlrewritefilter
     legacy_auth
-    server
+    public_service
+
+[app:public_service]
+paste.app_factory = keystone.server:app_factory
+
+[app:admin_service]
+paste.app_factory = keystone.server:admin_app_factory
+
+[filter:urlrewritefilter]
+paste.filter_factory = keystone.middleware.url:filter_factory
 
 [filter:legacy_auth]
 paste.filter_factory = keystone.frontends.legacy_token_auth:filter_factory
diff --git a/keystone/queryext/__init__.py b/keystone/backends/ldap/__init__.py
index e69de29b..e69de29b 100644
--- a/keystone/queryext/__init__.py
+++ b/keystone/backends/ldap/__init__.py
diff --git a/keystone/middleware/url.py b/keystone/middleware/url.py
new file mode 100644
index 00000000..3c876b8c
--- /dev/null
+++ b/keystone/middleware/url.py
@@ -0,0 +1,84 @@
+#!/usr/bin/env python
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+#
+# Copyright (c) 2010 OpenStack, LLC.
+#
+# 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.
+
+
+"""
+Auth Middleware that accepts URL query extension.
+
+This module can be installed as a filter in front of your service to
+detect extension in the resource URI (e.g., foo/resource.xml) to
+specify HTTP response body type. If an extension is specified, it
+overwrites the Accept header in the request, if present.
+
+"""
+
+CONTENT_TYPES = {'json': 'application/json', 'xml': 'application/xml'}
+DEFAULT_CONTENT_TYPE = CONTENT_TYPES['json']
+
+class UrlRewriteFilter(object):
+    """Middleware filter to handle URL rewriting"""
+
+    def __init__(self, app, conf):
+        # app is the next app in WSGI chain - eventually the OpenStack service
+        self.app = app
+        self.conf = conf
+
+    def __call__(self, env, start_response):
+        (env['PATH_INFO'], env['HTTP_ACCEPT']) = self.override_accept_header(
+            env.get('PATH_INFO'), env.get('HTTP_ACCEPT'))
+        
+        env['PATH_INFO'] = self.remove_trailing_slash(env.get('PATH_INFO'))
+        
+        return self.app(env, start_response)
+    
+    def override_accept_header(self, path_info, http_accept):
+        """Looks for an (.json/.xml) extension on the URL, removes it, and
+        overrides the Accept header if an extension was found"""
+        # try to split the extension from the rest of the path
+        parts = path_info.rsplit('.', 1)
+        if len(parts) > 1:
+            (path, ext) = parts
+        else:
+            (path, ext) = (parts[0], None)
+        
+        if ext in CONTENT_TYPES:
+            # Use the content type specified by the extension
+            return (path, CONTENT_TYPES[ext])
+        elif http_accept is None or http_accept == '*/*':
+            # TODO: This probably isn't the best place to handle "Accept: */*"
+            # No extension or Accept header specified, use default
+            return (path_info, DEFAULT_CONTENT_TYPE)
+        else:
+            # Return what we were given
+            return (path_info, http_accept)
+    
+    def remove_trailing_slash(self, path_info):
+        """Removes a trailing slash from the given path, if any"""
+        if path_info[-1] == '/':
+            return path_info[:-1]
+        else:
+            return path_info
+
+def filter_factory(global_conf, **local_conf):
+    """Returns a WSGI filter app for use with paste.deploy."""
+    conf = global_conf.copy()
+    conf.update(local_conf)
+
+    def ext_filter(app):
+        return UrlRewriteFilter(app, conf)
+    return ext_filter
diff --git a/keystone/queryext/exthandler.py b/keystone/queryext/exthandler.py
deleted file mode 100644
index cedf26fa..00000000
--- a/keystone/queryext/exthandler.py
+++ /dev/null
@@ -1,67 +0,0 @@
-#!/usr/bin/env python
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-#
-# Copyright (c) 2010 OpenStack, LLC.
-#
-# 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.
-
-
-"""
-Auth Middleware that accepts URL query extension.
-
-This module can be installed as a filter in front of your service to
-detect extension in the resource URI (e.g., foo/resource.xml) to
-specify HTTP response body type. If an extension is specified, it
-overwrites the Accept header in the request, if present.
-
-"""
-
-
-# Does this need to be configurable?
-DEFAULT_EXTS = {'xml': 'application/xml', 'json': 'application/json'}
-
-
-def scrub(uri, ext):
-    urisegs = uri.split('?')
-    first = urisegs[0][0: -(len(ext) + 1)]
-    if len(urisegs) > 1:
-        return '?'.join((first, urisegs[1], ))
-    else:
-        return first
-
-
-class UrlExtensionFilter(object):
-
-    def __init__(self, app, conf):
-        # app is the next app in WSGI chain - eventually the OpenStack service
-        self.app = app
-        self.conf = conf
-
-        print 'Starting extension handler middleware'
-
-    def __call__(self, env, start_response):
-        uri = env['PATH_INFO']
-        querysegs = uri.split('?')
-        ressegs = querysegs[0].split('.')
-        if len(ressegs) > 1:  # (Maybe) has an extension
-            ext = ressegs[-1]
-            if ext in DEFAULT_EXTS:
-                env['HTTP_ACCEPT'] = DEFAULT_EXTS[ext]
-                scrubbed = querysegs[0][0: -(len(ext) + 1)]  # Remove extension
-                if len(querysegs) > 1:  # Has query string
-                    env['PATH_INFO'] = '?'.join((scrubbed, querysegs[1], ))
-                else:
-                    env['PATH_INFO'] = scrubbed
-
-        return self.app(env, start_response)
diff --git a/keystone/server.py b/keystone/server.py
index 79ed98af..56cb4482 100755
--- a/keystone/server.py
+++ b/keystone/server.py
@@ -509,27 +509,18 @@ def get_marker_limit_and_url(req):
 
     if "limit" in req.GET:
         limit = req.GET["limit"]
+
     url = get_url(req)
+    
     return (marker, limit, url)
 
 
-def get_marker_and_limit(req):
-    marker = None
-    limit = 10
-
-    if "marker" in req.GET:
-        marker = req.GET["marker"]
-
-    if "limit" in req.GET:
-        limit = req.GET["limit"]
-
-
 def get_url(req):
-    url = '%s://%s:%s%s' % (req.environ['wsgi.url_scheme'],
-                     req.environ.get("SERVER_NAME"),
-                     req.environ.get("SERVER_PORT"),
-                     req.environ['PATH_INFO'])
-    return url
+    return '%s://%s:%s%s' % (
+        req.environ['wsgi.url_scheme'],
+        req.environ.get("SERVER_NAME"),
+        req.environ.get("SERVER_PORT"),
+        req.environ['PATH_INFO'])
 
 
 class KeystoneAPI(wsgi.Router):
@@ -552,9 +543,6 @@ class KeystoneAPI(wsgi.Router):
 
         # Miscellaneous Operations
         version_controller = VersionController(options)
-        mapper.connect("/v2.0/", controller=version_controller,
-                    action="get_version_info",
-                    conditions=dict(method=["GET"]))
         mapper.connect("/v2.0", controller=version_controller,
                     action="get_version_info",
                     conditions=dict(method=["GET"]))
@@ -764,9 +752,6 @@ class KeystoneAdminAPI(wsgi.Router):
 
         # Miscellaneous Operations
         version_controller = VersionController(options)
-        mapper.connect("/v2.0/", controller=version_controller,
-                    action="get_version_info",
-                    conditions=dict(method=["GET"]))
         mapper.connect("/v2.0", controller=version_controller,
                     action="get_version_info",
                     conditions=dict(method=["GET"]))
diff --git a/keystone/test/run_tests.py b/keystone/test/run_tests.py
index 190f632d..15fb8ede 100644
--- a/keystone/test/run_tests.py
+++ b/keystone/test/run_tests.py
@@ -1,4 +1,5 @@
 import os
+import sys
 import subprocess
 import time
 
@@ -20,7 +21,7 @@ if __name__ == '__main__':
         # blatent hack.
         time.sleep(3)
         if server.poll() is not None:
-            print >>sys.stderr, 'Failed to start server'
+            print >> sys.stderr, 'Failed to start server'
             sys.exit(-1)
         
         try:
diff --git a/keystone/test/unit/test_keystone.py b/keystone/test/unit/test_keystone.py
index 05fe1864..5909c08f 100755
--- a/keystone/test/unit/test_keystone.py
+++ b/keystone/test/unit/test_keystone.py
@@ -9,7 +9,7 @@ TEST_FILES = [
     #'test_authn_v2.py', # this is largely failing
     'test_common.py', # this doesn't actually contain tests
     'test_endpoints.py',
-    'test_exthandler.py',
+    'test_urlrewritefilter.py',
     'test_groups.py',
     'test_keystone.py', # not sure why this is referencing itself
     'test_roles.py',
diff --git a/keystone/test/unit/test_exthandler.py b/keystone/test/unit/test_urlrewritefilter.py
index 30a61f98..e22c362f 100644
--- a/keystone/test/unit/test_exthandler.py
+++ b/keystone/test/unit/test_urlrewritefilter.py
@@ -14,13 +14,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import os
-import sys
-# Need to access identity module
-sys.path.append(os.path.abspath(os.path.join(
-    os.getcwd(), '..', '..', 'keystone')))
-from keystone.queryext.exthandler import UrlExtensionFilter
+
 import unittest
+from keystone.middleware.url import UrlRewriteFilter
 
 
 class MockWsgiApp(object):
@@ -39,7 +35,23 @@ def _start_response():
 class UrlExtensionFilterTest(unittest.TestCase):
 
     def setUp(self):
-        self.filter = UrlExtensionFilter(MockWsgiApp(), {})
+        self.filter = UrlRewriteFilter(MockWsgiApp(), {})
+    
+    def test_remove_trailing_slash(self):
+        env = {'PATH_INFO': '/v2.0/'}
+        self.filter(env, _start_response)
+        self.assertEqual('/v2.0', env['PATH_INFO'])
+    
+    def test_remove_trailing_slash_from_empty_path(self):
+        env = {'PATH_INFO': '/'}
+        self.filter(env, _start_response)
+        self.assertEqual('', env['PATH_INFO'])
+    
+    def test_no_extension(self):
+        env = {'PATH_INFO': '/v2.0/someresource'}
+        self.filter(env, _start_response)
+        self.assertEqual('/v2.0/someresource', env['PATH_INFO'])
+        self.assertEqual('application/json', env['HTTP_ACCEPT'])
 
     def test_xml_extension(self):
         env = {'PATH_INFO': '/v2.0/someresource.xml'}
@@ -54,8 +66,9 @@ class UrlExtensionFilterTest(unittest.TestCase):
         self.assertEqual('application/json', env['HTTP_ACCEPT'])
 
     def test_extension_overrides_header(self):
-        env = {'PATH_INFO': '/v2.0/someresource.json',
-                'HTTP_ACCEPT': 'application/xml'}
+        env = {
+            'PATH_INFO': '/v2.0/someresource.json',
+            'HTTP_ACCEPT': 'application/xml'}
         self.filter(env, _start_response)
         self.assertEqual('/v2.0/someresource', env['PATH_INFO'])
         self.assertEqual('application/json', env['HTTP_ACCEPT'])
diff --git a/keystone/utils.py b/keystone/utils.py
index ddd63760..96991364 100755
--- a/keystone/utils.py
+++ b/keystone/utils.py
@@ -16,19 +16,10 @@
 
 
 import functools
-import httplib
-import json
 import logging
 import os
-import routes
 import sys
-import hashlib
 from webob import Response
-from webob import Request
-from webob import descriptors
-from webob.exc import (HTTPNotFound,
-                       HTTPConflict,
-                       HTTPBadRequest)
 
 POSSIBLE_TOPDIR = os.path.normpath(os.path.join(os.path.abspath(sys.argv[0]),
                                    os.pardir,
@@ -36,14 +27,12 @@ POSSIBLE_TOPDIR = os.path.normpath(os.path.join(os.path.abspath(sys.argv[0]),
 if os.path.exists(os.path.join(POSSIBLE_TOPDIR, 'keystone', '__init__.py')):
     sys.path.insert(0, POSSIBLE_TOPDIR)
 
-from queryext import exthandler
 import keystone.logic.types.fault as fault
 
 
 def is_xml_response(req):
-    if not "Accept" in req.headers:
-        return False
-    return req.content_type == "application/xml"
+    """Returns True when the request wants an XML response, False otherwise"""
+    return "Accept" in req.headers and "application/xml" in req.accept
 
 
 def get_app_root():
@@ -180,5 +169,5 @@ def import_module(module_name, class_name=None):
         __import__(module_name)
         return getattr(sys.modules[module_name], class_name)
     except (ImportError, ValueError, AttributeError), exception:
-        raise ImportError(_('Class %s.%s cannot be found (%s)') %
-            (module_name, class_name, exception))   
\ No newline at end of file
+        raise ImportError(_('Class %s.%s cannot be found (%s)') % 
+            (module_name, class_name, exception))   
diff --git a/setup.py b/setup.py
index f936b418..56444937 100755
--- a/setup.py
+++ b/setup.py
@@ -57,6 +57,7 @@ setup(
     entry_points={
         'paste.app_factory': ['main=identity:app_factory'],
         'paste.filter_factory': [
+            'extfilter=keystone.middleware.url:filter_factory',
             'remoteauth=keystone.middleware.remoteauth:remoteauth_factory',
             'tokenauth=keystone.auth_protocols.auth_token:filter_factory',
             'swiftauth=keystone.middleware.swift_auth:filter_factory',
diff --git a/tools/pip-requires b/tools/pip-requires
index 2b994af3..b9951cdf 100644
--- a/tools/pip-requires
+++ b/tools/pip-requires
@@ -7,4 +7,5 @@ pysqlite
 sqlalchemy
 webob
 Routes
-httplib2
\ No newline at end of file
+httplib2
+python-ldap # optional authentication backend
\ No newline at end of file