Merge "Move policy enforcement into REST API layer for v2.1 servers"
This commit is contained in:
commit
833f405ca7
|
@ -28,6 +28,23 @@
|
|||
"compute:volume_snapshot_delete": "",
|
||||
|
||||
"admin_api": "is_admin:True",
|
||||
"compute:v3:servers:confirm_resize": "rule:admin_or_owner",
|
||||
"compute:v3:servers:create": "",
|
||||
"compute:v3:servers:create:attach_network": "",
|
||||
"compute:v3:servers:create:attach_volume": "",
|
||||
"compute:v3:servers:create:forced_host": "",
|
||||
"compute:v3:servers:delete": "rule:admin_or_owner",
|
||||
"compute:v3:servers:detail": "rule:admin_or_owner",
|
||||
"compute:v3:servers:detail:get_all_tenants": "rule:admin_api",
|
||||
"compute:v3:servers:index": "rule:admin_or_owner",
|
||||
"compute:v3:servers:index:get_all_tenants": "rule:admin_api",
|
||||
"compute:v3:servers:reboot": "rule:admin_or_owner",
|
||||
"compute:v3:servers:rebuild": "rule:admin_or_owner",
|
||||
"compute:v3:servers:resize": "rule:admin_or_owner",
|
||||
"compute:v3:servers:revert_resize": "rule:admin_or_owner",
|
||||
"compute:v3:servers:show": "rule:admin_or_owner",
|
||||
"compute:v3:servers:create_image": "rule:admin_or_owner",
|
||||
"compute:v3:servers:update": "rule:admin_or_owner",
|
||||
"compute:v3:servers:start": "rule:admin_or_owner",
|
||||
"compute:v3:servers:stop": "rule:admin_or_owner",
|
||||
"compute_extension:v3:os-access-ips:discoverable": "",
|
||||
|
|
|
@ -41,7 +41,6 @@ from nova.i18n import _
|
|||
from nova.i18n import _LW
|
||||
from nova.image import glance
|
||||
from nova import objects
|
||||
from nova import policy
|
||||
from nova import utils
|
||||
|
||||
ALIAS = 'servers'
|
||||
|
@ -55,7 +54,7 @@ CONF.import_opt('extensions_blacklist', 'nova.api.openstack', group='osapi_v3')
|
|||
CONF.import_opt('extensions_whitelist', 'nova.api.openstack', group='osapi_v3')
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
authorizer = extensions.core_authorizer('compute:v3', 'servers')
|
||||
authorize = extensions.os_compute_authorizer(ALIAS, True)
|
||||
|
||||
|
||||
class ServersController(wsgi.Controller):
|
||||
|
@ -142,7 +141,7 @@ class ServersController(wsgi.Controller):
|
|||
|
||||
self.extension_info = kwargs.pop('extension_info')
|
||||
super(ServersController, self).__init__(**kwargs)
|
||||
self.compute_api = compute.API()
|
||||
self.compute_api = compute.API(skip_policy_check=True)
|
||||
|
||||
# Look for implementation of extension point of server creation
|
||||
self.create_extension_manager = \
|
||||
|
@ -247,6 +246,8 @@ class ServersController(wsgi.Controller):
|
|||
@extensions.expected_errors((400, 403))
|
||||
def index(self, req):
|
||||
"""Returns a list of server names and ids for a given user."""
|
||||
context = req.environ['nova.context']
|
||||
authorize(context, action="index")
|
||||
try:
|
||||
servers = self._get_servers(req, is_detail=False)
|
||||
except exception.Invalid as err:
|
||||
|
@ -256,6 +257,8 @@ class ServersController(wsgi.Controller):
|
|||
@extensions.expected_errors((400, 403))
|
||||
def detail(self, req):
|
||||
"""Returns a list of server details for a given user."""
|
||||
context = req.environ['nova.context']
|
||||
authorize(context, action="detail")
|
||||
try:
|
||||
servers = self._get_servers(req, is_detail=True)
|
||||
except exception.Invalid as err:
|
||||
|
@ -346,9 +349,10 @@ class ServersController(wsgi.Controller):
|
|||
raise exception.InvalidInput(six.text_type(err))
|
||||
|
||||
if 'all_tenants' in search_opts:
|
||||
policy.enforce(context, 'compute:get_all_tenants',
|
||||
{'project_id': context.project_id,
|
||||
'user_id': context.user_id})
|
||||
if is_detail:
|
||||
authorize(context, action="detail:get_all_tenants")
|
||||
else:
|
||||
authorize(context, action="index:get_all_tenants")
|
||||
del search_opts['all_tenants']
|
||||
else:
|
||||
if context.project_id:
|
||||
|
@ -476,6 +480,7 @@ class ServersController(wsgi.Controller):
|
|||
instance = common.get_instance(self.compute_api, context, id,
|
||||
expected_attrs=['pci_devices',
|
||||
'flavor'])
|
||||
authorize(context, action="show")
|
||||
req.cache_db_instance(instance)
|
||||
return self._view_builder.show(req, instance)
|
||||
|
||||
|
@ -504,6 +509,28 @@ class ServersController(wsgi.Controller):
|
|||
self.create_extension_manager.map(self._create_extension_point,
|
||||
server_dict, create_kwargs, body)
|
||||
|
||||
availability_zone = create_kwargs.get("availability_zone")
|
||||
|
||||
target = {
|
||||
'project_id': context.project_id,
|
||||
'user_id': context.user_id,
|
||||
'availability_zone': availability_zone}
|
||||
authorize(context, target, 'create')
|
||||
|
||||
# TODO(Shao He, Feng) move this policy check to os-availabilty-zone
|
||||
# extension after refactor it.
|
||||
if availability_zone:
|
||||
_dummy, host, node = self.compute_api._handle_availability_zone(
|
||||
context, availability_zone)
|
||||
if host or node:
|
||||
authorize(context, {}, 'create:forced_host')
|
||||
|
||||
block_device_mapping = create_kwargs.get("block_device_mapping")
|
||||
# TODO(Shao He, Feng) move this policy check to os-block-device-mapping
|
||||
# extension after refactor it.
|
||||
if block_device_mapping:
|
||||
authorize(context, target, 'create:attach_volume')
|
||||
|
||||
image_uuid = self._image_from_req_data(server_dict, create_kwargs)
|
||||
|
||||
# NOTE(cyeoh): Although an extension can set
|
||||
|
@ -525,6 +552,9 @@ class ServersController(wsgi.Controller):
|
|||
requested_networks = self._get_requested_networks(
|
||||
requested_networks)
|
||||
|
||||
if requested_networks and len(requested_networks):
|
||||
authorize(context, target, 'create:attach_network')
|
||||
|
||||
try:
|
||||
flavor_id = self._flavor_id_from_req_data(body)
|
||||
except ValueError as error:
|
||||
|
@ -689,6 +719,7 @@ class ServersController(wsgi.Controller):
|
|||
resize_schema['properties']['resize']['properties'].update(schema)
|
||||
|
||||
def _delete(self, context, req, instance_uuid):
|
||||
authorize(context, action='delete')
|
||||
instance = self._get_server(context, req, instance_uuid)
|
||||
if CONF.reclaim_instance_interval:
|
||||
try:
|
||||
|
@ -708,6 +739,7 @@ class ServersController(wsgi.Controller):
|
|||
|
||||
ctxt = req.environ['nova.context']
|
||||
update_dict = {}
|
||||
authorize(ctxt, action='update')
|
||||
|
||||
if 'name' in body['server']:
|
||||
update_dict['display_name'] = body['server']['name']
|
||||
|
@ -722,7 +754,6 @@ class ServersController(wsgi.Controller):
|
|||
# NOTE(mikal): this try block needs to stay because save() still
|
||||
# might throw an exception.
|
||||
req.cache_db_instance(instance)
|
||||
policy.enforce(ctxt, 'compute:update', instance)
|
||||
instance.update(update_dict)
|
||||
instance.save()
|
||||
return self._view_builder.show(req, instance,
|
||||
|
@ -739,6 +770,7 @@ class ServersController(wsgi.Controller):
|
|||
@wsgi.action('confirmResize')
|
||||
def _action_confirm_resize(self, req, id, body):
|
||||
context = req.environ['nova.context']
|
||||
authorize(context, action='confirm_resize')
|
||||
instance = self._get_server(context, req, id)
|
||||
try:
|
||||
self.compute_api.confirm_resize(context, instance)
|
||||
|
@ -756,6 +788,7 @@ class ServersController(wsgi.Controller):
|
|||
@wsgi.action('revertResize')
|
||||
def _action_revert_resize(self, req, id, body):
|
||||
context = req.environ['nova.context']
|
||||
authorize(context, action='revert_resize')
|
||||
instance = self._get_server(context, req, id)
|
||||
try:
|
||||
self.compute_api.revert_resize(context, instance)
|
||||
|
@ -779,6 +812,7 @@ class ServersController(wsgi.Controller):
|
|||
|
||||
reboot_type = body['reboot']['type'].upper()
|
||||
context = req.environ['nova.context']
|
||||
authorize(context, action='reboot')
|
||||
instance = self._get_server(context, req, id)
|
||||
|
||||
try:
|
||||
|
@ -792,6 +826,7 @@ class ServersController(wsgi.Controller):
|
|||
def _resize(self, req, instance_id, flavor_id, **kwargs):
|
||||
"""Begin the resize process with given instance/flavor."""
|
||||
context = req.environ["nova.context"]
|
||||
authorize(context, action='resize')
|
||||
instance = self._get_server(context, req, instance_id)
|
||||
|
||||
try:
|
||||
|
@ -907,6 +942,7 @@ class ServersController(wsgi.Controller):
|
|||
password = self._get_server_admin_password(rebuild_dict)
|
||||
|
||||
context = req.environ['nova.context']
|
||||
authorize(context, action='rebuild')
|
||||
instance = self._get_server(context, req, id)
|
||||
|
||||
attr_map = {
|
||||
|
@ -973,6 +1009,7 @@ class ServersController(wsgi.Controller):
|
|||
def _action_create_image(self, req, id, body):
|
||||
"""Snapshot a server instance."""
|
||||
context = req.environ['nova.context']
|
||||
authorize(context, action='create_image')
|
||||
|
||||
entity = body["createImage"]
|
||||
image_name = entity["name"]
|
||||
|
@ -1051,7 +1088,7 @@ class ServersController(wsgi.Controller):
|
|||
"""Start an instance."""
|
||||
context = req.environ['nova.context']
|
||||
instance = self._get_instance(context, id)
|
||||
authorizer(context, instance, 'start')
|
||||
authorize(context, instance, 'start')
|
||||
LOG.debug('start instance', instance=instance)
|
||||
try:
|
||||
self.compute_api.start(context, instance)
|
||||
|
@ -1068,7 +1105,7 @@ class ServersController(wsgi.Controller):
|
|||
"""Stop an instance."""
|
||||
context = req.environ['nova.context']
|
||||
instance = self._get_instance(context, id)
|
||||
authorizer(context, instance, 'stop')
|
||||
authorize(context, instance, 'stop')
|
||||
LOG.debug('stop instance', instance=instance)
|
||||
try:
|
||||
self.compute_api.stop(context, instance)
|
||||
|
|
|
@ -591,11 +591,6 @@ class API(base.Base):
|
|||
if not availability_zone:
|
||||
availability_zone = CONF.default_schedule_zone
|
||||
|
||||
if forced_host:
|
||||
check_policy(context, 'create:forced_host', {})
|
||||
if forced_node:
|
||||
check_policy(context, 'create:forced_host', {})
|
||||
|
||||
return availability_zone, forced_host, forced_node
|
||||
|
||||
def _ensure_auto_disk_config_is_valid(self, auto_disk_config_img,
|
||||
|
@ -1092,6 +1087,9 @@ class API(base.Base):
|
|||
availability_zone, forced_host, forced_node = handle_az(context,
|
||||
availability_zone)
|
||||
|
||||
if not self.skip_policy_check and (forced_host or forced_node):
|
||||
check_policy(context, 'create:forced_host', {})
|
||||
|
||||
base_options, max_net_count = self._validate_and_build_base_options(
|
||||
context,
|
||||
instance_type, boot_meta, image_href, image_id, kernel_id,
|
||||
|
@ -1414,11 +1412,11 @@ class API(base.Base):
|
|||
if not self.skip_policy_check:
|
||||
check_policy(context, 'create', target)
|
||||
|
||||
if requested_networks and len(requested_networks):
|
||||
check_policy(context, 'create:attach_network', target)
|
||||
if requested_networks and len(requested_networks):
|
||||
check_policy(context, 'create:attach_network', target)
|
||||
|
||||
if block_device_mapping:
|
||||
check_policy(context, 'create:attach_volume', target)
|
||||
if block_device_mapping:
|
||||
check_policy(context, 'create:attach_volume', target)
|
||||
|
||||
def _check_multiple_instances_neutron_ports(self, requested_networks):
|
||||
"""Check whether multiple instances are created from port id(s)."""
|
||||
|
|
|
@ -31,6 +31,7 @@ import six.moves.urllib.parse as urlparse
|
|||
import testtools
|
||||
import webob
|
||||
|
||||
from nova.api.openstack import common
|
||||
from nova.api.openstack import compute
|
||||
from nova.api.openstack.compute import plugins
|
||||
from nova.api.openstack.compute.plugins.v3 import disk_config
|
||||
|
@ -850,12 +851,11 @@ class ServersControllerTest(ControllerTest):
|
|||
fake_get_all)
|
||||
|
||||
rules = {
|
||||
"compute:get_all_tenants":
|
||||
common_policy.parse_rule("project_id:fake"),
|
||||
"compute:get_all":
|
||||
"compute:v3:servers:index":
|
||||
common_policy.parse_rule("project_id:fake"),
|
||||
"compute:v3:servers:index:get_all_tenants":
|
||||
common_policy.parse_rule("project_id:fake")
|
||||
}
|
||||
|
||||
policy.set_rules(rules)
|
||||
|
||||
req = fakes.HTTPRequestV3.blank('/servers?all_tenants=1')
|
||||
|
@ -869,9 +869,9 @@ class ServersControllerTest(ControllerTest):
|
|||
return [fakes.stub_instance(100)]
|
||||
|
||||
rules = {
|
||||
"compute:get_all_tenants":
|
||||
"compute:v3:servers:index:get_all_tenants":
|
||||
common_policy.parse_rule("project_id:non_fake"),
|
||||
"compute:get_all":
|
||||
"compute:v3:servers:get_all":
|
||||
common_policy.parse_rule("project_id:fake"),
|
||||
}
|
||||
|
||||
|
@ -3428,3 +3428,179 @@ class IPsPolicyEnforcementV21(test.NoDBTestCase):
|
|||
self.assertEqual(
|
||||
"Policy doesn't allow %s to be performed." % rule_name,
|
||||
exc.format_message())
|
||||
|
||||
|
||||
class ServersPolicyEnforcementV21(test.NoDBTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(ServersPolicyEnforcementV21, self).setUp()
|
||||
ext_info = plugins.LoadedExtensionInfo()
|
||||
ext_info.extensions.update({'os-networks': 'fake'})
|
||||
self.controller = servers.ServersController(extension_info=ext_info)
|
||||
self.req = fakes.HTTPRequest.blank('')
|
||||
self.image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6'
|
||||
|
||||
def _common_policy_check(self, rules, rule_name, func, *arg, **kwarg):
|
||||
self.policy.set_rules(rules)
|
||||
exc = self.assertRaises(
|
||||
exception.PolicyNotAuthorized, func, *arg, **kwarg)
|
||||
self.assertEqual(
|
||||
"Policy doesn't allow %s to be performed." % rule_name,
|
||||
exc.format_message())
|
||||
|
||||
@mock.patch.object(servers.ServersController, '_get_instance')
|
||||
def test_start_policy_failed(self, _get_instance_mock):
|
||||
_get_instance_mock.return_value = None
|
||||
rule_name = "compute:v3:servers:start"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._start_server,
|
||||
self.req, FAKE_UUID, body={})
|
||||
|
||||
@mock.patch.object(servers.ServersController, '_get_instance')
|
||||
def test_stop_policy_failed(self, _get_instance_mock):
|
||||
_get_instance_mock.return_value = None
|
||||
rule_name = "compute:v3:servers:stop"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._stop_server,
|
||||
self.req, FAKE_UUID, body={})
|
||||
|
||||
def test_index_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:index"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller.index, self.req)
|
||||
|
||||
def test_detail_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:detail"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller.detail, self.req)
|
||||
|
||||
def test_detail_get_tenants_policy_failed(self):
|
||||
req = fakes.HTTPRequest.blank('')
|
||||
req.GET["all_tenants"] = "True"
|
||||
rule_name = "compute:v3:servers:detail:get_all_tenants"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._get_servers, req, True)
|
||||
|
||||
def test_index_get_tenants_policy_failed(self):
|
||||
req = fakes.HTTPRequest.blank('')
|
||||
req.GET["all_tenants"] = "True"
|
||||
rule_name = "compute:v3:servers:index:get_all_tenants"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._get_servers, req, False)
|
||||
|
||||
@mock.patch.object(common, 'get_instance')
|
||||
def test_show_policy_failed(self, get_instance_mock):
|
||||
get_instance_mock.return_value = None
|
||||
rule_name = "compute:v3:servers:show"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller.show, self.req, FAKE_UUID)
|
||||
|
||||
def test_delete_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:delete"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller.delete, self.req, FAKE_UUID)
|
||||
|
||||
def test_update_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:update"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
body = {'server': {'name': 'server_test'}}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller.update, self.req,
|
||||
FAKE_UUID, body=body)
|
||||
|
||||
def test_confirm_resize_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:confirm_resize"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
body = {'server': {'name': 'server_test'}}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._action_confirm_resize,
|
||||
self.req, FAKE_UUID, body=body)
|
||||
|
||||
def test_revert_resize_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:revert_resize"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
body = {'server': {'name': 'server_test'}}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._action_revert_resize,
|
||||
self.req, FAKE_UUID, body=body)
|
||||
|
||||
def test_reboot_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:reboot"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
body = {'reboot': {'type': 'HARD'}}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._action_reboot,
|
||||
self.req, FAKE_UUID, body=body)
|
||||
|
||||
def test_resize_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:resize"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
flavor_id = 1
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._resize, self.req,
|
||||
FAKE_UUID, flavor_id)
|
||||
|
||||
def test_create_image_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:create_image"
|
||||
rule = {rule_name: "project:non_fake"}
|
||||
body = {
|
||||
'createImage': {
|
||||
'name': 'Snapshot 1',
|
||||
},
|
||||
}
|
||||
self._common_policy_check(
|
||||
rule, rule_name, self.controller._action_create_image,
|
||||
self.req, FAKE_UUID, body=body)
|
||||
|
||||
def _create_policy_check(self, rules, rule_name):
|
||||
flavor_ref = 'http://localhost/123/flavors/3'
|
||||
body = {
|
||||
'server': {
|
||||
'name': 'server_test',
|
||||
'imageRef': self.image_uuid,
|
||||
'flavorRef': flavor_ref,
|
||||
'availability_zone': "zone1:host1:node1",
|
||||
'block_device_mapping': [{'device_name': "/dev/sda1"}],
|
||||
'networks': [{'uuid': 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'}],
|
||||
'metadata': {
|
||||
'hello': 'world',
|
||||
'open': 'stack',
|
||||
},
|
||||
},
|
||||
}
|
||||
self._common_policy_check(
|
||||
rules, rule_name, self.controller.create, self.req, body=body)
|
||||
|
||||
def test_create_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:create"
|
||||
rules = {rule_name: "project:non_fake"}
|
||||
self._create_policy_check(rules, rule_name)
|
||||
|
||||
def test_create_forced_host_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:create:forced_host"
|
||||
rule = {"compute:v3:servers:create": "@",
|
||||
rule_name: "project:non_fake"}
|
||||
self._create_policy_check(rule, rule_name)
|
||||
|
||||
def test_create_attach_volume_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:create:attach_volume"
|
||||
rules = {"compute:v3:servers:create": "@",
|
||||
"compute:v3:servers:create:forced_host": "@",
|
||||
rule_name: "project:non_fake"}
|
||||
self._create_policy_check(rules, rule_name)
|
||||
|
||||
def test_create_attach_attach_network_policy_failed(self):
|
||||
rule_name = "compute:v3:servers:create:attach_network"
|
||||
rules = {"compute:v3:servers:create": "@",
|
||||
"compute:v3:servers:create:forced_host": "@",
|
||||
"compute:v3:servers:create:attach_volume": "@",
|
||||
rule_name: "project:non_fake"}
|
||||
self._create_policy_check(rules, rule_name)
|
||||
|
|
|
@ -104,6 +104,23 @@ policy_data = """
|
|||
"compute:volume_snapshot_create": "",
|
||||
"compute:volume_snapshot_delete": "",
|
||||
|
||||
"compute:v3:servers:confirm_resize": "",
|
||||
"compute:v3:servers:create": "",
|
||||
"compute:v3:servers:create:attach_network": "",
|
||||
"compute:v3:servers:create:attach_volume": "",
|
||||
"compute:v3:servers:create:forced_host": "",
|
||||
"compute:v3:servers:delete": "",
|
||||
"compute:v3:servers:detail": "",
|
||||
"compute:v3:servers:detail:get_all_tenants": "",
|
||||
"compute:v3:servers:index": "",
|
||||
"compute:v3:servers:index:get_all_tenants": "",
|
||||
"compute:v3:servers:reboot": "",
|
||||
"compute:v3:servers:rebuild": "",
|
||||
"compute:v3:servers:resize": "",
|
||||
"compute:v3:servers:revert_resize": "",
|
||||
"compute:v3:servers:show": "",
|
||||
"compute:v3:servers:create_image": "",
|
||||
"compute:v3:servers:update": "",
|
||||
"compute:v3:servers:start": "",
|
||||
"compute:v3:servers:stop": "",
|
||||
"compute_extension:v3:os-access-ips": "",
|
||||
|
|
Loading…
Reference in New Issue