From 0c6de9b7fd0968e2bd9a27184c837f8158014efd Mon Sep 17 00:00:00 2001 From: Domhnall Walsh Date: Tue, 28 Jun 2016 11:40:59 +0100 Subject: [PATCH] Fix Middleware loading for falcon >= 0.1.6 API now loads middleware correctly for all version of falcon specified in requirements.txt. Cherry-pick from https://review.openstack.org/#/c/334920/ and pre_test_hook.sh from https://review.openstack.org/#/c/319950/ Conflicts: freezer_api/api/common/middleware.py freezer_api/api/common/utils.py freezer_api/cmd/api.py freezer_api/service.py Change-Id: I337cff087b7b34e760e0555591e1929979450c39 Closes-Bug: #1593212 --- freezer_api/api/common/middleware.py | 130 ++++++++++++++++++++++++--- freezer_api/cmd/api.py | 81 +++++++++++++++-- freezer_api/tests/pre_test_hook.sh | 21 +++++ freezer_api/tests/unit/test_api.py | 72 ++++++++++++++- 4 files changed, 284 insertions(+), 20 deletions(-) create mode 100644 freezer_api/tests/pre_test_hook.sh diff --git a/freezer_api/api/common/middleware.py b/freezer_api/api/common/middleware.py index b096bbb8..dcdb7065 100644 --- a/freezer_api/api/common/middleware.py +++ b/freezer_api/api/common/middleware.py @@ -15,22 +15,51 @@ limitations under the License. """ -from freezer_api.common import exceptions as freezer_api_exc +import falcon import json -from webob.dec import wsgify +import logging + +import freezer_api.common.exceptions as freezer_api_exc -@wsgify.middleware -def json_translator(req, app): - resp = req.get_response(app) - if resp.body: - if isinstance(resp.body, dict): - try: - resp.body = json.dumps(resp.body) - except Exception: - raise freezer_api_exc.FreezerAPIException( - 'Internal server error: malformed json reply') - return resp +class Middleware(object): + """ + WSGI wrapper for all freezer middlewares. + """ + + def __init__(self, app): + self.app = app + + def process_request(self, req): + """ + implement this function in your middleware to change the request + if the function return None the request will be handled in the next + level functions + """ + return None + + def process_response(self, resp): + """ + Implement this to modify your response + """ + return None + + @classmethod + def factory(cls, global_conf, **local_conf): + def filter(app): + return cls(app) + return filter + + def __call__(self, req, resp): + response = self.process_request(req) + if response: + return response + response = req.get_response(self.app) + response.req = req + try: + self.process_response(response) + except falcon.HTTPError as e: + logging.error(e) class HealthApp(object): @@ -48,3 +77,78 @@ class HealthApp(object): start_response('200 OK', []) return [] return self.app(environ, start_response) + + +class HookableMiddlewareMixin(object): + """Provides methods to extract before and after hooks from WSGI Middleware + + Prior to falcon 0.2.0b1, it's necessary to provide falcon with middleware + as "hook" functions that are either invoked before (to process requests) + or after (to process responses) the API endpoint code runs. + + This mixin allows the process_request and process_response methods from a + typical WSGI middleware object to be extracted for use as these hooks, with + the appropriate method signatures. + """ + + def as_before_hook(self): + """Extract process_request method as "before" hook + + :return: before hook function + """ + + # Need to wrap this up in a closure because the parameter counts + # differ + def before_hook(req, resp, params=None): + return self.process_request(req, resp) + + try: + return before_hook + except AttributeError as ex: + # No such method, we presume. + message_template = "Failed to get before hook from middleware {0} - {1}" + message = message_template.format(self.__name__, ex.message) + logging.error(message) + logging.exception(ex) + raise freezer_api_exc.FreezerAPIException(message) + + def as_after_hook(self): + """Extract process_response method as "after" hook + + :return: after hook function + """ + # Need to wrap this up in a closure because the parameter counts + # differ + def after_hook(req, resp, resource=None): + return self.process_response(req, resp, resource) + + try: + return after_hook + except AttributeError as ex: + # No such method, we presume. + message_template = "Failed to get after hook from middleware {0} - {1}" + message = message_template.format(self.__name__, ex.message) + logging.error(message) + logging.exception(ex) + raise freezer_api_exc.FreezerAPIException(message) + + +class RequireJSON(HookableMiddlewareMixin, object): + + def process_request(self, req, resp): + if not req.client_accepts_json: + raise falcon.HTTPNotAcceptable( + 'Freezer-api only supports responses encoded as JSON.', + href='http://docs.examples.com/api/json') + + +class JSONTranslator(HookableMiddlewareMixin, object): + + def process_response(self, req, resp, resource): + if not hasattr(resp, 'body'): + return + if isinstance(resp.data, dict): + resp.data = json.dumps(resp.data) + + if isinstance(resp.body, dict): + resp.body = json.dumps(resp.body) diff --git a/freezer_api/cmd/api.py b/freezer_api/cmd/api.py index 069d81ba..3b7b4528 100644 --- a/freezer_api/cmd/api.py +++ b/freezer_api/cmd/api.py @@ -23,6 +23,7 @@ import sys from keystonemiddleware import auth_token from oslo_config import cfg +from pkg_resources import parse_version from wsgiref import simple_server from freezer_api.api.common import middleware @@ -35,9 +36,21 @@ from freezer_api.common import exceptions as freezer_api_exc from freezer_api.common import log from freezer_api.storage import driver +# Define the minimum version of falcon at which we can use the "new" invocation +# style for middleware (aka v1), i.e. the "middleware" named argument for +# falcon.API. +FALCON_MINVERSION_MIDDLEWARE = parse_version('0.2.0b1') -def get_application(db): - app = falcon.API() + +def configure_app(app, db=None): + """Build routes and exception handlers + + :param app: falcon WSGI app + :param db: Database engine (ElasticSearch) + :return: + """ + if not db: + db = driver.get_db() for exception_class in freezer_api_exc.exception_handlers_catalog: app.add_error_handler(exception_class, exception_class.handle) @@ -50,9 +63,6 @@ def get_application(db): for route, resource in endpoints: app.add_route(version_path + route, resource) - # pylint: disable=no-value-for-parameter - app = middleware.json_translator(app) - if 'keystone_authtoken' in config.CONF: app = auth_token.AuthProtocol(app, {}) else: @@ -62,6 +72,67 @@ def get_application(db): return app + +def build_app_v0(): + """Instantiate the root freezer-api app + + Old versions of falcon (< 0.2.0) don't have a named 'middleware' argument. + This was introduced in version 0.2.0b1, so before that we need to instead + provide "before" hooks (request processing) and "after" hooks (response + processing). + + :return: falcon WSGI app + """ + + # injecting FreezerContext & hooks + before_hooks = [middleware.RequireJSON().as_before_hook()] + after_hooks = [middleware.JSONTranslator().as_after_hook()] + + # The signature of falcon.API() differs between versions, suppress pylint: + # pylint: disable=unexpected-keyword-arg + app = falcon.API(before=before_hooks, after=after_hooks) + + app = configure_app(app) + return app + + +def build_app_v1(): + """Building routes and forming the root freezer-api app + + This uses the 'middleware' named argument to specify middleware for falcon + instead of the 'before' and 'after' hooks that were removed after 0.3.0 + (both approaches were available for versions 0.2.0 - 0.3.0) + + :return: falcon WSGI app + """ + middleware_list = [middleware.RequireJSON(), middleware.JSONTranslator()] + + # The signature of falcon.API() differs between versions, suppress pylint: + # pylint: disable=unexpected-keyword-arg + app = falcon.API(middleware=middleware_list) + + app = configure_app(app) + return app + + +def get_application(db): + """Launch the falcon API with the correct middleware arguments + + :param db: ElasticSearch instance + :return: Falcon API instance + """ + # Special case handling for python 0.1.6 (from 0.1.7, falcon exposes + # __version__ like a python module *should*)... + current_version = parse_version( + falcon.__version__ if hasattr(falcon, '__version__') else falcon.version) + + # Check the currently installed version of falcon in order to invoke it + # correctly. + if current_version < FALCON_MINVERSION_MIDDLEWARE: + return build_app_v0() + else: + return build_app_v1() + config_file = '/etc/freezer-api.conf' config_files_list = [config_file] if os.path.isfile(config_file) else [] config.parse_args(args=[], default_config_files=config_files_list) diff --git a/freezer_api/tests/pre_test_hook.sh b/freezer_api/tests/pre_test_hook.sh new file mode 100644 index 00000000..78630a27 --- /dev/null +++ b/freezer_api/tests/pre_test_hook.sh @@ -0,0 +1,21 @@ +#!/bin/bash -xe + +# (C) Copyright 2016 Hewlett Packard Enterprise Development Company LP +# +# 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. + +# Called by gate-freezer-api-devstack-dsvm job. + +echo 'Running freezer-api pre_test_hook' + +# Nothing to do. Can be used when needed. diff --git a/freezer_api/tests/unit/test_api.py b/freezer_api/tests/unit/test_api.py index 3be0d879..ef465a1d 100644 --- a/freezer_api/tests/unit/test_api.py +++ b/freezer_api/tests/unit/test_api.py @@ -17,14 +17,82 @@ limitations under the License. """ import unittest -from mock import Mock, patch +from mock import patch -from freezer_api.cmd import api from freezer_api.api import common +from freezer_api.cmd import api + class TestAPI(unittest.TestCase): + def setUp(self): + self.falcon_versions_hooks = ['0.1.6', '0.1.7', '0.1.8', '0.1.9', + '0.1.10'] + self.falcon_versions_middleware = ['0.2.0', '0.3.0', '1.0.0'] + @patch('freezer_api.storage.elastic.logging') def test_auth_install(self, mock_logging): app = api.get_application(None) assert isinstance(app, common.middleware.HealthApp) + + @patch('freezer_api.cmd.api.v1') + @patch('freezer_api.cmd.api.driver') + @patch('freezer_api.cmd.api.falcon') + def test_on_old_falcon_builds_v0(self, mock_falcon, mock_driver, mock_v1): + """Test that falcon versions that should use old middleware syntax do so + + :param mock_falcon: The falcon import freezer-api will try to + start up + :param mock_driver: Database driver + :param mock_v1: List of endpoints for v1 for the freezer API. + """ + mock_driver.get_db.return_value = None + mock_v1.endpoints = [] + + # Iterate through all of the versions of falcon that should be using the + # old before=,after= invocation and ensure that freezer-api isn't trying + # to invoke it in the old style. + for version_string in self.falcon_versions_hooks: + mock_falcon.__version__ = version_string + # Attempt to invoke a mocked version of falcon to see what args it + # was called with + _ = api.get_application(None) + + # Check kwargs to see if the correct arguments are being passed + _, named_args = mock_falcon.API.call_args + + self.assertIn('before', named_args) + self.assertIn('after', named_args) + self.assertNotIn('middleware', named_args) + + @patch('freezer_api.cmd.api.v1') + @patch('freezer_api.cmd.api.driver') + @patch('freezer_api.cmd.api.falcon') + def test_on_new_falcon_builds_v1(self, mock_falcon, mock_driver, mock_v1): + """Test that falcon versions that should use new middleware syntax do so + + :param mock_falcon: The falcon import freezer-api will try to + start up + :param mock_driver: Database driver + :param mock_v1: List of endpoints for v1 for the freezer API. + """ + mock_driver.get_db.return_value = None + mock_v1.endpoints = [] + + # Iterate through all of the versions of falcon that should be using the + # old before=,after= invocation and ensure that freezer-api isn't trying + # to invoke it in the old style. + for version_string in self.falcon_versions_middleware: + mock_falcon.__version__ = version_string + # Attempt to invoke a mocked version of falcon to see what args it + # was called with + _ = api.get_application(None) + + # Check kwargs to see if the correct arguments are being passed + _, kwargs = mock_falcon.API.call_args + + named_args = kwargs.keys() + + self.assertNotIn('before', named_args) + self.assertNotIn('after', named_args) + self.assertIn('middleware', named_args) \ No newline at end of file