From 6eac05abafecac2f3cd943c9ecf7096e9aceee6b Mon Sep 17 00:00:00 2001
From: Joey Orlando <joey.orlando@grafana.com>
Date: Fri, 9 Aug 2024 13:45:09 -0400
Subject: [PATCH] `apps.webhooks.tasks.trigger_webhook.execute_webhook` task -
 don't retry on `requests.exceptions.SSLError` (#4796)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Which issue(s) this PR closes

Address retrying `apps.webhooks.tasks.trigger_webhook.execute_webhook`
task when `requests.exceptions.SSLError` is raised
([logs](https://ops.grafana-ops.net/goto/vqrouqrIR?orgId=1)). Don't
retry the task on these exceptions as retrying will not help. From the
[`request`'s
docs](https://requests.readthedocs.io/en/latest/user/advanced/#ssl-cert-verification):
> By default, SSL verification is enabled, and Requests will throw a
SSLError if it’s unable to verify the certificate

## 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] Added the relevant release notes label (see labels prefixed w/
`release:`). These labels dictate how your PR will
    show up in the autogenerated release notes.
---
 engine/apps/webhooks/tasks/trigger_webhook.py |  7 +++
 .../webhooks/tests/test_trigger_webhook.py    | 50 +++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/engine/apps/webhooks/tasks/trigger_webhook.py b/engine/apps/webhooks/tasks/trigger_webhook.py
index 93bd2c14..ea89b2e2 100644
--- a/engine/apps/webhooks/tasks/trigger_webhook.py
+++ b/engine/apps/webhooks/tasks/trigger_webhook.py
@@ -198,6 +198,13 @@ def make_request(
         status["request_headers"] = error = e.message
     except InvalidWebhookData as e:
         status["request_data"] = error = e.message
+    except requests.exceptions.SSLError as e:
+        # Don't raise an exception for SSL errors, as they are out of our control and retrying
+        # isn't going to help. Just show the error to the user and give up
+        #
+        # from the docs (https://requests.readthedocs.io/en/latest/user/advanced/#ssl-cert-verification)
+        # "Requests will throw a SSLError if it’s unable to verify the certificate"
+        status["content"] = error = str(e)
     except Exception as e:
         status["content"] = error = str(e)
         exception = e
diff --git a/engine/apps/webhooks/tests/test_trigger_webhook.py b/engine/apps/webhooks/tests/test_trigger_webhook.py
index 381b79eb..38445a8a 100644
--- a/engine/apps/webhooks/tests/test_trigger_webhook.py
+++ b/engine/apps/webhooks/tests/test_trigger_webhook.py
@@ -736,6 +736,56 @@ def test_execute_webhook_errors(
     )
 
 
+@patch(
+    "apps.webhooks.models.webhook.WebhookSession.request",
+    side_effect=requests.exceptions.SSLError("SSL error - foo bar"),
+)
+@patch("apps.webhooks.utils.socket.gethostbyname", return_value="8.8.8.8")  # make it a valid URL when resolving name
+@pytest.mark.django_db
+def test_execute_webhook_ssl_error(
+    _mock_socket_gethostbyname,
+    mock_request,
+    make_organization,
+    make_alert_receive_channel,
+    make_alert_group,
+    make_custom_webhook,
+):
+    url = "https://something.cool/"
+    expected_error = "SSL error - foo bar"
+
+    organization = make_organization()
+    alert_receive_channel = make_alert_receive_channel(organization)
+    alert_group = make_alert_group(alert_receive_channel, resolved_at=timezone.now(), resolved=True)
+    webhook = make_custom_webhook(
+        organization=organization,
+        http_method="POST",
+        trigger_type=Webhook.TRIGGER_RESOLVE,
+        forward_all=False,
+        url=url,
+    )
+
+    execute_webhook(webhook.pk, alert_group.pk, None, None)
+
+    mock_request.assert_has_calls([call("POST", url, timeout=4, headers={})])
+
+    log = webhook.responses.all()[0]
+    assert log.status_code is None
+    assert log.content == expected_error
+
+    # check log record
+    log_record = alert_group.log_records.last()
+    assert log_record.type == AlertGroupLogRecord.ERROR_ESCALATION_TRIGGER_CUSTOM_WEBHOOK_ERROR
+    assert log_record.step_specific_info == {
+        "trigger": "resolve",
+        "webhook_id": webhook.public_primary_key,
+        "webhook_name": webhook.name,
+    }
+    assert log_record.reason == expected_error
+    assert (
+        log_record.rendered_log_line_action() == f"skipped resolve outgoing webhook `{webhook.name}`: {expected_error}"
+    )
+
+
 @pytest.mark.django_db
 def test_response_content_limit(
     make_organization, make_user_for_organization, make_alert_receive_channel, make_alert_group, make_custom_webhook