From 533492b4d36ded28fe9aadd1c51c2260cccafedb Mon Sep 17 00:00:00 2001 From: git-harry Date: Mon, 20 Mar 2017 16:08:49 +0000 Subject: [PATCH] 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 | 22 +- craton/api/middleware.py | 30 +- craton/api/v1/__init__.py | 12 +- craton/api/v1/base.py | 30 - craton/api/v1/resources/inventory/cells.py | 5 - craton/api/v1/resources/inventory/clouds.py | 5 - craton/api/v1/resources/inventory/devices.py | 1 - craton/api/v1/resources/inventory/hosts.py | 8 - craton/api/v1/resources/inventory/networks.py | 18 - craton/api/v1/resources/inventory/regions.py | 5 - craton/api/v1/resources/projects.py | 4 - craton/api/v1/resources/users.py | 4 - craton/api/v1/resources/variables.py | 3 - craton/api/v1/schemas.py | 668 ------------------ craton/api/v1/validators.py | 30 +- craton/exceptions.py | 5 + craton/tests/functional/__init__.py | 11 + craton/tests/functional/test_cell_calls.py | 37 +- craton/tests/functional/test_cloud_calls.py | 42 +- craton/tests/functional/test_host_calls.py | 39 +- craton/tests/functional/test_network_calls.py | 54 +- .../functional/test_network_device_calls.py | 14 + .../test_network_interface_calls.py | 13 + craton/tests/functional/test_project_calls.py | 10 + craton/tests/functional/test_region_calls.py | 50 +- craton/util.py | 64 ++ 26 files changed, 328 insertions(+), 856 deletions(-) diff --git a/craton/api/__init__.py b/craton/api/__init__.py index 767cadb..02724e2 100644 --- a/craton/api/__init__.py +++ b/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 diff --git a/craton/api/middleware.py b/craton/api/middleware.py index 7042e75..1b8919d 100644 --- a/craton/api/middleware.py +++ b/craton/api/middleware.py @@ -4,11 +4,9 @@ from oslo_context import context 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 @@ class ContextMiddleware(base.Middleware): 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 @@ class LocalAuthContextMiddleware(ContextMiddleware): 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 @@ class LocalAuthContextMiddleware(ContextMiddleware): 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 @@ class LocalAuthContextMiddleware(ContextMiddleware): 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']) diff --git a/craton/api/v1/__init__.py b/craton/api/v1/__init__.py index 5cdeffb..c465243 100644 --- a/craton/api/v1/__init__.py +++ b/craton/api/v1/__init__.py @@ -2,10 +2,20 @@ from flask import Blueprint 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) diff --git a/craton/api/v1/base.py b/craton/api/v1/base.py index 8404268..6cb6685 100644 --- a/craton/api/v1/base.py +++ b/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) diff --git a/craton/api/v1/resources/inventory/cells.py b/craton/api/v1/resources/inventory/cells.py index 4911c20..6fc32b9 100644 --- a/craton/api/v1/resources/inventory/cells.py +++ b/craton/api/v1/resources/inventory/cells.py @@ -13,7 +13,6 @@ LOG = log.getLogger(__name__) 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 @@ class Cells(base.Resource): 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 @@ class Cells(base.Resource): 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) diff --git a/craton/api/v1/resources/inventory/clouds.py b/craton/api/v1/resources/inventory/clouds.py index 372d3a6..5e9e951 100644 --- a/craton/api/v1/resources/inventory/clouds.py +++ b/craton/api/v1/resources/inventory/clouds.py @@ -13,7 +13,6 @@ LOG = log.getLogger(__name__) 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 @@ class Clouds(base.Resource): 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 @@ class Clouds(base.Resource): 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) diff --git a/craton/api/v1/resources/inventory/devices.py b/craton/api/v1/resources/inventory/devices.py index d2df276..339ef9f 100644 --- a/craton/api/v1/resources/inventory/devices.py +++ b/craton/api/v1/resources/inventory/devices.py @@ -13,7 +13,6 @@ LOG = log.getLogger(__name__) class Devices(base.Resource): - @base.http_codes @base.pagination_context def get(self, context, request_args, pagination_params): """Get all devices, with optional filtering.""" diff --git a/craton/api/v1/resources/inventory/hosts.py b/craton/api/v1/resources/inventory/hosts.py index 0a129ab..eb8dd80 100644 --- a/craton/api/v1/resources/inventory/hosts.py +++ b/craton/api/v1/resources/inventory/hosts.py @@ -13,7 +13,6 @@ LOG = log.getLogger(__name__) 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 @@ class Hosts(base.Resource): 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 @@ class Hosts(base.Resource): 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 @@ class HostById(base.Resource): 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 @@ class HostById(base.Resource): return host, 200, None - @base.http_codes def delete(self, context, id): """Delete existing host.""" dbapi.hosts_delete(context, id) @@ -88,14 +83,12 @@ class HostById(base.Resource): 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 @@ class HostsLabels(base.Resource): 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) diff --git a/craton/api/v1/resources/inventory/networks.py b/craton/api/v1/resources/inventory/networks.py index 02eaa8d..2158b38 100644 --- a/craton/api/v1/resources/inventory/networks.py +++ b/craton/api/v1/resources/inventory/networks.py @@ -14,7 +14,6 @@ LOG = log.getLogger(__name__) class Networks(base.Resource): """Controller for Networks resources.""" - @base.http_codes @base.pagination_context def get(self, context, request_args, pagination_params): """Get all networks, with optional filtering.""" @@ -30,7 +29,6 @@ class Networks(base.Resource): response_body = {'networks': networks_obj, 'links': links} return jsonutils.to_primitive(response_body), 200, None - @base.http_codes def post(self, context, request_data): """Create a new network.""" json = util.copy_project_id_into_json(context, request_data) @@ -47,7 +45,6 @@ class Networks(base.Resource): class NetworkById(base.Resource): """Controller for Networks by ID.""" - @base.http_codes def get(self, context, id): """Get network by given id""" obj = dbapi.networks_get_by_id(context, id) @@ -55,13 +52,11 @@ class NetworkById(base.Resource): device['variables'] = jsonutils.to_primitive(obj.variables) return device, 200, None - @base.http_codes def put(self, context, id, request_data): """Update existing network values.""" net_obj = dbapi.networks_update(context, id, request_data) return jsonutils.to_primitive(net_obj), 200, None - @base.http_codes def delete(self, context, id): """Delete existing network.""" dbapi.networks_delete(context, id) @@ -71,7 +66,6 @@ class NetworkById(base.Resource): class NetworkDevices(base.Resource): """Controller for Network Device resources.""" - @base.http_codes @base.pagination_context def get(self, context, request_args, pagination_params): """Get all network devices.""" @@ -93,7 +87,6 @@ class NetworkDevices(base.Resource): return response_body, 200, None - @base.http_codes def post(self, context, request_data): """Create a new network device.""" json = util.copy_project_id_into_json(context, request_data) @@ -113,7 +106,6 @@ class NetworkDevices(base.Resource): class NetworkDeviceById(base.Resource): """Controller for Network Devices by ID.""" - @base.http_codes def get(self, context, id, request_args): """Get network device by given id""" obj = dbapi.network_devices_get_by_id(context, id) @@ -125,7 +117,6 @@ class NetworkDeviceById(base.Resource): return device, 200, None - @base.http_codes def put(self, context, id, request_data): """Update existing device values.""" net_obj = dbapi.network_devices_update(context, id, request_data) @@ -135,7 +126,6 @@ class NetworkDeviceById(base.Resource): return device, 200, None - @base.http_codes def delete(self, context, id): """Delete existing network device.""" dbapi.network_devices_delete(context, id) @@ -145,21 +135,18 @@ class NetworkDeviceById(base.Resource): class NetworkDeviceLabels(base.Resource): """Controller for Netowrk Device Labels.""" - @base.http_codes def get(self, context, id): """Get labels for given network device.""" obj = dbapi.network_devices_get_by_id(context, id) response = {"labels": list(obj.labels)} return response, 200, None - @base.http_codes def put(self, context, id, request_data): """Update existing device label. Adds if it does not exist.""" resp = dbapi.network_devices_labels_update(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(s).""" dbapi.network_devices_labels_delete(context, id, request_data) @@ -169,7 +156,6 @@ class NetworkDeviceLabels(base.Resource): class NetworkInterfaces(base.Resource): """Controller for Netowrk Interfaces.""" - @base.http_codes @base.pagination_context def get(self, context, request_args, pagination_params): """Get all network interfaces.""" @@ -180,7 +166,6 @@ class NetworkInterfaces(base.Resource): response_body = {'network_interfaces': interfaces_obj, 'links': links} return jsonutils.to_primitive(response_body), 200, None - @base.http_codes def post(self, context, request_data): """Create a new network interface.""" json = util.copy_project_id_into_json(context, request_data) @@ -197,7 +182,6 @@ class NetworkInterfaces(base.Resource): class NetworkInterfaceById(base.Resource): - @base.http_codes def get(self, context, id): """Get network interface by given id""" obj = dbapi.network_interfaces_get_by_id(context, id) @@ -205,13 +189,11 @@ class NetworkInterfaceById(base.Resource): interface['variables'] = jsonutils.to_primitive(obj.variables) return interface, 200, None - @base.http_codes def put(self, context, id, request_data): """Update existing network interface values.""" net_obj = dbapi.network_interfaces_update(context, id, request_data) return jsonutils.to_primitive(net_obj), 200, None - @base.http_codes def delete(self, context, id): """Delete existing network interface.""" dbapi.network_interfaces_delete(context, id) diff --git a/craton/api/v1/resources/inventory/regions.py b/craton/api/v1/resources/inventory/regions.py index 0172bbb..68e36de 100644 --- a/craton/api/v1/resources/inventory/regions.py +++ b/craton/api/v1/resources/inventory/regions.py @@ -13,7 +13,6 @@ LOG = log.getLogger(__name__) class Regions(base.Resource): - @base.http_codes @base.pagination_context def get(self, context, request_args, pagination_params): """Get region(s) for the project. Get region details if @@ -46,7 +45,6 @@ class Regions(base.Resource): response_body = {'regions': regions_obj, 'links': links} return jsonutils.to_primitive(response_body), 200, None - @base.http_codes def post(self, context, request_data): """Create a new region.""" json = util.copy_project_id_into_json(context, request_data) @@ -67,19 +65,16 @@ class Regions(base.Resource): class RegionsById(base.Resource): - @base.http_codes def get(self, context, id, request_args): region_obj = dbapi.regions_get_by_id(context, id) region = utils.get_resource_with_vars(request_args, region_obj) return region, 200, None - @base.http_codes def put(self, context, id, request_data): """Update existing region.""" region_obj = dbapi.regions_update(context, id, request_data) return jsonutils.to_primitive(region_obj), 200, None - @base.http_codes def delete(self, context, id): """Delete existing region.""" dbapi.regions_delete(context, id) diff --git a/craton/api/v1/resources/projects.py b/craton/api/v1/resources/projects.py index 921892e..9f2236b 100644 --- a/craton/api/v1/resources/projects.py +++ b/craton/api/v1/resources/projects.py @@ -12,7 +12,6 @@ LOG = log.getLogger(__name__) class Projects(base.Resource): - @base.http_codes @base.pagination_context def get(self, context, request_args, pagination_params): """Get all projects. Requires super admin privileges.""" @@ -35,7 +34,6 @@ class Projects(base.Resource): response_body = {'projects': projects_obj, 'links': links} return jsonutils.to_primitive(response_body), 200, None - @base.http_codes def post(self, context, request_data): """Create a new project. Requires super admin privileges.""" project_obj = dbapi.projects_create(context, request_data) @@ -56,7 +54,6 @@ class Projects(base.Resource): class ProjectById(base.Resource): - @base.http_codes def get(self, context, id): """Get a project details by id. Requires super admin privileges.""" project_obj = dbapi.projects_get_by_id(context, id) @@ -64,7 +61,6 @@ class ProjectById(base.Resource): project['variables'] = jsonutils.to_primitive(project_obj.variables) return project, 200, None - @base.http_codes def delete(self, context, id): """Delete existing project. Requires super admin privileges.""" dbapi.projects_delete(context, id) diff --git a/craton/api/v1/resources/users.py b/craton/api/v1/resources/users.py index 9b5fb90..4043b80 100644 --- a/craton/api/v1/resources/users.py +++ b/craton/api/v1/resources/users.py @@ -13,7 +13,6 @@ LOG = log.getLogger(__name__) class Users(base.Resource): - @base.http_codes @base.pagination_context def get(self, context, request_args, pagination_params): """Get all users. Requires project admin privileges.""" @@ -38,7 +37,6 @@ class Users(base.Resource): response_body = {'users': users_obj, 'links': links} return jsonutils.to_primitive(response_body), 200, None - @base.http_codes def post(self, context, request_data): """Create a new user. Requires project admin privileges.""" json = util.copy_project_id_into_json(context, request_data) @@ -58,13 +56,11 @@ class Users(base.Resource): class UserById(base.Resource): - @base.http_codes def get(self, context, id): """Get a user details by id. Requires project admin privileges.""" user_obj = dbapi.users_get_by_id(context, id) return jsonutils.to_primitive(user_obj), 200, None - @base.http_codes def delete(self, context, id): """Delete existing user. Requires project admin privileges.""" dbapi.users_delete(context, id) diff --git a/craton/api/v1/resources/variables.py b/craton/api/v1/resources/variables.py index 9c7276a..526e78c 100644 --- a/craton/api/v1/resources/variables.py +++ b/craton/api/v1/resources/variables.py @@ -14,7 +14,6 @@ LOG = log.getLogger(__name__) class Variables(base.Resource): - @base.http_codes def get(self, context, resources, id, request_args=None): """Get variables for given resource.""" obj = dbapi.resource_get_by_id(context, resources, id) @@ -22,7 +21,6 @@ class Variables(base.Resource): resp = {"variables": jsonutils.to_primitive(obj.vars)} return resp, 200, None - @base.http_codes def put(self, context, resources, id, request_data): """ Update existing resource variables, or create if it does @@ -34,7 +32,6 @@ class Variables(base.Resource): resp = {"variables": jsonutils.to_primitive(obj.variables)} return resp, 200, None - @base.http_codes def delete(self, context, resources, id, request_data): """Delete resource variables.""" # NOTE(sulo): this is not that great. Find a better way to do this. diff --git a/craton/api/v1/schemas.py b/craton/api/v1/schemas.py index 549feac..83bfcb8 100644 --- a/craton/api/v1/schemas.py +++ b/craton/api/v1/schemas.py @@ -1484,578 +1484,198 @@ filters = { }, }, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("devices", "GET"): { 200: { "headers": None, "schema": DefinitionDevicesPaginated, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("hosts_id", "GET"): { 200: { "headers": None, "schema": DefinitionsHostId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("hosts_id", "PUT"): { 200: { "headers": None, "schema": DefinitionsHostId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("hosts_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("hosts_labels", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("hosts_labels", "GET"): { 200: { "headers": None, "schema": DefinitionsLabel, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("hosts_labels", "PUT"): { 200: { "headers": None, "schema": DefinitionsLabel, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("hosts", "POST"): { 201: { "headers": None, "schema": DefinitionsHost, }, - 400: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("hosts", "GET"): { 200: { "headers": None, "schema": paginated_resource("hosts", DefinitionsHost), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("cells_id", "GET"): { 200: { "headers": None, "schema": DefinitionsCellId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("cells_id", "PUT"): { 200: { "headers": None, "schema": DefinitionsCellId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("cells_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("cells", "POST"): { 201: { "headers": None, "schema": DefinitionsCell, }, - 400: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("cells", "GET"): { 200: { "headers": None, "schema": paginated_resource("cells", DefinitionsCell), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("regions", "POST"): { 201: { "headers": None, "schema": DefinitionsRegion, }, - 400: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("regions", "GET"): { 200: { "headers": None, "schema": paginated_resource("regions", DefinitionsRegion), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("regions_id", "GET"): { 200: { "headers": None, "schema": DefinitionsRegionId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("regions_id", "PUT"): { 200: { "headers": None, "schema": DefinitionsRegionId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("regions_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("clouds", "POST"): { 201: { "headers": None, "schema": DefinitionsCloud, }, - 400: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("clouds", "GET"): { 200: { "headers": None, "schema": paginated_resource("clouds", DefinitionsCloud), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("clouds_id", "GET"): { 200: { "headers": None, "schema": DefinitionsCloudId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("clouds_id", "PUT"): { 200: { "headers": None, "schema": DefinitionsCloudId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("clouds_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("projects", "GET"): { 200: { "headers": None, "schema": paginated_resource("projects", DefinitionProject), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("projects", "POST"): { 201: { "headers": None, "schema": DefinitionProject, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("users", "GET"): { 200: { "headers": None, "schema": paginated_resource("users", DefinitionUser), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("users", "POST"): { 201: { "headers": None, "schema": DefinitionUser, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("projects_id", "GET"): { 200: { "headers": None, "schema": DefinitionProject, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("projects_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("users_id", "GET"): { 200: { "headers": None, "schema": DefinitionUser, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("users_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_devices", "GET"): { 200: { @@ -2063,234 +1683,78 @@ filters = { "schema": paginated_resource("network_devices", DefinitionNetworkDeviceId), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_devices", "POST"): { 201: { "headers": None, "schema": DefinitionNetworkDeviceId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_devices_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_devices_id", "GET"): { 200: { "headers": None, "schema": DefinitionNetworkDeviceId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_devices_labels", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_devices_labels", "GET"): { 200: { "headers": None, "schema": DefinitionsLabel, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_devices_labels", "PUT"): { 200: { "headers": None, "schema": DefinitionsLabel, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_devices_id", "PUT"): { 200: { "headers": None, "schema": DefinitionNetworkDeviceId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("networks", "GET"): { 200: { "headers": None, "schema": paginated_resource("networks", DefinitionNetwork), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("networks", "POST"): { 201: { "headers": None, "schema": DefinitionNetwork, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("networks_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("networks_id", "GET"): { 200: { "headers": None, "schema": DefinitionNetworkId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("networks_id", "PUT"): { 200: { "headers": None, "schema": DefinitionNetworkId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_interfaces", "GET"): { 200: { @@ -2298,90 +1762,30 @@ filters = { "schema": paginated_resource("network_interfaces", DefinitionNetworkInterface), }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_interfaces", "POST"): { 201: { "headers": None, "schema": DefinitionNetworkInterface, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_interfaces_id", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_interfaces_id", "GET"): { 200: { "headers": None, "schema": DefinitionNetworkInterfaceId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("network_interfaces_id", "PUT"): { 200: { "headers": None, "schema": DefinitionNetworkInterfaceId, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("variables_with_resolve", "GET"): { 200: { @@ -2394,18 +1798,6 @@ filters = { }, }, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("variables_with_resolve", "PUT"): { 200: { @@ -2418,54 +1810,18 @@ filters = { }, }, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("variables_with_resolve", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("variables_without_resolve", "DELETE"): { 204: { "headers": None, "schema": None, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("variables_without_resolve", "GET"): { 200: { @@ -2478,18 +1834,6 @@ filters = { }, }, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, ("variables_without_resolve", "PUT"): { 200: { @@ -2502,17 +1846,5 @@ filters = { }, }, }, - 400: { - "headers": None, - "schema": None, - }, - 404: { - "headers": None, - "schema": None, - }, - 405: { - "headers": None, - "schema": None, - }, }, } diff --git a/craton/api/v1/validators.py b/craton/api/v1/validators.py index 05f18ba..014105f 100644 --- a/craton/api/v1/validators.py +++ b/craton/api/v1/validators.py @@ -4,9 +4,7 @@ from functools import wraps from werkzeug.datastructures import MultiDict, Headers -from flask import request, current_app -from flask_restful import abort -from flask_restful.utils import unpack +from flask import request from jsonschema import Draft4Validator from oslo_log import log @@ -182,9 +180,12 @@ class FlaskValidatorAdaptor(object): def validate(self, value): value = self.type_convert(value) - errors = list(e.message for e in self.validator.iter_errors(value)) + errors = sorted(e.message for e in self.validator.iter_errors(value)) if errors: - abort(400, message='Bad Request', errors=errors) + msg = "The request included the following errors:\n- {}".format( + "\n- ".join(errors) + ) + raise exceptions.BadRequest(message=msg) return merge_default(self.validator.schema, value) @@ -233,9 +234,6 @@ def response_filter(view): def wrapper(*args, **kwargs): resp = view(*args, **kwargs) - if isinstance(resp, current_app.response_class): - return resp - endpoint = request.endpoint.partition('.')[-1] method = request.method if method == 'HEAD': @@ -248,12 +246,9 @@ def response_filter(view): 'filters.', {"endpoint": endpoint, "method": method} ) - abort(500) + raise exceptions.UnknownException - headers = None - status = None - if isinstance(resp, tuple): - resp, status, headers = unpack(resp) + body, status, headers = resp try: schemas = resp_filter[status] @@ -263,17 +258,18 @@ def response_filter(view): 'filter "(%(endpoint)s, %(method)s)".', {"status": status, "endpoint": endpoint, "method": method} ) - abort(500) + raise exceptions.UnknownException - resp, errors = normalize(schemas['schema'], resp) + body, errors = normalize(schemas['schema'], body) if schemas['headers']: headers, header_errors = normalize( {'properties': schemas['headers']}, headers) errors.extend(header_errors) if errors: - abort(500, message='Expectation Failed', errors=errors) + LOG.error('Expectation Failed: %s', errors) + raise exceptions.UnknownException - return resp, status, headers + return body, status, headers return wrapper diff --git a/craton/exceptions.py b/craton/exceptions.py index c6128b8..b26d73f 100644 --- a/craton/exceptions.py +++ b/craton/exceptions.py @@ -65,6 +65,11 @@ class DeviceNotFound(Base): msg = "%(device_type)s device not found for ID %(id)s." +class AuthenticationError(Base): + code = 401 + msg = "The request could not be authenticated." + + class AdminRequired(Base): code = 401 msg = "This action requires the 'admin' role" diff --git a/craton/tests/functional/__init__.py b/craton/tests/functional/__init__.py index 8aad04a..cc2d462 100644 --- a/craton/tests/functional/__init__.py +++ b/craton/tests/functional/__init__.py @@ -238,11 +238,19 @@ class TestCase(testtools.TestCase): response.text ) + def assertFailureFormat(self, response): + if response.status_code >= 400: + body = response.json() + self.assertEqual(2, len(body)) + self.assertEqual(response.status_code, body["status"]) + self.assertIn("message", body) + def get(self, url, headers=None, **params): resp = self.session.get( url, verify=False, headers=headers, params=params, ) self.assertJSON(resp) + self.assertFailureFormat(resp) return resp def post(self, url, headers=None, data=None): @@ -250,6 +258,7 @@ class TestCase(testtools.TestCase): url, verify=False, headers=headers, json=data, ) self.assertJSON(resp) + self.assertFailureFormat(resp) return resp def put(self, url, headers=None, data=None): @@ -257,6 +266,7 @@ class TestCase(testtools.TestCase): url, verify=False, headers=headers, json=data, ) self.assertJSON(resp) + self.assertFailureFormat(resp) return resp def delete(self, url, headers=None, body=None): @@ -264,6 +274,7 @@ class TestCase(testtools.TestCase): url, verify=False, headers=headers, json=body, ) self.assertJSON(resp) + self.assertFailureFormat(resp) return resp def create_cloud(self, name, variables=None): diff --git a/craton/tests/functional/test_cell_calls.py b/craton/tests/functional/test_cell_calls.py index 2367904..2f760b7 100644 --- a/craton/tests/functional/test_cell_calls.py +++ b/craton/tests/functional/test_cell_calls.py @@ -64,8 +64,11 @@ class APIV1CellTest(APIV1ResourceWithVariablesTestCase): 'cloud_id': self.cloud['id'], 'name': 'a', 'id': 3} cell = self.post(url, data=payload) self.assertEqual(400, cell.status_code) - msg = ["Additional properties are not allowed ('id' was unexpected)"] - self.assertEqual(cell.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed ('id' was unexpected)" + ) + self.assertEqual(cell.json()['message'], msg) def test_cell_create_with_extra_created_at_property_fails(self): url = self.url + '/v1/cells' @@ -74,9 +77,12 @@ class APIV1CellTest(APIV1ResourceWithVariablesTestCase): 'created_at': "some date"} cell = self.post(url, data=payload) self.assertEqual(400, cell.status_code) - msg = ["Additional properties are not allowed " - "('created_at' was unexpected)"] - self.assertEqual(cell.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed " + "('created_at' was unexpected)" + ) + self.assertEqual(cell.json()['message'], msg) def test_cell_create_with_extra_updated_at_property_fails(self): url = self.url + '/v1/cells' @@ -85,9 +91,24 @@ class APIV1CellTest(APIV1ResourceWithVariablesTestCase): 'updated_at': "some date"} cell = self.post(url, data=payload) self.assertEqual(400, cell.status_code) - msg = ["Additional properties are not allowed " - "('updated_at' was unexpected)"] - self.assertEqual(cell.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed " + "('updated_at' was unexpected)" + ) + self.assertEqual(cell.json()['message'], msg) + + def test_cell_create_missing_all_properties_fails(self): + url = self.url + '/v1/cells' + cell = self.post(url, data={}) + self.assertEqual(400, cell.status_code) + msg = ( + "The request included the following errors:\n" + "- 'cloud_id' is a required property\n" + "- 'name' is a required property\n" + "- 'region_id' is a required property" + ) + self.assertEqual(cell.json()['message'], msg) def test_cells_get_all_with_details(self): self.create_cell('cell1', variables={'a': 'b'}) diff --git a/craton/tests/functional/test_cloud_calls.py b/craton/tests/functional/test_cloud_calls.py index 60e3b3a..7626b5e 100644 --- a/craton/tests/functional/test_cloud_calls.py +++ b/craton/tests/functional/test_cloud_calls.py @@ -40,8 +40,11 @@ class APIV1CloudTest(TestCase): url = self.url + '/v1/clouds' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - err_msg = ["'name' is a required property"] - self.assertEqual(resp.json()['errors'], err_msg) + err_msg = ( + "The request included the following errors:\n" + "- 'name' is a required property" + ) + self.assertEqual(resp.json()['message'], err_msg) def test_create_cloud_with_duplicate_name_fails(self): self.create_cloud("ORD135") @@ -56,26 +59,45 @@ class APIV1CloudTest(TestCase): url = self.url + '/v1/clouds' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - msg = ["Additional properties are not allowed ('id' was unexpected)"] - self.assertEqual(resp.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed ('id' was unexpected)" + ) + self.assertEqual(resp.json()['message'], msg) def test_create_region_with_extra_created_at_property_fails(self): values = {"name": "test", "created_at": "some date"} url = self.url + '/v1/clouds' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - msg = ["Additional properties are not allowed " - "('created_at' was unexpected)"] - self.assertEqual(resp.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed " + "('created_at' was unexpected)" + ) + self.assertEqual(resp.json()['message'], msg) def test_create_region_with_extra_updated_at_property_fails(self): values = {"name": "test", "updated_at": "some date"} url = self.url + '/v1/clouds' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - msg = ["Additional properties are not allowed " - "('updated_at' was unexpected)"] - self.assertEqual(resp.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed " + "('updated_at' was unexpected)" + ) + self.assertEqual(resp.json()['message'], msg) + + def test_cloud_create_missing_all_properties_fails(self): + url = self.url + '/v1/clouds' + cloud = self.post(url, data={}) + self.assertEqual(400, cloud.status_code) + msg = ( + "The request included the following errors:\n" + "- 'name' is a required property" + ) + self.assertEqual(cloud.json()['message'], msg) def test_clouds_get_all(self): self.create_cloud("ORD1") diff --git a/craton/tests/functional/test_host_calls.py b/craton/tests/functional/test_host_calls.py index a380025..47abac8 100644 --- a/craton/tests/functional/test_host_calls.py +++ b/craton/tests/functional/test_host_calls.py @@ -69,8 +69,11 @@ class APIV1HostTest(DeviceTestBase, APIV1ResourceWithVariablesTestCase): 'cloud_id': self.cloud['id'], 'name': 'a', 'id': 1} host = self.post(url, data=payload) self.assertEqual(400, host.status_code) - msg = ["Additional properties are not allowed ('id' was unexpected)"] - self.assertEqual(host.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed ('id' was unexpected)" + ) + self.assertEqual(host.json()['message'], msg) def test_create_with_extra_created_at_property_fails(self): url = self.url + '/v1/hosts' @@ -80,9 +83,12 @@ class APIV1HostTest(DeviceTestBase, APIV1ResourceWithVariablesTestCase): 'created_at': 'some date'} host = self.post(url, data=payload) self.assertEqual(400, host.status_code) - msg = ["Additional properties are not allowed " - "('created_at' was unexpected)"] - self.assertEqual(host.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed " + "('created_at' was unexpected)" + ) + self.assertEqual(host.json()['message'], msg) def test_create_with_extra_updated_at_property_fails(self): url = self.url + '/v1/hosts' @@ -92,9 +98,26 @@ class APIV1HostTest(DeviceTestBase, APIV1ResourceWithVariablesTestCase): 'updated_at': 'some date'} host = self.post(url, data=payload) self.assertEqual(400, host.status_code) - msg = ["Additional properties are not allowed " - "('updated_at' was unexpected)"] - self.assertEqual(host.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed " + "('updated_at' was unexpected)" + ) + self.assertEqual(host.json()['message'], msg) + + def test_create_missing_all_properties_fails(self): + url = self.url + '/v1/hosts' + host = self.post(url, data={}) + self.assertEqual(400, host.status_code) + msg = ( + "The request included the following errors:\n" + "- 'cloud_id' is a required property\n" + "- 'device_type' is a required property\n" + "- 'ip_address' is a required property\n" + "- 'name' is a required property\n" + "- 'region_id' is a required property" + ) + self.assertEqual(host.json()['message'], msg) def test_create_with_parent_id(self): parent = self.create_host( diff --git a/craton/tests/functional/test_network_calls.py b/craton/tests/functional/test_network_calls.py index 3f0104d..05f2d13 100644 --- a/craton/tests/functional/test_network_calls.py +++ b/craton/tests/functional/test_network_calls.py @@ -42,8 +42,11 @@ class APIV1NetworkSchemaTest(TestCase): } network = self.post(self.networks_url, data=payload) self.assertEqual(400, network.status_code) - msg = ["'region_id' is a required property"] - self.assertEqual(network.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- 'region_id' is a required property" + ) + self.assertEqual(network.json()['message'], msg) def test_network_create_without_cloud_id_fails(self): payload = { @@ -55,8 +58,11 @@ class APIV1NetworkSchemaTest(TestCase): } network = self.post(self.networks_url, data=payload) self.assertEqual(400, network.status_code) - msg = ["'cloud_id' is a required property"] - self.assertEqual(network.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- 'cloud_id' is a required property" + ) + self.assertEqual(network.json()['message'], msg) def test_network_create_with_extra_id_property_fails(self): payload = { @@ -70,8 +76,11 @@ class APIV1NetworkSchemaTest(TestCase): } network = self.post(self.networks_url, data=payload) self.assertEqual(400, network.status_code) - msg = ["Additional properties are not allowed ('id' was unexpected)"] - self.assertEqual(network.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed ('id' was unexpected)" + ) + self.assertEqual(network.json()['message'], msg) def test_network_create_with_extra_created_at_property_fails(self): payload = { @@ -85,9 +94,12 @@ class APIV1NetworkSchemaTest(TestCase): } network = self.post(self.networks_url, data=payload) self.assertEqual(400, network.status_code) - msg = ["Additional properties are not allowed ('created_at' was " - "unexpected)"] - self.assertEqual(network.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed ('created_at' was " + "unexpected)" + ) + self.assertEqual(network.json()['message'], msg) def test_network_create_with_extra_updated_at_property_fails(self): payload = { @@ -101,9 +113,27 @@ class APIV1NetworkSchemaTest(TestCase): } network = self.post(self.networks_url, data=payload) self.assertEqual(400, network.status_code) - msg = ["Additional properties are not allowed ('updated_at' was " - "unexpected)"] - self.assertEqual(network.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed ('updated_at' was " + "unexpected)" + ) + self.assertEqual(network.json()['message'], msg) + + def test_network_create_missing_all_properties_fails(self): + url = self.url + '/v1/networks' + network = self.post(url, data={}) + self.assertEqual(400, network.status_code) + msg = ( + "The request included the following errors:\n" + "- 'cidr' is a required property\n" + "- 'cloud_id' is a required property\n" + "- 'gateway' is a required property\n" + "- 'name' is a required property\n" + "- 'netmask' is a required property\n" + "- 'region_id' is a required property" + ) + self.assertEqual(network.json()['message'], msg) def test_network_get_all_with_details(self): payload = { diff --git a/craton/tests/functional/test_network_device_calls.py b/craton/tests/functional/test_network_device_calls.py index e0b7980..f5363b2 100644 --- a/craton/tests/functional/test_network_device_calls.py +++ b/craton/tests/functional/test_network_device_calls.py @@ -99,3 +99,17 @@ class APIV1NetworkDeviceTest(DeviceTestBase): url, data={'parent_id': grandchild['id']} ) self.assertEqual(400, parent_update_resp.status_code) + + def test_network_device_create_missing_all_properties_fails(self): + url = self.url + '/v1/network-devices' + network_device = self.post(url, data={}) + self.assertEqual(400, network_device.status_code) + msg = ( + "The request included the following errors:\n" + "- 'cloud_id' is a required property\n" + "- 'device_type' is a required property\n" + "- 'ip_address' is a required property\n" + "- 'name' is a required property\n" + "- 'region_id' is a required property" + ) + self.assertEqual(network_device.json()['message'], msg) diff --git a/craton/tests/functional/test_network_interface_calls.py b/craton/tests/functional/test_network_interface_calls.py index 65cac55..01e4134 100644 --- a/craton/tests/functional/test_network_interface_calls.py +++ b/craton/tests/functional/test_network_interface_calls.py @@ -55,3 +55,16 @@ class APIv1NetworkInterfacesTest(functional.DeviceTestBase): payload = {'port': 'asdf'} response = self.put(url, data=payload) self.assertBadRequest(response) + + def test_network_interface_create_missing_all_properties_fails(self): + url = self.url + '/v1/network-interfaces' + network_interface = self.post(url, data={}) + self.assertEqual(400, network_interface.status_code) + msg = ( + "The request included the following errors:\n" + "- 'device_id' is a required property\n" + "- 'interface_type' is a required property\n" + "- 'ip_address' is a required property\n" + "- 'name' is a required property" + ) + self.assertEqual(network_interface.json()['message'], msg) diff --git a/craton/tests/functional/test_project_calls.py b/craton/tests/functional/test_project_calls.py index 373cd2e..5d6ee3a 100644 --- a/craton/tests/functional/test_project_calls.py +++ b/craton/tests/functional/test_project_calls.py @@ -152,3 +152,13 @@ class APIV1ProjectTest(ProjectTests, APIV1ResourceWithVariablesTestCase): project = self.create_project(project_name, variables=variables) self.assert_vars_get_expected(project['id'], variables) self.assert_vars_can_be_deleted(project['id']) + + def test_project_create_missing_all_properties_fails(self): + url = self.url + '/v1/projects' + project = self.post(url, data={}) + self.assertEqual(400, project.status_code) + msg = ( + "The request included the following errors:\n" + "- 'name' is a required property" + ) + self.assertEqual(project.json()['message'], msg) diff --git a/craton/tests/functional/test_region_calls.py b/craton/tests/functional/test_region_calls.py index bbaaf85..76bc58a 100644 --- a/craton/tests/functional/test_region_calls.py +++ b/craton/tests/functional/test_region_calls.py @@ -63,16 +63,22 @@ class APIV1RegionTest(RegionTests): url = self.url + '/v1/regions' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - err_msg = ["'name' is a required property"] - self.assertEqual(resp.json()['errors'], err_msg) + err_msg = ( + "The request included the following errors:\n" + "- 'name' is a required property" + ) + self.assertEqual(resp.json()['message'], err_msg) def test_create_region_with_no_cloud_id_fails(self): values = {"name": "I don't work at all, you know."} url = self.url + '/v1/regions' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - err_msg = ["'cloud_id' is a required property"] - self.assertEqual(resp.json()['errors'], err_msg) + err_msg = ( + "The request included the following errors:\n" + "- 'cloud_id' is a required property" + ) + self.assertEqual(resp.json()['message'], err_msg) def test_create_region_with_duplicate_name_fails(self): self.create_region("ORD135") @@ -87,8 +93,11 @@ class APIV1RegionTest(RegionTests): url = self.url + '/v1/regions' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - msg = ["Additional properties are not allowed ('id' was unexpected)"] - self.assertEqual(resp.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed ('id' was unexpected)" + ) + self.assertEqual(resp.json()['message'], msg) def test_create_region_with_extra_created_at_property_fails(self): values = {"name": "test", 'cloud_id': self.cloud['id'], @@ -96,9 +105,12 @@ class APIV1RegionTest(RegionTests): url = self.url + '/v1/regions' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - msg = ["Additional properties are not allowed " - "('created_at' was unexpected)"] - self.assertEqual(resp.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed " + "('created_at' was unexpected)" + ) + self.assertEqual(resp.json()['message'], msg) def test_create_region_with_extra_updated_at_property_fails(self): values = {"name": "test", 'cloud_id': self.cloud['id'], @@ -106,9 +118,23 @@ class APIV1RegionTest(RegionTests): url = self.url + '/v1/regions' resp = self.post(url, data=values) self.assertEqual(resp.status_code, 400) - msg = ["Additional properties are not allowed " - "('updated_at' was unexpected)"] - self.assertEqual(resp.json()['errors'], msg) + msg = ( + "The request included the following errors:\n" + "- Additional properties are not allowed " + "('updated_at' was unexpected)" + ) + self.assertEqual(resp.json()['message'], msg) + + def test_region_create_missing_all_properties_fails(self): + url = self.url + '/v1/regions' + region = self.post(url, data={}) + self.assertEqual(400, region.status_code) + msg = ( + "The request included the following errors:\n" + "- 'cloud_id' is a required property\n" + "- 'name' is a required property" + ) + self.assertEqual(region.json()['message'], msg) def test_regions_get_all(self): self.create_region("ORD1") diff --git a/craton/util.py b/craton/util.py index a5558a9..76052bf 100644 --- a/craton/util.py +++ b/craton/util.py @@ -1,4 +1,14 @@ """Module containing generic utilies for Craton.""" +from datetime import date +from decorator import decorator +from flask import json, Response +import werkzeug.exceptions + +from oslo_log import log + +import craton.exceptions as exceptions + +LOG = log.getLogger(__name__) def copy_project_id_into_json(context, json, project_id_key='project_id'): @@ -16,3 +26,57 @@ def copy_project_id_into_json(context, json, project_id_key='project_id'): """ json[project_id_key] = getattr(context, 'tenant', '') return json + + +class JSONEncoder(json.JSONEncoder): + + def default(self, o): + if isinstance(o, date): + return o.isoformat() + return json.JSONEncoder.default(self, o) + + +JSON_KWARGS = { + "indent": 2, + "sort_keys": True, + "cls": JSONEncoder, + "separators": (",", ": "), +} + + +def handle_all_exceptions(e): + """Generate error Flask response object from exception.""" + headers = [("Content-Type", "application/json")] + if isinstance(e, exceptions.Base): + message = e.message + status = e.code + elif isinstance(e, werkzeug.exceptions.HTTPException): + message = e.description + status = e.code + # Werkzeug exceptions can include additional headers, those should be + # kept unless the header is "Content-Type" which is set by this + # function. + headers.extend( + h for h in e.get_headers(None) if h[0].lower() != "content-type" + ) + else: + LOG.exception(e) + e_ = exceptions.UnknownException + message = e_.message + status = e_.code + + body = { + "message": message, + "status": status, + } + + body_ = "{}\n".format(json.dumps(body, **JSON_KWARGS)) + return Response(body_, status, headers) + + +@decorator +def handle_all_exceptions_decorator(fn, *args, **kwargs): + try: + return fn(*args, **kwargs) + except Exception as e: + return handle_all_exceptions(e)