Add Nova notifier hook calls to pecan
This patch adds a the Nova notifier calls to the pecan notification hook and adds some simple functional tests for them as well. This patch also alters the operations of the policy enforcement hook, which now stores in the request context the original value of the object rather than the value of the object as it would have appeared to the plugin - that is to say a merge between the original objects and the parameters passed on to the request body. Such 'frankenobject' is indeed of no use for the notifiers. Partial-Bug: #1552979 Change-Id: I4a37197eb38afe15d2b368f4e355226824478792
This commit is contained in:
parent
48649d83aa
commit
fc954c9426
@ -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(
|
||||
|
@ -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 = []
|
||||
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})
|
||||
|
@ -36,10 +36,7 @@ def _custom_getter(resource, resource_id):
|
||||
return quota.get_tenant_quotas(resource_id)[quotasv2.RESOURCE_NAME]
|
||||
|
||||
|
||||
class PolicyHook(hooks.PecanHook):
|
||||
priority = 140
|
||||
|
||||
def _fetch_resource(self, neutron_context, resource, resource_id):
|
||||
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
|
||||
@ -61,6 +58,10 @@ class PolicyHook(hooks.PecanHook):
|
||||
# for policy checks.
|
||||
return _custom_getter(resource, resource_id)
|
||||
|
||||
|
||||
class PolicyHook(hooks.PecanHook):
|
||||
priority = 140
|
||||
|
||||
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_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(
|
||||
|
@ -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)
|
||||
|
Loading…
Reference in New Issue
Block a user