From 800d3abf00ae98c87af372761acfc10f63f625c1 Mon Sep 17 00:00:00 2001 From: Haiwei Xu Date: Wed, 15 Jan 2014 04:15:06 +0900 Subject: [PATCH] Add API schema for v2.1/v3 hosts API By defining the API schema, it is possible to separate the validation code from the API method. The API method can be more simple. In addition, a response of API validation error can be consistent for whole of Nova APIs. Partially implements blueprint v3-api-schema Change-Id: Ie252dce67f8e9d3292d8dad767b6113f7affcffd --- .../api/openstack/compute/plugins/v3/hosts.py | 40 +++++------------ .../api/openstack/compute/schemas/v3/hosts.py | 43 ++++++++++++++++++ .../compute/plugins/v3/test_hosts.py | 44 ++++++++++++------- 3 files changed, 81 insertions(+), 46 deletions(-) create mode 100644 nova/api/openstack/compute/schemas/v3/hosts.py diff --git a/nova/api/openstack/compute/plugins/v3/hosts.py b/nova/api/openstack/compute/plugins/v3/hosts.py index 5087b0171e7..cea2804290c 100644 --- a/nova/api/openstack/compute/plugins/v3/hosts.py +++ b/nova/api/openstack/compute/plugins/v3/hosts.py @@ -17,8 +17,10 @@ import webob.exc +from nova.api.openstack.compute.schemas.v3 import hosts from nova.api.openstack import extensions from nova.api.openstack import wsgi +from nova.api import validation from nova import compute from nova import exception from nova.openstack.common.gettextutils import _ @@ -92,49 +94,29 @@ def index(self, req): return {'hosts': hosts} @extensions.expected_errors((400, 404, 501)) + @validation.schema(hosts.update) def update(self, req, id, body): """:param body: example format {'host': {'status': 'enable', 'maintenance_mode': 'enable'}} :returns: """ - def read_enabled(orig_val, msg): + def read_enabled(orig_val): """:param orig_val: A string with either 'enable' or 'disable'. May be surrounded by whitespace, and case doesn't matter - :param msg: The message to be passed to HTTPBadRequest. A single - %s will be replaced with orig_val. :returns: True for 'enabled' and False for 'disabled' """ val = orig_val.strip().lower() - if val == "enable": - return True - elif val == "disable": - return False - else: - raise webob.exc.HTTPBadRequest(explanation=msg % orig_val) + return val == "enable" context = req.environ['nova.context'] authorize(context) # See what the user wants to 'update' - if not self.is_valid_body(body, 'host'): - raise webob.exc.HTTPBadRequest( - explanation=_("The request body invalid")) - params = dict([(k.strip().lower(), v) - for k, v in body['host'].iteritems()]) - orig_status = status = params.pop('status', None) - orig_maint_mode = maint_mode = params.pop('maintenance_mode', None) - # Validate the request - if len(params) > 0: - # Some extra param was passed. Fail. - explanation = _("Invalid update setting: '%s'") % params.keys()[0] - raise webob.exc.HTTPBadRequest(explanation=explanation) - if orig_status is not None: - status = read_enabled(orig_status, _("Invalid status: '%s'")) - if orig_maint_mode is not None: - maint_mode = read_enabled(orig_maint_mode, _("Invalid mode: '%s'")) - if status is None and maint_mode is None: - explanation = _("'status' or 'maintenance_mode' needed for " - "host update") - raise webob.exc.HTTPBadRequest(explanation=explanation) + status = body['host'].get('status') + maint_mode = body['host'].get('maintenance_mode') + if status is not None: + status = read_enabled(status) + if maint_mode is not None: + maint_mode = read_enabled(maint_mode) # Make the calls and merge the results result = {'host': id} if status is not None: diff --git a/nova/api/openstack/compute/schemas/v3/hosts.py b/nova/api/openstack/compute/schemas/v3/hosts.py new file mode 100644 index 00000000000..30ec09f40a1 --- /dev/null +++ b/nova/api/openstack/compute/schemas/v3/hosts.py @@ -0,0 +1,43 @@ +# Copyright 2014 NEC Corporation. All rights reserved. +# +# 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. + +update = { + 'type': 'object', + 'properties': { + 'host': { + 'type': 'object', + 'properties': { + 'status': { + 'type': 'string', + 'enum': ['enable', 'disable', + 'Enable', 'Disable', + 'ENABLE', 'DISABLE'], + }, + 'maintenance_mode': { + 'type': 'string', + 'enum': ['enable', 'disable', + 'Enable', 'Disable', + 'ENABLE', 'DISABLE'], + }, + }, + 'anyOf': [ + {'required': ['status']}, + {'required': ['maintenance_mode']} + ], + 'additionalProperties': False, + }, + }, + 'required': ['host'], + 'additionalProperties': False, +} diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_hosts.py b/nova/tests/api/openstack/compute/plugins/v3/test_hosts.py index 2029ae9002a..d57f5361163 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_hosts.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_hosts.py @@ -167,7 +167,7 @@ def setUp(self): def _test_host_update(self, host, key, val, expected_value): body = {'host': {key: val}} - result = self.controller.update(self.req, host, body) + result = self.controller.update(self.req, host, body=body) self.assertEqual(result['host'][key], expected_value) def test_list_hosts(self): @@ -211,7 +211,7 @@ def _test_host_update_service_unavailable(self, key, val): body = {'host': {key: val}} host = "serviceunavailable" self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, host, body) + self.req, host, body=body) def test_enable_host_service_unavailable(self): self._test_host_update_service_unavailable('status', 'enable') @@ -307,45 +307,55 @@ def test_host_power_action_bad_host(self): def test_bad_status_value(self): bad_body = {"host": {"status": "bad"}} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", bad_body) + self.assertRaises(exception.ValidationError, self.controller.update, + self.req, "host", body=bad_body) bad_body2 = {"host": {"status": "disablabc"}} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", bad_body2) + self.assertRaises(exception.ValidationError, self.controller.update, + self.req, "host", body=bad_body2) def test_bad_update_key(self): bad_body = {"host": {"crazy": "bad"}} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", bad_body) + self.assertRaises(exception.ValidationError, self.controller.update, + self.req, "host", body=bad_body) def test_bad_update_key_type(self): bad_body = {"host": "abc"} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", bad_body) + self.assertRaises(exception.ValidationError, self.controller.update, + self.req, "host", body=bad_body) bad_body = {"host": None} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", bad_body) + self.assertRaises(exception.ValidationError, self.controller.update, + self.req, "host", body=bad_body) def test_bad_update_empty(self): bad_body = {} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", bad_body) + self.assertRaises(exception.ValidationError, self.controller.update, + self.req, "host", body=bad_body) def test_bad_update_key_and_correct_update_key(self): bad_body = {"host": {"status": "disable", "crazy": "bad"}} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", bad_body) + self.assertRaises(exception.ValidationError, self.controller.update, + self.req, "host", body=bad_body) def test_good_update_keys(self): body = {"host": {"status": "disable", "maintenance_mode": "enable"}} - result = self.controller.update(self.req, 'host_c1', body) + result = self.controller.update(self.req, 'host_c1', body=body) self.assertEqual(result["host"]["host"], "host_c1") self.assertEqual(result["host"]["status"], "disabled") self.assertEqual(result["host"]["maintenance_mode"], "on_maintenance") + def test_update_with_status_key_only(self): + body = {"host": {"status": "enable"}} + result = self.controller.update(self.req, 'host_c1', body=body) + self.assertEqual("enabled", result["host"]["status"]) + + def test_update_with_maintenance_mode_key_only(self): + body = {"host": {"maintenance_mode": "enable"}} + result = self.controller.update(self.req, 'host_c1', body=body) + self.assertEqual("on_maintenance", result["host"]["maintenance_mode"]) + def test_show_forbidden(self): self.req.environ["nova.context"].is_admin = False dest = 'dummydest'