From ebc06cf9de086ff6a2001d20fc10f05360a0aa7c Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Thu, 12 Jan 2012 13:00:45 -0800 Subject: Add policy checks to Volume.API Change-Id: If4b37c1041a10c3c0697724281aadb9a17b51373 --- etc/nova/policy.json | 24 +++++++++++++++++++++++- nova/tests/policy.json | 24 +++++++++++++++++++++++- nova/tests/test_volume.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ nova/volume/api.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 2 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 343ee75cc..00140886b 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -69,5 +69,27 @@ "compute:delete": [["rule:admin_or_owner"]], "compute:soft_delete": [["rule:admin_or_owner"]], "compute:force_delete": [["rule:admin_or_owner"]], - "compute:restore": [["rule:admin_or_owner"]] + "compute:restore": [["rule:admin_or_owner"]], + + + "volume:create": [], + "volume:get": [], + "volume:get_all": [], + "volume:get_volume_metadata": [], + "volume:delete": [], + "volume:update": [], + "volume:delete_volume_metadata": [], + "volume:update_volume_metadata": [], + + "volume:attach": [], + "volume:detach": [], + "volume:check_attach": [], + "volume:check_detach": [], + "volume:initialize_connection": [], + "volume:terminate_connection": [], + + "volume:create_snapshot": [], + "volume:delete_snapshot": [], + "volume:get_snapshot": [], + "volume:get_all_snapshots": [] } diff --git a/nova/tests/policy.json b/nova/tests/policy.json index 7dae81a48..c96908fff 100644 --- a/nova/tests/policy.json +++ b/nova/tests/policy.json @@ -66,5 +66,27 @@ "compute:delete": [], "compute:soft_delete": [], "compute:force_delete": [], - "compute:restore": [] + "compute:restore": [], + + + "volume:create": [], + "volume:get": [], + "volume:get_all": [], + "volume:get_volume_metadata": [], + "volume:delete": [], + "volume:update": [], + "volume:delete_volume_metadata": [], + "volume:update_volume_metadata": [], + + "volume:attach": [], + "volume:detach": [], + "volume:check_attach": [], + "volume:check_detach": [], + "volume:initialize_connection": [], + "volume:terminate_connection": [], + + "volume:create_snapshot": [], + "volume:delete_snapshot": [], + "volume:get_snapshot": [], + "volume:get_all_snapshots": [] } diff --git a/nova/tests/test_volume.py b/nova/tests/test_volume.py index aeefcd020..fec394984 100644 --- a/nova/tests/test_volume.py +++ b/nova/tests/test_volume.py @@ -27,6 +27,7 @@ from nova import exception from nova import db from nova import flags from nova import log as logging +import nova.policy from nova import rpc from nova import test from nova import utils @@ -399,3 +400,47 @@ class ISCSITestCase(DriverTestCase): self.mox.UnsetStubs() self._detach_volume(volume_id_list) + + +class VolumePolicyTestCase(test.TestCase): + + def setUp(self): + super(VolumePolicyTestCase, self).setUp() + + nova.policy.reset() + nova.policy.init() + + self.context = context.get_admin_context() + self.volume_api = nova.volume.api.API() + + def tearDown(self): + super(VolumePolicyTestCase, self).tearDown() + nova.policy.reset() + + def _set_rules(self, rules): + nova.common.policy.set_brain(nova.common.policy.HttpBrain(rules)) + + def test_check_policy(self): + self.mox.StubOutWithMock(nova.policy, 'enforce') + target = { + 'project_id': self.context.project_id, + 'user_id': self.context.user_id, + } + nova.policy.enforce(self.context, 'volume:attach', target) + self.mox.ReplayAll() + nova.volume.api.check_policy(self.context, 'attach') + self.mox.UnsetStubs() + self.mox.VerifyAll() + + def test_check_policy_with_target(self): + self.mox.StubOutWithMock(nova.policy, 'enforce') + target = { + 'project_id': self.context.project_id, + 'user_id': self.context.user_id, + 'id': 2, + } + nova.policy.enforce(self.context, 'volume:attach', target) + self.mox.ReplayAll() + nova.volume.api.check_policy(self.context, 'attach', {'id': 2}) + self.mox.UnsetStubs() + self.mox.VerifyAll() diff --git a/nova/volume/api.py b/nova/volume/api.py index a41e01274..6f5921f7c 100644 --- a/nova/volume/api.py +++ b/nova/volume/api.py @@ -20,12 +20,14 @@ Handles all requests relating to volumes. """ +import functools from eventlet import greenthread from nova import exception from nova import flags from nova import log as logging +import nova.policy from nova import quota from nova import rpc from nova import utils @@ -37,11 +39,36 @@ flags.DECLARE('storage_availability_zone', 'nova.volume.manager') LOG = logging.getLogger('nova.volume') +def wrap_check_policy(func): + """Check policy corresponding to the wrapped methods prior to execution + + This decorator requires the first 3 args of the wrapped function + to be (self, context, volume) + """ + @functools.wraps(func) + def wrapped(self, context, target_obj, *args, **kwargs): + check_policy(context, func.__name__, target_obj) + return func(self, context, target_obj, *args, **kwargs) + + return wrapped + + +def check_policy(context, action, target_obj=None): + target = { + 'project_id': context.project_id, + 'user_id': context.user_id, + } + target.update(target_obj or {}) + _action = 'volume:%s' % action + nova.policy.enforce(context, _action, target) + + class API(base.Base): """API for interacting with the volume manager.""" def create(self, context, size, name, description, snapshot=None, volume_type=None, metadata=None, availability_zone=None): + check_policy(context, 'create') if snapshot is not None: if snapshot['status'] != "available": raise exception.ApiError( @@ -100,6 +127,7 @@ class API(base.Base): return greenthread.sleep(1) + @wrap_check_policy def delete(self, context, volume): volume_id = volume['id'] if volume['status'] != "available": @@ -113,14 +141,17 @@ class API(base.Base): {"method": "delete_volume", "args": {"volume_id": volume_id}}) + @wrap_check_policy def update(self, context, volume, fields): self.db.volume_update(context, volume['id'], fields) def get(self, context, volume_id): + check_policy(context, 'get', {'id': volume_id}) rv = self.db.volume_get(context, volume_id) return dict(rv.iteritems()) def get_all(self, context, search_opts={}): + check_policy(context, 'get_all') if context.is_admin: volumes = self.db.volume_get_all(context) else: @@ -161,14 +192,17 @@ class API(base.Base): return volumes def get_snapshot(self, context, snapshot_id): + check_policy(context, 'get_snapshot') rv = self.db.snapshot_get(context, snapshot_id) return dict(rv.iteritems()) def get_all_snapshots(self, context): + check_policy(context, 'get_all_snapshots') if context.is_admin: return self.db.snapshot_get_all(context) return self.db.snapshot_get_all_by_project(context, context.project_id) + @wrap_check_policy def check_attach(self, context, volume): # TODO(vish): abstract status checking? if volume['status'] != "available": @@ -176,6 +210,7 @@ class API(base.Base): if volume['attach_status'] == "attached": raise exception.ApiError(_("Volume is already attached")) + @wrap_check_policy def check_detach(self, context, volume): # TODO(vish): abstract status checking? if volume['status'] == "available": @@ -189,6 +224,7 @@ class API(base.Base): "args": {'instance_id': instance_id, 'volume_id': volume['id']}}) + @wrap_check_policy def attach(self, context, volume, instance_id, mountpoint): host = volume['host'] queue = self.db.queue_get_for(context, FLAGS.volume_topic, host) @@ -198,6 +234,7 @@ class API(base.Base): "instance_id": instance_id, "mountpoint": mountpoint}}) + @wrap_check_policy def detach(self, context, volume): host = volume['host'] queue = self.db.queue_get_for(context, FLAGS.volume_topic, host) @@ -205,6 +242,7 @@ class API(base.Base): {"method": "detach_volume", "args": {"volume_id": volume['id']}}) + @wrap_check_policy def initialize_connection(self, context, volume, address): host = volume['host'] queue = self.db.queue_get_for(context, FLAGS.volume_topic, host) @@ -213,6 +251,7 @@ class API(base.Base): "args": {"volume_id": volume['id'], "address": address}}) + @wrap_check_policy def terminate_connection(self, context, volume, address): host = volume['host'] queue = self.db.queue_get_for(context, FLAGS.volume_topic, host) @@ -223,6 +262,8 @@ class API(base.Base): def _create_snapshot(self, context, volume, name, description, force=False): + check_policy(context, 'create_snapshot') + if ((not force) and (volume['status'] != "available")): raise exception.ApiError(_("Volume status must be available")) @@ -253,6 +294,7 @@ class API(base.Base): return self._create_snapshot(context, volume, name, description, True) + @wrap_check_policy def delete_snapshot(self, context, snapshot): if snapshot['status'] != "available": raise exception.ApiError(_("Snapshot status must be available")) @@ -264,15 +306,18 @@ class API(base.Base): "args": {"topic": FLAGS.volume_topic, "snapshot_id": snapshot['id']}}) + @wrap_check_policy def get_volume_metadata(self, context, volume): """Get all metadata associated with a volume.""" rv = self.db.volume_metadata_get(context, volume['id']) return dict(rv.iteritems()) + @wrap_check_policy def delete_volume_metadata(self, context, volume, key): """Delete the given metadata item from an volume.""" self.db.volume_metadata_delete(context, volume['id'], key) + @wrap_check_policy def update_volume_metadata(self, context, volume, metadata, delete=False): """Updates or creates volume metadata. -- cgit