openstack/nova
Code Issues Proposed changes

Remove policy checkpoints for SecurityGroupAPI and NetworkAPI

Browse Source 
As legacy v2 API removed and v2.1 API check policy at API layer,
those policy checks under API layer are useless. This patch
cleanup them.

Partially implements blueprint remove-legacy-v2-api-code

Change-Id: Id64892e5837a61fa04437c79df01c23e5e1a9b25
This commit is contained in:
He Jie Xu
2016-06-05 17:49:55 +08:00
parent 54a5e73bb9
commit 0cb9182394
4 changed files with 0 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
nova/compute/api.py
View File

@@ -159,27 +159,6 @@ def check_instance_lock(function):
return inner return inner
def policy_decorator(scope):
"""Check corresponding policy prior of wrapped method to execution."""
def outer(func):
@functools.wraps(func)
def wrapped(self, context, target, *args, **kwargs):
if not self.skip_policy_check:
check_policy(context, func.__name__, target, scope)
return func(self, context, target, *args, **kwargs)
return wrapped
return outer
wrap_check_security_groups_policy = policy_decorator(
scope='compute:security_groups')
def check_policy(context, action, target, scope='compute'):
_action = '%s:%s' % (scope, action)
nova.policy.enforce(context, _action, target)
def check_instance_cell(fn): def check_instance_cell(fn):
def _wrapped(self, context, instance, *args, **kwargs): def _wrapped(self, context, instance, *args, **kwargs):
self._validate_cell(instance) self._validate_cell(instance)
@@ -4184,7 +4163,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
return False return False
@wrap_check_security_groups_policy
def add_to_instance(self, context, instance, security_group_name): def add_to_instance(self, context, instance, security_group_name):
"""Add security group to the instance.""" """Add security group to the instance."""
security_group = self.db.security_group_get_by_name(context, security_group = self.db.security_group_get_by_name(context,
@@ -4206,7 +4184,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
self.compute_rpcapi.refresh_instance_security_rules( self.compute_rpcapi.refresh_instance_security_rules(
context, instance.host, instance) context, instance.host, instance)
@wrap_check_security_groups_policy
def remove_from_instance(self, context, instance, security_group_name): def remove_from_instance(self, context, instance, security_group_name):
"""Remove the security group associated with the instance.""" """Remove the security group associated with the instance."""
security_group = self.db.security_group_get_by_name(context, security_group = self.db.security_group_get_by_name(context,

63
nova/network/api.py
View File

@@ -16,8 +16,6 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import functools
from oslo_config import cfg from oslo_config import cfg
from oslo_log import log as logging from oslo_log import log as logging
from oslo_utils import strutils from oslo_utils import strutils
@@ -30,7 +28,6 @@ from nova.network import model as network_model
from nova.network import rpcapi as network_rpcapi from nova.network import rpcapi as network_rpcapi
from nova import objects from nova import objects
from nova.objects import base as obj_base from nova.objects import base as obj_base
from nova import policy
from nova import utils from nova import utils
CONF = cfg.CONF CONF = cfg.CONF
@@ -38,28 +35,6 @@ CONF = cfg.CONF
LOG = logging.getLogger(__name__) LOG = logging.getLogger(__name__)
def wrap_check_policy(func):
"""Check policy corresponding to the wrapped methods prior to execution."""
@functools.wraps(func)
def wrapped(self, context, *args, **kwargs):
action = func.__name__
if not self.skip_policy_check:
check_policy(context, action)
return func(self, context, *args, **kwargs)
return wrapped
def check_policy(context, action):
target = {
'project_id': context.project_id,
'user_id': context.user_id,
}
_action = 'network:%s' % action
policy.enforce(context, _action, target)
class API(base_api.NetworkAPI): class API(base_api.NetworkAPI):
"""API for doing networking via the nova-network network manager. """API for doing networking via the nova-network network manager.
@@ -74,7 +49,6 @@ class API(base_api.NetworkAPI):
self.floating_manager = helper(floating_ips.LocalManager()) self.floating_manager = helper(floating_ips.LocalManager())
super(API, self).__init__(**kwargs) super(API, self).__init__(**kwargs)
@wrap_check_policy
def get_all(self, context): def get_all(self, context):
"""Get all the networks. """Get all the networks.
@@ -94,55 +68,44 @@ class API(base_api.NetworkAPI):
except exception.NoNetworksFound: except exception.NoNetworksFound:
return [] return []
@wrap_check_policy
def get(self, context, network_uuid): def get(self, context, network_uuid):
return objects.Network.get_by_uuid(context, network_uuid) return objects.Network.get_by_uuid(context, network_uuid)
@wrap_check_policy
def create(self, context, **kwargs): def create(self, context, **kwargs):
return self.network_rpcapi.create_networks(context, **kwargs) return self.network_rpcapi.create_networks(context, **kwargs)
@wrap_check_policy
def delete(self, context, network_uuid): def delete(self, context, network_uuid):
network = self.get(context, network_uuid) network = self.get(context, network_uuid)
if network.project_id is not None: if network.project_id is not None:
raise exception.NetworkInUse(network_id=network_uuid) raise exception.NetworkInUse(network_id=network_uuid)
return self.network_rpcapi.delete_network(context, network_uuid, None) return self.network_rpcapi.delete_network(context, network_uuid, None)
@wrap_check_policy
def disassociate(self, context, network_uuid): def disassociate(self, context, network_uuid):
network = self.get(context, network_uuid) network = self.get(context, network_uuid)
objects.Network.disassociate(context, network.id, objects.Network.disassociate(context, network.id,
host=True, project=True) host=True, project=True)
@wrap_check_policy
def get_fixed_ip(self, context, id): def get_fixed_ip(self, context, id):
return objects.FixedIP.get_by_id(context, id) return objects.FixedIP.get_by_id(context, id)
@wrap_check_policy
def get_fixed_ip_by_address(self, context, address): def get_fixed_ip_by_address(self, context, address):
return objects.FixedIP.get_by_address(context, address) return objects.FixedIP.get_by_address(context, address)
@wrap_check_policy
def get_floating_ip(self, context, id): def get_floating_ip(self, context, id):
if not strutils.is_int_like(id): if not strutils.is_int_like(id):
raise exception.InvalidID(id=id) raise exception.InvalidID(id=id)
return objects.FloatingIP.get_by_id(context, id) return objects.FloatingIP.get_by_id(context, id)
@wrap_check_policy
def get_floating_ip_pools(self, context): def get_floating_ip_pools(self, context):
return objects.FloatingIP.get_pool_names(context) return objects.FloatingIP.get_pool_names(context)
@wrap_check_policy
def get_floating_ip_by_address(self, context, address): def get_floating_ip_by_address(self, context, address):
return objects.FloatingIP.get_by_address(context, address) return objects.FloatingIP.get_by_address(context, address)
@wrap_check_policy
def get_floating_ips_by_project(self, context): def get_floating_ips_by_project(self, context):
return objects.FloatingIPList.get_by_project(context, return objects.FloatingIPList.get_by_project(context,
context.project_id) context.project_id)
@wrap_check_policy
def get_instance_id_by_floating_address(self, context, address): def get_instance_id_by_floating_address(self, context, address):
fixed_ip = objects.FixedIP.get_by_floating_address(context, address) fixed_ip = objects.FixedIP.get_by_floating_address(context, address)
if fixed_ip is None: if fixed_ip is None:
@@ -150,7 +113,6 @@ class API(base_api.NetworkAPI):
else: else:
return fixed_ip.instance_uuid return fixed_ip.instance_uuid
@wrap_check_policy
def get_vifs_by_instance(self, context, instance): def get_vifs_by_instance(self, context, instance):
vifs = objects.VirtualInterfaceList.get_by_instance_uuid(context, vifs = objects.VirtualInterfaceList.get_by_instance_uuid(context,
instance.uuid) instance.uuid)
@@ -161,7 +123,6 @@ class API(base_api.NetworkAPI):
vif.net_uuid = network.uuid vif.net_uuid = network.uuid
return vifs return vifs
@wrap_check_policy
def get_vif_by_mac_address(self, context, mac_address): def get_vif_by_mac_address(self, context, mac_address):
vif = objects.VirtualInterface.get_by_address(context, vif = objects.VirtualInterface.get_by_address(context,
mac_address) mac_address)
@@ -171,13 +132,11 @@ class API(base_api.NetworkAPI):
vif.net_uuid = network.uuid vif.net_uuid = network.uuid
return vif return vif
@wrap_check_policy
def allocate_floating_ip(self, context, pool=None): def allocate_floating_ip(self, context, pool=None):
"""Adds (allocates) a floating IP to a project from a pool.""" """Adds (allocates) a floating IP to a project from a pool."""
return self.floating_manager.allocate_floating_ip(context, return self.floating_manager.allocate_floating_ip(context,
context.project_id, False, pool) context.project_id, False, pool)
@wrap_check_policy
def release_floating_ip(self, context, address, def release_floating_ip(self, context, address,
affect_auto_assigned=False): affect_auto_assigned=False):
"""Removes (deallocates) a floating IP with address from a project.""" """Removes (deallocates) a floating IP with address from a project."""
@@ -204,7 +163,6 @@ class API(base_api.NetworkAPI):
# release ip from project # release ip from project
return self.release_floating_ip(context, address) return self.release_floating_ip(context, address)
@wrap_check_policy
@base_api.refresh_cache @base_api.refresh_cache
def associate_floating_ip(self, context, instance, def associate_floating_ip(self, context, instance,
floating_address, fixed_address, floating_address, fixed_address,
@@ -231,7 +189,6 @@ class API(base_api.NetworkAPI):
base_api.update_instance_cache_with_nw_info(self, context, base_api.update_instance_cache_with_nw_info(self, context,
orig_instance) orig_instance)
@wrap_check_policy
@base_api.refresh_cache @base_api.refresh_cache
def disassociate_floating_ip(self, context, instance, address, def disassociate_floating_ip(self, context, instance, address,
affect_auto_assigned=False): affect_auto_assigned=False):
@@ -239,7 +196,6 @@ class API(base_api.NetworkAPI):
return self.floating_manager.disassociate_floating_ip(context, address, return self.floating_manager.disassociate_floating_ip(context, address,
affect_auto_assigned) affect_auto_assigned)
@wrap_check_policy
@base_api.refresh_cache @base_api.refresh_cache
def allocate_for_instance(self, context, instance, vpn, def allocate_for_instance(self, context, instance, vpn,
requested_networks, macs=None, requested_networks, macs=None,
@@ -284,7 +240,6 @@ class API(base_api.NetworkAPI):
return network_model.NetworkInfo.hydrate(nw_info) return network_model.NetworkInfo.hydrate(nw_info)
@wrap_check_policy
def deallocate_for_instance(self, context, instance, def deallocate_for_instance(self, context, instance,
requested_networks=None): requested_networks=None):
"""Deallocates all network structures related to instance.""" """Deallocates all network structures related to instance."""
@@ -316,7 +271,6 @@ class API(base_api.NetworkAPI):
def show_port(self, *args, **kwargs): def show_port(self, *args, **kwargs):
raise NotImplementedError() raise NotImplementedError()
@wrap_check_policy
@base_api.refresh_cache @base_api.refresh_cache
def add_fixed_ip_to_instance(self, context, instance, network_id): def add_fixed_ip_to_instance(self, context, instance, network_id):
"""Adds a fixed IP to instance from specified network.""" """Adds a fixed IP to instance from specified network."""
@@ -329,7 +283,6 @@ class API(base_api.NetworkAPI):
context, **args) context, **args)
return network_model.NetworkInfo.hydrate(nw_info) return network_model.NetworkInfo.hydrate(nw_info)
@wrap_check_policy
@base_api.refresh_cache @base_api.refresh_cache
def remove_fixed_ip_from_instance(self, context, instance, address): def remove_fixed_ip_from_instance(self, context, instance, address):
"""Removes a fixed IP from instance from specified network.""" """Removes a fixed IP from instance from specified network."""
@@ -343,13 +296,11 @@ class API(base_api.NetworkAPI):
context, **args) context, **args)
return network_model.NetworkInfo.hydrate(nw_info) return network_model.NetworkInfo.hydrate(nw_info)
@wrap_check_policy
def add_network_to_project(self, context, project_id, network_uuid=None): def add_network_to_project(self, context, project_id, network_uuid=None):
"""Force adds another network to a project.""" """Force adds another network to a project."""
self.network_rpcapi.add_network_to_project(context, project_id, self.network_rpcapi.add_network_to_project(context, project_id,
network_uuid) network_uuid)
@wrap_check_policy
def associate(self, context, network_uuid, host=base_api.SENTINEL, def associate(self, context, network_uuid, host=base_api.SENTINEL,
project=base_api.SENTINEL): project=base_api.SENTINEL):
"""Associate or disassociate host or project to network.""" """Associate or disassociate host or project to network."""
@@ -369,7 +320,6 @@ class API(base_api.NetworkAPI):
objects.Network.associate(context, project, objects.Network.associate(context, project,
network_id=network.id, force=True) network_id=network.id, force=True)
@wrap_check_policy
def get_instance_nw_info(self, context, instance, **kwargs): def get_instance_nw_info(self, context, instance, **kwargs):
"""Returns all network info related to an instance.""" """Returns all network info related to an instance."""
return super(API, self).get_instance_nw_info(context, instance, return super(API, self).get_instance_nw_info(context, instance,
@@ -386,7 +336,6 @@ class API(base_api.NetworkAPI):
return network_model.NetworkInfo.hydrate(nw_info) return network_model.NetworkInfo.hydrate(nw_info)
@wrap_check_policy
def validate_networks(self, context, requested_networks, num_instances): def validate_networks(self, context, requested_networks, num_instances):
"""validate the networks passed at the time of creating """validate the networks passed at the time of creating
the server. the server.
@@ -415,14 +364,12 @@ class API(base_api.NetworkAPI):
# This is NOOP for Nova network since it doesn't support SR-IOV. # This is NOOP for Nova network since it doesn't support SR-IOV.
pass pass
@wrap_check_policy
def get_dns_domains(self, context): def get_dns_domains(self, context):
"""Returns a list of available dns domains. """Returns a list of available dns domains.
These can be used to create DNS entries for floating IPs. These can be used to create DNS entries for floating IPs.
""" """
return self.network_rpcapi.get_dns_domains(context) return self.network_rpcapi.get_dns_domains(context)
@wrap_check_policy
def add_dns_entry(self, context, address, name, dns_type, domain): def add_dns_entry(self, context, address, name, dns_type, domain):
"""Create specified DNS entry for address.""" """Create specified DNS entry for address."""
args = {'address': address, args = {'address': address,
@@ -431,7 +378,6 @@ class API(base_api.NetworkAPI):
'domain': domain} 'domain': domain}
return self.network_rpcapi.add_dns_entry(context, **args) return self.network_rpcapi.add_dns_entry(context, **args)
@wrap_check_policy
def modify_dns_entry(self, context, name, address, domain): def modify_dns_entry(self, context, name, address, domain):
"""Create specified DNS entry for address.""" """Create specified DNS entry for address."""
args = {'address': address, args = {'address': address,
@@ -439,42 +385,35 @@ class API(base_api.NetworkAPI):
'domain': domain} 'domain': domain}
return self.network_rpcapi.modify_dns_entry(context, **args) return self.network_rpcapi.modify_dns_entry(context, **args)
@wrap_check_policy
def delete_dns_entry(self, context, name, domain): def delete_dns_entry(self, context, name, domain):
"""Delete the specified dns entry.""" """Delete the specified dns entry."""
args = {'name': name, 'domain': domain} args = {'name': name, 'domain': domain}
return self.network_rpcapi.delete_dns_entry(context, **args) return self.network_rpcapi.delete_dns_entry(context, **args)
@wrap_check_policy
def delete_dns_domain(self, context, domain): def delete_dns_domain(self, context, domain):
"""Delete the specified dns domain.""" """Delete the specified dns domain."""
return self.network_rpcapi.delete_dns_domain(context, domain=domain) return self.network_rpcapi.delete_dns_domain(context, domain=domain)
@wrap_check_policy
def get_dns_entries_by_address(self, context, address, domain): def get_dns_entries_by_address(self, context, address, domain):
"""Get entries for address and domain.""" """Get entries for address and domain."""
args = {'address': address, 'domain': domain} args = {'address': address, 'domain': domain}
return self.network_rpcapi.get_dns_entries_by_address(context, **args) return self.network_rpcapi.get_dns_entries_by_address(context, **args)
@wrap_check_policy
def get_dns_entries_by_name(self, context, name, domain): def get_dns_entries_by_name(self, context, name, domain):
"""Get entries for name and domain.""" """Get entries for name and domain."""
args = {'name': name, 'domain': domain} args = {'name': name, 'domain': domain}
return self.network_rpcapi.get_dns_entries_by_name(context, **args) return self.network_rpcapi.get_dns_entries_by_name(context, **args)
@wrap_check_policy
def create_private_dns_domain(self, context, domain, availability_zone): def create_private_dns_domain(self, context, domain, availability_zone):
"""Create a private DNS domain with nova availability zone.""" """Create a private DNS domain with nova availability zone."""
args = {'domain': domain, 'av_zone': availability_zone} args = {'domain': domain, 'av_zone': availability_zone}
return self.network_rpcapi.create_private_dns_domain(context, **args) return self.network_rpcapi.create_private_dns_domain(context, **args)
@wrap_check_policy
def create_public_dns_domain(self, context, domain, project=None): def create_public_dns_domain(self, context, domain, project=None):
"""Create a public DNS domain with optional nova project.""" """Create a public DNS domain with optional nova project."""
args = {'domain': domain, 'project': project} args = {'domain': domain, 'project': project}
return self.network_rpcapi.create_public_dns_domain(context, **args) return self.network_rpcapi.create_public_dns_domain(context, **args)
@wrap_check_policy
def setup_networks_on_host(self, context, instance, host=None, def setup_networks_on_host(self, context, instance, host=None,
teardown=False): teardown=False):
"""Setup or teardown the network structures on hosts related to """Setup or teardown the network structures on hosts related to
@@ -503,7 +442,6 @@ class API(base_api.NetworkAPI):
addresses.append(floating.address) addresses.append(floating.address)
return fixed_ips[0].network.multi_host, addresses return fixed_ips[0].network.multi_host, addresses
@wrap_check_policy
def migrate_instance_start(self, context, instance, migration): def migrate_instance_start(self, context, instance, migration):
"""Start to migrate the network of an instance.""" """Start to migrate the network of an instance."""
flavor = instance.get_flavor() flavor = instance.get_flavor()
@@ -523,7 +461,6 @@ class API(base_api.NetworkAPI):
self.network_rpcapi.migrate_instance_start(context, **args) self.network_rpcapi.migrate_instance_start(context, **args)
@wrap_check_policy
def migrate_instance_finish(self, context, instance, migration): def migrate_instance_finish(self, context, instance, migration):
"""Finish migrating the network of an instance.""" """Finish migrating the network of an instance."""
flavor = instance.get_flavor() flavor = instance.get_flavor()

3
nova/network/security_group/neutron_driver.py
View File

@@ -23,7 +23,6 @@ from oslo_utils import uuidutils
import six import six
from webob import exc from webob import exc
from nova.compute import api as compute_api
from nova import exception from nova import exception
from nova.i18n import _, _LE, _LI, _LW from nova.i18n import _, _LE, _LI, _LW
from nova.network.neutronv2 import api as neutronapi from nova.network.neutronv2 import api as neutronapi
@@ -424,7 +423,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
return port_security_enabled return port_security_enabled
return False return False
@compute_api.wrap_check_security_groups_policy
def add_to_instance(self, context, instance, security_group_name): def add_to_instance(self, context, instance, security_group_name):
"""Add security group to the instance.""" """Add security group to the instance."""
@@ -482,7 +480,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
with excutils.save_and_reraise_exception(): with excutils.save_and_reraise_exception():
LOG.exception(_LE("Neutron Error:")) LOG.exception(_LE("Neutron Error:"))
@compute_api.wrap_check_security_groups_policy
def remove_from_instance(self, context, instance, security_group_name): def remove_from_instance(self, context, instance, security_group_name):
"""Remove the security group associated with the instance.""" """Remove the security group associated with the instance."""
neutron = neutronapi.get_client(context) neutron = neutronapi.get_client(context)

36
nova/tests/unit/network/test_api.py
View File

@@ -19,7 +19,6 @@ import itertools
import uuid import uuid
import mock import mock
from oslo_policy import policy as oslo_policy
from nova.compute import flavors from nova.compute import flavors
from nova import context from nova import context
@@ -31,7 +30,6 @@ from nova.network import floating_ips
from nova.network import model as network_model from nova.network import model as network_model
from nova import objects from nova import objects
from nova.objects import fields from nova.objects import fields
from nova import policy
from nova import test from nova import test
from nova.tests.unit.api.openstack import fakes from nova.tests.unit.api.openstack import fakes
from nova.tests.unit import fake_instance from nova.tests.unit import fake_instance
@@ -51,40 +49,6 @@ fake_info_cache = {
} }
class NetworkPolicyTestCase(test.TestCase):
def setUp(self):
super(NetworkPolicyTestCase, self).setUp()
policy.reset()
policy.init()
self.context = context.get_admin_context()
def tearDown(self):
super(NetworkPolicyTestCase, self).tearDown()
policy.reset()
@mock.patch.object(policy, 'enforce')
def test_check_policy(self, mock_enforce):
target = {
'project_id': self.context.project_id,
'user_id': self.context.user_id,
}
api.check_policy(self.context, 'get_all')
mock_enforce.assert_called_once_with(
self.context, 'network:get_all', target)
def test_skip_policy(self):
policy.reset()
rules = {'network:get_all': '!'}
policy.set_rules(oslo_policy.Rules.from_dict(rules))
api = network.API()
self.assertRaises(exception.PolicyNotAuthorized,
api.get_all, self.context)
api = network.API(skip_policy_check=True)
api.get_all(self.context)
class ApiTestCase(test.TestCase): class ApiTestCase(test.TestCase):
def setUp(self): def setUp(self):
super(ApiTestCase, self).setUp() super(ApiTestCase, self).setUp()