openstack/zun
Code Issues Proposed changes

Merge "Use policy for control the 'host' field"

Browse Source
This commit is contained in:
Zuul 2018-05-18 19:29:09 +00:00 committed by Gerrit Code Review
commit 2066587d7a
6 changed files with 181 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
zun/api/controllers/v1/containers.py
View File

@ -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

7
zun/api/controllers/v1/views/containers_view.py
View File

@ -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(

8
zun/common/context.py
View File

@ -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

23
zun/common/policies/container.py
View File

@ -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,

16
zun/common/policy.py
View File

@ -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)

125
zun/tests/unit/api/controllers/v1/test_containers.py
View File

@ -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=''):