Merge "Ensure JSON responses result from failure"

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)