openstack
/
nova
Code Issues Proposed changes

Merge "Remove policy check in db layer for aggregates"

This commit is contained in:
Jenkins 2014-02-12 08:48:24 +00:00 committed by Gerrit Code Review
parent 3528e80017 81ace28111
commit 07c44c6859
4 changed files with 89 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
nova/db/sqlalchemy/api.py
View File

@ -4936,7 +4936,6 @@ def _aggregate_get_query(context, model_class, id_field=None, id=None,
return query
@require_admin_context
def aggregate_create(context, values, metadata=None):
session = get_session()
query = _aggregate_get_query(context,
@ -4961,7 +4960,6 @@ def aggregate_create(context, values, metadata=None):
return aggregate_get(context, aggregate.id)
@require_admin_context
def aggregate_get(context, aggregate_id):
query = _aggregate_get_query(context,
models.Aggregate,
@ -4975,7 +4973,6 @@ def aggregate_get(context, aggregate_id):
return aggregate
@require_admin_context
def aggregate_get_by_host(context, host, key=None):
"""Return rows that match host (mandatory) and metadata key (optional).
@ -4994,7 +4991,6 @@ def aggregate_get_by_host(context, host, key=None):
return query.all()
@require_admin_context
def aggregate_metadata_get_by_host(context, host, key=None):
query = model_query(context, models.Aggregate)
query = query.join("_hosts")
@ -5013,7 +5009,6 @@ def aggregate_metadata_get_by_host(context, host, key=None):
return dict(metadata)
@require_admin_context
def aggregate_metadata_get_by_metadata_key(context, aggregate_id, key):
query = model_query(context, models.Aggregate)
query = query.join("_metadata")
@ -5029,7 +5024,6 @@ def aggregate_metadata_get_by_metadata_key(context, aggregate_id, key):
return dict(metadata)
@require_admin_context
def aggregate_host_get_by_metadata_key(context, key):
query = model_query(context, models.Aggregate)
query = query.join("_metadata")
@ -5045,7 +5039,6 @@ def aggregate_host_get_by_metadata_key(context, key):
return dict(metadata)
@require_admin_context
def aggregate_update(context, aggregate_id, values):
session = get_session()
aggregate = (_aggregate_get_query(context,
@ -5078,7 +5071,6 @@ def aggregate_update(context, aggregate_id, values):
raise exception.AggregateNotFound(aggregate_id=aggregate_id)
@require_admin_context
def aggregate_delete(context, aggregate_id):
session = get_session()
with session.begin():
@ -5098,7 +5090,6 @@ def aggregate_delete(context, aggregate_id):
soft_delete()
@require_admin_context
def aggregate_get_all(context):
return _aggregate_get_query(context, models.Aggregate).all()
@ -5112,7 +5103,6 @@ def _aggregate_metadata_get_query(context, aggregate_id, session=None,
filter_by(aggregate_id=aggregate_id)
@require_admin_context
@require_aggregate_exists
def aggregate_metadata_get(context, aggregate_id):
rows = model_query(context,
@ -5122,7 +5112,6 @@ def aggregate_metadata_get(context, aggregate_id):
return dict([(r['key'], r['value']) for r in rows])
@require_admin_context
@require_aggregate_exists
def aggregate_metadata_delete(context, aggregate_id, key):
count = _aggregate_get_query(context,
@ -5136,7 +5125,6 @@ def aggregate_metadata_delete(context, aggregate_id, key):
metadata_key=key)
@require_admin_context
@require_aggregate_exists
def aggregate_metadata_add(context, aggregate_id, metadata, set_delete=False,
max_retries=10):
@ -5183,7 +5171,6 @@ def aggregate_metadata_add(context, aggregate_id, metadata, set_delete=False,
LOG.warn(msg)
@require_admin_context
@require_aggregate_exists
def aggregate_host_get_all(context, aggregate_id):
rows = model_query(context,
@ -5193,7 +5180,6 @@ def aggregate_host_get_all(context, aggregate_id):
return [r.host for r in rows]
@require_admin_context
@require_aggregate_exists
def aggregate_host_delete(context, aggregate_id, host):
count = _aggregate_get_query(context,
@ -5207,7 +5193,6 @@ def aggregate_host_delete(context, aggregate_id, host):
host=host)
@require_admin_context
@require_aggregate_exists
def aggregate_host_add(context, aggregate_id, host):
host_ref = models.AggregateHost()

48
nova/tests/api/openstack/compute/contrib/test_aggregates.py
View File

@ -21,6 +21,7 @@ from nova.api.openstack.compute.contrib import aggregates
from nova import context
from nova import exception
from nova import test
from nova.tests.api.openstack import fakes
from nova.tests import matchers
AGGREGATE_LIST = [
@ -46,6 +47,7 @@ class AggregateTestCase(test.NoDBTestCase):
super(AggregateTestCase, self).setUp()
self.controller = aggregates.AggregateController()
self.req = FakeRequest()
self.user_req = fakes.HTTPRequest.blank('/v2/os-aggregates')
self.context = self.req.environ['nova.context']
def test_index(self):
@ -60,6 +62,11 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE_LIST, result["aggregates"])
def test_index_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.index,
self.user_req)
def test_create(self):
def stub_create_aggregate(context, name, availability_zone):
self.assertEqual(context, self.context, "context")
@ -74,6 +81,13 @@ class AggregateTestCase(test.NoDBTestCase):
"availability_zone": "nova1"}})
self.assertEqual(AGGREGATE, result["aggregate"])
def test_create_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.create, self.user_req,
{"aggregate":
{"name": "test",
"availability_zone": "nova1"}})
def test_create_with_duplicate_aggregate_name(self):
def stub_create_aggregate(context, name, availability_zone):
raise exception.AggregateNameExists(aggregate_name=name)
@ -155,6 +169,11 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE, aggregate["aggregate"])
def test_show_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.show,
self.user_req, "1")
def test_show_with_invalid_id(self):
def stub_get_aggregate(context, id):
raise exception.AggregateNotFound(aggregate_id=2)
@ -181,6 +200,11 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE, result["aggregate"])
def test_update_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.update,
self.user_req, "1", body={})
def test_update_with_only_name(self):
body = {"aggregate": {"name": "new_name"}}
@ -260,6 +284,12 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(aggregate["aggregate"], AGGREGATE)
def test_add_host_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.action,
self.user_req, "1",
body={"add_host": {"host": "host1"}})
def test_add_host_with_already_added_host(self):
def stub_add_host_to_aggregate(context, aggregate, host):
raise exception.AggregateHostExists(aggregate_id=aggregate,
@ -322,6 +352,12 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertTrue(stub_remove_host_from_aggregate.called)
def test_remove_host_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.action,
self.user_req, "1",
body={"remove_host": {"host": "host1"}})
def test_remove_host_with_bad_aggregate(self):
def stub_remove_host_from_aggregate(context, aggregate, host):
raise exception.AggregateNotFound(aggregate_id=aggregate)
@ -381,6 +417,13 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE, result["aggregate"])
def test_set_metadata_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller._set_metadata,
self.user_req, "1",
body={"set_metadata": {"metadata":
{"foo": "bar"}}})
def test_set_metadata_with_bad_aggregate(self):
body = {"set_metadata": {"metadata": {"foo": "bar"}}}
@ -414,6 +457,11 @@ class AggregateTestCase(test.NoDBTestCase):
self.controller.delete(self.req, "1")
self.assertTrue(stub_delete_aggregate.called)
def test_delete_aggregate_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.delete,
self.user_req, "1")
def test_delete_aggregate_with_bad_aggregate(self):
def stub_delete_aggregate(context, aggregate):
raise exception.AggregateNotFound(aggregate_id=aggregate)

68
nova/tests/api/openstack/compute/plugins/v3/test_aggregates.py
View File

@ -59,9 +59,11 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE_LIST, result["aggregates"])
def test_index_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.index,
self.user_req)
exc = self.assertRaises(exception.PolicyNotAuthorized,
self.controller.index,
self.user_req)
self.assertIn("compute_extension:v3:os-aggregates:index",
exc.format_message())
def test_create(self):
def stub_create_aggregate(context, name, availability_zone):
@ -78,11 +80,13 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE, result["aggregate"])
def test_create_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.create, self.user_req,
{"aggregate":
{"name": "test",
"availability_zone": "nova1"}})
exc = self.assertRaises(exception.PolicyNotAuthorized,
self.controller.create, self.user_req,
{"aggregate":
{"name": "test",
"availability_zone": "nova1"}})
self.assertIn("compute_extension:v3:os-aggregates:create",
exc.format_message())
def test_create_with_duplicate_aggregate_name(self):
def stub_create_aggregate(context, name, availability_zone):
@ -159,9 +163,11 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE, aggregate["aggregate"])
def test_show_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.show,
self.user_req, "1")
exc = self.assertRaises(exception.PolicyNotAuthorized,
self.controller.show,
self.user_req, "1")
self.assertIn("compute_extension:v3:os-aggregates:show",
exc.format_message())
def test_show_with_invalid_id(self):
def stub_get_aggregate(context, id):
@ -190,9 +196,11 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE, result["aggregate"])
def test_update_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.update,
self.user_req, "1", body={})
exc = self.assertRaises(exception.PolicyNotAuthorized,
self.controller.update,
self.user_req, "1", body={})
self.assertIn("compute_extension:v3:os-aggregates:update",
exc.format_message())
def test_update_with_only_name(self):
body = {"aggregate": {"name": "new_name"}}
@ -270,10 +278,12 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(self.controller._add_host.wsgi_code, 202)
def test_add_host_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller._add_host,
self.user_req, "1",
body={"add_host": {"host": "host1"}})
exc = self.assertRaises(exception.PolicyNotAuthorized,
self.controller._add_host,
self.user_req, "1",
body={"add_host": {"host": "host1"}})
self.assertIn("compute_extension:v3:os-aggregates:add_host",
exc.format_message())
def test_add_host_with_already_added_host(self):
def stub_add_host_to_aggregate(context, aggregate, host):
@ -335,10 +345,12 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(self.controller._remove_host.wsgi_code, 202)
def test_remove_host_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller._remove_host,
self.user_req, "1",
body={"remove_host": {"host": "host1"}})
exc = self.assertRaises(exception.PolicyNotAuthorized,
self.controller._remove_host,
self.user_req, "1",
body={"remove_host": {"host": "host1"}})
self.assertIn("compute_extension:v3:os-aggregates:remove_host",
exc.format_message())
def test_remove_host_with_host_not_in_aggregate(self):
def stub_remove_host_from_aggregate(context, aggregate, host):
@ -392,11 +404,13 @@ class AggregateTestCase(test.NoDBTestCase):
self.assertEqual(AGGREGATE, result["aggregate"])
def test_set_metadata_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
self.controller._set_metadata,
self.user_req, "1",
body={"set_metadata": {"metadata":
{"foo": "bar"}}})
exc = self.assertRaises(exception.PolicyNotAuthorized,
self.controller._set_metadata,
self.user_req, "1",
body={"set_metadata": {"metadata":
{"foo": "bar"}}})
self.assertIn("compute_extension:v3:os-aggregates:set_metadata",
exc.format_message())
def test_set_metadata_with_bad_aggregate(self):
body = {"set_metadata": {"metadata": {"foo": "bar"}}}

5
nova/tests/db/test_db_api.py
View File

@ -339,11 +339,6 @@ class AggregateDBApiTestCase(test.TestCase):
self.assertEqual(expected_metadata, {'availability_zone':
'fake_avail_zone'})
def test_aggregate_create_low_privi_context(self):
self.assertRaises(exception.AdminRequired,
db.aggregate_create,
self.context, _get_fake_aggr_values())
def test_aggregate_get(self):
ctxt = context.get_admin_context()
result = _create_aggregate_with_hosts(context=ctxt)