Ensure JSON responses result from failure

Responses generated from failure should be JSON formatted.

flask_restful.Api is subclassed to control the format of all error
responses generated by the API Flask app.

The API middleware is modified to use the same error response function
given that it is not part of the Flask app for the API. Any Response
object created by the middleware has been converted to an exception to
ensure that all error responses are generated using the same code.

The use of abort has been replaced with raising exceptions, this further
consolidates the error response mechanism.

response_filter has been modified so that it always expects to receive a
response tuple now that http_codes is no longer there to return a
response object.

The error filters in schemas.py have been removed. These were not used
by the existing code.

Closes-bug: 1665015

Change-Id: I2be40e9493f313b3fe0173c34d371659039c1bae

craton/api/__init__.py

@@ -1,13 +1,13 @@
-from datetime import date
 import os
 from paste import deploy
-from flask import Flask, json
+from flask import Flask
 
 
 from oslo_config import cfg
 from oslo_log import log as logging
 
 from craton.api import v1
+from craton.util import JSON_KWARGS
 
 
 LOG = logging.getLogger(__name__)
@@ -49,27 +49,11 @@ def create_app(global_config, **local_config):
     return setup_app()
 
 
-class JSONEncoder(json.JSONEncoder):
-
-    def default(self, o):
-        if isinstance(o, date):
-            return o.isoformat()
-        return json.JSONEncoder.default(self, o)
-
-
-RESTFUL_JSON = {
-    "indent": 2,
-    "sort_keys": True,
-    "cls": JSONEncoder,
-    "separators": (",", ": "),
-}
-
-
 def setup_app(config=None):
     app = Flask(__name__, static_folder=None)
     app.config.update(
         PROPAGATE_EXCEPTIONS=True,
-        RESTFUL_JSON=RESTFUL_JSON,
+        RESTFUL_JSON=JSON_KWARGS,
     )
     app.register_blueprint(v1.bp, url_prefix='/v1')
     return app

craton/api/middleware.py

@@ -4,11 +4,9 @@
 from oslo_log import log
 from oslo_utils import uuidutils
 
-import flask
-import json
-
 from craton.db import api as dbapi
 from craton import exceptions
+from craton.util import handle_all_exceptions_decorator
 
 
 LOG = log.getLogger(__name__)
@@ -34,20 +32,13 @@ def make_context(self, request, *args, **kwargs):
         request.environ['context'] = ctxt
         return ctxt
 
-    def _invalid_project_id(self, project_id):
-        err_msg = json.dumps({
-            "message": "Project ID ('{}') is not a valid UUID".format(
-                project_id)
-        })
-        return flask.Response(response=err_msg, status=401,
-                              headers={'Content-Type': 'application/json'})
-
 
 class NoAuthContextMiddleware(ContextMiddleware):
 
     def __init__(self, application):
         self.application = application
 
+    @handle_all_exceptions_decorator
     def process_request(self, request):
         # Simply insert some dummy context info
         self.make_context(
@@ -72,11 +63,16 @@ class LocalAuthContextMiddleware(ContextMiddleware):
     def __init__(self, application):
         self.application = application
 
+    @handle_all_exceptions_decorator
     def process_request(self, request):
         headers = request.headers
         project_id = headers.get('X-Auth-Project')
         if not uuidutils.is_uuid_like(project_id):
-            return self._invalid_project_id(project_id)
+            raise exceptions.AuthenticationError(
+                message="Project ID ('{}') is not a valid UUID".format(
+                    project_id
+                )
+            )
 
         ctx = self.make_context(
             request,
@@ -91,7 +87,7 @@ def process_request(self, request):
             user_info = dbapi.get_user_info(ctx,
                                             headers.get('X-Auth-User', None))
             if user_info.api_key != headers.get('X-Auth-Token', None):
-                return flask.Response(status=401)
+                raise exceptions.AuthenticationError
             if user_info.is_root:
                 ctx.is_admin = True
                 ctx.is_admin_project = True
@@ -102,10 +98,7 @@ def process_request(self, request):
                 ctx.is_admin = False
                 ctx.is_admin_project = False
         except exceptions.NotFound:
-            return flask.Response(status=401)
-        except Exception as err:
-            LOG.error(err)
-            return flask.Response(status=500)
+            raise exceptions.AuthenticationError
 
     @classmethod
     def factory(cls, global_config, **local_config):
@@ -116,11 +109,12 @@ def _factory(application):
 
 class KeystoneContextMiddleware(ContextMiddleware):
 
+    @handle_all_exceptions_decorator
     def process_request(self, request):
         headers = request.headers
         environ = request.environ
         if headers.get('X-Identity-Status', '').lower() != 'confirmed':
-            return flask.Response(status=401)
+            raise exceptions.AuthenticationError
 
         token_info = environ['keystone.token_info']['token']
         roles = (role['name'] for role in token_info['roles'])

craton/api/v1/__init__.py

@@ -2,10 +2,20 @@
 import flask_restful as restful
 
 from craton.api.v1.routes import routes
+from craton.util import handle_all_exceptions
+
+
+class CratonApi(restful.Api):
+
+    def error_router(self, _, e):
+        return self.handle_error(e)
+
+    def handle_error(self, e):
+        return handle_all_exceptions(e)
 
 
 bp = Blueprint('v1', __name__)
-api = restful.Api(bp, catch_all_404s=True)
+api = CratonApi(bp, catch_all_404s=False)
 
 for route in routes:
     api.add_resource(route.pop('resource'), *route.pop('urls'), **route)

craton/api/v1/base.py

@@ -1,19 +1,13 @@
 import functools
-import inspect
-import json
 import re
 import urllib.parse as urllib
 
-import decorator
-
 import flask
 import flask_restful as restful
 
-from craton import api
 from craton.api.v1.validators import ensure_project_exists
 from craton.api.v1.validators import request_validate
 from craton.api.v1.validators import response_filter
-from craton import exceptions
 
 
 SORT_KEY_SPLITTER = re.compile('[ ,]')
@@ -23,30 +17,6 @@ class Resource(restful.Resource):
     method_decorators = [request_validate, ensure_project_exists,
                          response_filter]
 
-    def error_response(self, status_code, message):
-        body = json.dumps(
-            {
-                'status': status_code,
-                'message': message
-            },
-            **api.RESTFUL_JSON,
-        )
-        resp = flask.make_response("{body}\n".format(body=body))
-        resp.status_code = status_code
-        return resp
-
-
-@decorator.decorator
-def http_codes(f, *args, **kwargs):
-    try:
-        return f(*args, **kwargs)
-    except exceptions.Base as err:
-        return args[0].error_response(err.code, err.message)
-    except Exception as err:
-        inspect.getmodule(f).LOG.error(
-            'Error during %s: %s' % (f.__qualname__, err))
-        return args[0].error_response(500, 'Unknown Error')
-
 
 def pagination_context(function):
     @functools.wraps(function)

craton/api/v1/resources/inventory/cells.py

@@ -13,7 +13,6 @@
 
 class Cells(base.Resource):
 
-    @base.http_codes
     @base.pagination_context
     def get(self, context, request_args, pagination_params):
         """Get all cells, with optional filtering."""
@@ -30,7 +29,6 @@ def get(self, context, request_args, pagination_params):
         response_body = {'cells': cells_obj, 'links': links}
         return jsonutils.to_primitive(response_body), 200, None
 
-    @base.http_codes
     def post(self, context, request_data):
         """Create a new cell."""
         json = util.copy_project_id_into_json(context, request_data)
@@ -51,19 +49,16 @@ def post(self, context, request_data):
 
 class CellById(base.Resource):
 
-    @base.http_codes
     def get(self, context, id, request_args):
         cell_obj = dbapi.cells_get_by_id(context, id)
         cell = utils.get_resource_with_vars(request_args, cell_obj)
         return cell, 200, None
 
-    @base.http_codes
     def put(self, context, id, request_data):
         """Update existing cell."""
         cell_obj = dbapi.cells_update(context, id, request_data)
         return jsonutils.to_primitive(cell_obj), 200, None
 
-    @base.http_codes
     def delete(self, context, id):
         """Delete existing cell."""
         dbapi.cells_delete(context, id)

craton/api/v1/resources/inventory/clouds.py

@@ -13,7 +13,6 @@
 
 class Clouds(base.Resource):
 
-    @base.http_codes
     @base.pagination_context
     def get(self, context, request_args, pagination_params):
         """Get cloud(s) for the project. Get cloud details if
@@ -46,7 +45,6 @@ def get(self, context, request_args, pagination_params):
         response_body = {'clouds': clouds_obj, 'links': links}
         return jsonutils.to_primitive(response_body), 200, None
 
-    @base.http_codes
     def post(self, context, request_data):
         """Create a new cloud."""
         json = util.copy_project_id_into_json(context, request_data)
@@ -67,20 +65,17 @@ def post(self, context, request_data):
 
 class CloudsById(base.Resource):
 
-    @base.http_codes
     def get(self, context, id):
         cloud_obj = dbapi.clouds_get_by_id(context, id)
         cloud = jsonutils.to_primitive(cloud_obj)
         cloud['variables'] = jsonutils.to_primitive(cloud_obj.variables)
         return cloud, 200, None
 
-    @base.http_codes
     def put(self, context, id, request_data):
         """Update existing cloud."""
         cloud_obj = dbapi.clouds_update(context, id, request_data)
         return jsonutils.to_primitive(cloud_obj), 200, None
 
-    @base.http_codes
     def delete(self, context, id):
         """Delete existing cloud."""
         dbapi.clouds_delete(context, id)

craton/api/v1/resources/inventory/devices.py

@@ -13,7 +13,6 @@
 
 class Devices(base.Resource):
 
-    @base.http_codes
     @base.pagination_context
     def get(self, context, request_args, pagination_params):
         """Get all devices, with optional filtering."""

craton/api/v1/resources/inventory/hosts.py

@@ -13,7 +13,6 @@
 
 class Hosts(base.Resource):
 
-    @base.http_codes
     @base.pagination_context
     def get(self, context, request_args, pagination_params):
         """Get all hosts for region, with optional filtering."""
@@ -35,7 +34,6 @@ def get(self, context, request_args, pagination_params):
 
         return response_body, 200, None
 
-    @base.http_codes
     def post(self, context, request_data):
         """Create a new host."""
         json = util.copy_project_id_into_json(context, request_data)
@@ -58,7 +56,6 @@ def post(self, context, request_data):
 
 class HostById(base.Resource):
 
-    @base.http_codes
     def get(self, context, id, request_args):
         """Get host by given id"""
         host_obj = dbapi.hosts_get_by_id(context, id)
@@ -68,7 +65,6 @@ def get(self, context, id, request_args):
 
         return host, 200, None
 
-    @base.http_codes
     def put(self, context, id, request_data):
         """Update existing host data, or create if it does not exist."""
         host_obj = dbapi.hosts_update(context, id, request_data)
@@ -79,7 +75,6 @@ def put(self, context, id, request_data):
 
         return host, 200, None
 
-    @base.http_codes
     def delete(self, context, id):
         """Delete existing host."""
         dbapi.hosts_delete(context, id)
@@ -88,14 +83,12 @@ def delete(self, context, id):
 
 class HostsLabels(base.Resource):
 
-    @base.http_codes
     def get(self, context, id):
         """Get labels for given host device."""
         host_obj = dbapi.hosts_get_by_id(context, id)
         response = {"labels": list(host_obj.labels)}
         return response, 200, None
 
-    @base.http_codes
     def put(self, context, id, request_data):
         """
         Update existing device label entirely, or add if it does
@@ -105,7 +98,6 @@ def put(self, context, id, request_data):
         response = {"labels": list(resp.labels)}
         return response, 200, None
 
-    @base.http_codes
     def delete(self, context, id, request_data):
         """Delete device label entirely."""
         dbapi.hosts_labels_delete(context, id, request_data)