Simplify ceilometer-api and checks Keystone middleware parsing

This adds some test to be sure options passed to the middleware are read and
user properly.

This fixes bug #1099739

Change-Id: I491db4d2c7387cc2848ebefbbb579597234a82b0
Signed-off-by: Julien Danjou <julien@danjou.info>

bin/ceilometer-api

@@ -20,27 +20,14 @@
 """
 import sys
 
-from ceilometer.api import acl
 from ceilometer.api.v1 import app
 from ceilometer import service
 from ceilometer.openstack.common import cfg
-from ceilometer.openstack.common import log as logging
 
 
 if __name__ == '__main__':
-    # Register keystone middleware option before
-    # parsing the config file and command line
-    # inputs.
-    acl.register_opts(cfg.CONF)
-
     # Parse config file and command line options,
     # then configure logging.
-    service.prepare_service()
+    service.prepare_service(sys.argv)
-
+    root = app.make_app(cfg.CONF)
-    root = app.make_app()
-
-    # Enable debug mode
-    if cfg.CONF.verbose or cfg.CONF.debug:
-        root.debug = True
-
     root.run(host='0.0.0.0', port=cfg.CONF.metering_api_port)

bin/ceilometer-api-v2

@@ -22,34 +22,19 @@ import os
 import sys
 from wsgiref import simple_server
 
-from pecan import configuration
-
-from ceilometer.api import acl
 from ceilometer.api import app
 from ceilometer import service
-from ceilometer.api import config as api_config
 from ceilometer.openstack.common import cfg
 from ceilometer.openstack.common import log as logging
 
 
 if __name__ == '__main__':
-    # Register keystone middleware option before
-    # parsing the config file and command line
-    # inputs.
-    acl.register_opts(cfg.CONF)
-
     # Parse OpenStack config file and command line options, then
     # configure logging.
-    service.prepare_service()
+    service.prepare_service(sys.argv)
-
-    # Set up the pecan configuration
-    filename = api_config.__file__.replace('.pyc', '.py')
-    pecan_config = configuration.conf_from_file(filename)
 
     # Build the WSGI app
-    root = app.setup_app(pecan_config,
+    root = app.setup_app()
-                         extra_hooks=[acl.AdminAuthHook()])
-    root = acl.install(root, cfg.CONF)
 
     # Create the WSGI server and start it
     host, port = '0.0.0.0', int(cfg.CONF.metering_api_port)

ceilometer/api/acl.py

@@ -18,6 +18,7 @@
 """Set up the ACL to acces the API server."""
 
 from ceilometer import policy
+from ceilometer.openstack.common import cfg
 
 from pecan import hooks
 
@@ -36,9 +37,11 @@ def register_opts(conf):
     auth_token.CONF = conf
 
 
+register_opts(cfg.CONF)
+
+
 def install(app, conf):
     """Install ACL check on application."""
-    register_opts(conf)
     return auth_token.AuthProtocol(app,
                                    conf=dict(conf.get(OPT_GROUP_NAME)))
 

ceilometer/api/app.py

@@ -17,28 +17,48 @@
 # under the License.
 
 from pecan import make_app
+from pecan import configuration
+
+from ceilometer.api import config as api_config
+from ceilometer.api import acl
 from ceilometer.api import hooks
 from ceilometer.api import middleware
-from ceilometer.service import prepare_service
+from ceilometer.openstack.common import cfg
 
 
-def setup_app(config, extra_hooks=[]):
+def get_pecan_config():
+    # Set up the pecan configuration
+    filename = api_config.__file__.replace('.pyc', '.py')
+    return configuration.conf_from_file(filename)
 
-    # Initialize the cfg.CONF object
-    prepare_service([])
 
+def setup_app(pecan_config=None, extra_hooks=None):
     # FIXME: Replace DBHook with a hooks.TransactionHook
     app_hooks = [hooks.ConfigHook(),
                  hooks.DBHook()]
-    app_hooks.extend(extra_hooks)
+    if extra_hooks:
+        app_hooks.extend(extra_hooks)
 
-    return make_app(
+    if not pecan_config:
-        config.app.root,
+        pecan_config = get_pecan_config()
-        static_root=config.app.static_root,
+
-        template_path=config.app.template_path,
+    if pecan_config.app.enable_acl:
-        logging=getattr(config, 'logging', {}),
+        app_hooks.append(acl.AdminAuthHook())
-        debug=getattr(config.app, 'debug', False),
+
-        force_canonical=getattr(config.app, 'force_canonical', True),
+    configuration.set_config(dict(pecan_config), overwrite=True)
+
+    app = make_app(
+        pecan_config.app.root,
+        static_root=pecan_config.app.static_root,
+        template_path=pecan_config.app.template_path,
+        logging=getattr(pecan_config, 'logging', {}),
+        debug=getattr(pecan_config.app, 'debug', False),
+        force_canonical=getattr(pecan_config.app, 'force_canonical', True),
         hooks=app_hooks,
         wrap_app=middleware.ParsableErrorMiddleware,
     )
+
+    if pecan_config.app.enable_acl:
+        return acl.install(app, cfg.CONF)
+
+    return app

ceilometer/api/config.py

@@ -11,6 +11,7 @@ app = {
     'static_root': '%(confdir)s/public',
     'template_path': '%(confdir)s/ceilometer/api/templates',
     'debug': False,
+    'enable_acl': True,
 }
 
 logging = {

ceilometer/api/v1/app.py

@@ -30,7 +30,7 @@ from ceilometer.api import acl
 storage.register_opts(cfg.CONF)
 
 
-def make_app(enable_acl=True, attach_storage=True):
+def make_app(conf, enable_acl=True, attach_storage=True):
     app = flask.Flask('ceilometer.api')
     app.register_blueprint(v1_blueprint.blueprint, url_prefix='/v1')
 
@@ -42,21 +42,22 @@ def make_app(enable_acl=True, attach_storage=True):
 
     @app.before_request
     def attach_config():
-        flask.request.cfg = cfg.CONF
+        flask.request.cfg = conf
         flask.request.sources = sources
 
     if attach_storage:
         @app.before_request
         def attach_storage():
-            storage_engine = storage.get_engine(cfg.CONF)
+            storage_engine = storage.get_engine(conf)
             flask.request.storage_engine = storage_engine
             flask.request.storage_conn = \
-                storage_engine.get_connection(cfg.CONF)
+                storage_engine.get_connection(conf)
 
     # Install the middleware wrapper
     if enable_acl:
-        app.wsgi_app = acl.install(app.wsgi_app, cfg.CONF)
+        app.wsgi_app = acl.install(app.wsgi_app, conf)
+
     return app
 
 # For documentation
-app = make_app(enable_acl=False, attach_storage=False)
+app = make_app(cfg.CONF, enable_acl=False, attach_storage=False)

ceilometer/tests/api.py

@@ -21,7 +21,6 @@
 import json
 import os
 import urllib
-import unittest
 
 import flask
 from pecan import set_config
@@ -36,6 +35,7 @@ from ceilometer.api.v1 import blueprint as v1_blueprint
 from ceilometer.api.controllers import v2
 from ceilometer.openstack.common import cfg
 from ceilometer.tests import db as db_test_base
+from ceilometer.tests import base
 
 
 class TestBase(db_test_base.TestBase):
@@ -44,7 +44,9 @@ class TestBase(db_test_base.TestBase):
 
     def setUp(self):
         super(TestBase, self).setUp()
-        self.app = v1_app.make_app(enable_acl=False, attach_storage=False)
+        self.app = v1_app.make_app(cfg.CONF,
+                                   enable_acl=False,
+                                   attach_storage=False)
         self.app.register_blueprint(v1_blueprint.blueprint)
         self.test_app = self.app.test_client()
 
@@ -68,7 +70,7 @@ class TestBase(db_test_base.TestBase):
         return rv
 
 
-class FunctionalTest(unittest.TestCase):
+class FunctionalTest(base.TestCase):
     """
     Used for functional tests of Pecan controllers where you need to
     test your literal application and its integration with the
@@ -82,6 +84,7 @@ class FunctionalTest(unittest.TestCase):
     SOURCE_DATA = {'test_source': {'somekey': '666'}}
 
     def setUp(self):
+        super(FunctionalTest, self).setUp()
 
         cfg.CONF.database_connection = 'test://localhost/%s' % self.DBNAME
         self.conn = storage.get_connection(cfg.CONF)
@@ -90,12 +93,16 @@ class FunctionalTest(unittest.TestCase):
         # http://davisp.lighthouseapp.com/projects/26898/tickets/22
         self.conn.conn[self.DBNAME].clear()
 
+        self.app = self._make_app()
+
+    def _make_app(self, enable_acl=False):
         # Determine where we are so we can set up paths in the config
         root_dir = os.path.abspath(os.path.join(os.path.dirname(__file__),
                                                 '..',
                                                 '..',
                                                 )
                                    )
+
         self.config = {
 
             'app': {
@@ -103,6 +110,7 @@ class FunctionalTest(unittest.TestCase):
                 'modules': ['ceilometer.api'],
                 'static_root': '%s/public' % root_dir,
                 'template_path': '%s/ceilometer/api/templates' % root_dir,
+                'enable_acl': enable_acl,
             },
 
             'logging': {
@@ -129,19 +137,10 @@ class FunctionalTest(unittest.TestCase):
             },
         }
 
-        self.mox = mox.Mox()
-        self.stubs = stubout.StubOutForTesting()
-
-        self.app = self._make_app()
-
-    def _make_app(self):
         return load_test_app(self.config)
 
     def tearDown(self):
-        self.mox.UnsetStubs()
+        super(FunctionalTest, self).tearDown()
-        self.stubs.UnsetAll()
-        self.stubs.SmartUnsetAll()
-        self.mox.VerifyAll()
         set_config({}, overwrite=True)
 
     def get_json(self, path, expect_errors=False, headers=None,

ceilometer/tests/db.py

@@ -49,6 +49,7 @@ class TestBase(test_base.TestCase):
 
     def tearDown(self):
         self.conn.drop_database(self.DBNAME)
+        super(TestBase, self).tearDown()
 
 
 class TestConnection(impl_mongodb.Connection):

tests/api/v1/test_app.py

@@ -0,0 +1,49 @@
+# -*- encoding: utf-8 -*-
+#
+# Copyright © 2013 Julien Danjou
+#
+# Author: Julien Danjou <julien@danjou.info>
+#
+# 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.
+"""Test basic ceilometer-api app
+"""
+import os
+import tempfile
+import unittest
+
+from ceilometer.api.v1 import app
+from ceilometer.api import acl
+from ceilometer.openstack.common import cfg
+from ceilometer import service
+
+
+class TestApp(unittest.TestCase):
+
+    def tearDown(self):
+        cfg.CONF.reset()
+
+    def test_keystone_middleware_conf(self):
+        cfg.CONF.set_override("auth_protocol", "foottp",
+                              group=acl.OPT_GROUP_NAME)
+        api_app = app.make_app(cfg.CONF, attach_storage=False)
+        self.assertEqual(api_app.wsgi_app.auth_protocol, 'foottp')
+
+    def test_keystone_middleware_parse_conffile(self):
+        tmpfile = tempfile.mktemp()
+        with open(tmpfile, "w") as f:
+            f.write("[%s]\nauth_protocol = barttp" % acl.OPT_GROUP_NAME)
+        service.prepare_service(['ceilometer-api',
+                                 '--config-file=%s' % tmpfile])
+        api_app = app.make_app(cfg.CONF, attach_storage=False)
+        self.assertEqual(api_app.wsgi_app.auth_protocol, 'barttp')
+        os.unlink(tmpfile)

tests/api/v2/base.py

@@ -22,9 +22,3 @@ from ceilometer.tests import api
 class FunctionalTest(api.FunctionalTest):
 
     PATH_PREFIX = '/v2'
-
-    def setUp(self):
-        super(FunctionalTest, self).setUp()
-
-    def tearDown(self):
-        super(FunctionalTest, self).tearDown()

tests/api/v2/test_acl.py

@@ -17,31 +17,47 @@
 # under the License.
 """Test ACL."""
 
-from ceilometer.api import acl
+import datetime
-from ceilometer.api import app
+
-from ceilometer.openstack.common import cfg
 from .base import FunctionalTest
 
+VALID_TOKEN = '4562138218392831'
+
+
+class FakeMemcache(object):
+    def __init__(self):
+        self.set_key = None
+        self.set_value = None
+        self.token_expiration = None
+
+    def get(self, key):
+        if key == "tokens/%s" % VALID_TOKEN:
+            dt = datetime.datetime.now() + datetime.timedelta(minutes=5)
+            return ({'access': {
+                'token': {'id': VALID_TOKEN},
+                'user': {
+                    'id': 'user_id1',
+                    'name': 'user_name1',
+                    'tenantId': '123i2910',
+                    'tenantName': 'mytenant',
+                    'roles': [
+                        {'name': 'admin'},
+                    ]},
+            }}, dt.strftime("%s"))
+
+    def set(self, key, value, time=None):
+        self.set_value = value
+        self.set_key = key
+
 
 class TestAPIACL(FunctionalTest):
 
+    def setUp(self):
+        super(TestAPIACL, self).setUp()
+        self.app.app._cache = FakeMemcache()
+
     def _make_app(self):
-        # Save the original app constructor so
+        return super(TestAPIACL, self)._make_app(enable_acl=True)
-        # we can use it in our wrapper
-        original_setup_app = app.setup_app
-
-        # Wrap application construction with
-        # a function that ensures the AdminAuthHook
-        # is provided.
-        def setup_app(config, extra_hooks=[]):
-            extra_hooks = extra_hooks[:]
-            extra_hooks.append(acl.AdminAuthHook())
-            return original_setup_app(config, extra_hooks)
-
-        self.stubs.Set(app, 'setup_app', setup_app)
-        result = super(TestAPIACL, self)._make_app()
-        acl.install(result, cfg.CONF)
-        return result
 
     def test_non_authenticated(self):
         response = self.get_json('/meters', expect_errors=True)
@@ -77,6 +93,7 @@ class TestAPIACL(FunctionalTest):
         response = self.get_json('/meters',
                                  expect_errors=True,
                                  headers={
+                                     "X-Auth-Token": VALID_TOKEN,
                                      "X-Roles": "admin",
                                      "X-Tenant-Name": "admin",
                                      "X-Tenant-Id":

tests/api/v2/test_app.py

@@ -0,0 +1,49 @@
+# -*- encoding: utf-8 -*-
+#
+# Copyright © 2013 Julien Danjou
+#
+# Author: Julien Danjou <julien@danjou.info>
+#
+# 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.
+"""Test basic ceilometer-api app
+"""
+import os
+import tempfile
+import unittest
+
+from ceilometer.api import app
+from ceilometer.api import acl
+from ceilometer.openstack.common import cfg
+from ceilometer import service
+
+
+class TestApp(unittest.TestCase):
+
+    def tearDown(self):
+        cfg.CONF.reset()
+
+    def test_keystone_middleware_conf(self):
+        cfg.CONF.set_override("auth_protocol", "foottp",
+                              group=acl.OPT_GROUP_NAME)
+        api_app = app.setup_app()
+        self.assertEqual(api_app.auth_protocol, 'foottp')
+
+    def test_keystone_middleware_parse_conffile(self):
+        tmpfile = tempfile.mktemp()
+        with open(tmpfile, "w") as f:
+            f.write("[%s]\nauth_protocol = barttp" % acl.OPT_GROUP_NAME)
+        service.prepare_service(['ceilometer-api',
+                                 '--config-file=%s' % tmpfile])
+        api_app = app.setup_app()
+        self.assertEqual(api_app.auth_protocol, 'barttp')
+        os.unlink(tmpfile)