From e666839bc170ee3a529e11a78500d34357c8f6ff Mon Sep 17 00:00:00 2001 From: Morgan Fainberg Date: Wed, 10 Oct 2018 14:23:12 -0700 Subject: [PATCH] Cleanup keystone.server.flask.application Remove a chunk of the compat code for legacy dispatching. This moves the logging about the request to it's own before_request function. Change-Id: I0b1a4ca9a95489e410f055ff47f3399feba3a8f1 Partial-Bug: #1776504 --- keystone/server/flask/application.py | 177 +----------------- .../flask/request_processing/req_logging.py | 31 +++ 2 files changed, 41 insertions(+), 167 deletions(-) create mode 100644 keystone/server/flask/request_processing/req_logging.py diff --git a/keystone/server/flask/application.py b/keystone/server/flask/application.py index 9e4258d406..acc883a624 100644 --- a/keystone/server/flask/application.py +++ b/keystone/server/flask/application.py @@ -12,58 +12,21 @@ from __future__ import absolute_import -import collections import functools -import itertools import sys import flask from oslo_log import log from oslo_middleware import healthcheck -import routes import werkzeug.wsgi import keystone.api -from keystone.common import wsgi as keystone_wsgi from keystone.server.flask.request_processing import json_body - -# TODO(morgan): _MOVED_API_PREFIXES to be removed when the legacy dispatch -# support is removed. -_MOVED_API_PREFIXES = frozenset( - ['auth', - 'credentials', - 'domains', - 'ec2tokens', - 'endpoints', - 'groups', - 'OS-EP-FILTER', - 'OS-FEDERATION', - 'OS-INHERIT', - 'OS-OAUTH1', - 'OS-REVOKE', - 'OS-SIMPLE-CERT', - 'OS-TRUST', - 'limits', - 'policy', - 'projects', - 'regions', - 'registered_limits', - 'role_assignments', - 'role_inferences', - 'roles', - 's3tokens', - 'services', - 'system', - 'users', - ] -) +from keystone.server.flask.request_processing import req_logging LOG = log.getLogger(__name__) -ALL_API_ROUTERS = [] - - def fail_gracefully(f): """Log exceptions and aborts.""" @functools.wraps(f) @@ -80,90 +43,6 @@ def fail_gracefully(f): return wrapper -class KeystoneDispatcherMiddleware(werkzeug.wsgi.DispatcherMiddleware): - """Allows one to mount middlewares or applications in a WSGI application. - - This is useful if you want to combine multiple WSGI applications:: - - app = DispatcherMiddleware(app, { - '/app2': app2, - '/app3': app3 - }) - - This is a modified version of the werkzeurg.wsgi.DispatchMiddleware to - handle the "SCRIPT_NAME" and "PATH_INFO" mangling in a way that is - compatible with the way paste.deploy and routes.Mapper works. For - Migration from legacy routes.Mapper to native flask blueprints, we are - treating each subsystem as their own "app". - - This Dispatcher also logs (debug) if we are dispatching a request to - a non-native flask Mapper. - """ - - @property - def config(self): - return self.app.config - - def __call__(self, environ, start_response): - script = environ.get('PATH_INFO', '') - original_script_name = environ.get('SCRIPT_NAME', '') - last_element = '' - path_info = '' - while '/' in script: - if script in self.mounts: - LOG.debug('Dispatching request to legacy mapper: %s', - script) - app = self.mounts[script] - # NOTE(morgan): Simply because we're doing something "odd" - # here and internally routing magically to another "wsgi" - # router even though we're already deep in the stack we - # need to re-add the last element pulled off. This is 100% - # legacy and only applies to the "apps" that make up each - # keystone subsystem. - # - # This middleware is only used in support of the transition - # process from webob and home-rolled WSGI framework to - # Flask - if script.rindex('/') > 0: - script, last_element = script.rsplit('/', 1) - last_element = '/%s' % last_element - environ['SCRIPT_NAME'] = original_script_name + script - # Ensure there is only 1 slash between these items, the - # mapper gets horribly confused if we have // in there, - # which occasionally. As this is temporary to dispatch - # to the Legacy mapper, fix the string until we no longer - # need this logic. - environ['PATH_INFO'] = '%s/%s' % (last_element.rstrip('/'), - path_info.strip('/')) - break - script, last_item = script.rsplit('/', 1) - path_info = '/%s%s' % (last_item, path_info) - else: - app = self.mounts.get(script, self.app) - if app != self.app: - LOG.debug('Dispatching (fallthrough) request to legacy ' - 'mapper: %s', script) - else: - LOG.debug('Dispatching back to Flask native app.') - environ['SCRIPT_NAME'] = original_script_name + script - environ['PATH_INFO'] = path_info - - # NOTE(morgan): remove extra trailing slashes so the mapper can do the - # right thing and get the requests mapped to the right place. For - # example, "/v3/projects/" is not the same as "/v3/projects". We do not - # want to blindly rstrip for the case of '/'. - if environ['PATH_INFO'][-1] == '/' and len(environ['PATH_INFO']) > 1: - environ['PATH_INFO'] = environ['PATH_INFO'][0:-1] - LOG.debug('SCRIPT_NAME: `%s`, PATH_INFO: `%s`', - environ['SCRIPT_NAME'], environ['PATH_INFO']) - return app(environ, start_response) - - -class _ComposibleRouterStub(keystone_wsgi.ComposableRouter): - def __init__(self, routers): - self._routers = routers - - def _add_vary_x_auth_token_header(response): # Add the expected Vary Header, this is run after every request in the # response-phase @@ -177,11 +56,10 @@ def application_factory(name='public'): raise RuntimeError('Application name (for base_url lookup) must be ' 'either `admin` or `public`.') - # NOTE(morgan): The Flask App actually dispatches nothing until we migrate - # some routers to Flask-Blueprints, it is simply a placeholder. app = flask.Flask(name) # Add core before request functions + app.before_request(req_logging.log_request_info) app.before_request(json_body.json_body_before_request) # Add core after request functions @@ -192,57 +70,22 @@ def application_factory(name='public'): # We want to bubble up Flask Exceptions (for now) PROPAGATE_EXCEPTIONS=True) - # TODO(morgan): Convert Subsystems over to Flask-Native, for now, we simply - # dispatch to another "application" [e.g "keystone"] - # NOTE(morgan): as each router is converted to flask-native blueprint, - # remove from this list. WARNING ORDER MATTERS; ordered dict used to - # ensure sane ordering of the routers in the legacy-dispatch model. - dispatch_map = collections.OrderedDict() - - # Load in Healthcheck and map it to /healthcheck - hc_app = healthcheck.Healthcheck.app_factory( - {}, oslo_config_project='keystone') - dispatch_map['/healthcheck'] = hc_app - - # More legacy code to instantiate all the magic for the dispatchers. - # The move to blueprints (FLASK) will allow this to be eliminated. - _routers = [] - sub_routers = [] - mapper = routes.Mapper() - for api_routers in ALL_API_ROUTERS: - moved_found = [pfx for - pfx in getattr(api_routers, '_path_prefixes', []) - if pfx in _MOVED_API_PREFIXES] - if moved_found: - raise RuntimeError('An API Router is trying to register path ' - 'prefix(s) `%(pfx)s` that is handled by the ' - 'native Flask app. Keystone cannot ' - 'start.' % - {'pfx': ', '.join([p for p in moved_found])}) - - routers_instance = api_routers.Routers() - _routers.append(routers_instance) - routers_instance.append_v3_routers(mapper, sub_routers) - # TODO(morgan): Remove "API version registration". For now this is kept # for ease of conversion (minimal changes) keystone.api.discovery.register_version('v3') - # NOTE(morgan): We add in all the keystone.api blueprints here, this - # replaces (as they are implemented) the legacy dispatcher work. for api in keystone.api.__apis__: for api_bp in api.APIs: api_bp.instantiate_and_register_to_app(app) - # Build and construct the dispatching for the Legacy dispatching model - sub_routers.append(_ComposibleRouterStub(_routers)) - legacy_dispatcher = keystone_wsgi.ComposingRouter(mapper, sub_routers) + # Load in Healthcheck and map it to /healthcheck + hc_app = healthcheck.Healthcheck.app_factory( + {}, oslo_config_project='keystone') - for pfx in itertools.chain(*[rtr.Routers._path_prefixes for - rtr in ALL_API_ROUTERS]): - dispatch_map['/v3/%s' % pfx] = legacy_dispatcher - - app.wsgi_app = KeystoneDispatcherMiddleware( + # Use the simple form of the dispatch middleware, no extra logic needed + # for legacy dispatching. This is to mount /healthcheck at a consistent + # place + app.wsgi_app = werkzeug.wsgi.DispatcherMiddleware( app.wsgi_app, - dispatch_map) + {'/healthcheck': hc_app}) return app diff --git a/keystone/server/flask/request_processing/req_logging.py b/keystone/server/flask/request_processing/req_logging.py new file mode 100644 index 0000000000..acae061590 --- /dev/null +++ b/keystone/server/flask/request_processing/req_logging.py @@ -0,0 +1,31 @@ +# 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. + +# LOG some debug output about the request. This was originally in the +# dispatch middleware + +import flask +from oslo_log import log + + +LOG = log.getLogger(__name__) +_ENVIRON_KEYS = ('SCRIPT_NAME', 'PATH_INFO') + + +def log_request_info(): + # Add in any extra debug logging about the request that is desired + # note that this is executed prior to routing the request to a resource + # so the data is somewhat raw. + for element in _ENVIRON_KEYS: + LOG.debug("environ['%(key)s']: %(value)s", + {'key': element, + 'value': flask.request.environ.get(element, '<>')})