From 563da722673a3106e6f47fb76b3de354de0a7b4d Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Mon, 3 Jul 2017 23:14:10 +0000 Subject: [PATCH] Support show/delete with all_tenants Users have administrator privilege can list containers in all tenants. This commit add the similar capability for 'show' and 'delete'. This allows cloud administrator to manage containers in other tenants, which could be useful. Cloess-Bug: #1681589 Change-Id: Ica579c2f297c6ee08118859b1571b084e40c9bf9 --- etc/zun/policy.json | 2 + zun/api/controllers/v1/containers.py | 41 +++++++----- zun/api/controllers/v1/schemas/containers.py | 3 +- zun/tests/fake_policy.py | 4 +- .../api/controllers/v1/test_containers.py | 65 ++++++++++++++++++- 5 files changed, 95 insertions(+), 20 deletions(-) diff --git a/etc/zun/policy.json b/etc/zun/policy.json index 9e708378a..ca42b447e 100644 --- a/etc/zun/policy.json +++ b/etc/zun/policy.json @@ -6,7 +6,9 @@ "container:create": "rule:default", "container:delete": "rule:default", + "container:delete_all_tenants": "rule:admin_api", "container:get": "rule:default", + "container:get_one_all_tenants": "rule:admin_api", "container:get_all": "rule:default", "container:get_all_all_tenants": "rule:admin_api", "container:update": "rule:default", diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index 79e706805..3c2de1865 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -51,6 +51,18 @@ def check_policy_on_container(container, action): policy.enforce(context, action, container, action=action) +def is_all_tenants(search_opts): + all_tenants = search_opts.get('all_tenants') + if all_tenants: + try: + all_tenants = strutils.bool_from_string(all_tenants, True) + except ValueError as err: + raise exception.InvalidInput(six.text_type(err)) + else: + all_tenants = False + return all_tenants + + class ContainerCollection(collection.Collection): """API representation of a collection of containers.""" @@ -112,16 +124,7 @@ class ContainersController(base.Controller): def _get_containers_collection(self, **kwargs): context = pecan.request.context - all_tenants = kwargs.get('all_tenants') - if all_tenants: - try: - all_tenants = strutils.bool_from_string(all_tenants, True) - except ValueError as err: - raise exception.InvalidInput(six.text_type(err)) - else: - # If no value, it's considered to disable all_tenants - all_tenants = False - if all_tenants: + if is_all_tenants(kwargs): policy.enforce(context, "container:get_all_all_tenants", action="container:get_all_all_tenants") context.all_tenants = True @@ -162,14 +165,18 @@ class ContainersController(base.Controller): @pecan.expose('json') @exception.wrap_pecan_controller_exception - def get_one(self, container_id): + def get_one(self, container_id, **kwargs): """Retrieve information about the given container. :param container_ident: UUID or name of a container. """ - container = _get_container(container_id) - check_policy_on_container(container.as_dict(), "container:get") context = pecan.request.context + if is_all_tenants(kwargs): + policy.enforce(context, "container:get_one_all_tenants", + action="container:get_one_all_tenants") + context.all_tenants = True + container = _get_container(container_id) + check_policy_on_container(container.as_dict(), "container:get_one") compute_api = pecan.request.compute_api container = compute_api.container_show(context, container) return view.format_container(pecan.request.host_url, container) @@ -335,11 +342,16 @@ class ContainersController(base.Controller): @pecan.expose('json') @exception.wrap_pecan_controller_exception @validation.validate_query_param(pecan.request, schema.query_param_delete) - def delete(self, container_id, force=False): + def delete(self, container_id, force=False, **kwargs): """Delete a container. :param container_ident: UUID or Name of a container. """ + context = pecan.request.context + if is_all_tenants(kwargs): + policy.enforce(context, "container:delete_all_tenants", + action="container:delete_all_tenants") + context.all_tenants = True container = _get_container(container_id) check_policy_on_container(container.as_dict(), "container:delete") try: @@ -351,7 +363,6 @@ class ContainersController(base.Controller): utils.validate_container_state(container, 'delete') else: utils.validate_container_state(container, 'delete_force') - context = pecan.request.context compute_api = pecan.request.compute_api compute_api.container_delete(context, container, force) pecan.response.status = 204 diff --git a/zun/api/controllers/v1/schemas/containers.py b/zun/api/controllers/v1/schemas/containers.py index cda48e8fa..89fbd45d1 100644 --- a/zun/api/controllers/v1/schemas/containers.py +++ b/zun/api/controllers/v1/schemas/containers.py @@ -69,7 +69,8 @@ container_update = { query_param_delete = { 'type': 'object', 'properties': { - 'force': parameter_types.boolean_extended + 'force': parameter_types.boolean_extended, + 'all_tenants': parameter_types.boolean_extended }, 'additionalProperties': False } diff --git a/zun/tests/fake_policy.py b/zun/tests/fake_policy.py index 993060ed8..2e15c3863 100644 --- a/zun/tests/fake_policy.py +++ b/zun/tests/fake_policy.py @@ -22,7 +22,9 @@ policy_data = """ "container:create": "", "container:delete": "", - "container:get": "", + "container:delete_all_tenants": "", + "container:get_one": "", + "container:get_one_all_tenants": "", "container:get_all": "", "container:get_all_all_tenants": "", "container:update": "", diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index e892b16bf..505bd291b 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -580,6 +580,29 @@ class TestContainerController(api_base.FunctionalTest): mock_container_get_by_uuid.assert_called_once_with( mock.ANY, test_container['uuid']) + context = mock_container_get_by_uuid.call_args[0][0] + self.assertIs(False, context.all_tenants) + self.assertEqual(200, response.status_int) + self.assertEqual(test_container['uuid'], + response.json['uuid']) + + @patch('zun.compute.api.API.container_show') + @patch('zun.objects.Container.get_by_uuid') + def test_get_one_by_uuid_all_tenants(self, mock_container_get_by_uuid, + mock_container_show): + test_container = utils.get_test_container() + test_container_obj = objects.Container(self.context, **test_container) + mock_container_get_by_uuid.return_value = test_container_obj + mock_container_show.return_value = test_container_obj + + response = self.app.get('/v1/containers/%s/?all_tenants=1' % + test_container['uuid']) + + mock_container_get_by_uuid.assert_called_once_with( + mock.ANY, + test_container['uuid']) + context = mock_container_get_by_uuid.call_args[0][0] + self.assertIs(True, context.all_tenants) self.assertEqual(200, response.status_int) self.assertEqual(test_container['uuid'], response.json['uuid']) @@ -924,6 +947,28 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(204, response.status_int) mock_container_delete.assert_called_once_with( mock.ANY, test_container_obj, False) + context = mock_container_delete.call_args[0][0] + self.assertIs(False, context.all_tenants) + + @patch('zun.common.utils.validate_container_state') + @patch('zun.compute.api.API.container_delete') + @patch('zun.objects.Container.get_by_uuid') + def test_delete_container_by_uuid_all_tenants(self, mock_get_by_uuid, + mock_container_delete, + mock_validate): + test_container = utils.get_test_container() + test_container_obj = objects.Container(self.context, **test_container) + mock_get_by_uuid.return_value = test_container_obj + + container_uuid = test_container.get('uuid') + response = self.app.delete('/v1/containers/%s/?all_tenants=1' % + container_uuid) + + self.assertEqual(204, response.status_int) + mock_container_delete.assert_called_once_with( + mock.ANY, test_container_obj, False) + context = mock_container_delete.call_args[0][0] + self.assertIs(True, context.all_tenants) def test_delete_by_uuid_invalid_state(self): uuid = uuidutils.generate_uuid() @@ -1353,8 +1398,15 @@ class TestContainerEnforcement(api_base.FunctionalTest): def test_policy_disallow_get_one(self): container = obj_utils.create_test_container(self.context) self._common_policy_check( - 'container:get', self.get_json, - '/containers/%s/' % container.uuid, + 'container:get_one', self.app.get, + '/v1/containers/%s/' % container.uuid, + expect_errors=True) + + def test_policy_disallow_get_one_all_tenants(self): + container = obj_utils.create_test_container(self.context) + self._common_policy_check( + 'container:get_one_all_tenants', self.app.get, + '/v1/containers/%s/?all_tenants=1' % container.uuid, expect_errors=True) def test_policy_disallow_update(self): @@ -1382,6 +1434,13 @@ class TestContainerEnforcement(api_base.FunctionalTest): '/v1/containers/%s/' % container.uuid, expect_errors=True) + def test_policy_disallow_delete_all_tenants(self): + container = obj_utils.create_test_container(self.context) + self._common_policy_check( + 'container:delete_all_tenants', self.app.delete, + '/v1/containers/%s/?all_tenants=1' % container.uuid, + expect_errors=True) + def _owner_check(self, rule, func, *args, **kwargs): self.policy.set_rules({rule: "user_id:%(user_id)s"}) response = func(*args, **kwargs) @@ -1394,7 +1453,7 @@ class TestContainerEnforcement(api_base.FunctionalTest): def test_policy_only_owner_get_one(self): container = obj_utils.create_test_container(self.context, user_id='another') - self._owner_check("container:get", self.get_json, + self._owner_check("container:get_one", self.get_json, '/containers/%s/' % container.uuid, expect_errors=True)