Fix #181: check webhooks status in heartbeat (#395)

* Fix #181: check webhooks status in heartbeat

* Show warning if a hook has errors

* Remove comment about product==Any

Author: leplatrem

jbi/models.py

@@ -346,6 +346,26 @@ class BugzillaApiResponse(BaseModel):
     bugs: Optional[list[BugzillaBug]]
 
 
+class BugzillaWebhook(BaseModel):
+    """Bugzilla Webhook"""
+
+    id: int
+    creator: str
+    name: str
+    url: str
+    event: str
+    product: str
+    component: str
+    enabled: bool
+    errors: int
+
+
+class BugzillaWebhooksResponse(BaseModel):
+    """Bugzilla Webhooks List Response Object"""
+
+    webhooks: Optional[list[BugzillaWebhook]]
+
+
 class Context(BaseModel):
     """Generic log context throughout JBI"""
 

jbi/services/bugzilla.py

@@ -9,7 +9,13 @@
 from pydantic import parse_obj_as
 
 from jbi import Operation, environment
-from jbi.models import ActionContext, BugzillaApiResponse, BugzillaBug, BugzillaComment
+from jbi.models import (
+    ActionContext,
+    BugzillaApiResponse,
+    BugzillaBug,
+    BugzillaComment,
+    BugzillaWebhooksResponse,
+)
 
 from .common import InstrumentedClient, ServiceHealth
 
@@ -95,6 +101,17 @@ def update_bug(self, bugid, **fields) -> BugzillaBug:
             )
         return parsed.bugs[0]
 
+    def list_webhooks(self):
+        """List the currently configured webhooks, including their status."""
+        url = f"{self.base_url}/rest/webhooks/list"
+        webhooks_info = self._call("GET", url)
+        parsed = BugzillaWebhooksResponse.parse_obj(webhooks_info)
+        if parsed.webhooks is None:
+            raise BugzillaClientError(
+                f"Unexpected response content from 'GET {url}' (no 'webhooks' field)"
+            )
+        return parsed.webhooks
+
 
 @lru_cache(maxsize=1)
 def get_client():
@@ -109,6 +126,7 @@ def get_client():
             "get_bug",
             "get_comments",
             "update_bugs",
+            "list_webhooks",
         ),
         exceptions=(
             BugzillaClientError,
@@ -120,7 +138,30 @@ def get_client():
 def check_health() -> ServiceHealth:
     """Check health for Bugzilla Service"""
     client = get_client()
-    health: ServiceHealth = {"up": client.logged_in}
+    logged_in = client.logged_in
+
+    # Check that all JBI webhooks are enabled in Bugzilla,
+    # and report disabled ones.
+    all_webhooks_enabled = False
+    if logged_in:
+        webhooks = client.list_webhooks()
+        jbi_webhooks = [wh for wh in webhooks if "/bugzilla_webhook" in wh.url]
+        all_webhooks_enabled = len(jbi_webhooks) > 0
+        for webhook in jbi_webhooks:
+            if webhook.errors > 0:
+                logger.warning(
+                    "Webhook %s has %s error(s)", webhook.name, webhook.errors
+                )
+            if not webhook.enabled:
+                all_webhooks_enabled = False
+                logger.error(
+                    "Webhook %s is disabled (%s errors)", webhook.name, webhook.errors
+                )
+
+    health: ServiceHealth = {
+        "up": logged_in,
+        "all_webhooks_enabled": all_webhooks_enabled,
+    }
     return health
 
 

tests/unit/services/test_bugzilla.py

@@ -179,3 +179,44 @@ def test_bugzilla_missing_private_comment(
     expanded = get_client().get_bug(webhook_private_comment_example.bug.id)
 
     assert not expanded.comment
+
+
+@pytest.mark.no_mocked_bugzilla
+def test_bugzilla_list_webhooks(mocked_responses):
+    url = f"{get_settings().bugzilla_base_url}/rest/webhooks/list"
+    mocked_responses.add(
+        responses.GET,
+        url,
+        json={
+            "webhooks": [
+                {
+                    "id": 0,
+                    "creator": "Bob",
+                    "name": "",
+                    "url": "http://server/bugzilla_webhook",
+                    "event": "create,change,comment",
+                    "product": "Any",
+                    "component": "Any",
+                    "enabled": True,
+                    "errors": 0,
+                }
+            ]
+        },
+    )
+
+    webhooks = get_client().list_webhooks()
+
+    assert len(webhooks) == 1
+    assert webhooks[0].creator == "Bob"
+    assert "/bugzilla_webhook" in webhooks[0].url
+
+
+@pytest.mark.no_mocked_bugzilla
+def test_bugzilla_list_webhooks_raises_if_response_has_no_webhooks(mocked_responses):
+    url = f"{get_settings().bugzilla_base_url}/rest/webhooks/list"
+    mocked_responses.add(responses.GET, url, json={})
+
+    with pytest.raises(BugzillaClientError) as exc:
+        get_client().list_webhooks()
+
+    assert "Unexpected response" in str(exc)

tests/unit/test_router.py

@@ -6,7 +6,19 @@
 
 from jbi.app import app
 from jbi.environment import get_settings
-from jbi.models import BugzillaWebhookRequest
+from jbi.models import BugzillaWebhook, BugzillaWebhookRequest
+
+EXAMPLE_WEBHOOK = BugzillaWebhook(
+    id=0,
+    creator="",
+    name="",
+    url="http://server/bugzilla_webhook",
+    event="create,change,comment",
+    product="Any",
+    component="Any",
+    enabled=True,
+    errors=0,
+)
 
 
 def test_read_root(anon_client):
@@ -158,28 +170,40 @@ def test_read_heartbeat_all_services_fail(anon_client, mocked_jira, mocked_bugzi
         },
         "bugzilla": {
             "up": False,
+            "all_webhooks_enabled": False,
         },
     }
 
 
 def test_read_heartbeat_jira_services_fails(anon_client, mocked_jira, mocked_bugzilla):
     """/__heartbeat__ returns 503 when one service is unavailable."""
-    mocked_bugzilla.logged_in = True
     mocked_jira.get_server_info.return_value = None
 
     resp = anon_client.get("/__heartbeat__")
 
     assert resp.status_code == 503
-    assert resp.json() == {
-        "jira": {
-            "up": False,
-            "all_projects_are_visible": False,
-            "all_projects_have_permissions": False,
-            "all_projects_components_exist": False,
-        },
-        "bugzilla": {
-            "up": True,
-        },
+    assert resp.json()["jira"] == {
+        "up": False,
+        "all_projects_are_visible": False,
+        "all_projects_have_permissions": False,
+        "all_projects_components_exist": False,
+    }
+
+
+def test_read_heartbeat_bugzilla_webhooks_fails(
+    anon_client, mocked_jira, mocked_bugzilla
+):
+    mocked_bugzilla.logged_in = True
+    mocked_bugzilla.list_webhooks.return_value = [
+        EXAMPLE_WEBHOOK.copy(update={"enabled": False})
+    ]
+
+    resp = anon_client.get("/__heartbeat__")
+
+    assert resp.status_code == 503
+    assert resp.json()["bugzilla"] == {
+        "up": True,
+        "all_webhooks_enabled": False,
     }
 
 
@@ -194,22 +218,16 @@ def test_read_heartbeat_bugzilla_services_fails(
     resp = anon_client.get("/__heartbeat__")
 
     assert resp.status_code == 503
-    assert resp.json() == {
-        "jira": {
-            "up": True,
-            "all_projects_are_visible": True,
-            "all_projects_have_permissions": False,
-            "all_projects_components_exist": False,
-        },
-        "bugzilla": {
-            "up": False,
-        },
+    assert resp.json()["bugzilla"] == {
+        "up": False,
+        "all_webhooks_enabled": False,
     }
 
 
 def test_read_heartbeat_success(anon_client, mocked_jira, mocked_bugzilla):
     """/__heartbeat__ returns 200 when checks succeed."""
     mocked_bugzilla.logged_in = True
+    mocked_bugzilla.list_webhooks.return_value = [EXAMPLE_WEBHOOK]
     mocked_jira.get_server_info.return_value = {}
     mocked_jira.projects.return_value = [{"key": "DevTest"}]
     mocked_jira.get_project_components.return_value = [{"name": "Main"}]
@@ -234,34 +252,28 @@ def test_read_heartbeat_success(anon_client, mocked_jira, mocked_bugzilla):
         },
         "bugzilla": {
             "up": True,
+            "all_webhooks_enabled": True,
         },
     }
 
 
 def test_jira_heartbeat_visible_projects(anon_client, mocked_jira, mocked_bugzilla):
     """/__heartbeat__ fails if configured projects don't match."""
-    mocked_bugzilla.logged_in = True
     mocked_jira.get_server_info.return_value = {}
 
     resp = anon_client.get("/__heartbeat__")
 
     assert resp.status_code == 503
-    assert resp.json() == {
-        "jira": {
-            "up": True,
-            "all_projects_are_visible": False,
-            "all_projects_have_permissions": False,
-            "all_projects_components_exist": False,
-        },
-        "bugzilla": {
-            "up": True,
-        },
+    assert resp.json()["jira"] == {
+        "up": True,
+        "all_projects_are_visible": False,
+        "all_projects_have_permissions": False,
+        "all_projects_components_exist": False,
     }
 
 
 def test_jira_heartbeat_missing_permissions(anon_client, mocked_jira, mocked_bugzilla):
     """/__heartbeat__ fails if configured projects don't match."""
-    mocked_bugzilla.logged_in = True
     mocked_jira.get_server_info.return_value = {}
     mocked_jira.get_project_components.return_value = [{"name": "Main"}]
     mocked_jira.get_project_permission_scheme.return_value = {
@@ -276,21 +288,17 @@ def test_jira_heartbeat_missing_permissions(anon_client, mocked_jira, mocked_bug
     resp = anon_client.get("/__heartbeat__")
 
     assert resp.status_code == 503
-    assert resp.json() == {
-        "jira": {
-            "up": True,
-            "all_projects_are_visible": False,
-            "all_projects_have_permissions": False,
-            "all_projects_components_exist": True,
-        },
-        "bugzilla": {
-            "up": True,
-        },
+    assert resp.json()["jira"] == {
+        "up": True,
+        "all_projects_are_visible": False,
+        "all_projects_have_permissions": False,
+        "all_projects_components_exist": True,
     }
 
 
 def test_jira_heartbeat_unknown_components(anon_client, mocked_jira, mocked_bugzilla):
     mocked_bugzilla.logged_in = True
+    mocked_bugzilla.list_webhooks.return_value = [EXAMPLE_WEBHOOK]
     mocked_jira.get_server_info.return_value = {}
 
     resp = anon_client.get("/__heartbeat__")
@@ -299,9 +307,10 @@ def test_jira_heartbeat_unknown_components(anon_client, mocked_jira, mocked_bugz
     assert not resp.json()["jira"]["all_projects_components_exist"]
 
 
-def test_head_heartbeat(anon_client, mocked_jira, mocked_bugzilla):
+def test_head_heartbeat_success(anon_client, mocked_jira, mocked_bugzilla):
     """/__heartbeat__ support head requests"""
     mocked_bugzilla.logged_in = True
+    mocked_bugzilla.list_webhooks.return_value = [EXAMPLE_WEBHOOK]
     mocked_jira.get_server_info.return_value = {}
     mocked_jira.projects.return_value = [{"key": "DevTest"}]
     mocked_jira.get_project_components.return_value = [{"name": "Main"}]