From 0cb91823945a9fb9a91c248ebdb592375e90bf08 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Sun, 5 Jun 2016 17:49:55 +0800 Subject: [PATCH] Remove policy checkpoints for SecurityGroupAPI and NetworkAPI 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 --- nova/compute/api.py | 23 ------- nova/network/api.py | 63 ------------------- nova/network/security_group/neutron_driver.py | 3 - nova/tests/unit/network/test_api.py | 36 ----------- 4 files changed, 125 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 831112a3ca14..8d582f82abb5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -159,27 +159,6 @@ def check_instance_lock(function): 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 _wrapped(self, context, instance, *args, **kwargs): self._validate_cell(instance) @@ -4184,7 +4163,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase): return False - @wrap_check_security_groups_policy def add_to_instance(self, context, instance, security_group_name): """Add security group to the instance.""" 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( context, instance.host, instance) - @wrap_check_security_groups_policy def remove_from_instance(self, context, instance, security_group_name): """Remove the security group associated with the instance.""" security_group = self.db.security_group_get_by_name(context, diff --git a/nova/network/api.py b/nova/network/api.py index 6ab0c2b48add..b4260add9de6 100644 --- a/nova/network/api.py +++ b/nova/network/api.py @@ -16,8 +16,6 @@ # License for the specific language governing permissions and limitations # under the License. -import functools - from oslo_config import cfg from oslo_log import log as logging 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 import objects from nova.objects import base as obj_base -from nova import policy from nova import utils CONF = cfg.CONF @@ -38,28 +35,6 @@ CONF = cfg.CONF 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): """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()) super(API, self).__init__(**kwargs) - @wrap_check_policy def get_all(self, context): """Get all the networks. @@ -94,55 +68,44 @@ class API(base_api.NetworkAPI): except exception.NoNetworksFound: return [] - @wrap_check_policy def get(self, context, network_uuid): return objects.Network.get_by_uuid(context, network_uuid) - @wrap_check_policy def create(self, context, **kwargs): return self.network_rpcapi.create_networks(context, **kwargs) - @wrap_check_policy def delete(self, context, network_uuid): network = self.get(context, network_uuid) if network.project_id is not None: raise exception.NetworkInUse(network_id=network_uuid) return self.network_rpcapi.delete_network(context, network_uuid, None) - @wrap_check_policy def disassociate(self, context, network_uuid): network = self.get(context, network_uuid) objects.Network.disassociate(context, network.id, host=True, project=True) - @wrap_check_policy def get_fixed_ip(self, context, id): return objects.FixedIP.get_by_id(context, id) - @wrap_check_policy def get_fixed_ip_by_address(self, context, address): return objects.FixedIP.get_by_address(context, address) - @wrap_check_policy def get_floating_ip(self, context, id): if not strutils.is_int_like(id): raise exception.InvalidID(id=id) return objects.FloatingIP.get_by_id(context, id) - @wrap_check_policy def get_floating_ip_pools(self, context): return objects.FloatingIP.get_pool_names(context) - @wrap_check_policy def get_floating_ip_by_address(self, context, address): return objects.FloatingIP.get_by_address(context, address) - @wrap_check_policy def get_floating_ips_by_project(self, context): return objects.FloatingIPList.get_by_project(context, context.project_id) - @wrap_check_policy def get_instance_id_by_floating_address(self, context, address): fixed_ip = objects.FixedIP.get_by_floating_address(context, address) if fixed_ip is None: @@ -150,7 +113,6 @@ class API(base_api.NetworkAPI): else: return fixed_ip.instance_uuid - @wrap_check_policy def get_vifs_by_instance(self, context, instance): vifs = objects.VirtualInterfaceList.get_by_instance_uuid(context, instance.uuid) @@ -161,7 +123,6 @@ class API(base_api.NetworkAPI): vif.net_uuid = network.uuid return vifs - @wrap_check_policy def get_vif_by_mac_address(self, context, mac_address): vif = objects.VirtualInterface.get_by_address(context, mac_address) @@ -171,13 +132,11 @@ class API(base_api.NetworkAPI): vif.net_uuid = network.uuid return vif - @wrap_check_policy def allocate_floating_ip(self, context, pool=None): """Adds (allocates) a floating IP to a project from a pool.""" return self.floating_manager.allocate_floating_ip(context, context.project_id, False, pool) - @wrap_check_policy def release_floating_ip(self, context, address, affect_auto_assigned=False): """Removes (deallocates) a floating IP with address from a project.""" @@ -204,7 +163,6 @@ class API(base_api.NetworkAPI): # release ip from project return self.release_floating_ip(context, address) - @wrap_check_policy @base_api.refresh_cache def associate_floating_ip(self, context, instance, floating_address, fixed_address, @@ -231,7 +189,6 @@ class API(base_api.NetworkAPI): base_api.update_instance_cache_with_nw_info(self, context, orig_instance) - @wrap_check_policy @base_api.refresh_cache def disassociate_floating_ip(self, context, instance, address, affect_auto_assigned=False): @@ -239,7 +196,6 @@ class API(base_api.NetworkAPI): return self.floating_manager.disassociate_floating_ip(context, address, affect_auto_assigned) - @wrap_check_policy @base_api.refresh_cache def allocate_for_instance(self, context, instance, vpn, requested_networks, macs=None, @@ -284,7 +240,6 @@ class API(base_api.NetworkAPI): return network_model.NetworkInfo.hydrate(nw_info) - @wrap_check_policy def deallocate_for_instance(self, context, instance, requested_networks=None): """Deallocates all network structures related to instance.""" @@ -316,7 +271,6 @@ class API(base_api.NetworkAPI): def show_port(self, *args, **kwargs): raise NotImplementedError() - @wrap_check_policy @base_api.refresh_cache def add_fixed_ip_to_instance(self, context, instance, network_id): """Adds a fixed IP to instance from specified network.""" @@ -329,7 +283,6 @@ class API(base_api.NetworkAPI): context, **args) return network_model.NetworkInfo.hydrate(nw_info) - @wrap_check_policy @base_api.refresh_cache def remove_fixed_ip_from_instance(self, context, instance, address): """Removes a fixed IP from instance from specified network.""" @@ -343,13 +296,11 @@ class API(base_api.NetworkAPI): context, **args) return network_model.NetworkInfo.hydrate(nw_info) - @wrap_check_policy def add_network_to_project(self, context, project_id, network_uuid=None): """Force adds another network to a project.""" self.network_rpcapi.add_network_to_project(context, project_id, network_uuid) - @wrap_check_policy def associate(self, context, network_uuid, host=base_api.SENTINEL, project=base_api.SENTINEL): """Associate or disassociate host or project to network.""" @@ -369,7 +320,6 @@ class API(base_api.NetworkAPI): objects.Network.associate(context, project, network_id=network.id, force=True) - @wrap_check_policy def get_instance_nw_info(self, context, instance, **kwargs): """Returns all network info related to an 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) - @wrap_check_policy def validate_networks(self, context, requested_networks, num_instances): """validate the networks passed at the time of creating the server. @@ -415,14 +364,12 @@ class API(base_api.NetworkAPI): # This is NOOP for Nova network since it doesn't support SR-IOV. pass - @wrap_check_policy def get_dns_domains(self, context): """Returns a list of available dns domains. These can be used to create DNS entries for floating IPs. """ return self.network_rpcapi.get_dns_domains(context) - @wrap_check_policy def add_dns_entry(self, context, address, name, dns_type, domain): """Create specified DNS entry for address.""" args = {'address': address, @@ -431,7 +378,6 @@ class API(base_api.NetworkAPI): 'domain': domain} return self.network_rpcapi.add_dns_entry(context, **args) - @wrap_check_policy def modify_dns_entry(self, context, name, address, domain): """Create specified DNS entry for address.""" args = {'address': address, @@ -439,42 +385,35 @@ class API(base_api.NetworkAPI): 'domain': domain} return self.network_rpcapi.modify_dns_entry(context, **args) - @wrap_check_policy def delete_dns_entry(self, context, name, domain): """Delete the specified dns entry.""" args = {'name': name, 'domain': domain} return self.network_rpcapi.delete_dns_entry(context, **args) - @wrap_check_policy def delete_dns_domain(self, context, domain): """Delete the specified dns domain.""" return self.network_rpcapi.delete_dns_domain(context, domain=domain) - @wrap_check_policy def get_dns_entries_by_address(self, context, address, domain): """Get entries for address and domain.""" args = {'address': address, 'domain': domain} return self.network_rpcapi.get_dns_entries_by_address(context, **args) - @wrap_check_policy def get_dns_entries_by_name(self, context, name, domain): """Get entries for name and domain.""" args = {'name': name, 'domain': domain} return self.network_rpcapi.get_dns_entries_by_name(context, **args) - @wrap_check_policy def create_private_dns_domain(self, context, domain, availability_zone): """Create a private DNS domain with nova availability zone.""" args = {'domain': domain, 'av_zone': availability_zone} return self.network_rpcapi.create_private_dns_domain(context, **args) - @wrap_check_policy def create_public_dns_domain(self, context, domain, project=None): """Create a public DNS domain with optional nova project.""" args = {'domain': domain, 'project': project} return self.network_rpcapi.create_public_dns_domain(context, **args) - @wrap_check_policy def setup_networks_on_host(self, context, instance, host=None, teardown=False): """Setup or teardown the network structures on hosts related to @@ -503,7 +442,6 @@ class API(base_api.NetworkAPI): addresses.append(floating.address) return fixed_ips[0].network.multi_host, addresses - @wrap_check_policy def migrate_instance_start(self, context, instance, migration): """Start to migrate the network of an instance.""" flavor = instance.get_flavor() @@ -523,7 +461,6 @@ class API(base_api.NetworkAPI): self.network_rpcapi.migrate_instance_start(context, **args) - @wrap_check_policy def migrate_instance_finish(self, context, instance, migration): """Finish migrating the network of an instance.""" flavor = instance.get_flavor() diff --git a/nova/network/security_group/neutron_driver.py b/nova/network/security_group/neutron_driver.py index b35ab6a6d702..f5f904779de2 100644 --- a/nova/network/security_group/neutron_driver.py +++ b/nova/network/security_group/neutron_driver.py @@ -23,7 +23,6 @@ from oslo_utils import uuidutils import six from webob import exc -from nova.compute import api as compute_api from nova import exception from nova.i18n import _, _LE, _LI, _LW from nova.network.neutronv2 import api as neutronapi @@ -424,7 +423,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase): return port_security_enabled return False - @compute_api.wrap_check_security_groups_policy def add_to_instance(self, context, instance, security_group_name): """Add security group to the instance.""" @@ -482,7 +480,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase): with excutils.save_and_reraise_exception(): LOG.exception(_LE("Neutron Error:")) - @compute_api.wrap_check_security_groups_policy def remove_from_instance(self, context, instance, security_group_name): """Remove the security group associated with the instance.""" neutron = neutronapi.get_client(context) diff --git a/nova/tests/unit/network/test_api.py b/nova/tests/unit/network/test_api.py index cd3e6d1d00d3..426c0934d54b 100644 --- a/nova/tests/unit/network/test_api.py +++ b/nova/tests/unit/network/test_api.py @@ -19,7 +19,6 @@ import itertools import uuid import mock -from oslo_policy import policy as oslo_policy from nova.compute import flavors from nova import context @@ -31,7 +30,6 @@ from nova.network import floating_ips from nova.network import model as network_model from nova import objects from nova.objects import fields -from nova import policy from nova import test from nova.tests.unit.api.openstack import fakes 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): def setUp(self): super(ApiTestCase, self).setUp()