diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index e55d7d163..1b6a226e1 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -281,6 +281,7 @@ webhook_params: webhook_version: type: string in: query + required: True description: | The webhook implementation version requested. diff --git a/api-ref/source/webhooks.inc b/api-ref/source/webhooks.inc index d19fdb497..75b50846a 100644 --- a/api-ref/source/webhooks.inc +++ b/api-ref/source/webhooks.inc @@ -20,6 +20,7 @@ Response Codes .. rest_status_code:: error status.yaml + - 400 - 403 - 404 - 503 diff --git a/senlin/api/middleware/webhook.py b/senlin/api/middleware/webhook.py index c36fd8b42..221c0c6b1 100644 --- a/senlin/api/middleware/webhook.py +++ b/senlin/api/middleware/webhook.py @@ -13,6 +13,7 @@ from oslo_log import log as logging import six from six.moves.urllib import parse as urlparse +import webob from senlin.api.common import util from senlin.api.common import wsgi @@ -75,6 +76,7 @@ class WebhookMiddleware(wsgi.Middleware): parts = urlparse.urlparse(url) p = parts.path.split('/') + # check if URL is a webhook trigger request # expected: ['', 'v1', 'webhooks', 'webhook-id', 'trigger'] if len(p) != 5: return None @@ -82,10 +84,16 @@ class WebhookMiddleware(wsgi.Middleware): if any((p[0] != '', p[2] != 'webhooks', p[4] != 'trigger')): return None + # at this point it has been determined that the URL is a webhook + # trigger request qs = urlparse.parse_qs(parts.query) - if 'V' not in qs: - return None - qs.pop('V') + if 'V' in qs: + qs.pop('V') + else: + raise webob.exc.HTTPBadRequest( + explanation=_('V query parameter is required in webhook ' + 'trigger URL')) + params = dict((k, v[0]) for k, v in qs.items()) return p[3], params diff --git a/senlin/tests/unit/api/middleware/test_webhook.py b/senlin/tests/unit/api/middleware/test_webhook.py index d1985a342..52ad9c070 100644 --- a/senlin/tests/unit/api/middleware/test_webhook.py +++ b/senlin/tests/unit/api/middleware/test_webhook.py @@ -13,6 +13,8 @@ import mock from oslo_config import cfg +import six +import webob from senlin.api.common import util as common_util from senlin.api.common import version_request as vr @@ -55,6 +57,40 @@ class TestWebhookMiddleware(base.SenlinTestCase): res = self.middleware._parse_url(self._generate_url()) self.assertEqual(('WEBHOOK_ID', {'key': 'TEST_KEY'}), res) + def test_parse_url_version_provided_no_key(self): + # The structure //trigger?V=1 should be valid + self.url_slices.pop('05_params') + + res = self.middleware._parse_url(self._generate_url()) + self.assertEqual(('WEBHOOK_ID', {}), res) + + def test_parse_url_no_version_provided_no_key_provided(self): + # The structure //trigger should be invalid + # because version is missing + self.url_slices.pop('04_version') + self.url_slices.pop('05_params') + + ex = self.assertRaises(webob.exc.HTTPBadRequest, + self.middleware._parse_url, + self._generate_url()) + + self.assertEqual("V query parameter is required in webhook trigger " + "URL", six.text_type(ex)) + + def test_parse_url_no_version_provided_key_provided(self): + # The structure //trigger?key=value should be invalid + # because version is missing + self.url_slices.pop('04_version') + self.url_slices.pop('05_params') + self.url_slices['05_params'] = 'key=TEST_KEY' + + ex = self.assertRaises(webob.exc.HTTPBadRequest, + self.middleware._parse_url, + self._generate_url()) + + self.assertEqual("V query parameter is required in webhook trigger " + "URL", six.text_type(ex)) + def test_parse_url_webhooks_not_found(self): # String 'webhooks' is not found in url self.url_slices['01_webhook_str'] = '/foo/' @@ -79,12 +115,6 @@ class TestWebhookMiddleware(base.SenlinTestCase): res = self.middleware._parse_url(self._generate_url()) self.assertIsNone(res) - def test_parse_url_no_key_provided(self): - # Bottom string of the url does not start with 'trigger' - self.url_slices['04_key_str'] = 'version=1' - res = self.middleware._parse_url(self._generate_url()) - self.assertIsNone(res) - @mock.patch.object(driver_base, 'SenlinDriver') def test_get_token_succeeded(self, mock_senlindriver): class FakeAccessInfo(object):