From 5ecb2abcddc2ca07577b0186bf1ecbb8a1b0a632 Mon Sep 17 00:00:00 2001 From: Mehdi Abaakouk Date: Mon, 10 Oct 2016 14:02:17 +0200 Subject: [PATCH] api: Remove global conf Change-Id: Idfb9bc3e67e963cad5277a559d8b07cd7790cdac --- ceilometer/api/app.py | 50 +++++++++++++------ ceilometer/api/app.wsgi | 4 +- ceilometer/api/controllers/v2/meters.py | 4 +- ceilometer/api/controllers/v2/root.py | 21 ++++---- ceilometer/api/controllers/v2/utils.py | 2 +- ceilometer/api/hooks.py | 26 ++++++---- ceilometer/api/rbac.py | 31 ++---------- ceilometer/service.py | 25 ++++++---- ceilometer/tests/functional/api/__init__.py | 4 +- .../functional/api/v2/test_acl_scenarios.py | 2 +- ceilometer/tests/functional/gabbi/fixtures.py | 18 +++++++ .../tests/functional/gabbi/test_gabbi.py | 3 +- .../functional/gabbi/test_gabbi_prefix.py | 3 +- ceilometer/tests/unit/api/test_app.py | 5 +- ceilometer/tests/unit/api/test_hooks.py | 2 +- 15 files changed, 114 insertions(+), 86 deletions(-) diff --git a/ceilometer/api/app.py b/ceilometer/api/app.py index e97754a078..497c0513e4 100644 --- a/ceilometer/api/app.py +++ b/ceilometer/api/app.py @@ -15,6 +15,7 @@ # under the License. import os +import uuid from oslo_config import cfg from oslo_log import log @@ -52,11 +53,14 @@ CONF.register_opts(OPTS) CONF.register_opts(API_OPTS, group='api') -def setup_app(pecan_config=None): +def setup_app(pecan_config=None, conf=None): + if conf is None: + raise RuntimeError("No configuration passed") + # FIXME: Replace DBHook with a hooks.TransactionHook - app_hooks = [hooks.ConfigHook(), - hooks.DBHook(), - hooks.NotifierHook(), + app_hooks = [hooks.ConfigHook(conf), + hooks.DBHook(conf), + hooks.NotifierHook(conf), hooks.TranslationHook()] pecan_config = pecan_config or { @@ -70,7 +74,7 @@ def setup_app(pecan_config=None): app = pecan.make_app( pecan_config['app']['root'], - debug=CONF.api.pecan_debug, + debug=conf.api.pecan_debug, hooks=app_hooks, wrap_app=middleware.ParsableErrorMiddleware, guess_content_type_from_ext=False @@ -79,25 +83,43 @@ def setup_app(pecan_config=None): return app -def load_app(): +# NOTE(sileht): pastedeploy uses ConfigParser to handle +# global_conf, since python 3 ConfigParser doesn't +# allow to store object as config value, only strings are +# permit, so to be able to pass an object created before paste load +# the app, we store them into a global var. But the each loaded app +# store it's configuration in unique key to be concurrency safe. +global APPCONFIGS +APPCONFIGS = {} + + +def load_app(conf): + global APPCONFIGS + # Build the WSGI app cfg_file = None - cfg_path = cfg.CONF.api_paste_config + cfg_path = conf.api_paste_config if not os.path.isabs(cfg_path): - cfg_file = CONF.find_file(cfg_path) + cfg_file = conf.find_file(cfg_path) elif os.path.exists(cfg_path): cfg_file = cfg_path if not cfg_file: - raise cfg.ConfigFilesNotFoundError([cfg.CONF.api_paste_config]) + raise cfg.ConfigFilesNotFoundError([conf.api_paste_config]) + + configkey = str(uuid.uuid4()) + APPCONFIGS[configkey] = conf + LOG.info("Full WSGI config used: %s" % cfg_file) - return deploy.loadapp("config:" + cfg_file) + return deploy.loadapp("config:" + cfg_file, + global_conf={'configkey': configkey}) def app_factory(global_config, **local_conf): - return setup_app() + global APPCONFIGS + conf = APPCONFIGS.get(global_config.get('configkey')) + return setup_app(conf=conf) -def build_wsgi_app(): - service.prepare_service() - return load_app() +def build_wsgi_app(argv=None): + return load_app(service.prepare_service(argv=argv)) diff --git a/ceilometer/api/app.wsgi b/ceilometer/api/app.wsgi index a5a4856ecf..130a63aab9 100644 --- a/ceilometer/api/app.wsgi +++ b/ceilometer/api/app.wsgi @@ -21,5 +21,5 @@ from ceilometer import service from ceilometer.api import app # Initialize the oslo configuration library and logging -service.prepare_service([]) -application = app.load_app() +conf = service.prepare_service([]) +application = app.load_app(conf) diff --git a/ceilometer/api/controllers/v2/meters.py b/ceilometer/api/controllers/v2/meters.py index 9aa500ebc8..0a63a305ef 100644 --- a/ceilometer/api/controllers/v2/meters.py +++ b/ceilometer/api/controllers/v2/meters.py @@ -21,7 +21,6 @@ import base64 import datetime -from oslo_config import cfg from oslo_log import log from oslo_utils import strutils from oslo_utils import timeutils @@ -360,7 +359,8 @@ class MeterController(rest.RestController): s.message_id = published_sample.id sample_dict = publisher_utils.meter_message_from_counter( - published_sample, cfg.CONF.publisher.telemetry_secret) + published_sample, + pecan.request.cfg.publisher.telemetry_secret) if direct: ts = timeutils.parse_isotime(sample_dict['timestamp']) sample_dict['timestamp'] = timeutils.normalize_time(ts) diff --git a/ceilometer/api/controllers/v2/root.py b/ceilometer/api/controllers/v2/root.py index 08f7fc35eb..fb3bb2ce03 100644 --- a/ceilometer/api/controllers/v2/root.py +++ b/ceilometer/api/controllers/v2/root.py @@ -116,11 +116,12 @@ class V2Controller(object): @property def gnocchi_is_enabled(self): if self._gnocchi_is_enabled is None: - if cfg.CONF.api.gnocchi_is_enabled is not None: - self._gnocchi_is_enabled = cfg.CONF.api.gnocchi_is_enabled + if pecan.request.cfg.api.gnocchi_is_enabled is not None: + self._gnocchi_is_enabled = ( + pecan.request.cfg.api.gnocchi_is_enabled) - elif ("gnocchi" not in cfg.CONF.meter_dispatchers - or "database" in cfg.CONF.meter_dispatchers): + elif ("gnocchi" not in pecan.request.cfg.meter_dispatchers + or "database" in pecan.request.cfg.meter_dispatchers): self._gnocchi_is_enabled = False else: try: @@ -142,11 +143,11 @@ class V2Controller(object): @property def aodh_url(self): if self._aodh_url is None: - if cfg.CONF.api.aodh_is_enabled is False: + if pecan.request.cfg.api.aodh_is_enabled is False: self._aodh_url = "" - elif cfg.CONF.api.aodh_url is not None: + elif pecan.request.cfg.api.aodh_url is not None: self._aodh_url = self._normalize_url( - cfg.CONF.api.aodh_url) + pecan.request.cfg.api.aodh_url) else: try: catalog = keystone_client.get_service_catalog( @@ -167,11 +168,11 @@ class V2Controller(object): @property def panko_url(self): if self._panko_url is None: - if cfg.CONF.api.panko_is_enabled is False: + if pecan.request.cfg.api.panko_is_enabled is False: self._panko_url = "" - elif cfg.CONF.api.panko_url is not None: + elif pecan.request.cfg.api.panko_url is not None: self._panko_url = self._normalize_url( - cfg.CONF.api.panko_url) + pecan.request.cfg.api.panko_url) else: try: catalog = keystone_client.get_service_catalog( diff --git a/ceilometer/api/controllers/v2/utils.py b/ceilometer/api/controllers/v2/utils.py index 88142cbda2..f7b64e5479 100644 --- a/ceilometer/api/controllers/v2/utils.py +++ b/ceilometer/api/controllers/v2/utils.py @@ -43,7 +43,7 @@ cfg.CONF.import_opt('default_api_return_limit', 'ceilometer.api.app', def enforce_limit(limit): """Ensure limit is defined and is valid. if not, set a default.""" if limit is None: - limit = cfg.CONF.api.default_api_return_limit + limit = pecan.request.cfg.api.default_api_return_limit LOG.info(_LI('No limit value provided, result set will be' ' limited to %(limit)d.'), {'limit': limit}) if not limit or limit <= 0: diff --git a/ceilometer/api/hooks.py b/ceilometer/api/hooks.py index 003a236303..fd3c954b83 100644 --- a/ceilometer/api/hooks.py +++ b/ceilometer/api/hooks.py @@ -16,6 +16,7 @@ from oslo_config import cfg from oslo_log import log import oslo_messaging +from oslo_policy import policy from pecan import hooks @@ -34,17 +35,22 @@ class ConfigHook(hooks.PecanHook): That allows controllers to get it. """ + def __init__(self, conf): + super(ConfigHook, self).__init__() + self.conf = conf + self.enforcer = policy.Enforcer(conf) + self.enforcer.load_rules() - @staticmethod - def before(state): - state.request.cfg = cfg.CONF + def on_route(self, state): + state.request.cfg = self.conf + state.request.enforcer = self.enforcer class DBHook(hooks.PecanHook): - def __init__(self): - self.storage_connection = DBHook.get_connection('metering') - self.event_storage_connection = DBHook.get_connection('event') + def __init__(self, conf): + self.storage_connection = self.get_connection(conf, 'metering') + self.event_storage_connection = self.get_connection(conf, 'event') if (not self.storage_connection and not self.event_storage_connection): @@ -57,9 +63,9 @@ class DBHook(hooks.PecanHook): state.request.event_storage_conn = self.event_storage_connection @staticmethod - def get_connection(purpose): + def get_connection(conf, purpose): try: - return storage.get_connection_from_config(cfg.CONF, purpose) + return storage.get_connection_from_config(conf, purpose) except Exception as err: params = {"purpose": purpose, "err": err} LOG.exception(_LE("Failed to connect to db, purpose %(purpose)s " @@ -73,10 +79,10 @@ class NotifierHook(hooks.PecanHook): are posted via /v2/meters/ API. """ - def __init__(self): + def __init__(self, conf): transport = messaging.get_transport() self.notifier = oslo_messaging.Notifier( - transport, driver=cfg.CONF.publisher_notifier.telemetry_driver, + transport, driver=conf.publisher_notifier.telemetry_driver, publisher_id="ceilometer.api") def before(self, state): diff --git a/ceilometer/api/rbac.py b/ceilometer/api/rbac.py index d4443e4337..06731a7489 100644 --- a/ceilometer/api/rbac.py +++ b/ceilometer/api/rbac.py @@ -16,24 +16,11 @@ """Access Control Lists (ACL's) control access the API server.""" -from oslo_config import cfg -from oslo_policy import policy import pecan -_ENFORCER = None - -CONF = cfg.CONF - - -def reset(): - global _ENFORCER - if _ENFORCER: - _ENFORCER.clear() - _ENFORCER = None - def _has_rule(name): - return name in _ENFORCER.rules.keys() + return name in pecan.request.enforcer.rules.keys() def enforce(policy_name, request): @@ -44,10 +31,6 @@ def enforce(policy_name, request): """ - global _ENFORCER - if not _ENFORCER: - _ENFORCER = policy.Enforcer(CONF) - _ENFORCER.load_rules() rule_method = "telemetry:" + policy_name headers = request.headers @@ -60,7 +43,7 @@ def enforce(policy_name, request): # maintain backward compat with Juno and previous by allowing the action if # there is no rule defined for it if ((_has_rule('default') or _has_rule(rule_method)) and - not _ENFORCER.enforce(rule_method, {}, policy_dict)): + not pecan.request.enforcer.enforce(rule_method, {}, policy_dict)): pecan.core.abort(status_code=403, detail='RBAC Authorization Failed') @@ -75,10 +58,6 @@ def get_limited_to(headers): one of these. """ - global _ENFORCER - if not _ENFORCER: - _ENFORCER = policy.Enforcer(CONF) - _ENFORCER.load_rules() policy_dict = dict() policy_dict['roles'] = headers.get('X-Roles', "").split(",") @@ -89,9 +68,9 @@ def get_limited_to(headers): # rule if the segregation rule (added in Kilo) is not defined rule_name = 'segregation' if _has_rule( 'segregation') else 'context_is_admin' - if not _ENFORCER.enforce(rule_name, - {}, - policy_dict): + if not pecan.request.enforcer.enforce(rule_name, + {}, + policy_dict): return headers.get('X-User-Id'), headers.get('X-Project-Id') return None, None diff --git a/ceilometer/service.py b/ceilometer/service.py index 5961160de6..a1bb03c07c 100644 --- a/ceilometer/service.py +++ b/ceilometer/service.py @@ -19,6 +19,7 @@ from keystoneauth1 import loading as ka_loading from oslo_config import cfg import oslo_i18n from oslo_log import log +from oslo_policy import opts as policy_opts from oslo_reports import guru_meditation_report as gmr from ceilometer.conf import defaults @@ -62,26 +63,32 @@ keystone_client.register_keystoneauth_opts(cfg.CONF) def prepare_service(argv=None, config_files=None): + if argv is None: + argv = sys.argv + + # FIXME(sileht): Use ConfigOpts() instead + conf = cfg.CONF + oslo_i18n.enable_lazy() - log.register_options(cfg.CONF) - log_levels = (cfg.CONF.default_log_levels + + log.register_options(conf) + log_levels = (conf.default_log_levels + ['futurist=INFO', 'neutronclient=INFO', 'keystoneclient=INFO']) log.set_defaults(default_log_levels=log_levels) defaults.set_cors_middleware_defaults() + policy_opts.set_defaults(conf) - if argv is None: - argv = sys.argv - cfg.CONF(argv[1:], project='ceilometer', validate_default_values=True, - version=version.version_info.version_string(), - default_config_files=config_files) + conf(argv[1:], project='ceilometer', validate_default_values=True, + version=version.version_info.version_string(), + default_config_files=config_files) - ka_loading.load_auth_from_conf_options(cfg.CONF, "service_credentials") + ka_loading.load_auth_from_conf_options(conf, "service_credentials") - log.setup(cfg.CONF, 'ceilometer') + log.setup(conf, 'ceilometer') # NOTE(liusheng): guru cannot run with service under apache daemon, so when # ceilometer-api running with mod_wsgi, the argv is [], we don't start # guru. if argv: gmr.TextGuruMeditation.setup_autorun(version) messaging.setup() + return conf diff --git a/ceilometer/tests/functional/api/__init__.py b/ceilometer/tests/functional/api/__init__.py index 65e987022d..5de6d80445 100644 --- a/ceilometer/tests/functional/api/__init__.py +++ b/ceilometer/tests/functional/api/__init__.py @@ -21,7 +21,6 @@ from oslo_policy import opts import pecan import pecan.testing -from ceilometer.api import rbac from ceilometer.tests import db as db_test_base cfg.CONF.import_group('api', 'ceilometer.api.controllers.v2.root') @@ -64,11 +63,10 @@ class FunctionalTest(db_test_base.TestBase): }, } - return pecan.testing.load_test_app(self.config) + return pecan.testing.load_test_app(self.config, conf=self.CONF) def tearDown(self): super(FunctionalTest, self).tearDown() - rbac.reset() pecan.set_config({}, overwrite=True) def put_json(self, path, params, expect_errors=False, headers=None, diff --git a/ceilometer/tests/functional/api/v2/test_acl_scenarios.py b/ceilometer/tests/functional/api/v2/test_acl_scenarios.py index f30e090d26..49a4463f2b 100644 --- a/ceilometer/tests/functional/api/v2/test_acl_scenarios.py +++ b/ceilometer/tests/functional/api/v2/test_acl_scenarios.py @@ -97,7 +97,7 @@ class TestAPIACL(v2.FunctionalTest): def _make_app(self): file_name = self.path_get('etc/ceilometer/api_paste.ini') self.CONF.set_override("api_paste_config", file_name) - return webtest.TestApp(app.load_app()) + return webtest.TestApp(app.load_app(self.CONF)) def test_non_authenticated(self): response = self.get_json('/meters', expect_errors=True) diff --git a/ceilometer/tests/functional/gabbi/fixtures.py b/ceilometer/tests/functional/gabbi/fixtures.py index 257fc7d267..4104d771c6 100644 --- a/ceilometer/tests/functional/gabbi/fixtures.py +++ b/ceilometer/tests/functional/gabbi/fixtures.py @@ -29,6 +29,7 @@ from oslo_utils import fileutils import six from six.moves.urllib import parse as urlparse +from ceilometer.api import app from ceilometer.event.storage import models from ceilometer.publisher import utils from ceilometer import sample @@ -39,6 +40,17 @@ from ceilometer import storage # data store. ENGINES = ['mongodb'] +# NOTE(chdent): Hack to restore semblance of global configuration to +# pass to the WSGI app used per test suite. LOAD_APP_KWARGS are the olso +# configuration, and the pecan application configuration of +# which the critical part is a reference to the current indexer. +LOAD_APP_KWARGS = None + + +def setup_app(): + global LOAD_APP_KWARGS + return app.load_app(**LOAD_APP_KWARGS) + class ConfigFixture(fixture.GabbiFixture): """Establish the relevant configuration for a test run.""" @@ -46,6 +58,8 @@ class ConfigFixture(fixture.GabbiFixture): def start_fixture(self): """Set up config.""" + global LOAD_APP_KWARGS + self.conf = None # Determine the database connection. @@ -94,6 +108,10 @@ class ConfigFixture(fixture.GabbiFixture): conf.set_override('aodh_is_enabled', False, group='api') conf.set_override('panko_is_enabled', False, group='api') + LOAD_APP_KWARGS = { + 'conf': conf, + } + def stop_fixture(self): """Reset the config and remove data.""" if self.conf: diff --git a/ceilometer/tests/functional/gabbi/test_gabbi.py b/ceilometer/tests/functional/gabbi/test_gabbi.py index 8c3670c8c1..162e426f1a 100644 --- a/ceilometer/tests/functional/gabbi/test_gabbi.py +++ b/ceilometer/tests/functional/gabbi/test_gabbi.py @@ -22,7 +22,6 @@ import os from gabbi import driver -from ceilometer.api import app from ceilometer.tests.functional.gabbi import fixtures as fixture_module TESTS_DIR = 'gabbits' @@ -32,5 +31,5 @@ def load_tests(loader, tests, pattern): """Provide a TestSuite to the discovery process.""" test_dir = os.path.join(os.path.dirname(__file__), TESTS_DIR) return driver.build_tests(test_dir, loader, host=None, - intercept=app.load_app, + intercept=fixture_module.setup_app, fixture_module=fixture_module) diff --git a/ceilometer/tests/functional/gabbi/test_gabbi_prefix.py b/ceilometer/tests/functional/gabbi/test_gabbi_prefix.py index 6f9da5e359..04f337c97c 100644 --- a/ceilometer/tests/functional/gabbi/test_gabbi_prefix.py +++ b/ceilometer/tests/functional/gabbi/test_gabbi_prefix.py @@ -19,7 +19,6 @@ import os from gabbi import driver -from ceilometer.api import app from ceilometer.tests.functional.gabbi import fixtures as fixture_module TESTS_DIR = 'gabbits_prefix' @@ -30,5 +29,5 @@ def load_tests(loader, tests, pattern): test_dir = os.path.join(os.path.dirname(__file__), TESTS_DIR) return driver.build_tests(test_dir, loader, host=None, prefix='/telemetry', - intercept=app.setup_app, + intercept=fixture_module.setup_app, fixture_module=fixture_module) diff --git a/ceilometer/tests/unit/api/test_app.py b/ceilometer/tests/unit/api/test_app.py index e507e0b426..08d3cf2816 100644 --- a/ceilometer/tests/unit/api/test_app.py +++ b/ceilometer/tests/unit/api/test_app.py @@ -16,7 +16,6 @@ import mock from oslo_config import cfg from oslo_config import fixture as fixture_config -from oslo_log import log from ceilometer.api import app from ceilometer.tests import base @@ -27,10 +26,10 @@ class TestApp(base.BaseTestCase): def setUp(self): super(TestApp, self).setUp() self.CONF = self.useFixture(fixture_config.Config()).conf - log.register_options(cfg.CONF) def test_api_paste_file_not_exist(self): self.CONF.set_override('api_paste_config', 'non-existent-file') with mock.patch.object(self.CONF, 'find_file') as ff: ff.return_value = None - self.assertRaises(cfg.ConfigFilesNotFoundError, app.load_app) + self.assertRaises(cfg.ConfigFilesNotFoundError, app.load_app, + self.CONF) diff --git a/ceilometer/tests/unit/api/test_hooks.py b/ceilometer/tests/unit/api/test_hooks.py index 96bc023b2c..4885448a8b 100644 --- a/ceilometer/tests/unit/api/test_hooks.py +++ b/ceilometer/tests/unit/api/test_hooks.py @@ -29,7 +29,7 @@ class TestTestNotifierHook(base.BaseTestCase): def test_init_notifier_with_drivers(self): self.CONF.set_override('telemetry_driver', 'messagingv2', group='publisher_notifier') - hook = hooks.NotifierHook() + hook = hooks.NotifierHook(self.CONF) notifier = hook.notifier self.assertIsInstance(notifier, oslo_messaging.Notifier) self.assertEqual(['messagingv2'], notifier._driver_names)