From a5bc34d932d91931a5e81a72c686a0fe5037d36e Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Wed, 16 Jan 2013 14:55:57 +0100 Subject: [PATCH] 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 --- bin/ceilometer-api | 17 ++----------- bin/ceilometer-api-v2 | 19 ++------------ ceilometer/api/acl.py | 5 +++- ceilometer/api/app.py | 44 +++++++++++++++++++++++--------- ceilometer/api/config.py | 1 + ceilometer/api/v1/app.py | 13 +++++----- ceilometer/tests/api.py | 25 +++++++++--------- ceilometer/tests/db.py | 1 + tests/api/v1/test_app.py | 49 +++++++++++++++++++++++++++++++++++ tests/api/v2/base.py | 6 ----- tests/api/v2/test_acl.py | 55 ++++++++++++++++++++++++++-------------- tests/api/v2/test_app.py | 49 +++++++++++++++++++++++++++++++++++ 12 files changed, 195 insertions(+), 89 deletions(-) create mode 100644 tests/api/v1/test_app.py create mode 100644 tests/api/v2/test_app.py diff --git a/bin/ceilometer-api b/bin/ceilometer-api index 5350fc49a..c2cff2a88 100755 --- a/bin/ceilometer-api +++ b/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() - - root = app.make_app() - - # Enable debug mode - if cfg.CONF.verbose or cfg.CONF.debug: - root.debug = True - + service.prepare_service(sys.argv) + root = app.make_app(cfg.CONF) root.run(host='0.0.0.0', port=cfg.CONF.metering_api_port) diff --git a/bin/ceilometer-api-v2 b/bin/ceilometer-api-v2 index 0388987f2..40ff83ed3 100755 --- a/bin/ceilometer-api-v2 +++ b/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() - - # Set up the pecan configuration - filename = api_config.__file__.replace('.pyc', '.py') - pecan_config = configuration.conf_from_file(filename) + service.prepare_service(sys.argv) # Build the WSGI app - root = app.setup_app(pecan_config, - extra_hooks=[acl.AdminAuthHook()]) - root = acl.install(root, cfg.CONF) + root = app.setup_app() # Create the WSGI server and start it host, port = '0.0.0.0', int(cfg.CONF.metering_api_port) diff --git a/ceilometer/api/acl.py b/ceilometer/api/acl.py index a912d7f15..51d39c8da 100644 --- a/ceilometer/api/acl.py +++ b/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))) diff --git a/ceilometer/api/app.py b/ceilometer/api/app.py index aba226264..8544bf81b 100644 --- a/ceilometer/api/app.py +++ b/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( - config.app.root, - static_root=config.app.static_root, - template_path=config.app.template_path, - logging=getattr(config, 'logging', {}), - debug=getattr(config.app, 'debug', False), - force_canonical=getattr(config.app, 'force_canonical', True), + if not pecan_config: + pecan_config = get_pecan_config() + + if pecan_config.app.enable_acl: + app_hooks.append(acl.AdminAuthHook()) + + 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 diff --git a/ceilometer/api/config.py b/ceilometer/api/config.py index fb85fee65..02c0e3ff5 100644 --- a/ceilometer/api/config.py +++ b/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 = { diff --git a/ceilometer/api/v1/app.py b/ceilometer/api/v1/app.py index d19d06454..10345978d 100644 --- a/ceilometer/api/v1/app.py +++ b/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) diff --git a/ceilometer/tests/api.py b/ceilometer/tests/api.py index eb19968f5..20dd832ae 100644 --- a/ceilometer/tests/api.py +++ b/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() - self.stubs.UnsetAll() - self.stubs.SmartUnsetAll() - self.mox.VerifyAll() + super(FunctionalTest, self).tearDown() set_config({}, overwrite=True) def get_json(self, path, expect_errors=False, headers=None, diff --git a/ceilometer/tests/db.py b/ceilometer/tests/db.py index cff0d04b8..786a4a9ed 100644 --- a/ceilometer/tests/db.py +++ b/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): diff --git a/tests/api/v1/test_app.py b/tests/api/v1/test_app.py new file mode 100644 index 000000000..cafd40eed --- /dev/null +++ b/tests/api/v1/test_app.py @@ -0,0 +1,49 @@ +# -*- encoding: utf-8 -*- +# +# Copyright © 2013 Julien Danjou +# +# Author: Julien Danjou +# +# 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) diff --git a/tests/api/v2/base.py b/tests/api/v2/base.py index d6af44906..0eaeca4bd 100644 --- a/tests/api/v2/base.py +++ b/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() diff --git a/tests/api/v2/test_acl.py b/tests/api/v2/test_acl.py index 224b3012f..2bb7078c5 100644 --- a/tests/api/v2/test_acl.py +++ b/tests/api/v2/test_acl.py @@ -17,31 +17,47 @@ # under the License. """Test ACL.""" -from ceilometer.api import acl -from ceilometer.api import app -from ceilometer.openstack.common import cfg +import datetime + 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 - # 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 + return super(TestAPIACL, self)._make_app(enable_acl=True) 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": diff --git a/tests/api/v2/test_app.py b/tests/api/v2/test_app.py new file mode 100644 index 000000000..968c30307 --- /dev/null +++ b/tests/api/v2/test_app.py @@ -0,0 +1,49 @@ +# -*- encoding: utf-8 -*- +# +# Copyright © 2013 Julien Danjou +# +# Author: Julien Danjou +# +# 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)