From 05c84d7976e47ac7dd83b313d1ce22548fb75811 Mon Sep 17 00:00:00 2001 From: someposer Date: Fri, 3 Nov 2017 19:40:43 -0500 Subject: [PATCH] Add support for arbitrary headers using a JSON body for webhooks. --- hc/api/models.py | 38 +++++++--- hc/api/tests/test_notify.py | 94 ++++++++----------------- hc/api/transports.py | 16 ++--- hc/front/forms.py | 10 +-- hc/front/tests/test_add_webhook.py | 25 +++---- templates/front/channels.html | 14 ++-- templates/front/log.html | 2 +- templates/integrations/add_webhook.html | 34 ++++----- 8 files changed, 109 insertions(+), 124 deletions(-) diff --git a/hc/api/models.py b/hc/api/models.py index eca63700..05ae3ac6 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -304,28 +304,44 @@ class Channel(models.Model): return user_key, prio, PO_PRIORITIES[prio] @property - def value_down(self): + def url_down(self): assert self.kind == "webhook" - parts = self.value.split("\n") - return parts[0] + if not self.value.startswith("{"): + parts = self.value.split("\n") + return parts[0] + + doc = json.loads(self.value) + return doc["url_down"] + @property - def value_up(self): + def url_up(self): assert self.kind == "webhook" - parts = self.value.split("\n") - return parts[1] if len(parts) > 1 else "" + if not self.value.startswith("{"): + parts = self.value.split("\n") + return parts[1] if len(parts) > 1 else "" + + doc = json.loads(self.value) + return doc["url_up"] @property def post_data(self): assert self.kind == "webhook" - parts = self.value.split("\n") - return parts[2] if len(parts) > 2 else "" + if not self.value.startswith("{"): + parts = self.value.split("\n") + return parts[2] if len(parts) > 2 else "" + + doc = json.loads(self.value) + return doc["post_data"] @property - def content_type(self): + def headers(self): assert self.kind == "webhook" - parts = self.value.split("\n") - return parts[3] if len(parts) > 3 else "" + if not self.value.startswith("{"): + return "" + + doc = json.loads(self.value) + return doc["headers"] @property def slack_team(self): diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 12fc7779..48627710 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -5,7 +5,6 @@ import json from django.core import mail from django.utils.timezone import now -from hc.api.transports import Transport from hc.api.models import Channel, Check, Notification from hc.test import BaseTestCase from mock import patch @@ -63,14 +62,6 @@ class NotifyTestCase(BaseTestCase): self.assertFalse(mock_get.called) self.assertEqual(Notification.objects.count(), 0) - @patch("hc.api.transports.requests.request") - def test_webhooks_ignore_down_events(self, mock_get): - self._setup_data("webhook", "\nhttp://example", status="down") - self.channel.notify(self.check) - - self.assertFalse(mock_get.called) - self.assertEqual(Notification.objects.count(), 0) - @patch("hc.api.transports.requests.request") def test_webhooks_handle_500(self, mock_get): self._setup_data("webhook", "http://example") @@ -155,20 +146,44 @@ class NotifyTestCase(BaseTestCase): self.assertTrue(isinstance(kwargs["data"], binary_type)) @patch("hc.api.transports.requests.request") - def test_webhooks_handle_content_type(self, mock_request): - template = u"http://example.com\n\n{}\napplication/json" - self._setup_data("webhook", template) - self.check.save() + def test_webhooks_handle_json_value(self, mock_request): + self._setup_data("webhook", '{"url_down": "http://foo.com", ' + '"url_up": "", "post_data": "", "headers": ""}') + self.channel.notify(self.check) headers = { - "User-Agent": "healthchecks.io", - "Content-Type": "application/json" + "User-Agent": "healthchecks.io" + } + mock_request.assert_called_with( + "get", "http://foo.com", headers=headers, + timeout=5) + + @patch("hc.api.transports.requests.request") + def test_webhooks_handle_json_up_event(self, mock_request): + self._setup_data("webhook", '{"url_down": "", ' + '"url_up": "http://bar", "post_data": "", "headers": ""}', status="up") + self.channel.notify(self.check) + + headers = { + "User-Agent": "healthchecks.io" } + mock_request.assert_called_with( + "get", "http://bar", headers=headers, + timeout=5) + @patch("hc.api.transports.requests.request") + def test_webhooks_handle_headers(self, mock_request): + self._setup_data("webhook", '{"url_down": "http://foo.com", ' + '"url_up": "", "post_data": "data", "headers": ' + '"{\\\"Content-Type\\\": \\\"application/json\\\"}"}') self.channel.notify(self.check) + headers = { + "User-Agent": "healthchecks.io", + "Content-Type": "application/json" + } mock_request.assert_called_with( - "post", "http://example.com", data=b"{}", headers=headers, timeout=5) + "post", "http://foo.com", data=b"data", headers=headers, timeout=5) def test_email(self): self._setup_data("email", "alice@example.org") @@ -192,17 +207,6 @@ class NotifyTestCase(BaseTestCase): self.assertEqual(n.error, "Email not verified") self.assertEqual(len(mail.outbox), 0) - @patch("hc.api.transports.emails.alert") - def test_email_missing_profile(self, mock_emails): - self._setup_data("email", "not_alice@example.org") - self.profile.sort = "name" - self.profile.save() - self.channel.notify(self.check) - - args, kwargs = mock_emails.call_args - self.assertEqual(args[0], "not_alice@example.org") - self.assertEqual(args[1]["sort"], "created") - @patch("hc.api.transports.requests.request") def test_pd(self, mock_post): self._setup_data("pd", "123") @@ -312,21 +316,6 @@ class NotifyTestCase(BaseTestCase): payload = kwargs["json"] self.assertIn("DOWN", payload["message"]) - @patch("hc.api.transports.requests.request") - def test_opsgenie_up(self, mock_post): - self._setup_data("opsgenie", "123", status="up") - mock_post.return_value.status_code = 200 - - self.channel.notify(self.check) - n = Notification.objects.first() - self.assertEqual(n.error, "") - - args, kwargs = mock_post.call_args - payload = kwargs["json"] - self.assertEqual(args[0], "post") - self.assertTrue(args[1].endswith("/close")) - self.assertNotIn("message", payload) - @patch("hc.api.transports.requests.request") def test_pushover(self, mock_post): self._setup_data("po", "123|0") @@ -338,22 +327,6 @@ class NotifyTestCase(BaseTestCase): args, kwargs = mock_post.call_args payload = kwargs["data"] self.assertIn("DOWN", payload["title"]) - self.assertNotIn("retry", payload) - self.assertNotIn("expire", payload) - - @patch("hc.api.transports.requests.request") - def test_pushover_emergency(self, mock_post): - self._setup_data("po", "123|2") - mock_post.return_value.status_code = 200 - - self.channel.notify(self.check) - assert Notification.objects.count() == 1 - - args, kwargs = mock_post.call_args - payload = kwargs["data"] - self.assertIn("DOWN", payload["title"]) - self.assertIn("retry", payload) - self.assertIn("expire", payload) @patch("hc.api.transports.requests.request") def test_victorops(self, mock_post): @@ -454,8 +427,3 @@ class NotifyTestCase(BaseTestCase): self.channel.notify(self.check) self.assertTrue(mock_post.called) - - def test_transport_notify(self): - self._setup_data("webhook", "http://example") - with self.assertRaises(NotImplementedError): - Transport(self.channel).notify(self.check) diff --git a/hc/api/transports.py b/hc/api/transports.py index 260c9634..aeaac2e7 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -147,27 +147,27 @@ class Webhook(HttpTransport): return result def is_noop(self, check): - if check.status == "down" and not self.channel.value_down: + if check.status == "down" and not self.channel.url_down: return True - if check.status == "up" and not self.channel.value_up: + if check.status == "up" and not self.channel.url_up: return True return False def notify(self, check): - url = self.channel.value_down + url = self.channel.url_down if check.status == "up": - url = self.channel.value_up + url = self.channel.url_up assert url url = self.prepare(url, check, urlencode=True) if self.channel.post_data: - headers = {} - if self.channel.content_type: - headers["Content-Type"] = self.channel.content_type payload = self.prepare(self.channel.post_data, check) + headers = {} + if self.channel.headers: + headers = json.loads(self.channel.headers) return self.post(url, data=payload.encode("utf-8"), headers=headers) else: return self.get(url) @@ -233,7 +233,7 @@ class Pushbullet(HttpTransport): url = "https://api.pushbullet.com/v2/pushes" headers = { "Access-Token": self.channel.value, - "Content-Type": "application/json" + "Conent-Type": "application/json" } payload = { "type": "note", diff --git a/hc/front/forms.py b/hc/front/forms.py index 2eb76934..7f65659d 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -1,3 +1,4 @@ +import json from datetime import timedelta as td from django import forms @@ -57,19 +58,18 @@ class AddUrlForm(forms.Form): class AddWebhookForm(forms.Form): error_css_class = "has-error" - value_down = forms.URLField(max_length=1000, required=False, + url_down = forms.URLField(max_length=1000, required=False, validators=[WebhookValidator()]) - value_up = forms.URLField(max_length=1000, required=False, + url_up = forms.URLField(max_length=1000, required=False, validators=[WebhookValidator()]) post_data = forms.CharField(max_length=1000, required=False) - content_type = forms.CharField(max_length=1000, required=False) + headers = forms.CharField(max_length=1000, required=False) def get_value(self): - d = self.cleaned_data - return "\n".join((d["value_down"], d["value_up"], d["post_data"], d["content_type"])) + return json.dumps(self.cleaned_data) phone_validator = RegexValidator(regex='^\+\d{5,15}$', diff --git a/hc/front/tests/test_add_webhook.py b/hc/front/tests/test_add_webhook.py index dbda57da..1f5d09ae 100644 --- a/hc/front/tests/test_add_webhook.py +++ b/hc/front/tests/test_add_webhook.py @@ -11,17 +11,17 @@ class AddWebhookTestCase(BaseTestCase): self.assertContains(r, "Runs a HTTP GET or HTTP POST") def test_it_adds_two_webhook_urls_and_redirects(self): - form = {"value_down": "http://foo.com", "value_up": "https://bar.com"} + form = {"url_down": "http://foo.com", "url_up": "https://bar.com"} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, "http://foo.com\nhttps://bar.com\n\n") + self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "https://bar.com", "post_data": "", "headers": ""}') def test_it_adds_webhook_using_team_access(self): - form = {"value_down": "http://foo.com", "value_up": "https://bar.com"} + form = {"url_down": "http://foo.com", "url_up": "https://bar.com"} # Logging in as bob, not alice. Bob has team access so this # should work. @@ -30,7 +30,7 @@ class AddWebhookTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.user, self.alice) - self.assertEqual(c.value, "http://foo.com\nhttps://bar.com\n\n") + self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "https://bar.com", "post_data": "", "headers": ""}') def test_it_rejects_bad_urls(self): urls = [ @@ -45,7 +45,7 @@ class AddWebhookTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") for url in urls: - form = {"value_down": url, "value_up": ""} + form = {"url_down": url, "url_up": ""} r = self.client.post(self.url, form) self.assertContains(r, "Enter a valid URL.", msg_prefix=url) @@ -53,30 +53,31 @@ class AddWebhookTestCase(BaseTestCase): self.assertEqual(Channel.objects.count(), 0) def test_it_handles_empty_down_url(self): - form = {"value_down": "", "value_up": "http://foo.com"} + form = {"url_down": "", "url_up": "http://foo.com"} self.client.login(username="alice@example.org", password="password") self.client.post(self.url, form) c = Channel.objects.get() - self.assertEqual(c.value, "\nhttp://foo.com\n\n") + self.assertEqual(c.value, '{"url_down": "", "url_up": "http://foo.com", "post_data": "", "headers": ""}') def test_it_adds_post_data(self): - form = {"value_down": "http://foo.com", "post_data": "hello"} + form = {"url_down": "http://foo.com", "post_data": "hello"} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, "http://foo.com\n\nhello\n") + self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "", "post_data": "hello", "headers": ""}') - def test_it_adds_content_type(self): - form = {"value_down": "http://foo.com", "post_data": "hello", "content_type": "application/json"} + def test_it_adds_headers(self): + form = {"url_down": "http://foo.com", "headers": '{"test": "123"}'} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, "http://foo.com\n\nhello\napplication/json") + self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "", "post_data": "", "headers": "{\\\"test\\\": \\\"123\\\"}"}') + diff --git a/templates/front/channels.html b/templates/front/channels.html index 8c0cc719..141bd569 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -62,16 +62,16 @@ {% endif %} {% elif ch.kind == "webhook" %} - {% if ch.value_down %} + {% if ch.url_down %} - + {% endif %} - {% if ch.value_up %} + {% if ch.url_up %} - + {% endif %} {% if ch.post_data %} @@ -80,10 +80,10 @@ {% endif %} - {% if ch.content_type %} + {% if ch.headers %} - - + + {% endif %}
down {{ ch.value_down }}{{ ch.url_down }}
up {{ ch.value_up }}{{ ch.url_up }}
{{ ch.post_data }}
type {{ ch.content_type }}headers {{ ch.headers }}
diff --git a/templates/front/log.html b/templates/front/log.html index fbd1e60f..42422737 100644 --- a/templates/front/log.html +++ b/templates/front/log.html @@ -96,7 +96,7 @@ {% elif event.channel.kind == "po" %} Sent a Pushover notification {% elif event.channel.kind == "webhook" %} - Called webhook {{ event.channel.value_down }} + Called webhook {{ event.channel.url_down }} {% else %} Sent alert to {{ event.channel.kind|capfirst }} {% endif %} diff --git a/templates/integrations/add_webhook.html b/templates/integrations/add_webhook.html index 3432bb08..1aacfee0 100644 --- a/templates/integrations/add_webhook.html +++ b/templates/integrations/add_webhook.html @@ -57,34 +57,34 @@
{% csrf_token %} -
+
- {% if form.value_down.errors %} + value="{{ form.url_down.value|default:"" }}"> + {% if form.url_down.errors %}
- {{ form.value_down.errors|join:"" }} + {{ form.url_down.errors|join:"" }}
{% endif %}
-
+
- {% if form.value_up.errors %} + value="{{ form.url_up.value|default:"" }}"> + {% if form.url_up.errors %}
- {{ form.value_up.errors|join:"" }} + {{ form.url_up.errors|join:"" }}
{% endif %}
@@ -105,18 +105,18 @@ {% endif %}
-
- +
+
- {% if form.content_type.errors %} + name="headers" + placeholder='{"Content-Type": "application/json"}' + value="{{ form.headers.value|default:"" }}"> + {% if form.headers.errors %}
- {{ form.content_type.errors|join:"" }} + {{ form.headers.errors|join:"" }}
{% endif %}