From e2bdfa13f5943af50636ef334f857c410b7888fc Mon Sep 17 00:00:00 2001 From: Hieu LE Date: Fri, 6 Oct 2017 17:42:19 +0700 Subject: [PATCH] Remove unprotected policies in unit test The Zun test suits rely on unprotected policies in order to test things. This patch remove usage of fake unprotected policies and advance to use native policies in code. Change-Id: I3b699991ebed9f88a61825596fc51617a3ce4ed2 Closes-Bug: #1721584 --- zun/tests/fake_policy.py | 69 ------------------- zun/tests/policy_fixture.py | 16 ----- .../controllers/experimental/test_capsules.py | 5 +- .../api/controllers/v1/test_containers.py | 40 ++++++++--- .../unit/api/controllers/v1/test_hosts.py | 13 +++- .../api/controllers/v1/test_zun_service.py | 24 +++++-- 6 files changed, 62 insertions(+), 105 deletions(-) delete mode 100644 zun/tests/fake_policy.py diff --git a/zun/tests/fake_policy.py b/zun/tests/fake_policy.py deleted file mode 100644 index 43e957a96..000000000 --- a/zun/tests/fake_policy.py +++ /dev/null @@ -1,69 +0,0 @@ -# Copyright (c) 2012 OpenStack Foundation -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - - -policy_data = """ -{ - "default": "rule:admin_or_owner", - - "container:create": "", - "container:delete": "", - "container:delete_all_tenants": "", - "container:delete_force": "", - "container:get_one": "", - "container:get_one_all_tenants": "", - "container:get_all": "", - "container:get_all_all_tenants": "", - "container:update": "", - "container:start": "", - "container:stop": "", - "container:reboot": "", - "container:pause": "", - "container:unpause": "", - "container:logs": "", - "container:execute": "", - "container:execute_resize": "", - "container:kill": "", - "container:rename": "", - "container:attach": "", - "container:resize": "", - "container:top": "", - "container:get_archive": "", - "container:put_archive": "", - "container:stats": "", - "container:commit": "", - "container:add_security_group": "", - - "image:pull": "", - "image:get_all": "", - "image:search": "", - - "zun-service:delete": "", - "zun-service:disable": "", - "zun-service:enable": "", - "zun-service:force_down": "", - "zun-service:get_all": "", - - "host:get_all": "", - "host:get": "", - - "capsule:create": "", - "capsule:delete": "", - "capsule:delete_all_tenants": "", - "capsule:get": "", - "capsule:get_one_all_tenants": "", - "capsule:get_all": "", - "capsule:get_all_all_tenants": "" -} -""" diff --git a/zun/tests/policy_fixture.py b/zun/tests/policy_fixture.py index f82107345..760b93b04 100644 --- a/zun/tests/policy_fixture.py +++ b/zun/tests/policy_fixture.py @@ -10,16 +10,12 @@ # License for the specific language governing permissions and limitations # under the License. -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 -from zun.tests import fake_policy CONF = zun.conf.CONF @@ -28,23 +24,11 @@ CONF = zun.conf.CONF class PolicyFixture(fixtures.Fixture): def _setUp(self): - self.policy_dir = self.useFixture(fixtures.TempDir()) - self.policy_file_name = os.path.join(self.policy_dir.path, - 'policy.json') - with open(self.policy_file_name, 'w') as policy_file: - policy_file.write(fake_policy.policy_data) policy_opts.set_defaults(CONF) - CONF.set_override('policy_file', self.policy_file_name, 'oslo_policy') zun_policy._ENFORCER = None 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/experimental/test_capsules.py b/zun/tests/unit/api/controllers/experimental/test_capsules.py index 4d287d097..72368af2b 100644 --- a/zun/tests/unit/api/controllers/experimental/test_capsules.py +++ b/zun/tests/unit/api/controllers/experimental/test_capsules.py @@ -217,6 +217,7 @@ class TestCapsuleController(api_base.FunctionalTest): context = mock_capsule_save.call_args[0][0] self.assertIs(False, context.all_tenants) + @patch('zun.common.policy.enforce') @patch('zun.compute.api.API.capsule_delete') @patch('zun.objects.Capsule.get_by_uuid') @patch('zun.objects.Container.get_by_uuid') @@ -225,7 +226,9 @@ class TestCapsuleController(api_base.FunctionalTest): mock_capsule_save, mock_container_get_by_uuid, mock_capsule_get_by_uuid, - mock_capsule_delete): + mock_capsule_delete, + mock_policy): + mock_policy.return_value = True 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 diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index e70afd29a..85e4955fa 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -247,6 +247,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertIn('status_reason', response.json.keys()) mock_neutron_get_network.assert_called_once() + @patch('zun.common.policy.enforce') @patch('zun.network.neutron.NeutronAPI.get_available_network') @patch('zun.compute.api.API.container_show') @patch('zun.compute.api.API.container_create') @@ -256,7 +257,9 @@ class TestContainerController(api_base.FunctionalTest): mock_container_delete, mock_container_create, mock_container_show, - mock_neutron_get_network): + mock_neutron_get_network, + mock_policy): + mock_policy.return_value = True mock_container_create.side_effect = lambda x, y, **z: y fake_network = {'id': 'foo'} mock_neutron_get_network.return_value = fake_network @@ -593,6 +596,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(1, len(requested_networks)) self.assertEqual(fake_network['id'], requested_networks[0]['network']) + @patch('zun.common.policy.enforce') @patch('zun.network.neutron.NeutronAPI.get_neutron_network') @patch('zun.network.neutron.NeutronAPI.get_neutron_port') @patch('zun.network.neutron.NeutronAPI.ensure_neutron_port_usable') @@ -603,7 +607,8 @@ class TestContainerController(api_base.FunctionalTest): def test_create_container_with_requested_neutron_port( self, mock_search, mock_container_delete, mock_container_create, mock_container_show, mock_ensure_port_usable, mock_get_port, - mock_get_network): + mock_get_network, mock_policy): + mock_policy.return_value = True mock_container_create.side_effect = lambda x, y, **z: y fake_port = {'network_id': 'foo', 'id': 'bar'} fake_private_network = {'router:external': False, 'shared': False} @@ -773,10 +778,12 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(test_container['uuid'], actual_containers[0].get('uuid')) + @patch('zun.common.policy.enforce') @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): + mock_container_show, mock_policy): + mock_policy.return_value = True test_container = utils.get_test_container() containers = [objects.Container(self.context, **test_container)] mock_container_list.return_value = containers @@ -859,10 +866,12 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(consts.UNKNOWN, actual_containers[0].get('status')) + @patch('zun.common.policy.enforce') @patch('zun.compute.api.API.container_show') @patch('zun.objects.Container.get_by_uuid') def test_get_one_by_uuid(self, mock_container_get_by_uuid, - mock_container_show): + mock_container_show, mock_policy): + mock_policy.return_value = True 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 @@ -879,10 +888,12 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(test_container['uuid'], response.json['uuid']) + @patch('zun.common.policy.enforce') @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): + mock_container_show, mock_policy): + mock_policy.return_value = True 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 @@ -1243,12 +1254,14 @@ class TestContainerController(api_base.FunctionalTest): context = mock_container_delete.call_args[0][0] self.assertIs(False, context.all_tenants) + @patch('zun.common.policy.enforce') @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): + mock_validate, mock_policy): + mock_policy.return_value = True test_container = utils.get_test_container() test_container_obj = objects.Container(self.context, **test_container) mock_get_by_uuid.return_value = test_container_obj @@ -1281,8 +1294,11 @@ class TestContainerController(api_base.FunctionalTest): "Cannot delete_force container %s in Paused state" % uuid): self.delete('/v1/containers/%s?force=True' % test_object.uuid) + @patch('zun.common.policy.enforce') @patch('zun.compute.api.API.container_delete') - def test_delete_by_uuid_invalid_state_force_true(self, mock_delete): + def test_delete_by_uuid_invalid_state_force_true(self, mock_delete, + mock_policy): + mock_policy.return_value = True uuid = uuidutils.generate_uuid() test_object = utils.create_test_container(context=self.context, uuid=uuid, status='Running') @@ -1760,7 +1776,9 @@ class TestContainerController(api_base.FunctionalTest): class TestContainerEnforcement(api_base.FunctionalTest): def _common_policy_check(self, rule, func, *arg, **kwarg): - self.policy.set_rules({rule: 'project_id:non_fake'}) + rules = dict({rule: 'project_id:non_fake'}, + **kwarg.pop('bypass_rules', {})) + self.policy.set_rules(rules) response = func(*arg, **kwarg) self.assertEqual(403, response.status_int) self.assertEqual('application/json', response.content_type) @@ -1777,7 +1795,8 @@ class TestContainerEnforcement(api_base.FunctionalTest): self._common_policy_check( 'container:get_all_all_tenants', self.get, '/v1/containers/?all_tenants=1', - expect_errors=True) + expect_errors=True, + bypass_rules={'container:get_all': 'project_id:fake_project'}) def test_policy_disallow_get_one(self): container = obj_utils.create_test_container(self.context) @@ -1830,7 +1849,8 @@ class TestContainerEnforcement(api_base.FunctionalTest): self._common_policy_check( 'container:delete_force', self.delete, '/v1/containers/%s/?force=True' % container.uuid, - expect_errors=True) + expect_errors=True, + bypass_rules={'container:delete': 'project_id:fake_project'}) def _owner_check(self, rule, func, *args, **kwargs): self.policy.set_rules({rule: "user_id:%(user_id)s"}) diff --git a/zun/tests/unit/api/controllers/v1/test_hosts.py b/zun/tests/unit/api/controllers/v1/test_hosts.py index 6f261785e..1ed5ebebf 100644 --- a/zun/tests/unit/api/controllers/v1/test_hosts.py +++ b/zun/tests/unit/api/controllers/v1/test_hosts.py @@ -23,8 +23,10 @@ from zun.tests.unit.db import utils class TestHostController(api_base.FunctionalTest): + @mock.patch('zun.common.policy.enforce') @patch('zun.objects.ComputeNode.list') - def test_get_all_hosts(self, mock_host_list): + def test_get_all_hosts(self, mock_host_list, mock_policy): + mock_policy.return_value = True test_host = utils.get_test_compute_node() numat = numa.NUMATopology._from_dict(test_host['numa_topology']) test_host['numa_topology'] = numat @@ -42,8 +44,11 @@ class TestHostController(api_base.FunctionalTest): self.assertEqual(test_host['uuid'], actual_hosts[0].get('uuid')) + @mock.patch('zun.common.policy.enforce') @patch('zun.objects.ComputeNode.list') - def test_get_all_hosts_with_pagination_marker(self, mock_host_list): + def test_get_all_hosts_with_pagination_marker(self, mock_host_list, + mock_policy): + mock_policy.return_value = True host_list = [] for id_ in range(4): test_host = utils.create_test_compute_node( @@ -63,8 +68,10 @@ class TestHostController(api_base.FunctionalTest): self.assertEqual(host_list[-1].uuid, actual_hosts[0].get('uuid')) + @mock.patch('zun.common.policy.enforce') @patch('zun.objects.ComputeNode.get_by_uuid') - def test_get_one_host(self, mock_get_by_uuid): + def test_get_one_host(self, mock_get_by_uuid, mock_policy): + mock_policy.return_value = True test_host = utils.get_test_compute_node() numat = numa.NUMATopology._from_dict(test_host['numa_topology']) test_host['numa_topology'] = numat diff --git a/zun/tests/unit/api/controllers/v1/test_zun_service.py b/zun/tests/unit/api/controllers/v1/test_zun_service.py index 85b9bebf3..5b1a507fd 100644 --- a/zun/tests/unit/api/controllers/v1/test_zun_service.py +++ b/zun/tests/unit/api/controllers/v1/test_zun_service.py @@ -29,7 +29,9 @@ class DbRec(object): class TestZunServiceController(api_base.FunctionalTest): - def test_empty(self): + @mock.patch('zun.common.policy.enforce') + def test_empty(self, mock_policy): + mock_policy.return_value = True response = self.get_json('/services') self.assertEqual([], response['services']) @@ -42,9 +44,11 @@ class TestZunServiceController(api_base.FunctionalTest): reclist.append(rec) return reclist + @mock.patch('zun.common.policy.enforce') @mock.patch.object(objects.ZunService, 'list') @mock.patch.object(servicegroup.ServiceGroup, 'service_is_up') - def test_get_one(self, svc_up, mock_list): + def test_get_one(self, svc_up, mock_list, mock_policy): + mock_policy.return_value = True mock_list.return_value = self._rpc_api_reply() svc_up.return_value = "up" @@ -52,9 +56,11 @@ class TestZunServiceController(api_base.FunctionalTest): self.assertEqual(1, len(response['services'])) self.assertEqual(1, response['services'][0]['id']) + @mock.patch('zun.common.policy.enforce') @mock.patch.object(objects.ZunService, 'list') @mock.patch.object(servicegroup.ServiceGroup, 'service_is_up') - def test_get_many(self, svc_up, mock_list): + def test_get_many(self, svc_up, mock_list, mock_policy): + mock_policy.return_value = True svc_num = 5 mock_list.return_value = self._rpc_api_reply(svc_num) svc_up.return_value = "up" @@ -65,9 +71,11 @@ class TestZunServiceController(api_base.FunctionalTest): elem = response['services'][i] self.assertEqual(elem['id'], i + 1) + @mock.patch('zun.common.policy.enforce') @mock.patch.object(objects.ZunService, 'get_by_host_and_binary') @mock.patch.object(objects.ZunService, 'update') - def test_enable(self, mock_update, mock_get_host): + def test_enable(self, mock_update, mock_get_host, mock_policy): + mock_policy.return_value = True return_value = { 'service': { 'host': 'fake-host', @@ -81,9 +89,11 @@ class TestZunServiceController(api_base.FunctionalTest): self.assertFalse(response.json['service']['disabled']) self.assertEqual(return_value, response.json) + @mock.patch('zun.common.policy.enforce') @mock.patch.object(objects.ZunService, 'get_by_host_and_binary') @mock.patch.object(objects.ZunService, 'update') - def test_disable(self, mock_update, mock_get_host): + def test_disable(self, mock_update, mock_get_host, mock_policy): + mock_policy.return_value = True return_value = { 'service': { 'host': 'fake-host', @@ -99,9 +109,11 @@ class TestZunServiceController(api_base.FunctionalTest): self.assertEqual('abc', response.json['service']['disabled_reason']) self.assertEqual(return_value, response.json) + @mock.patch('zun.common.policy.enforce') @mock.patch.object(objects.ZunService, 'get_by_host_and_binary') @mock.patch.object(objects.ZunService, 'update') - def test_force_down(self, mock_force_down, mock_get_host): + def test_force_down(self, mock_force_down, mock_get_host, mock_policy): + mock_policy.return_value = True return_value = { 'service': { 'host': 'fake-host',