From fd591409c49789972c6cfdb42c0f86c8ca4960ce Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Fri, 7 Jul 2017 21:04:00 +0000 Subject: [PATCH] Introduce policy rule for all tenants List containers from all tenants should have a dedicated policy rule since this is a privilage operations. In addition, serveral issues of unit tests were fixed. Closes-Bug: #1703024 Change-Id: Id9b3bfee0ff63173b7bf3fecd1ad50c40211035d --- etc/zun/policy.json | 1 + zun/api/controllers/v1/containers.py | 2 + zun/tests/fake_policy.py | 1 + zun/tests/policy_fixture.py | 7 ++ .../api/controllers/v1/test_containers.py | 72 +++++++++++++------ 5 files changed, 62 insertions(+), 21 deletions(-) diff --git a/etc/zun/policy.json b/etc/zun/policy.json index 2e031ad0b..910bca34a 100644 --- a/etc/zun/policy.json +++ b/etc/zun/policy.json @@ -8,6 +8,7 @@ "container:delete": "rule:default", "container:get": "rule:default", "container:get_all": "rule:default", + "container:get_all_all_tenants": "rule:admin_api", "container:update": "rule:default", "container:start": "rule:default", "container:stop": "rule:default", diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index f6190154e..0e17633a4 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -121,6 +121,8 @@ class ContainersController(base.Controller): # If no value, it's considered to disable all_tenants all_tenants = False if all_tenants: + policy.enforce(context, "container:get_all_all_tenants", + action="container:get_all_all_tenants") context.all_tenants = True compute_api = pecan.request.compute_api limit = api_utils.validate_limit(kwargs.get('limit')) diff --git a/zun/tests/fake_policy.py b/zun/tests/fake_policy.py index 870751f43..401842821 100644 --- a/zun/tests/fake_policy.py +++ b/zun/tests/fake_policy.py @@ -26,6 +26,7 @@ policy_data = """ "container:detail": "", "container:get": "", "container:get_all": "", + "container:get_all_all_tenants": "", "container:update": "", "container:start": "", "container:stop": "", diff --git a/zun/tests/policy_fixture.py b/zun/tests/policy_fixture.py index cb67e26f7..f82107345 100644 --- a/zun/tests/policy_fixture.py +++ b/zun/tests/policy_fixture.py @@ -15,6 +15,7 @@ import os import fixtures from oslo_policy import _parser from oslo_policy import opts as policy_opts +from oslo_serialization import jsonutils from zun.common import policy as zun_policy import zun.conf @@ -38,6 +39,12 @@ class PolicyFixture(fixtures.Fixture): self.addCleanup(zun_policy.init().clear) def set_rules(self, rules): + self._add_default_rules(rules) policy = zun_policy._ENFORCER policy.set_rules({k: _parser.parse_rule(v) for k, v in rules.items()}) + + def _add_default_rules(self, rules): + default_rules = jsonutils.loads(fake_policy.policy_data) + for k, v in default_rules.items(): + rules.setdefault(k, v) diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index c4bce2ff2..e892b16bf 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -472,6 +472,30 @@ class TestContainerController(api_base.FunctionalTest): mock_container_list.assert_called_once_with(mock.ANY, 1000, None, 'id', 'asc', filters=None) + context = mock_container_list.call_args[0][0] + self.assertIs(False, context.all_tenants) + self.assertEqual(200, response.status_int) + actual_containers = response.json['containers'] + self.assertEqual(1, len(actual_containers)) + self.assertEqual(test_container['uuid'], + actual_containers[0].get('uuid')) + + @patch('zun.compute.api.API.container_show') + @patch('zun.objects.Container.list') + def test_get_all_containers_all_tenants(self, mock_container_list, + mock_container_show): + test_container = utils.get_test_container() + containers = [objects.Container(self.context, **test_container)] + mock_container_list.return_value = containers + mock_container_show.return_value = containers[0] + + response = self.app.get('/v1/containers/?all_tenants=1') + + mock_container_list.assert_called_once_with(mock.ANY, + 1000, None, 'id', 'asc', + filters=None) + context = mock_container_list.call_args[0][0] + self.assertIs(True, context.all_tenants) self.assertEqual(200, response.status_int) actual_containers = response.json['containers'] self.assertEqual(1, len(actual_containers)) @@ -1284,6 +1308,25 @@ class TestContainerController(api_base.FunctionalTest): AppError, "Cannot commit container %s in Error state" % uuid): self.app.post('/v1/containers/%s/commit/' % uuid, cmd) + @patch('zun.common.utils.validate_container_state') + @patch('zun.compute.api.API.container_exec_resize') + @patch('zun.api.utils.get_resource') + def test_execute_resize_container_exec( + self, mock_get_resource, mock_exec_resize, mock_validate): + test_container = utils.get_test_container() + test_container_obj = objects.Container(self.context, **test_container) + mock_get_resource.return_value = test_container_obj + container_name = test_container.get('name') + url = '/v1/containers/%s/%s/' % (container_name, 'execute_resize') + fake_exec_id = ('7df36611fa1fc855618c2c643835d41d' + 'ac3fe568e7688f0bae66f7bcb3cccc6c') + kwargs = {'exec_id': fake_exec_id, 'h': '100', 'w': '100'} + response = self.app.post(url, kwargs) + self.assertEqual(200, response.status_int) + mock_exec_resize.assert_called_once_with( + mock.ANY, test_container_obj, fake_exec_id, kwargs['h'], + kwargs['w']) + class TestContainerEnforcement(api_base.FunctionalTest): @@ -1292,13 +1335,19 @@ class TestContainerEnforcement(api_base.FunctionalTest): response = func(*arg, **kwarg) self.assertEqual(403, response.status_int) self.assertEqual('application/json', response.content_type) - self.assertTrue( + self.assertEqual( "Policy doesn't allow %s to be performed." % rule, response.json['errors'][0]['detail']) def test_policy_disallow_get_all(self): self._common_policy_check( - 'container:get_all', self.get_json, '/containers/', + 'container:get_all', self.app.get, '/v1/containers/', + expect_errors=True) + + def test_policy_disallow_get_all_all_tenants(self): + self._common_policy_check( + 'container:get_all_all_tenants', + self.app.get, '/v1/containers/?all_tenants=1', expect_errors=True) def test_policy_disallow_get_one(self): @@ -1388,22 +1437,3 @@ class TestContainerEnforcement(api_base.FunctionalTest): self._owner_check('container:%s' % action, self.post_json, '/containers/%s/%s/' % (container.uuid, action), {}, expect_errors=True) - - @patch('zun.common.utils.validate_container_state') - @patch('zun.compute.api.API.container_exec_resize') - @patch('zun.api.utils.get_resource') - def test_execute_resize_container_exec( - self, mock_get_resource, mock_exec_resize, mock_validate): - test_container = utils.get_test_container() - test_container_obj = objects.Container(self.context, **test_container) - mock_get_resource.return_value = test_container_obj - container_name = test_container.get('name') - url = '/v1/containers/%s/%s/' % (container_name, 'execute_resize') - fake_exec_id = ('7df36611fa1fc855618c2c643835d41d' - 'ac3fe568e7688f0bae66f7bcb3cccc6c') - kwargs = {'exec_id': fake_exec_id, 'h': '100', 'w': '100'} - response = self.app.post(url, kwargs) - self.assertEqual(200, response.status_int) - mock_exec_resize.assert_called_once_with( - mock.ANY, test_container_obj, fake_exec_id, kwargs['h'], - kwargs['w'])