Add webhook v2 support
Previously webhook API introduced microversion 1.10 to allow callers to pass arbritary data in the body along with the webhook call. This was done so that webhooks would work with aodh again. However, aodh and most webhook callers cannot pass in the header necessary to specify the microversion. Thus, I'm introducing webhook v2 here so that webhooks behave like in microversion 1.10 but without the need to specify that microversion header. Closes-Bug: #1828856 Change-Id: I7f5cdb2ea19c6ca1a9ea68e64515804f219a73ee
This commit is contained in:
parent
4764595ac7
commit
7775928abc
|
@ -7,7 +7,7 @@
|
|||
]
|
||||
},
|
||||
"channel": {
|
||||
"alarm_url": "http://node1:8778/v1/webhooks/e03dd2e5-8f2e-4ec1-8c6a-74ba891e5422/trigger?V=1&count=1"
|
||||
"alarm_url": "http://node1:8778/v1/webhooks/e03dd2e5-8f2e-4ec1-8c6a-74ba891e5422/trigger?V=2&count=1"
|
||||
},
|
||||
"cluster_id": "ae63a10b-4a90-452c-aef1-113a0b255ee3",
|
||||
"created_at": "2015-06-27T05:09:43",
|
||||
|
|
|
@ -7,7 +7,7 @@
|
|||
]
|
||||
},
|
||||
"channel": {
|
||||
"alarm_url": "http://node1:8778/v1/webhooks/e03dd2e5-8f2e-4ec1-8c6a-74ba891e5422/trigger?V=1&count=1"
|
||||
"alarm_url": "http://node1:8778/v1/webhooks/e03dd2e5-8f2e-4ec1-8c6a-74ba891e5422/trigger?V=2&count=1"
|
||||
},
|
||||
"cluster_id": "ae63a10b-4a90-452c-aef1-113a0b255ee3",
|
||||
"created_at": "2015-06-27T05:09:43",
|
||||
|
|
|
@ -7,7 +7,7 @@
|
|||
]
|
||||
},
|
||||
"channel": {
|
||||
"alarm_url": "http://node1:8778/v1/webhooks/e03dd2e5-8f2e-4ec1-8c6a-74ba891e5422/trigger?V=1&count=2"
|
||||
"alarm_url": "http://node1:8778/v1/webhooks/e03dd2e5-8f2e-4ec1-8c6a-74ba891e5422/trigger?V=2&count=2"
|
||||
},
|
||||
"cluster_id": "ae63a10b-4a90-452c-aef1-113a0b255ee3",
|
||||
"created_at": "2015-06-27T05:09:43",
|
||||
|
|
|
@ -8,7 +8,7 @@
|
|||
]
|
||||
},
|
||||
"channel": {
|
||||
"alarm_url": "http://node1:8778/v1/webhooks/e03dd2e5-8f2e-4ec1-8c6a-74ba891e5422/trigger?V=1&count=1"
|
||||
"alarm_url": "http://node1:8778/v1/webhooks/e03dd2e5-8f2e-4ec1-8c6a-74ba891e5422/trigger?V=2&count=1"
|
||||
},
|
||||
"cluster_id": "ae63a10b-4a90-452c-aef1-113a0b255ee3",
|
||||
"created_at": "2015-06-27T05:09:43",
|
||||
|
|
|
@ -78,9 +78,9 @@ When the Senlin API service receives the request, it does three things:
|
|||
a cluster action. For the ``webhook`` receiver, this is a URL stored in
|
||||
the ``alarm_url`` field and it looks like::
|
||||
|
||||
http://{host:port}/v1/webhooks/{webhook_id}/trigger?V=1
|
||||
http://{host:port}/v1/webhooks/{webhook_id}/trigger?V=2
|
||||
|
||||
**NOTE**: The ``V=1`` above is used to encode the current webhook triggering
|
||||
**NOTE**: The ``V=2`` above is used to encode the current webhook triggering
|
||||
protocol. When the protocol changes in future, the value will be changed
|
||||
accordingly.
|
||||
|
||||
|
|
|
@ -145,7 +145,7 @@ by two nodes every time it is triggered:
|
|||
| | "trust_id": "432f81d339444cac959bab2fd9ba92fa" |
|
||||
| | } |
|
||||
| channel | { |
|
||||
| | "alarm_url": "http://node1:8778/v1/webhooks/ba...5a/trigger?V=1&count=2 |
|
||||
| | "alarm_url": "http://node1:8778/v1/webhooks/ba...5a/trigger?V=2&count=2 |
|
||||
| | } |
|
||||
| cluster_id | b75d25e7-e84d-4742-abf7-d8a3001e25a9 |
|
||||
| created_at | 2016-08-01T02:17:14Z |
|
||||
|
@ -191,7 +191,7 @@ triggering. For convenience, we export that value to an environment variable:
|
|||
|
||||
.. code-block:: console
|
||||
|
||||
$ export ALRM_URL01="http://node1:8778/v1/webhooks/ba...5a/trigger?V=1&count=2"
|
||||
$ export ALRM_URL01="http://node1:8778/v1/webhooks/ba...5a/trigger?V=2&count=2"
|
||||
|
||||
Similar to the example above, you can create other receivers for different
|
||||
kinds of cluster operations or the same cluster operation with different
|
||||
|
|
|
@ -48,7 +48,7 @@ The output from the command will be something like this:
|
|||
| | "trust_id": "1bc958f5780b4ad38fb6583701a9f39b" |
|
||||
| | } |
|
||||
| channel | { |
|
||||
| | "alarm_url": "http://node1:8778/v1/webhooks/5dacde18-.../trigger?V=1" |
|
||||
| | "alarm_url": "http://node1:8778/v1/webhooks/5dacde18-.../trigger?V=2" |
|
||||
| | } |
|
||||
| cluster_id | 7fb3d988-3bc1-4539-bd5d-3f72e8d6e0c7 |
|
||||
| created_at | 2016-05-23T01:36:39 |
|
||||
|
@ -79,7 +79,7 @@ action using tools like ``curl``.
|
|||
|
||||
.. code-block:: console
|
||||
|
||||
$ curl -X POST http://node1:8778/v1/webhooks/5dacde18-661e-4db4-b7a8-f2a6e3466f98/trigger?V=1
|
||||
$ curl -X POST http://node1:8778/v1/webhooks/5dacde18-661e-4db4-b7a8-f2a6e3466f98/trigger?V=2
|
||||
|
||||
After a while, you can check that the cluster has been shrunk by 1 node.
|
||||
|
||||
|
|
|
@ -0,0 +1,12 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixes bug where the webhook rejected additional parameters in the body for
|
||||
mircoversion less than 1.10. Now with new webhook version 2, additional
|
||||
parameters in the body will always be accepted regardless of the
|
||||
microversion API passed in.
|
||||
other:
|
||||
- |
|
||||
Introduces webhook version 2 that is returned when creating new webhook
|
||||
receivers. Webhook version 1 receivers are still valid and will
|
||||
continue to be accepted.
|
|
@ -30,11 +30,18 @@ class WebhookController(wsgi.Controller):
|
|||
if body is None:
|
||||
body = {'params': None}
|
||||
|
||||
body = obj_base.SenlinObject.normalize_req(
|
||||
'WebhookTriggerRequestBody', body)
|
||||
obj = util.parse_request(
|
||||
'WebhookTriggerRequest', req, {'identity': webhook_id,
|
||||
'body': body})
|
||||
webhook_version = req.params.getall('V')
|
||||
if webhook_version == ['1']:
|
||||
body = obj_base.SenlinObject.normalize_req(
|
||||
'WebhookTriggerRequestBody', body)
|
||||
obj = util.parse_request(
|
||||
'WebhookTriggerRequest', req, {'identity': webhook_id,
|
||||
'body': body})
|
||||
else:
|
||||
# webhook version 2 and greater accept parameters other than param
|
||||
obj = util.parse_request(
|
||||
'WebhookTriggerRequestParamsInBody', req,
|
||||
{'identity': webhook_id, 'body': body})
|
||||
|
||||
res = self.rpc_client.call(req.context, 'webhook_trigger', obj)
|
||||
location = {'location': '/actions/%s' % res['action']}
|
||||
|
|
|
@ -25,6 +25,7 @@ CONF = cfg.CONF
|
|||
|
||||
class Webhook(base.Receiver):
|
||||
"""Webhook flavor of receivers."""
|
||||
WEBHOOK_VERSION = 2
|
||||
|
||||
def initialize_channel(self, context):
|
||||
host = CONF.receiver.host
|
||||
|
@ -52,9 +53,11 @@ class Webhook(base.Receiver):
|
|||
if self.params:
|
||||
normalized = sorted(self.params.items(), key=lambda d: d[0])
|
||||
qstr = parse.urlencode(normalized)
|
||||
url = "".join([base, webhook, '?V=1&', qstr])
|
||||
url = "".join(
|
||||
[base, webhook, '?V={}&'.format(self.WEBHOOK_VERSION), qstr])
|
||||
else:
|
||||
url = "".join([base, webhook, '?V=1'])
|
||||
url = "".join(
|
||||
[base, webhook, '?V={}'.format(self.WEBHOOK_VERSION)])
|
||||
|
||||
self.channel = {
|
||||
'alarm_url': url
|
||||
|
|
|
@ -39,7 +39,7 @@ class TestWebhookMiddleware(base.SenlinTestCase):
|
|||
'01_webhook_str': '/webhooks/',
|
||||
'02_webhook_id': 'WEBHOOK_ID',
|
||||
'03_trigger_str': '/trigger?',
|
||||
'04_version': 'V=1',
|
||||
'04_version': 'V=2',
|
||||
'05_params': '&key=TEST_KEY',
|
||||
}
|
||||
self.credential = {
|
||||
|
|
|
@ -16,7 +16,6 @@ from webob import exc
|
|||
|
||||
from oslo_serialization import jsonutils
|
||||
|
||||
from senlin.api.common import util
|
||||
from senlin.api.openstack.v1 import webhooks
|
||||
from senlin.common import policy
|
||||
from senlin.rpc import client as rpc_client
|
||||
|
@ -25,9 +24,12 @@ from senlin.tests.unit.common import base
|
|||
|
||||
|
||||
@mock.patch.object(policy, 'enforce')
|
||||
class WebhookControllerTest(shared.ControllerTest, base.SenlinTestCase):
|
||||
class WebhookControllerBaseTest(shared.ControllerTest, base.SenlinTestCase):
|
||||
WEBHOOK_VERSION = '1'
|
||||
WEBHOOK_API_MICROVERSION = '1.0'
|
||||
|
||||
def setUp(self):
|
||||
super(WebhookControllerTest, self).setUp()
|
||||
super(WebhookControllerBaseTest, self).setUp()
|
||||
|
||||
class DummyConfig(object):
|
||||
bind_port = 8778
|
||||
|
@ -35,9 +37,8 @@ class WebhookControllerTest(shared.ControllerTest, base.SenlinTestCase):
|
|||
cfgopts = DummyConfig()
|
||||
self.controller = webhooks.WebhookController(options=cfgopts)
|
||||
|
||||
@mock.patch.object(util, 'parse_request')
|
||||
@mock.patch.object(rpc_client.EngineClient, 'call')
|
||||
def test_webhook_trigger(self, mock_call, mock_parse, mock_enforce):
|
||||
def test_webhook_trigger(self, mock_call, mock_enforce):
|
||||
self._mock_enforce_setup(mock_enforce, 'trigger', True)
|
||||
body = None
|
||||
webhook_id = 'test_webhook_id'
|
||||
|
@ -48,23 +49,18 @@ class WebhookControllerTest(shared.ControllerTest, base.SenlinTestCase):
|
|||
}
|
||||
|
||||
req = self._post('/webhooks/test_webhook_id/trigger',
|
||||
jsonutils.dumps(body), version='1.10')
|
||||
jsonutils.dumps(body),
|
||||
version=self.WEBHOOK_API_MICROVERSION,
|
||||
params={'V': self.WEBHOOK_VERSION})
|
||||
mock_call.return_value = engine_response
|
||||
obj = mock.Mock()
|
||||
mock_parse.return_value = obj
|
||||
|
||||
resp = self.controller.trigger(req, webhook_id=webhook_id, body=None)
|
||||
|
||||
self.assertEqual(action_id, resp['action'])
|
||||
self.assertEqual('/actions/test_action_id', resp['location'])
|
||||
mock_parse.assert_called_once_with(
|
||||
'WebhookTriggerRequestParamsInBody', req, mock.ANY)
|
||||
mock_call.assert_called_once_with(req.context, 'webhook_trigger', obj)
|
||||
|
||||
@mock.patch.object(util, 'parse_request')
|
||||
@mock.patch.object(rpc_client.EngineClient, 'call')
|
||||
def test_webhook_trigger_with_params(self, mock_call, mock_parse,
|
||||
mock_enforce):
|
||||
def test_webhook_trigger_with_params(self, mock_call, mock_enforce):
|
||||
self._mock_enforce_setup(mock_enforce, 'trigger', True)
|
||||
body = {'params': {'key': 'value'}}
|
||||
webhook_id = 'test_webhook_id'
|
||||
|
@ -72,50 +68,106 @@ class WebhookControllerTest(shared.ControllerTest, base.SenlinTestCase):
|
|||
engine_response = {'action': 'FAKE_ACTION'}
|
||||
|
||||
req = self._post('/webhooks/test_webhook_id/trigger',
|
||||
jsonutils.dumps(body), version='1.10')
|
||||
jsonutils.dumps(body),
|
||||
version=self.WEBHOOK_API_MICROVERSION,
|
||||
params={'V': self.WEBHOOK_VERSION})
|
||||
mock_call.return_value = engine_response
|
||||
obj = mock.Mock()
|
||||
mock_parse.return_value = obj
|
||||
|
||||
resp = self.controller.trigger(req, webhook_id=webhook_id, body=body)
|
||||
|
||||
self.assertEqual('FAKE_ACTION', resp['action'])
|
||||
self.assertEqual('/actions/FAKE_ACTION', resp['location'])
|
||||
mock_parse.assert_called_once_with(
|
||||
'WebhookTriggerRequestParamsInBody', req, mock.ANY)
|
||||
mock_call.assert_called_once_with(req.context, 'webhook_trigger', obj)
|
||||
|
||||
@mock.patch.object(util, 'parse_request')
|
||||
|
||||
class WebhookV1ControllerInvalidParamsTest(WebhookControllerBaseTest):
|
||||
WEBHOOK_VERSION = '1'
|
||||
WEBHOOK_API_MICROVERSION = '1.0'
|
||||
|
||||
@mock.patch.object(policy, 'enforce')
|
||||
@mock.patch.object(rpc_client.EngineClient, 'call')
|
||||
def test_webhook_trigger_invalid_params(self, mock_call, mock_parse,
|
||||
mock_enforce):
|
||||
def test_webhook_trigger_invalid_params(self, mock_call, mock_enforce):
|
||||
self._mock_enforce_setup(mock_enforce, 'trigger', True)
|
||||
webhook_id = 'fake'
|
||||
body = {"bad": "boo"}
|
||||
req = self._patch('/webhooks/%s/trigger' % webhook_id,
|
||||
jsonutils.dumps(body))
|
||||
req = self._patch('/webhooks/{}/trigger'.format(webhook_id),
|
||||
jsonutils.dumps(body),
|
||||
version=self.WEBHOOK_API_MICROVERSION,
|
||||
params={'V': self.WEBHOOK_VERSION})
|
||||
|
||||
mock_parse.side_effect = exc.HTTPBadRequest("bad param")
|
||||
ex = self.assertRaises(exc.HTTPBadRequest,
|
||||
self.controller.trigger,
|
||||
req, webhook_id=webhook_id, body=body)
|
||||
|
||||
self.assertEqual("bad param", six.text_type(ex))
|
||||
self.assertEqual(
|
||||
"Additional properties are not allowed ('bad' was unexpected)",
|
||||
six.text_type(ex))
|
||||
self.assertFalse(mock_call.called)
|
||||
|
||||
@mock.patch.object(util, 'parse_request')
|
||||
@mock.patch.object(policy, 'enforce')
|
||||
@mock.patch.object(rpc_client.EngineClient, 'call')
|
||||
def test_webhook_trigger_invalid_json(self, mock_call, mock_parse,
|
||||
mock_enforce):
|
||||
def test_webhook_trigger_invalid_json(self, mock_call, mock_enforce):
|
||||
self._mock_enforce_setup(mock_enforce, 'trigger', True)
|
||||
webhook_id = 'fake'
|
||||
body = {"params": "boo"}
|
||||
req = self._patch('/webhooks/%s/trigger' % webhook_id,
|
||||
jsonutils.dumps(body))
|
||||
req = self._patch('/webhooks/{}/trigger'.format(webhook_id),
|
||||
jsonutils.dumps(body),
|
||||
version=self.WEBHOOK_API_MICROVERSION,
|
||||
params={'V': self.WEBHOOK_VERSION})
|
||||
|
||||
mock_parse.side_effect = exc.HTTPBadRequest("invalid param")
|
||||
ex = self.assertRaises(exc.HTTPBadRequest,
|
||||
self.controller.trigger,
|
||||
req, webhook_id=webhook_id, body=body)
|
||||
self.assertEqual("invalid param", six.text_type(ex))
|
||||
self.assertEqual("The value (boo) is not a valid JSON.",
|
||||
six.text_type(ex))
|
||||
self.assertFalse(mock_call.called)
|
||||
|
||||
|
||||
class WebhookV1ControllerValidParamsTest(WebhookControllerBaseTest):
|
||||
WEBHOOK_VERSION = '1'
|
||||
WEBHOOK_API_MICROVERSION = '1.10'
|
||||
|
||||
@mock.patch.object(policy, 'enforce')
|
||||
@mock.patch.object(rpc_client.EngineClient, 'call')
|
||||
def test_webhook_trigger_extra_params(self, mock_call, mock_enforce):
|
||||
self._mock_enforce_setup(mock_enforce, 'trigger', True)
|
||||
webhook_id = 'fake'
|
||||
body = {"bad": "boo"}
|
||||
engine_response = {'action': 'FAKE_ACTION'}
|
||||
mock_call.return_value = engine_response
|
||||
req = self._patch('/webhooks/{}/trigger'.format(webhook_id),
|
||||
jsonutils.dumps(body),
|
||||
version=self.WEBHOOK_API_MICROVERSION,
|
||||
params={'V': self.WEBHOOK_VERSION})
|
||||
|
||||
resp = self.controller.trigger(req, webhook_id=webhook_id, body=body)
|
||||
|
||||
self.assertEqual('FAKE_ACTION', resp['action'])
|
||||
self.assertEqual('/actions/FAKE_ACTION', resp['location'])
|
||||
|
||||
@mock.patch.object(policy, 'enforce')
|
||||
@mock.patch.object(rpc_client.EngineClient, 'call')
|
||||
def test_webhook_trigger_non_json_params(self, mock_call, mock_enforce):
|
||||
self._mock_enforce_setup(mock_enforce, 'trigger', True)
|
||||
webhook_id = 'fake'
|
||||
body = {"params": "boo"}
|
||||
engine_response = {'action': 'FAKE_ACTION'}
|
||||
mock_call.return_value = engine_response
|
||||
req = self._patch('/webhooks/{}/trigger'.format(webhook_id),
|
||||
jsonutils.dumps(body),
|
||||
version=self.WEBHOOK_API_MICROVERSION,
|
||||
params={'V': self.WEBHOOK_VERSION})
|
||||
|
||||
resp = self.controller.trigger(req, webhook_id=webhook_id, body=body)
|
||||
|
||||
self.assertEqual('FAKE_ACTION', resp['action'])
|
||||
self.assertEqual('/actions/FAKE_ACTION', resp['location'])
|
||||
|
||||
|
||||
class WebhookV2ControllerTest(WebhookV1ControllerValidParamsTest):
|
||||
WEBHOOK_VERSION = '2'
|
||||
WEBHOOK_API_MICROVERSION = '1.0'
|
||||
|
||||
|
||||
class WebhookV2_110_ControllerTest(WebhookV1ControllerValidParamsTest):
|
||||
WEBHOOK_VERSION = '2'
|
||||
WEBHOOK_API_MICROVERSION = '1.10'
|
||||
|
|
|
@ -101,8 +101,10 @@ class ControllerTest(object):
|
|||
req.body = encodeutils.safe_encode(data) if data else None
|
||||
return req
|
||||
|
||||
def _post(self, path, data, content_type='application/json', version=None):
|
||||
return self._data_request(path, data, content_type, version=version)
|
||||
def _post(self, path, data, content_type='application/json', version=None,
|
||||
params=None):
|
||||
return self._data_request(path, data, content_type, version=version,
|
||||
params=params)
|
||||
|
||||
def _put(self, path, data, content_type='application/json', version=None):
|
||||
return self._data_request(path, data, content_type, method='PUT',
|
||||
|
|
|
@ -38,7 +38,7 @@ class TestWebhook(base.SenlinTestCase):
|
|||
|
||||
expected = {
|
||||
'alarm_url': ('http://web.com:1234/v1/webhooks/%s/trigger'
|
||||
'?V=1' % UUID1)
|
||||
'?V=2' % UUID1)
|
||||
}
|
||||
self.assertEqual(expected, channel)
|
||||
self.assertEqual(expected, webhook.channel)
|
||||
|
@ -52,7 +52,7 @@ class TestWebhook(base.SenlinTestCase):
|
|||
|
||||
expected = {
|
||||
'alarm_url': ('http://web.com:1234/v1/webhooks/%s/trigger'
|
||||
'?V=1' % UUID1)
|
||||
'?V=2' % UUID1)
|
||||
}
|
||||
self.assertEqual(expected, channel)
|
||||
self.assertEqual(expected, webhook.channel)
|
||||
|
@ -69,7 +69,7 @@ class TestWebhook(base.SenlinTestCase):
|
|||
|
||||
expected = {
|
||||
'alarm_url': ('http://test-host:8778/v1/webhooks/%s/trigger'
|
||||
'?V=1' % UUID1)
|
||||
'?V=2' % UUID1)
|
||||
}
|
||||
self.assertEqual(expected, channel)
|
||||
self.assertEqual(expected, webhook.channel)
|
||||
|
@ -85,7 +85,7 @@ class TestWebhook(base.SenlinTestCase):
|
|||
|
||||
expected = {
|
||||
'alarm_url': ('http://web.com:1234/v1/webhooks/%s/trigger'
|
||||
'?V=1&FOO=BAR&KEY=884' % UUID1)
|
||||
'?V=2&FOO=BAR&KEY=884' % UUID1)
|
||||
}
|
||||
self.assertEqual(expected, channel)
|
||||
self.assertEqual(expected, webhook.channel)
|
||||
|
|
Loading…
Reference in New Issue