From fde94b28e3561d23eee1c30337f2df21165f7e67 Mon Sep 17 00:00:00 2001 From: jiaopengju Date: Tue, 17 Oct 2017 17:04:45 +0800 Subject: [PATCH] Add tenant isolation of checkpoints Change-Id: Iaae21b4c7339f6453c391cb958fb828694b083fd Implements: blueprint checkpoint-tenant-isolation --- karbor/api/v1/providers.py | 9 ++- karbor/exception.py | 8 ++ karbor/services/protection/checkpoint.py | 81 +++++++++++++------ karbor/services/protection/manager.py | 24 +++++- karbor/services/protection/provider.py | 12 +-- karbor/tests/unit/protection/fakes.py | 6 +- .../protection/test_checkpoint_collection.py | 43 ++++++---- karbor/tests/unit/protection/test_manager.py | 13 ++- 8 files changed, 139 insertions(+), 57 deletions(-) diff --git a/karbor/api/v1/providers.py b/karbor/api/v1/providers.py index eb14ec16..da652e57 100644 --- a/karbor/api/v1/providers.py +++ b/karbor/api/v1/providers.py @@ -431,6 +431,8 @@ class ProvidersController(wsgi.Controller): checkpoint_id) except exception.CheckpointNotFound as error: raise exc.HTTPNotFound(explanation=error.msg) + except exception.AccessCheckpointNotAllowed as error: + raise exc.HTTPForbidden(explanation=error.msg) LOG.info("Show checkpoint request issued successfully.") LOG.info("checkpoint: %s", checkpoint) @@ -479,9 +481,10 @@ class ProvidersController(wsgi.Controller): raise exc.HTTPBadRequest(explanation=msg) context.can(provider_policy.CHECKPOINT_DELETE_POLICY) - self.protection_api.delete(context, - provider_id, - checkpoint_id) + try: + self.protection_api.delete(context, provider_id, checkpoint_id) + except exception.DeleteCheckpointNotAllowed as error: + raise exc.HTTPForbidden(explantion=error.msg) LOG.info("Delete checkpoint request issued successfully.") return {} diff --git a/karbor/exception.py b/karbor/exception.py index 13b614f5..9c9a72ea 100644 --- a/karbor/exception.py +++ b/karbor/exception.py @@ -242,6 +242,14 @@ class DeleteTriggerNotAllowed(NotAuthorized): message = _("Can not delete trigger %(trigger_id)s") +class AccessCheckpointNotAllowed(NotAuthorized): + message = _("Access checkpoint %(checkpoint_id)s is not allowed") + + +class DeleteCheckpointNotAllowed(NotAuthorized): + message = _("Delete checkpoint %(checkpoint_id)s is not allowed") + + class ClassNotFound(NotFound): message = _("Class %(class_name)s could not be found: %(exception)s") diff --git a/karbor/services/protection/checkpoint.py b/karbor/services/protection/checkpoint.py index 5ff36d7c..f6005d29 100644 --- a/karbor/services/protection/checkpoint.py +++ b/karbor/services/protection/checkpoint.py @@ -139,6 +139,24 @@ class Checkpoint(object): return Checkpoint(checkpoint_section, indices_section, bank_lease, checkpoint_id) + @staticmethod + def _get_checkpoint_path_by_provider( + provider_id, project_id, timestamp, checkpoint_id): + return "/by-provider/%s/%s/%s@%s" % ( + provider_id, project_id, timestamp, checkpoint_id) + + @staticmethod + def _get_checkpoint_path_by_plan( + plan_id, project_id, created_at, timestamp, checkpoint_id): + return "/by-plan/%s/%s/%s/%s@%s" % ( + plan_id, project_id, created_at, timestamp, checkpoint_id) + + @staticmethod + def _get_checkpoint_path_by_date( + created_at, project_id, timestamp, checkpoint_id): + return "/by-date/%s/%s/%s@%s" % ( + created_at, project_id, timestamp, checkpoint_id) + @classmethod def create_in_section(cls, checkpoints_section, indices_section, bank_lease, owner_id, plan, @@ -150,6 +168,7 @@ class Checkpoint(object): created_at = timeutils.utcnow().strftime('%Y-%m-%d') provider_id = plan.get("provider_id") + project_id = plan.get("project_id") extra_info = None if checkpoint_properties: extra_info = checkpoint_properties.get("extra_info", None) @@ -161,7 +180,7 @@ class Checkpoint(object): "status": constants.CHECKPOINT_STATUS_PROTECTING, "owner_id": owner_id, "provider_id": provider_id, - "project_id": plan.get("project_id"), + "project_id": project_id, "protection_plan": { "id": plan.get("id"), "name": plan.get("name"), @@ -175,19 +194,21 @@ class Checkpoint(object): ) indices_section.update_object( - key="/by-provider/%s/%s@%s" % (provider_id, timestamp, - checkpoint_id), + key=cls._get_checkpoint_path_by_provider( + provider_id, project_id, timestamp, checkpoint_id), value=checkpoint_id ) indices_section.update_object( - key="/by-date/%s/%s@%s" % (created_at, timestamp, checkpoint_id), + key=cls._get_checkpoint_path_by_date( + created_at, project_id, timestamp, checkpoint_id), value=checkpoint_id ) indices_section.update_object( - key="/by-plan/%s/%s/%s@%s" % ( - plan.get("id"), created_at, timestamp, checkpoint_id), + key=cls._get_checkpoint_path_by_plan( + plan.get("id"), project_id, created_at, timestamp, + checkpoint_id), value=checkpoint_id) return Checkpoint(checkpoint_section, @@ -213,13 +234,16 @@ class Checkpoint(object): timestamp = self._md_cache["timestamp"] plan_id = self._md_cache["protection_plan"]["id"] provider_id = self._md_cache["protection_plan"]["provider_id"] + project_id = self._md_cache["project_id"] self._indices_section.delete_object( - "/by-provider/%s/%s@%s" % (provider_id, timestamp, self.id)) + self._get_checkpoint_path_by_provider( + provider_id, project_id, timestamp, self.id)) self._indices_section.delete_object( - "/by-date/%s/%s@%s" % (created_at, timestamp, self.id)) + self._get_checkpoint_path_by_date( + created_at, project_id, timestamp, self.id)) self._indices_section.delete_object( - "/by-plan/%s/%s/%s@%s" % ( - plan_id, created_at, timestamp, self.id)) + self._get_checkpoint_path_by_plan( + plan_id, project_id, created_at, timestamp, self.id)) self._checkpoint_section.delete_object(_INDEX_FILE_NAME) else: @@ -233,13 +257,16 @@ class Checkpoint(object): timestamp = self._md_cache["timestamp"] plan_id = self._md_cache["protection_plan"]["id"] provider_id = self._md_cache["protection_plan"]["provider_id"] + project_id = self._md_cache["project_id"] self._indices_section.delete_object( - "/by-provider/%s/%s@%s" % (provider_id, timestamp, self.id)) + self._get_checkpoint_path_by_provider( + provider_id, project_id, timestamp, self.id)) self._indices_section.delete_object( - "/by-date/%s/%s@%s" % (created_at, timestamp, self.id)) + self._get_checkpoint_path_by_date( + created_at, project_id, timestamp, self.id)) self._indices_section.delete_object( - "/by-plan/%s/%s/%s@%s" % ( - plan_id, created_at, timestamp, self.id)) + self._get_checkpoint_path_by_plan( + plan_id, project_id, created_at, timestamp, self.id)) def get_resource_bank_section(self, resource_id): prefix = "/resource-data/%s/" % resource_id @@ -255,8 +282,8 @@ class CheckpointCollection(object): self._checkpoints_section = bank.get_sub_section("/checkpoints") self._indices_section = bank.get_sub_section("/indices") - def list_ids(self, provider_id, limit=None, marker=None, plan_id=None, - start_date=None, end_date=None, sort_dir=None): + def list_ids(self, project_id, provider_id, limit=None, marker=None, + plan_id=None, start_date=None, end_date=None, sort_dir=None): marker_checkpoint = None if marker is not None: checkpoint_section = self._checkpoints_section.get_sub_section( @@ -270,24 +297,26 @@ class CheckpointCollection(object): end_date = timeutils.utcnow() if plan_id is None and start_date is None: - prefix = "/by-provider/%s/" % provider_id + prefix = "/by-provider/%s/%s/" % (provider_id, project_id) if marker is not None: marker = "/%s" % marker elif plan_id is not None: - prefix = "/by-plan/%s/" % plan_id + prefix = "/by-plan/%s/%s/" % (plan_id, project_id) if marker is not None: date = marker_checkpoint["created_at"] - marker = "/by-plan/%s/%s/%s" % (plan_id, date, marker) + marker = "/by-plan/%s/%s/%s/%s" % ( + plan_id, project_id, date, marker) else: prefix = "/by-date/" if marker is not None: date = marker_checkpoint["created_at"] - marker = "/by-date/%s/%s" % (date, marker) + marker = "/by-date/%s/%s/%s" % (date, project_id, marker) - return self._list_ids(prefix, limit, marker, start_date, end_date, - sort_dir) + return self._list_ids(project_id, prefix, limit, marker, start_date, + end_date, sort_dir) - def _list_ids(self, prefix, limit, marker, start_date, end_date, sort_dir): + def _list_ids(self, project_id, prefix, limit, marker, start_date, + end_date, sort_dir): if start_date is None: return [key[key.find("@") + 1:] for key in self._indices_section.list_objects( @@ -301,8 +330,10 @@ class CheckpointCollection(object): for key in self._indices_section.list_objects(prefix=prefix, marker=marker, sort_dir=sort_dir): - date = datetime.strptime(key.split("/")[-2], "%Y-%m-%d") - if start_date <= date <= end_date: + date = datetime.strptime(key.split("/")[-3], "%Y-%m-%d") + checkpoint_project_id = key.split("/")[-2] + if start_date <= date <= end_date and ( + checkpoint_project_id == project_id): ids.append(key[key.find("@") + 1:]) if limit is not None and len(ids) == limit: return ids diff --git a/karbor/services/protection/manager.py b/karbor/services/protection/manager.py index c196ffe7..8b2f52f4 100644 --- a/karbor/services/protection/manager.py +++ b/karbor/services/protection/manager.py @@ -197,6 +197,7 @@ class ProtectionManager(manager.Manager): "is invalid.") raise exception.InvalidInput(reason=msg) + @messaging.expected_exceptions(exception.DeleteCheckpointNotAllowed) def delete(self, context, provider_id, checkpoint_id): LOG.info("Starting protection service:delete action") LOG.debug('provider_id :%s checkpoint_id:%s', provider_id, @@ -211,6 +212,13 @@ class ProtectionManager(manager.Manager): raise exception.InvalidInput( reason=_("Invalid checkpoint_id or provider_id")) + checkpoint_dict = checkpoint.to_dict() + if not context.is_admin and ( + context.project_id != checkpoint_dict['project_id']): + LOG.warn("Delete checkpoint(%s) is not allowed." % checkpoint_id) + raise exception.DeleteCheckpointNotAllowed( + checkpoint_id=checkpoint_id) + if checkpoint.status not in [ constants.CHECKPOINT_STATUS_AVAILABLE, constants.CHECKPOINT_STATUS_ERROR, @@ -259,9 +267,11 @@ class ProtectionManager(manager.Manager): filters.get("end_date"), "%Y-%m-%d") sort_dir = None if sort_dirs is None else sort_dirs[0] provider = self.provider_registry.show_provider(provider_id) + project_id = context.project_id checkpoint_ids = provider.list_checkpoints( - provider_id, limit=limit, marker=marker, plan_id=plan_id, - start_date=start_date, end_date=end_date, sort_dir=sort_dir) + project_id, provider_id, limit=limit, marker=marker, + plan_id=plan_id, start_date=start_date, end_date=end_date, + sort_dir=sort_dir) checkpoints = [] for checkpoint_id in checkpoint_ids: checkpoint = provider.get_checkpoint(checkpoint_id) @@ -269,12 +279,18 @@ class ProtectionManager(manager.Manager): return checkpoints @messaging.expected_exceptions(exception.ProviderNotFound, - exception.CheckpointNotFound) + exception.CheckpointNotFound, + exception.AccessCheckpointNotAllowed) def show_checkpoint(self, context, provider_id, checkpoint_id): provider = self.provider_registry.show_provider(provider_id) checkpoint = provider.get_checkpoint(checkpoint_id) - return checkpoint.to_dict() + checkpoint_dict = checkpoint.to_dict() + if not context.is_admin and ( + context.project_id != checkpoint_dict['project_id']): + raise exception.AccessCheckpointNotAllowed( + checkpoint_id=checkpoint_id) + return checkpoint_dict def list_protectable_types(self, context): LOG.info("Start to list protectable types.") diff --git a/karbor/services/protection/provider.py b/karbor/services/protection/provider.py index 48e9a950..bc8094e1 100644 --- a/karbor/services/protection/provider.py +++ b/karbor/services/protection/provider.py @@ -147,14 +147,14 @@ class PluggableProtectionProvider(object): def get_checkpoint(self, checkpoint_id): return self.get_checkpoint_collection().get(checkpoint_id) - def list_checkpoints(self, provider_id, limit=None, marker=None, - plan_id=None, start_date=None, end_date=None, - sort_dir=None): + def list_checkpoints(self, project_id, provider_id, limit=None, + marker=None, plan_id=None, start_date=None, + end_date=None, sort_dir=None): checkpoint_collection = self.get_checkpoint_collection() return checkpoint_collection.list_ids( - provider_id=provider_id, limit=limit, marker=marker, - plan_id=plan_id, start_date=start_date, end_date=end_date, - sort_dir=sort_dir) + project_id=project_id, provider_id=provider_id, limit=limit, + marker=marker, plan_id=plan_id, start_date=start_date, + end_date=end_date, sort_dir=sort_dir) class ProviderRegistry(object): diff --git a/karbor/tests/unit/protection/fakes.py b/karbor/tests/unit/protection/fakes.py index 66c2b72f..cf9401f2 100644 --- a/karbor/tests/unit/protection/fakes.py +++ b/karbor/tests/unit/protection/fakes.py @@ -66,7 +66,8 @@ def fake_protection_plan(): {"id": "D", "type": "fake", "name": "fake"}], 'protection_provider': None, 'parameters': {}, - 'provider_id': 'fake_id' + 'provider_id': 'fake_id', + 'project_id': 'fake_project_id' } return protection_plan @@ -248,6 +249,7 @@ class FakeCheckpoint(object): super(FakeCheckpoint, self).__init__() self.id = 'fake_checkpoint' self.status = 'available' + self.project_id = 'fake_project_id' self.resource_graph = resource_graph def purge(self): @@ -266,7 +268,7 @@ class FakeCheckpoint(object): "status": self.status, "resource_graph": self.resource_graph, "protection_plan": None, - "project_id": None + "project_id": self.project_id } diff --git a/karbor/tests/unit/protection/test_checkpoint_collection.py b/karbor/tests/unit/protection/test_checkpoint_collection.py index b01cd84a..7e3d83b4 100644 --- a/karbor/tests/unit/protection/test_checkpoint_collection.py +++ b/karbor/tests/unit/protection/test_checkpoint_collection.py @@ -42,28 +42,32 @@ class CheckpointCollectionTest(base.TestCase): collection = self._create_test_collection() plan = fake_protection_plan() provider_id = plan['provider_id'] + project_id = plan['project_id'] result = {collection.create(plan).id for i in range(10)} - self.assertEqual(set(collection.list_ids(provider_id)), result) + self.assertEqual(set(collection.list_ids( + project_id=project_id, provider_id=provider_id)), result) def test_list_checkpoints_by_plan_id(self): collection = self._create_test_collection() plan_1 = fake_protection_plan() plan_1["id"] = "fake_plan_id_1" plan_1['provider_id'] = "fake_provider_id_1" + plan_1["project_id"] = "fake_project_id_1" provider_id_1 = plan_1['provider_id'] checkpoints_plan_1 = {collection.create(plan_1).id for i in range(10)} plan_2 = fake_protection_plan() plan_2["id"] = "fake_plan_id_2" plan_2['provider_id'] = "fake_provider_id_2" + plan_2["project_id"] = "fake_project_id_2" provider_id_2 = plan_1['provider_id'] checkpoints_plan_2 = {collection.create(plan_2).id for i in range(10)} - self.assertEqual(set(collection.list_ids(provider_id=provider_id_1, - plan_id="fake_plan_id_1")), - checkpoints_plan_1) - self.assertEqual(set(collection.list_ids(provider_id=provider_id_2, - plan_id="fake_plan_id_2")), - checkpoints_plan_2) + self.assertEqual(set(collection.list_ids( + project_id="fake_project_id_1", provider_id=provider_id_1, + plan_id="fake_plan_id_1")), checkpoints_plan_1) + self.assertEqual(set(collection.list_ids( + project_id="fake_project_id_2", provider_id=provider_id_2, + plan_id="fake_plan_id_2")), checkpoints_plan_2) def test_list_checkpoints_by_date(self): collection = self._create_test_collection() @@ -72,28 +76,35 @@ class CheckpointCollectionTest(base.TestCase): timeutils.utcnow.return_value = date1 plan = fake_protection_plan() provider_id = plan['provider_id'] + project_id = plan['project_id'] checkpoints_date_1 = {collection.create(plan).id for i in range(10)} date2 = datetime.strptime("2016-06-13", "%Y-%m-%d") timeutils.utcnow = mock.MagicMock() timeutils.utcnow.return_value = date2 checkpoints_date_2 = {collection.create(plan).id for i in range(10)} - self.assertEqual(set(collection.list_ids(provider_id=provider_id, - start_date=date1, - end_date=date1)), - checkpoints_date_1) - self.assertEqual(set(collection.list_ids(provider_id=provider_id, - start_date=date2, - end_date=date2)), - checkpoints_date_2) + self.assertEqual(set(collection.list_ids( + project_id=project_id, + provider_id=provider_id, + start_date=date1, + end_date=date1)), + checkpoints_date_1) + self.assertEqual(set(collection.list_ids( + project_id=project_id, + provider_id=provider_id, + start_date=date2, + end_date=date2)), + checkpoints_date_2) def test_delete_checkpoint(self): collection = self._create_test_collection() plan = fake_protection_plan() provider_id = plan['provider_id'] + project_id = plan['project_id'] result = {collection.create(plan).id for i in range(10)} checkpoint = collection.get(result.pop()) checkpoint.purge() - self.assertEqual(set(collection.list_ids(provider_id)), result) + self.assertEqual(set(collection.list_ids( + project_id=project_id, provider_id=provider_id)), result) def test_write_checkpoint_with_invalid_lease(self): collection = self._create_test_collection() diff --git a/karbor/tests/unit/protection/test_manager.py b/karbor/tests/unit/protection/test_manager.py index bb279b1e..ab23da39 100644 --- a/karbor/tests/unit/protection/test_manager.py +++ b/karbor/tests/unit/protection/test_manager.py @@ -149,11 +149,22 @@ class ProtectionServiceTest(base.TestCase): @mock.patch.object(provider.ProviderRegistry, 'show_provider') def test_show_checkpoint(self, mock_provider): mock_provider.return_value = fakes.FakeProvider() - context = mock.MagicMock() + context = mock.MagicMock(project_id='fake_project_id') cp = self.pro_manager.show_checkpoint(context, 'provider1', 'fake_checkpoint') self.assertEqual('fake_checkpoint', cp['id']) + @mock.patch.object(provider.ProviderRegistry, 'show_provider') + def test_show_checkpoint_not_allowed(self, mock_provider): + mock_provider.return_value = fakes.FakeProvider() + context = mock.MagicMock( + project_id='fake_project_id_1', + is_admin=False + ) + self.assertRaises(oslo_messaging.ExpectedException, + self.pro_manager.show_checkpoint, + context, 'provider1', 'fake_checkpoint') + @mock.patch.object(provider.ProviderRegistry, 'show_provider') @mock.patch.object(fakes.FakeCheckpointCollection, 'get') def test_show_checkpoint_not_found(self, mock_provider,