diff --git a/neutron/notifiers/nova.py b/neutron/notifiers/nova.py index c444c19afc6..57f636e0c06 100644 --- a/neutron/notifiers/nova.py +++ b/neutron/notifiers/nova.py @@ -46,6 +46,9 @@ class Notifier(object): # and each Notifier is handling it's own auth. That means that we are # authenticating the exact same thing len(controllers) times. This # should be an easy thing to optimize. + # FIXME(kevinbenton): remove this comment and the one above once the + # switch to pecan is complete since only one notifier is constructed + # in the pecan notification hook. auth = ks_loading.load_auth_from_conf_options(cfg.CONF, 'nova') session = ks_loading.load_session_from_conf_options( diff --git a/neutron/pecan_wsgi/hooks/notifier.py b/neutron/pecan_wsgi/hooks/notifier.py index 7bd2d6506b0..acfe3d56b15 100644 --- a/neutron/pecan_wsgi/hooks/notifier.py +++ b/neutron/pecan_wsgi/hooks/notifier.py @@ -29,9 +29,16 @@ LOG = log.getLogger(__name__) class NotifierHook(hooks.PecanHook): priority = 135 - # TODO(kevinbenton): implement - # ceilo notifier - # nova notifier + # TODO(kevinbenton): implement ceilo notifier + + def _nova_notify(self, action, resource, *args): + action_resource = '%s_%s' % (action, resource) + if not hasattr(self, '_nova_notifier'): + # this is scoped to avoid a dependency on nova client when nova + # notifications aren't enabled + from neutron.notifiers import nova + self._nova_notifier = nova.Notifier() + self._nova_notifier.send_network_change(action_resource, *args) def _notify_dhcp_agent(self, context, resource_name, action, resources): plugin = manager.NeutronManager.get_plugin_for_resource(resource_name) @@ -67,23 +74,38 @@ class NotifierHook(hooks.PecanHook): # The object has been deleted, so we must notify the agent with the # data of the original object data = {collection_name: - state.request.context.get('request_resources', [])} + state.request.context.get('original_resources', [])} else: try: data = jsonutils.loads(state.response.body) except ValueError: if not state.response.body: data = {} - # Send a notification only if a resource can be identified in the - # response. This means that for operations such as add_router_interface - # no notification will be sent - if cfg.CONF.dhcp_agent_notification and data: - resources = [] + resources = [] + if data: if resource_name in data: resources = [data[resource_name]] elif collection_name in data: # This was a bulk request resources = data[collection_name] + # Send a notification only if a resource can be identified in the + # response. This means that for operations such as add_router_interface + # no notification will be sent + if cfg.CONF.dhcp_agent_notification and data: self._notify_dhcp_agent( neutron_context, resource_name, action, resources) + + if cfg.CONF.notify_nova_on_port_data_changes: + orig = {} + if action == 'update': + orig = state.request.context.get('original_resources')[0] + elif action == 'delete': + # NOTE(kevinbenton): the nova notifier is a bit strange because + # it expects the original to be in the last argument on a + # delete rather than in the 'original_obj' position + resources = ( + state.request.context.get('original_resources') or []) + for resource in resources: + self._nova_notify(action, resource_name, orig, + {resource_name: resource}) diff --git a/neutron/pecan_wsgi/hooks/policy_enforcement.py b/neutron/pecan_wsgi/hooks/policy_enforcement.py index c855a1a66b7..582986a3e19 100644 --- a/neutron/pecan_wsgi/hooks/policy_enforcement.py +++ b/neutron/pecan_wsgi/hooks/policy_enforcement.py @@ -36,31 +36,32 @@ def _custom_getter(resource, resource_id): return quota.get_tenant_quotas(resource_id)[quotasv2.RESOURCE_NAME] +def fetch_resource(neutron_context, resource, resource_id): + attrs = v2_attributes.get_resource_info(resource) + if not attrs: + # this isn't a request for a normal resource. it could be + # an action like removing a network from a dhcp agent. + # return None and assume the custom controller for this will + # handle the necessary logic. + return + field_list = [name for (name, value) in attrs.items() + if (value.get('required_by_policy') or + value.get('primary_key') or 'default' not in value)] + plugin = manager.NeutronManager.get_plugin_for_resource(resource) + if plugin: + getter = getattr(plugin, 'get_%s' % resource) + # TODO(kevinbenton): the parent_id logic currently in base.py + return getter(neutron_context, resource_id, fields=field_list) + else: + # Some legit resources, like quota, do not have a plugin yet. + # Retrieving the original object is nevertheless important + # for policy checks. + return _custom_getter(resource, resource_id) + + class PolicyHook(hooks.PecanHook): priority = 140 - def _fetch_resource(self, neutron_context, resource, resource_id): - attrs = v2_attributes.get_resource_info(resource) - if not attrs: - # this isn't a request for a normal resource. it could be - # an action like removing a network from a dhcp agent. - # return None and assume the custom controller for this will - # handle the necessary logic. - return - field_list = [name for (name, value) in attrs.items() - if (value.get('required_by_policy') or - value.get('primary_key') or 'default' not in value)] - plugin = manager.NeutronManager.get_plugin_for_resource(resource) - if plugin: - getter = getattr(plugin, 'get_%s' % resource) - # TODO(kevinbenton): the parent_id logic currently in base.py - return getter(neutron_context, resource_id, fields=field_list) - else: - # Some legit resources, like quota, do not have a plugin yet. - # Retrieving the original object is nevertheless important - # for policy checks. - return _custom_getter(resource, resource_id) - def before(self, state): # This hook should be run only for PUT,POST and DELETE methods and for # requests targeting a neutron resource @@ -87,7 +88,7 @@ class PolicyHook(hooks.PecanHook): # identifier would have been already retrieved by the lookup process; # in the case of DELETE requests there won't be any item to process in # the request body - merged_resources = [] + original_resources = [] if needs_prefetch: try: item = resources_copy.pop() @@ -95,12 +96,12 @@ class PolicyHook(hooks.PecanHook): # Ops... this was a delete after all! item = {} resource_id = state.request.context.get('resource_id') - resource_obj = self._fetch_resource(neutron_context, - resource, resource_id) + resource_obj = fetch_resource(neutron_context, + resource, resource_id) if resource_obj: + original_resources.append(resource_obj) obj = copy.copy(resource_obj) obj.update(item) - merged_resources.append(obj.copy()) obj[const.ATTRIBUTES_TO_UPDATE] = item.keys() # Put back the item in the list so that policies could be # enforced @@ -108,7 +109,7 @@ class PolicyHook(hooks.PecanHook): # TODO(salv-orlando): as other hooks might need to prefetch resources, # store them in the request context. However, this should be done in a # separate hook which is conventietly called before all other hooks - state.request.context['request_resources'] = merged_resources + state.request.context['original_resources'] = original_resources for item in resources_copy: try: policy.enforce( diff --git a/neutron/tests/functional/pecan_wsgi/test_hooks.py b/neutron/tests/functional/pecan_wsgi/test_hooks.py index 1c81681ab0e..a408ef0a1b4 100644 --- a/neutron/tests/functional/pecan_wsgi/test_hooks.py +++ b/neutron/tests/functional/pecan_wsgi/test_hooks.py @@ -23,6 +23,7 @@ from neutron import context from neutron.db.quota import driver as quota_driver from neutron import manager from neutron.pecan_wsgi.controllers import resource +from neutron.pecan_wsgi.hooks import policy_enforcement as pe from neutron import policy from neutron.tests.functional.pecan_wsgi import test_functional @@ -220,7 +221,7 @@ class TestPolicyEnforcementHook(test_functional.PecanFunctionalTest): self.assertNotIn('restricted_attr', json_response['mehs'][0]) -class TestNotifierHook(test_functional.PecanFunctionalTest): +class TestDHCPNotifierHook(test_functional.PecanFunctionalTest): def setUp(self): # the DHCP notifier needs to be mocked so that correct operations can @@ -230,7 +231,7 @@ class TestNotifierHook(test_functional.PecanFunctionalTest): patcher = mock.patch('neutron.api.rpc.agentnotifiers.' 'dhcp_rpc_agent_api.DhcpAgentNotifyAPI.notify') self.mock_notifier = patcher.start() - super(TestNotifierHook, self).setUp() + super(TestDHCPNotifierHook, self).setUp() def test_dhcp_notifications_disabled(self): cfg.CONF.set_override('dhcp_agent_notification', False) @@ -290,3 +291,73 @@ class TestNotifierHook(test_functional.PecanFunctionalTest): self.mock_notifier.assert_has_calls( [mock.call(mock.ANY, {'network': item_1}, 'network.create.end'), mock.call(mock.ANY, {'network': item_2}, 'network.create.end')]) + + +class TestNovaNotifierHook(test_functional.PecanFunctionalTest): + + def setUp(self): + patcher = mock.patch('neutron.pecan_wsgi.hooks.notifier.NotifierHook.' + '_nova_notify') + self.mock_notifier = patcher.start() + super(TestNovaNotifierHook, self).setUp() + + def test_nova_notifications_disabled(self): + cfg.CONF.set_override('notify_nova_on_port_data_changes', False) + self.app.post_json( + '/v2.0/networks.json', + params={'network': {'name': 'meh'}}, + headers={'X-Project-Id': 'tenid'}) + self.assertFalse(self.mock_notifier.called) + + def test_post_put_delete_triggers_notification(self): + req_headers = {'X-Project-Id': 'tenid', 'X-Roles': 'admin'} + response = self.app.post_json( + '/v2.0/networks.json', + params={'network': {'name': 'meh'}}, headers=req_headers) + self.assertEqual(201, response.status_int) + json_body = jsonutils.loads(response.body) + self.mock_notifier.assert_called_once_with('create', 'network', {}, + json_body) + self.mock_notifier.reset_mock() + network_id = json_body['network']['id'] + + # NOTE(kevinbenton): the original passed into the notifier does + # not contain all of the fields of the object. Only those required + # by the policy engine are included. + orig = pe.fetch_resource(context.get_admin_context(), + 'network', network_id) + response = self.app.put_json( + '/v2.0/networks/%s.json' % network_id, + params={'network': {'name': 'meh-2'}}, + headers=req_headers) + self.assertEqual(200, response.status_int) + json_body = jsonutils.loads(response.body) + self.mock_notifier.assert_called_once_with('update', 'network', + orig, json_body) + self.mock_notifier.reset_mock() + + orig = pe.fetch_resource(context.get_admin_context(), + 'network', network_id) + response = self.app.delete( + '/v2.0/networks/%s.json' % network_id, headers=req_headers) + self.assertEqual(204, response.status_int) + # No need to validate data content sent to the notifier as it's just + # going to load the object from the database + self.mock_notifier.assert_called_once_with('delete', 'network', {}, + {'network': orig}) + + def test_bulk_create_triggers_notifications(self): + req_headers = {'X-Project-Id': 'tenid', 'X-Roles': 'admin'} + response = self.app.post_json( + '/v2.0/networks.json', + params={'networks': [{'name': 'meh_1'}, + {'name': 'meh_2'}]}, + headers=req_headers) + self.assertEqual(201, response.status_int) + json_body = jsonutils.loads(response.body) + item_1 = json_body['networks'][0] + item_2 = json_body['networks'][1] + self.assertEqual( + [mock.call('create', 'network', {}, {'network': item_1}), + mock.call('create', 'network', {}, {'network': item_2})], + self.mock_notifier.mock_calls)