womax/nova
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Validate id as integer for os-aggregates"

Browse Source
This commit is contained in:
Zuul
2020-03-09 18:17:44 +00:00
committed by Gerrit Code Review
parent abd1f05a0b 2e70a1717f
commit fa16a330f5
2 changed files with 98 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nova/api/openstack/compute/aggregates.py
+43 -3
View File
@@ -32,6 +32,7 @@ from nova.conductor import api as conductor
from nova import exception
from nova.i18n import _
from nova.policies import aggregates as aggr_policies
from nova import utils
LOG = logging.getLogger(__name__)
@@ -92,11 +93,17 @@ class AggregateController(wsgi.Controller):
return agg
@wsgi.expected_errors(404)
@wsgi.expected_errors((400, 404))
def show(self, req, id):
"""Shows the details of an aggregate, hosts and metadata included."""
context = _get_context(req)
context.can(aggr_policies.POLICY_ROOT % 'show')
try:
utils.validate_integer(id, 'id')
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
try:
aggregate = self.api.get_aggregate(context, id)
except exception.AggregateNotFound as e:
@@ -114,6 +121,11 @@ class AggregateController(wsgi.Controller):
if 'name' in updates:
updates['name'] = common.normalize_name(updates['name'])
try:
utils.validate_integer(id, 'id')
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
try:
aggregate = self.api.update_aggregate(context, id, updates)
except exception.AggregateNameExists as e:
@@ -133,6 +145,12 @@ class AggregateController(wsgi.Controller):
"""Removes an aggregate by id."""
context = _get_context(req)
context.can(aggr_policies.POLICY_ROOT % 'delete')
try:
utils.validate_integer(id, 'id')
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
try:
self.api.delete_aggregate(context, id)
except exception.AggregateNotFound as e:
@@ -143,7 +161,7 @@ class AggregateController(wsgi.Controller):
# NOTE(gmann): Returns 200 for backwards compatibility but should be 202
# for representing async API as this API just accepts the request and
# request hypervisor driver to complete the same in async mode.
@wsgi.expected_errors((404, 409))
@wsgi.expected_errors((400, 404, 409))
@wsgi.action('add_host')
@validation.schema(aggregates.add_host)
def _add_host(self, req, id, body):
@@ -152,6 +170,12 @@ class AggregateController(wsgi.Controller):
context = _get_context(req)
context.can(aggr_policies.POLICY_ROOT % 'add_host')
try:
utils.validate_integer(id, 'id')
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
try:
aggregate = self.api.add_host_to_aggregate(context, id, host)
except (exception.AggregateNotFound,
@@ -166,7 +190,7 @@ class AggregateController(wsgi.Controller):
# NOTE(gmann): Returns 200 for backwards compatibility but should be 202
# for representing async API as this API just accepts the request and
# request hypervisor driver to complete the same in async mode.
@wsgi.expected_errors((404, 409))
@wsgi.expected_errors((400, 404, 409))
@wsgi.action('remove_host')
@validation.schema(aggregates.remove_host)
def _remove_host(self, req, id, body):
@@ -175,6 +199,12 @@ class AggregateController(wsgi.Controller):
context = _get_context(req)
context.can(aggr_policies.POLICY_ROOT % 'remove_host')
try:
utils.validate_integer(id, 'id')
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
try:
aggregate = self.api.remove_host_from_aggregate(context, id, host)
except (exception.AggregateNotFound,
@@ -202,6 +232,11 @@ class AggregateController(wsgi.Controller):
context = _get_context(req)
context.can(aggr_policies.POLICY_ROOT % 'set_metadata')
try:
utils.validate_integer(id, 'id')
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
metadata = body["set_metadata"]["metadata"]
try:
aggregate = self.api.update_aggregate_metadata(context,
@@ -245,6 +280,11 @@ class AggregateController(wsgi.Controller):
context = _get_context(req)
context.can(aggr_policies.NEW_POLICY_ROOT % 'images')
try:
utils.validate_integer(id, 'id')
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
image_ids = []
for image_req in body.get('cache'):
image_ids.append(image_req['id'])
nova/tests/unit/api/openstack/compute/test_aggregates.py
+55 -18
View File
@@ -299,7 +299,7 @@ class AggregateTestCaseV21(test.NoDBTestCase):
self.controller.show,
self.user_req, "1")
def test_show_with_invalid_id(self):
def test_show_with_bad_aggregate(self):
side_effect = exception.AggregateNotFound(aggregate_id='2')
with mock.patch.object(self.controller.api, 'get_aggregate',
side_effect=side_effect) as mock_get:
@@ -307,6 +307,10 @@ class AggregateTestCaseV21(test.NoDBTestCase):
self.req, "2")
mock_get.assert_called_once_with(self.context, '2')
def test_show_with_invalid_id(self):
self.assertRaises(exc.HTTPBadRequest, self.controller.show,
self.req, 'foo')
def test_update(self):
body = {"aggregate": {"name": "new_name",
"availability_zone": "nova1"}}
@@ -390,10 +394,11 @@ class AggregateTestCaseV21(test.NoDBTestCase):
@mock.patch('nova.compute.api.AggregateAPI.update_aggregate')
def test_update_with_none_availability_zone(self, mock_update_agg):
agg_id = uuidsentinel.aggregate
agg_id = 173
mock_update_agg.return_value = objects.Aggregate(self.context,
name='test',
uuid=agg_id,
uuid=uuidsentinel.agg,
id=agg_id,
hosts=[],
metadata={})
body = {"aggregate": {"name": "test",
@@ -414,6 +419,11 @@ class AggregateTestCaseV21(test.NoDBTestCase):
mock_update.assert_called_once_with(self.context, '2',
body["aggregate"])
def test_update_with_invalid_id(self):
body = {"aggregate": {"name": "test_name"}}
self.assertRaises(exc.HTTPBadRequest, self.controller.update,
self.req, 'foo', body=body)
def test_update_with_duplicated_name(self):
body = {"aggregate": {"name": "test_name"}}
side_effect = exception.AggregateNameExists(aggregate_name="test_name")
@@ -433,7 +443,7 @@ class AggregateTestCaseV21(test.NoDBTestCase):
def test_update_with_invalid_action(self):
with mock.patch.object(self.controller.api, "update_aggregate",
side_effect=exception.InvalidAggregateAction(
action='invalid', aggregate_id='agg1', reason= "not empty")):
action='invalid', aggregate_id='1', reason= "not empty")):
body = {"aggregate": {"availability_zone": "nova"}}
self.assertRaises(exc.HTTPBadRequest, self.controller.update,
self.req, "1", body=body)
@@ -467,15 +477,20 @@ class AggregateTestCaseV21(test.NoDBTestCase):
def test_add_host_with_bad_aggregate(self):
side_effect = exception.AggregateNotFound(
aggregate_id="bogus_aggregate")
aggregate_id="2")
with mock.patch.object(self.controller.api, 'add_host_to_aggregate',
side_effect=side_effect) as mock_add:
self.assertRaises(exc.HTTPNotFound, eval(self.add_host),
self.req, "bogus_aggregate",
self.req, "2",
body={"add_host": {"host": "host1"}})
mock_add.assert_called_once_with(self.context, "bogus_aggregate",
mock_add.assert_called_once_with(self.context, "2",
"host1")
def test_add_host_with_invalid_id(self):
body = {"add_host": {"host": "host1"}}
self.assertRaises(exc.HTTPBadRequest, eval(self.add_host),
self.req, 'foo', body=body)
def test_add_host_with_bad_host(self):
side_effect = exception.ComputeHostNotFound(host="bogus_host")
with mock.patch.object(self.controller.api, 'add_host_to_aggregate',
@@ -534,16 +549,21 @@ class AggregateTestCaseV21(test.NoDBTestCase):
def test_remove_host_with_bad_aggregate(self):
side_effect = exception.AggregateNotFound(
aggregate_id="bogus_aggregate")
aggregate_id="2")
with mock.patch.object(self.controller.api,
'remove_host_from_aggregate',
side_effect=side_effect) as mock_rem:
self.assertRaises(exc.HTTPNotFound, eval(self.remove_host),
self.req, "bogus_aggregate",
self.req, "2",
body={"remove_host": {"host": "host1"}})
mock_rem.assert_called_once_with(self.context, "bogus_aggregate",
mock_rem.assert_called_once_with(self.context, "2",
"host1")
def test_remove_host_with_invalid_id(self):
body = {"remove_host": {"host": "host1"}}
self.assertRaises(exc.HTTPBadRequest, eval(self.remove_host),
self.req, 'foo', body=body)
def test_remove_host_with_host_not_in_aggregate(self):
side_effect = exception.AggregateHostNotFound(aggregate_id="1",
host="host1")
@@ -639,16 +659,21 @@ class AggregateTestCaseV21(test.NoDBTestCase):
def test_set_metadata_with_bad_aggregate(self):
body = {"set_metadata": {"metadata": {"foo": "bar"}}}
side_effect = exception.AggregateNotFound(aggregate_id="bad_aggregate")
side_effect = exception.AggregateNotFound(aggregate_id="2")
with mock.patch.object(self.controller.api,
'update_aggregate_metadata',
side_effect=side_effect) as mock_update:
self.assertRaises(exc.HTTPNotFound, eval(self.set_metadata),
self.req, "bad_aggregate", body=body)
mock_update.assert_called_once_with(self.context, "bad_aggregate",
self.req, "2", body=body)
mock_update.assert_called_once_with(self.context, "2",
body["set_metadata"]['metadata'])
def test_set_metadata_with_invalid_id(self):
body = {"set_metadata": {"metadata": {"foo": "bar"}}}
self.assertRaises(exc.HTTPBadRequest, eval(self.set_metadata),
self.req, 'foo', body=body)
def test_set_metadata_with_missing_metadata(self):
body = {"asdf": {"foo": "bar"}}
self.assertRaises(self.bad_request, eval(self.set_metadata),
@@ -697,21 +722,25 @@ class AggregateTestCaseV21(test.NoDBTestCase):
def test_delete_aggregate_with_bad_aggregate(self):
side_effect = exception.AggregateNotFound(
aggregate_id="bogus_aggregate")
aggregate_id="2")
with mock.patch.object(self.controller.api, 'delete_aggregate',
side_effect=side_effect) as mock_del:
self.assertRaises(exc.HTTPNotFound, self.controller.delete,
self.req, "bogus_aggregate")
mock_del.assert_called_once_with(self.context, "bogus_aggregate")
self.req, "2")
mock_del.assert_called_once_with(self.context, "2")
def test_delete_with_invalid_id(self):
self.assertRaises(exc.HTTPBadRequest, self.controller.delete,
self.req, 'foo')
def test_delete_aggregate_with_host(self):
with mock.patch.object(self.controller.api, "delete_aggregate",
side_effect=exception.InvalidAggregateAction(
action="delete", aggregate_id="agg1",
action="delete", aggregate_id="2",
reason="not empty")):
self.assertRaises(exc.HTTPBadRequest,
self.controller.delete,
self.req, "agg1")
self.req, "2")
def test_marshall_aggregate(self):
# _marshall_aggregate() just basically turns the aggregate returned
@@ -746,3 +775,11 @@ class AggregateTestCaseV21(test.NoDBTestCase):
def _assert_agg_data(self, expected, actual):
self.assertTrue(obj_base.obj_equal_prims(expected, actual),
"The aggregate objects were not equal")
def test_images_with_invalid_id(self):
body = {'cache': [{'id': uuidsentinel.cache}]}
req = fakes.HTTPRequest.blank('/v2/os-aggregates',
use_admin_context=True,
version='2.81')
self.assertRaises(exc.HTTPBadRequest, self.controller.images,
req, 'foo', body=body)