diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index 005e715fd..d70ce9280 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -31,6 +31,7 @@ from zun.common import context as zun_context from zun.common import exception from zun.common.i18n import _ from zun.common import name_generator +from zun.common.policies import container as policies from zun.common import policy from zun.common import utils import zun.conf @@ -66,9 +67,10 @@ class ContainerCollection(collection.Collection): @staticmethod def convert_with_links(rpc_containers, limit, url=None, expand=False, **kwargs): + context = pecan.request.context collection = ContainerCollection() collection.containers = \ - [view.format_container(url, p) for p in rpc_containers] + [view.format_container(url, p, context) for p in rpc_containers] collection.next = collection.get_next(limit, url=url, **kwargs) return collection @@ -210,6 +212,8 @@ class ContainersController(base.Controller): filters = {} for filter_key in container_allowed_filters: if filter_key in kwargs: + policy_action = policies.CONTAINER % ('get_one:' + filter_key) + context.can(policy_action, might_not_exist=True) filter_value = kwargs.pop(filter_key) filters[filter_key] = filter_value marker_obj = None @@ -228,9 +232,6 @@ class ContainersController(base.Controller): sort_key, sort_dir, filters=filters) - if not context.is_admin: - for container in containers: - del container.host return ContainerCollection.convert_with_links(containers, limit, url=resource_url, expand=expand, @@ -258,9 +259,8 @@ class ContainersController(base.Controller): except exception.ContainerHostNotUp: raise exception.ServerNotUsable - if not context.is_admin: - del container.host - return view.format_container(pecan.request.host_url, container) + return view.format_container(pecan.request.host_url, container, + context) def _generate_name_for_container(self): """Generate a random name like: zeta-22-container.""" @@ -367,7 +367,8 @@ class ContainersController(base.Controller): pecan.response.location = link.build_url('containers', new_container.uuid) pecan.response.status = 202 - return view.format_container(pecan.request.host_url, new_container) + return view.format_container(pecan.request.host_url, new_container, + context) def _set_default_resource_limit(self, container_dict): # NOTE(kiennt): Default disk size will be set later. @@ -568,7 +569,8 @@ class ContainersController(base.Controller): context = pecan.request.context compute_api = pecan.request.compute_api container = compute_api.container_update(context, container, patch) - return view.format_container(pecan.request.host_url, container) + return view.format_container(pecan.request.host_url, container, + context) @base.Controller.api_version("1.1", "1.13") @pecan.expose('json') @@ -588,7 +590,8 @@ class ContainersController(base.Controller): container.name = name context = pecan.request.context container.save(context) - return view.format_container(pecan.request.host_url, container) + return view.format_container(pecan.request.host_url, container, + context) @pecan.expose('json') @exception.wrap_pecan_controller_exception diff --git a/zun/api/controllers/v1/views/containers_view.py b/zun/api/controllers/v1/views/containers_view.py index 5d1f2d640..a457f4c81 100644 --- a/zun/api/controllers/v1/views/containers_view.py +++ b/zun/api/controllers/v1/views/containers_view.py @@ -14,6 +14,7 @@ import itertools from zun.api.controllers import link +from zun.common.policies import container as policies _basic_keys = ( 'uuid', @@ -49,10 +50,14 @@ _basic_keys = ( ) -def format_container(url, container): +def format_container(url, container, context): def transform(key, value): if key not in _basic_keys: return + # strip the key if it is not allowed by policy + policy_action = policies.CONTAINER % ('get_one:%s' % key) + if not context.can(policy_action, fatal=False, might_not_exist=True): + return if key == 'uuid': yield ('uuid', value) yield ('links', [link.make_link( diff --git a/zun/common/context.py b/zun/common/context.py index 2529fd53f..2398461d1 100644 --- a/zun/common/context.py +++ b/zun/common/context.py @@ -122,7 +122,7 @@ class RequestContext(context.RequestContext): return context - def can(self, action, target=None, fatal=True): + def can(self, action, target=None, fatal=True, might_not_exist=False): """Verifies that the given action is valid on the target in this context. :param action: string representing the action to be checked. @@ -133,6 +133,9 @@ class RequestContext(context.RequestContext): {'project_id': self.project_id, 'user_id': self.user_id} :param fatal: if False, will return False when an exception.NotAuthorized occurs. + :param might_not_exist: If True the policy check is skipped (and the + function returns True) if the specified policy does not exist. + Defaults to false. :raises zun.common.exception.NotAuthorized: if verification fails and fatal is True. @@ -145,7 +148,8 @@ class RequestContext(context.RequestContext): 'user_id': self.user_id} try: - return policy.authorize(self, action, target) + return policy.authorize(self, action, target, + might_not_exist=might_not_exist) except exception.NotAuthorized: if fatal: raise diff --git a/zun/common/policies/container.py b/zun/common/policies/container.py index 98939d527..a80ff1369 100644 --- a/zun/common/policies/container.py +++ b/zun/common/policies/container.py @@ -72,6 +72,29 @@ rules = [ } ] ), + policy.DocumentedRuleDefault( + name=CONTAINER % 'get_one:host', + check_str=base.RULE_ADMIN_API, + description='Retrieve the host field of containers.', + operations=[ + { + 'path': '/v1/containers/{container_ident}', + 'method': 'GET' + }, + { + 'path': '/v1/containers', + 'method': 'GET' + }, + { + 'path': '/v1/containers', + 'method': 'POST' + }, + { + 'path': '/v1/containers/{container_ident}', + 'method': 'PATCH' + } + ] + ), policy.DocumentedRuleDefault( name=CONTAINER % 'get_one_all_projects', check_str=base.RULE_ADMIN_API, diff --git a/zun/common/policy.py b/zun/common/policy.py index 2519730b9..54014a163 100644 --- a/zun/common/policy.py +++ b/zun/common/policy.py @@ -102,7 +102,8 @@ def enforce(context, rule=None, target=None, do_raise=do_raise, exc=exc, *args, **kwargs) -def authorize(context, action, target, do_raise=True, exc=None): +def authorize(context, action, target, do_raise=True, exc=None, + might_not_exist=False): """Verifies that the action is valid on the target in this context. :param context: zun context @@ -115,10 +116,13 @@ def authorize(context, action, target, do_raise=True, exc=None): :param do_raise: if True (the default), raises PolicyNotAuthorized; if False, returns False :param exc: Class of the exception to raise if the check fails. - Any remaining arguments passed to :meth:`authorize` (both - positional and keyword arguments) will be passed to - the exception class. If not specified, - :class:`PolicyNotAuthorized` will be used. + Any remaining arguments passed to :meth:`authorize` (both + positional and keyword arguments) will be passed to + the exception class. If not specified, + :class:`PolicyNotAuthorized` will be used. + :param might_not_exist: If True the policy check is skipped (and the + function returns True) if the specified policy does not exist. + Defaults to false. :raises zun.common.exception.PolicyNotAuthorized: if verification fails and do_raise is True. Or if 'exc' is specified it will raise an @@ -131,6 +135,8 @@ def authorize(context, action, target, do_raise=True, exc=None): credentials = context.to_policy_values() if not exc: exc = exception.PolicyNotAuthorized + if might_not_exist and not (_ENFORCER.rules and action in _ENFORCER.rules): + return True try: result = _ENFORCER.enforce(action, target, credentials, do_raise=do_raise, exc=exc, action=action) diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index 02fd18d72..bc7498e17 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -166,6 +166,29 @@ class TestContainerController(api_base.FunctionalTest): content_type='application/json') self.assertEqual(202, response.status_int) + self.assertNotIn('host', response.json) + self.assertTrue(mock_container_create.called) + self.assertTrue(mock_container_create.call_args[1]['run'] is False) + mock_neutron_get_network.assert_called_once() + + @patch('zun.common.context.RequestContext.can') + @patch('zun.network.neutron.NeutronAPI.get_available_network') + @patch('zun.compute.api.API.container_create') + @patch('zun.compute.api.API.image_search') + def test_create_container_by_admin( + self, mock_search, mock_container_create, mock_neutron_get_network, + mock_can): + mock_container_create.side_effect = lambda x, y, **z: y + mock_can.return_value = True + params = ('{"name": "MyDocker", "image": "ubuntu",' + '"command": "env", "memory": "512",' + '"environment": {"key1": "val1", "key2": "val2"}}') + response = self.post('/v1/containers/', + params=params, + content_type='application/json') + + self.assertEqual(202, response.status_int) + self.assertIn('host', response.json) self.assertTrue(mock_container_create.called) self.assertTrue(mock_container_create.call_args[1]['run'] is False) mock_neutron_get_network.assert_called_once() @@ -269,6 +292,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(20, c.get('disk')) self.assertEqual({"Name": "no", "MaximumRetryCount": "0"}, c.get('restart_policy')) + self.assertNotIn('host', c) requested_networks = \ mock_container_create.call_args[1]['requested_networks'] self.assertEqual(1, len(requested_networks)) @@ -328,6 +352,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertIsNone(c.get('runtime')) self.assertIsNone(c.get('hostname')) self.assertEqual({}, c.get('restart_policy')) + self.assertNotIn('host', c) mock_neutron_get_network.assert_called_once() requested_networks = \ mock_container_create.call_args[1]['requested_networks'] @@ -365,6 +390,7 @@ class TestContainerController(api_base.FunctionalTest): # [1] https://bugs.launchpad.net/zun/+bug/1746401 # self.assertEqual(10, c.get('disk')) self.assertEqual({}, c.get('environment')) + self.assertNotIn('host', c) mock_neutron_get_network.assert_called_once() extra_spec = \ mock_container_create.call_args[1]['extra_spec'] @@ -400,6 +426,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual('512', c.get('memory')) self.assertEqual({"Name": "no", "MaximumRetryCount": "0"}, c.get('restart_policy')) + self.assertNotIn('host', c) mock_neutron_get_network.assert_called_once() requested_networks = \ mock_container_create.call_args[1]['requested_networks'] @@ -436,6 +463,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual('512', c.get('memory')) self.assertEqual({"Name": "no", "MaximumRetryCount": "0"}, c.get('restart_policy')) + self.assertNotIn('host', c) mock_neutron_get_network.assert_called_once() requested_networks = \ mock_container_create.call_args[1]['requested_networks'] @@ -472,6 +500,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual('512', c.get('memory')) self.assertEqual({"Name": "unless-stopped", "MaximumRetryCount": "0"}, c.get('restart_policy')) + self.assertNotIn('host', c) mock_neutron_get_network.assert_called_once() requested_networks = \ mock_container_create.call_args[1]['requested_networks'] @@ -516,6 +545,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual('512', c.get('memory')) self.assertEqual({"key1": "val1", "key2": "val2"}, c.get('environment')) + self.assertNotIn('host', c) requested_networks = \ mock_container_create.call_args[1]['requested_networks'] self.assertEqual(1, len(requested_networks)) @@ -638,6 +668,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual('env', c.get('command')) self.assertEqual('Creating', c.get('status')) self.assertEqual('512', c.get('memory')) + self.assertIn('host', c) requested_networks = \ mock_container_create.call_args[1]['requested_networks'] self.assertEqual(1, len(requested_networks)) @@ -701,6 +732,29 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(1, len(actual_containers)) self.assertEqual(test_container['uuid'], actual_containers[0].get('uuid')) + self.assertNotIn('host', actual_containers[0]) + + @patch('zun.common.context.RequestContext.can') + @patch('zun.objects.Container.list') + def test_get_all_containers_by_admin(self, mock_container_list, mock_can): + mock_can.return_value = True + test_container = utils.get_test_container() + containers = [objects.Container(self.context, **test_container)] + mock_container_list.return_value = containers + + response = self.get('/v1/containers/') + + mock_container_list.assert_called_once_with(mock.ANY, + 1000, None, 'id', 'asc', + filters={}) + context = mock_container_list.call_args[0][0] + self.assertIs(False, context.all_projects) + 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')) + self.assertIn('host', actual_containers[0]) @patch('zun.common.policy.enforce') @patch('zun.objects.Container.list') @@ -776,6 +830,22 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(test_container['uuid'], actual_containers[0].get('uuid')) + @patch('zun.objects.Container.list') + def test_get_all_containers_with_filter_disallow( + self, mock_container_list): + test_container = utils.get_test_container() + containers = [objects.Container(self.context, **test_container)] + mock_container_list.return_value = containers + + response = self.get('/v1/containers/?host=fake-name', + expect_errors=True) + self.assertEqual(403, response.status_int) + self.assertEqual('application/json', response.content_type) + rule = 'container:get_one:host' + self.assertEqual( + "Policy doesn't allow %s to be performed." % rule, + response.json['errors'][0]['detail']) + @patch('zun.objects.Container.list') def test_get_all_containers_with_unknown_parameter( self, mock_container_list): @@ -807,12 +877,10 @@ 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.get_by_uuid') def test_get_one_by_uuid(self, mock_container_get_by_uuid, - mock_container_show, mock_policy): - mock_policy.return_value = True + 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 @@ -828,6 +896,30 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(200, response.status_int) self.assertEqual(test_container['uuid'], response.json['uuid']) + self.assertNotIn('host', response.json) + + @patch('zun.common.context.RequestContext.can') + @patch('zun.compute.api.API.container_show') + @patch('zun.objects.Container.get_by_uuid') + def test_get_one_by_admin(self, mock_container_get_by_uuid, + mock_container_show, mock_can): + mock_can.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 + mock_container_show.return_value = test_container_obj + + response = self.get('/v1/containers/%s/' % 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(False, context.all_projects) + self.assertEqual(200, response.status_int) + self.assertEqual(test_container['uuid'], + response.json['uuid']) + self.assertIn('host', response.json) @patch('zun.common.policy.enforce') @patch('zun.compute.api.API.container_show') @@ -874,6 +966,33 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(200, response.status_int) self.assertTrue(mock_update.called) + self.assertNotIn('host', response.json) + + @patch('zun.common.context.RequestContext.can') + @patch('zun.objects.ComputeNode.get_by_name') + @patch('zun.compute.api.API.container_update') + @patch('zun.objects.Container.get_by_uuid') + def test_patch_by_admin(self, mock_container_get_by_uuid, mock_update, + mock_computenode, mock_can): + mock_can.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 + mock_update.return_value = test_container_obj + test_host = utils.get_test_compute_node() + numa = objects.numa.NUMATopology._from_dict(test_host['numa_topology']) + test_host['numa_topology'] = numa + test_host_obj = objects.ComputeNode(self.context, **test_host) + mock_computenode.return_value = test_host_obj + params = {'cpu': 1} + container_uuid = test_container.get('uuid') + response = self.patch_json( + '/containers/%s/' % container_uuid, + params=params) + + self.assertEqual(200, response.status_int) + self.assertTrue(mock_update.called) + self.assertIn('host', response.json) def _action_test(self, test_container_obj, action, ident_field, mock_container_action, status_code, query_param=''):