From 4e3194c10647825cd6fd548a241f1511dc7905a3 Mon Sep 17 00:00:00 2001 From: KevinDW-Fluxys <122517058+KevinDW-Fluxys@users.noreply.github.com> Date: Thu, 1 Feb 2024 16:48:09 +0100 Subject: [PATCH] expose OUTGOING_WEBHOOK_TIMEOUT as env var (#3801) # What this PR does It adds functionality to be able to configure the outgoing webhook timeout from an environment variable. ## Which issue(s) this PR fixes Running into timeouts when outgoing webhooks take longer than 4 seconds (which is exceptional, but can happen) the webhook reports failure, while it still might have succeeded on the webhook side. ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --------- Co-authored-by: Joey Orlando Co-authored-by: Joey Orlando --- CHANGELOG.md | 1 + docs/sources/outgoing-webhooks/_index.md | 3 ++- engine/apps/webhooks/models/webhook.py | 13 ++++++------- engine/apps/webhooks/tests/test_webhook.py | 11 +++-------- engine/apps/webhooks/utils.py | 2 -- engine/settings/base.py | 1 + 6 files changed, 13 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f2ff4ed..0035ba4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Enable Grafana Alerting V2 feature flag by default +- Allow configuration of outgoing webhook timeout via `OUTGOING_WEBHOOK_TIMEOUT` environment variable @kevindw-fluxys ([#3801](https://github.com/grafana/oncall/pull/3801)) ## v1.3.98 (2024-02-01) diff --git a/docs/sources/outgoing-webhooks/_index.md b/docs/sources/outgoing-webhooks/_index.md index 70e61de7..1b9ceb51 100644 --- a/docs/sources/outgoing-webhooks/_index.md +++ b/docs/sources/outgoing-webhooks/_index.md @@ -111,7 +111,8 @@ If no integrations are selected the outgoing webhook will trigger for any integr The destination URL the outgoing webhook will make a request to. This must be a FQDN. -> ⚠️ **Note** the destination server must respond back within 4 seconds or it will result in a timeout +> ⚠️ **Note** the destination server must respond back within 4 seconds (by default) or it will result in a timeout +> For open source deployments, this timeout is configurable by setting the environment variable OUTGOING_WEBHOOK_TIMEOUT > (this can be seen in the "Response Body" under the "Last Run" section) | Required | [Template Accepted](#outgoing-webhook-templates) | Default Value | diff --git a/engine/apps/webhooks/models/webhook.py b/engine/apps/webhooks/models/webhook.py index 425fcda3..3a6df1c2 100644 --- a/engine/apps/webhooks/models/webhook.py +++ b/engine/apps/webhooks/models/webhook.py @@ -14,7 +14,6 @@ from mirage import fields as mirage_fields from requests.auth import HTTPBasicAuth from apps.webhooks.utils import ( - OUTGOING_WEBHOOK_TIMEOUT, InvalidWebhookData, InvalidWebhookHeaders, InvalidWebhookTrigger, @@ -246,17 +245,17 @@ class Webhook(models.Model): def make_request(self, url, request_kwargs): if self.http_method == "GET": - r = requests.get(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) + r = requests.get(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) elif self.http_method == "POST": - r = requests.post(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) + r = requests.post(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) elif self.http_method == "PUT": - r = requests.put(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) + r = requests.put(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) elif self.http_method == "DELETE": - r = requests.delete(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) + r = requests.delete(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) elif self.http_method == "OPTIONS": - r = requests.options(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) + r = requests.options(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) elif self.http_method == "PATCH": - r = requests.patch(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) + r = requests.patch(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs) else: raise ValueError(f"Unsupported http method: {self.http_method}") return r diff --git a/engine/apps/webhooks/tests/test_webhook.py b/engine/apps/webhooks/tests/test_webhook.py index a0be802a..cf840924 100644 --- a/engine/apps/webhooks/tests/test_webhook.py +++ b/engine/apps/webhooks/tests/test_webhook.py @@ -1,16 +1,11 @@ from unittest.mock import call, patch import pytest +from django.conf import settings from requests.auth import HTTPBasicAuth from apps.webhooks.models import Webhook -from apps.webhooks.utils import ( - OUTGOING_WEBHOOK_TIMEOUT, - InvalidWebhookData, - InvalidWebhookHeaders, - InvalidWebhookTrigger, - InvalidWebhookUrl, -) +from apps.webhooks.utils import InvalidWebhookData, InvalidWebhookHeaders, InvalidWebhookTrigger, InvalidWebhookUrl @pytest.mark.django_db @@ -236,7 +231,7 @@ def test_make_request(make_organization, make_custom_webhook): webhook.make_request("url", {"foo": "bar"}) expected_call = getattr(mock_requests, method.lower()) assert expected_call.called - assert expected_call.call_args == call("url", timeout=OUTGOING_WEBHOOK_TIMEOUT, foo="bar") + assert expected_call.call_args == call("url", timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, foo="bar") # invalid webhook = make_custom_webhook(organization=organization, http_method="NOT") diff --git a/engine/apps/webhooks/utils.py b/engine/apps/webhooks/utils.py index a7099559..016ce181 100644 --- a/engine/apps/webhooks/utils.py +++ b/engine/apps/webhooks/utils.py @@ -11,8 +11,6 @@ from apps.labels.utils import get_alert_group_labels_dict, get_labels_dict, is_l from apps.schedules.ical_utils import list_users_to_notify_from_ical from common.jinja_templater import apply_jinja_template -OUTGOING_WEBHOOK_TIMEOUT = 4 - class InvalidWebhookUrl(Exception): def __init__(self, message): diff --git a/engine/settings/base.py b/engine/settings/base.py index fa17ab4b..1c5b930a 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -91,6 +91,7 @@ GRAFANA_CLOUD_ONCALL_TOKEN = os.environ.get("GRAFANA_CLOUD_ONCALL_TOKEN", None) # Outgoing webhook settings DANGEROUS_WEBHOOKS_ENABLED = getenv_boolean("DANGEROUS_WEBHOOKS_ENABLED", default=False) +OUTGOING_WEBHOOK_TIMEOUT = getenv_integer("OUTGOING_WEBHOOK_TIMEOUT", default=4) WEBHOOK_RESPONSE_LIMIT = 50000 # Multiregion settings