webhook accept room_id param; config define list of allowed rooms ; check if duration is positive in command

matrix_alertbot/alertmanager.py

@@ -11,7 +11,6 @@ from diskcache import Cache
 from matrix_alertbot.errors import (
     AlertmanagerServerError,
     AlertNotFoundError,
-    InvalidDurationError,
     SilenceExpiredError,
     SilenceNotFoundError,
 )
@@ -74,11 +73,9 @@ class AlertmanagerClient:
         max_duration = timedelta(days=MAX_DURATION_DAYS)
         if duration_seconds is None or duration_seconds > max_duration.total_seconds():
             end_time = start_time + max_duration
-        elif duration_seconds > 0:
+        else:
             duration_delta = timedelta(seconds=duration_seconds)
             end_time = start_time + duration_delta
-        else:
-            raise InvalidDurationError(f"Duration must be positive: {duration_seconds}")
 
         silence = {
             "id": silence_id,

matrix_alertbot/callback.py

@@ -61,7 +61,7 @@ class Callbacks:
             return
 
         # Ignore messages from unauthorized room
-        if room.room_id != self.config.room_id:
+        if room.room_id not in self.config.allowed_rooms:
             return
 
         # Extract the message text
@@ -118,7 +118,7 @@ class Callbacks:
             event: The invite event.
         """
         # Ignore invites from unauthorized room
-        if room.room_id != self.config.room_id:
+        if room.room_id not in self.config.allowed_rooms:
             return
 
         logger.debug(f"Got invite to {room.room_id} from {event.sender}.")
@@ -166,7 +166,7 @@ class Callbacks:
             reacted_to_id: The event ID that the reaction points to.
         """
         # Ignore reactions from unauthorized room
-        if room.room_id != self.config.room_id:
+        if room.room_id not in self.config.allowed_rooms:
             return
 
         # Ignore reactions from ourselves
@@ -209,7 +209,7 @@ class Callbacks:
 
     async def redaction(self, room: MatrixRoom, event: RedactionEvent) -> None:
         # Ignore events from unauthorized room
-        if room.room_id != self.config.room_id:
+        if room.room_id not in self.config.allowed_rooms:
             return
 
         # Ignore redactions from ourselves
@@ -239,7 +239,7 @@ class Callbacks:
             event: The encrypted event that we were unable to decrypt.
         """
         # Ignore events from unauthorized room
-        if room.room_id != self.config.room_id:
+        if room.room_id not in self.config.allowed_rooms:
             return
 
         logger.error(
@@ -263,7 +263,7 @@ class Callbacks:
             event: The event itself.
         """
         # Ignore events from unauthorized room
-        if room.room_id != self.config.room_id:
+        if room.room_id not in self.config.allowed_rooms:
             return
 
         if event.type == "m.reaction":

matrix_alertbot/command.py

@@ -11,7 +11,6 @@ from matrix_alertbot.config import Config
 from matrix_alertbot.errors import (
     AlertmanagerError,
     AlertNotFoundError,
-    InvalidDurationError,
     SilenceExpiredError,
     SilenceNotFoundError,
 )
@@ -104,8 +103,19 @@ class AckAlertCommand(BaseAlertCommand):
                     f"I tried really hard, but I can't convert the duration '{duration}' to a number of seconds.",
                 )
                 return
+            elif duration_seconds < 0:
+                logger.error(f"Unable to create silence: Duration must be positive, got '{duration}'")
+                await send_text_to_room(
+                    self.client,
+                    self.room.room_id,
+                    "I can't create a silence with a negative duration!",
+                )
+                return
+
+            cache_expire_time = duration_seconds
         else:
             duration_seconds = None
+            cache_expire_time = self.config.cache_expire_time
             logger.debug(
                 "Receiving a command to create a silence for an indefinite period"
             )
@@ -138,7 +148,7 @@ class AckAlertCommand(BaseAlertCommand):
                 duration_seconds,
                 cached_silence_id,
             )
-        except (AlertNotFoundError, InvalidDurationError) as e:
+        except AlertNotFoundError as e:
             logger.warning(f"Unable to create silence: {e}")
             await send_text_to_room(
                 self.client,
@@ -146,7 +156,6 @@ class AckAlertCommand(BaseAlertCommand):
                 f"Sorry, I couldn't create silence for alert with fingerprint {alert_fingerprint}: {e}",
             )
             return
-            return
         except AlertmanagerError as e:
             logger.exception(f"Unable to create silence: {e}", exc_info=e)
             await send_text_to_room(
@@ -157,7 +166,7 @@ class AckAlertCommand(BaseAlertCommand):
             )
             return
 
-        self.cache.set(self.event_id, alert_fingerprint, expire=duration_seconds)
+        self.cache.set(self.event_id, alert_fingerprint, expire=cache_expire_time)
         self.cache.set(alert_fingerprint, silence_id, expire=duration_seconds)
 
         await send_text_to_room(
@@ -220,6 +229,8 @@ class UnackAlertCommand(BaseAlertCommand):
             )
             return
 
+        self.cache.delete(alert_fingerprint)
+
         await send_text_to_room(
             self.client,
             self.room.room_id,

matrix_alertbot/config.py

@@ -102,7 +102,9 @@ class Config:
             ["matrix", "device_name"], default="matrix-alertbot"
         )
         self.homeserver_url: str = self._get_cfg(["matrix", "url"], required=True)
-        self.room_id: str = self._get_cfg(["matrix", "room"], required=True)
+        self.allowed_rooms: list = self._get_cfg(
+            ["matrix", "allowed_rooms"], required=True
+        )
 
         self.address: str = self._get_cfg(["webhook", "address"], required=False)
         self.port: int = self._get_cfg(["webhook", "port"], required=False)

matrix_alertbot/errors.py

@@ -49,12 +49,6 @@ class SilenceExpiredError(AlertmanagerError):
     pass
 
 
-class InvalidDurationError(AlertmanagerError):
-    """An error encountered when an alert has an invalid duration."""
-
-    pass
-
-
 class AlertmanagerServerError(AlertmanagerError):
     """An error encountered with Alertmanager server."""
 

matrix_alertbot/webhook.py

@@ -23,27 +23,42 @@ async def get_health(request: web_request.Request) -> web.Response:
     return web.Response(status=200)
 
 
-@routes.post("/alerts")
+@routes.post("/alerts/{room_id}")
 async def create_alerts(request: web_request.Request) -> web.Response:
     data = await request.json()
-    logger.info(f"Received alerts: {data}")
+    room_id = request.match_info["room_id"]
+
     client: AsyncClient = request.app["client"]
     config: Config = request.app["config"]
     cache: Cache = request.app["cache"]
 
+    if room_id not in config.allowed_rooms:
+        logger.error("Cannot send alerts to room ID {room_id}.")
+        return web.Response(status=401, body=f"Cannot send alerts to room ID {room_id}.")
+
     if "alerts" not in data:
+        logger.error("Received data without 'alerts' key")
         return web.Response(status=400, body="Data must contain 'alerts' key.")
 
+    alerts = data["alerts"]
+
     if not isinstance(data["alerts"], list):
-        return web.Response(status=400, body="Alerts must be a list.")
+        alerts_type = type(alerts).__name__
+        logger.error(f"Received data with invalid alerts type '{alerts_type}'.")
+        return web.Response(
+            status=400, body=f"Alerts must be a list, got '{alerts_type}'."
+        )
+
+    logger.info(f"Received {len(alerts)} alerts for room ID {room_id}: {data}")
 
     if len(data["alerts"]) == 0:
         return web.Response(status=400, body="Alerts cannot be empty.")
 
-    for alert in data["alerts"]:
+    for alert in alerts:
         try:
             alert = Alert.from_dict(alert)
-        except KeyError:
+        except KeyError as e:
+            logger.error(f"Cannot parse alert dict: {e}")
             return web.Response(status=400, body=f"Invalid alert: {alert}.")
 
         plaintext = alert.plaintext()
@@ -51,7 +66,7 @@ async def create_alerts(request: web_request.Request) -> web.Response:
 
         try:
             event = await send_text_to_room(
-                client, config.room_id, plaintext, html, notice=False
+                client, room_id, plaintext, html, notice=False
             )
         except (LocalProtocolError, ClientError) as e:
             logger.error(
@@ -62,7 +77,10 @@ async def create_alerts(request: web_request.Request) -> web.Response:
                 body=f"An error occured when sending alert with fingerprint '{alert.fingerprint}' to Matrix room.",
             )
 
-        cache.set(event.event_id, alert.fingerprint, expire=config.cache_expire_time)
+        if alert.firing:
+            cache.set(
+                event.event_id, alert.fingerprint, expire=config.cache_expire_time
+            )
 
     return web.Response(status=200)
 

tests/resources/config/config.full.yml

@@ -20,7 +20,8 @@ matrix:
   device_id: ABCDEFGHIJ
   # What to name the logged in device
   device_name: fake_device_name
-  room: "!abcdefgh:matrix.example.com"
+  allowed_rooms:
+    - "!abcdefgh:matrix.example.com"
 
 webhook:
   socket: matrix-alertbot.socket

tests/resources/config/config.minimal.yml

@@ -15,7 +15,8 @@ matrix:
   # The device ID that is **non pre-existing** device
   # If this device ID already exists, messages will be dropped silently in encrypted rooms
   device_id: ABCDEFGHIJ
-  room: "!abcdefgh:matrix.example.com"
+  allowed_rooms:
+    - "!abcdefgh:matrix.example.com"
 
 webhook:
   address: 0.0.0.0

tests/test_alertmanager.py

@@ -17,7 +17,6 @@ from matrix_alertbot.alertmanager import AlertmanagerClient
 from matrix_alertbot.errors import (
     AlertmanagerServerError,
     AlertNotFoundError,
-    InvalidDurationError,
     SilenceExpiredError,
     SilenceNotFoundError,
 )
@@ -467,17 +466,6 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase):
             silence,
         )
 
-    @freeze_time(datetime.utcfromtimestamp(0))
-    async def test_create_silence_raise_duration_error(self) -> None:
-        async with FakeAlertmanagerServerWithoutSilence() as fake_alertmanager_server:
-            port = fake_alertmanager_server.port
-            alertmanager = AlertmanagerClient(
-                f"http://localhost:{port}", self.fake_cache
-            )
-            async with aiotools.closing_async(alertmanager):
-                with self.assertRaises(InvalidDurationError):
-                    await alertmanager.create_silence("fingerprint1", "user", -1)
-
     async def test_create_silence_raise_alert_not_found(self) -> None:
         async with FakeAlertmanagerServerWithoutAlert() as fake_alertmanager_server:
             port = fake_alertmanager_server.port

tests/test_callback.py

@@ -30,7 +30,7 @@ class CallbacksTestCase(unittest.IsolatedAsyncioTestCase):
 
         # We don't spec config, as it doesn't currently have well defined attributes
         self.fake_config = Mock()
-        self.fake_config.room_id = self.fake_room.room_id
+        self.fake_config.allowed_rooms = [self.fake_room.room_id]
         self.fake_config.command_prefix = "!alert "
 
         self.callbacks = Callbacks(
@@ -151,8 +151,6 @@ class CallbacksTestCase(unittest.IsolatedAsyncioTestCase):
         fake_message_event = Mock(spec=nio.RoomMessageText)
         fake_message_event.sender = "@some_other_fake_user:example.com"
 
-        self.assertNotEqual(self.fake_config.room_id, self.fake_room.room_id)
-
         # Pretend that we received a text message event
         await self.callbacks.message(self.fake_room, fake_message_event)
 

tests/test_command.py

@@ -103,7 +103,7 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase):
 
         # We don't spec config, as it doesn't currently have well defined attributes
         self.fake_config = Mock()
-        self.fake_config.room_id = self.fake_room.room_id
+        self.fake_config.allowed_rooms = [self.fake_room.room_id]
         self.fake_config.command_prefix = "!alert "
 
     @patch.object(matrix_alertbot.command.AckAlertCommand, "process")
@@ -246,7 +246,11 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase):
         self.fake_cache.get.assert_called_once_with("fingerprint1")
         self.fake_cache.set.assert_has_calls(
             [
-                call("some event id", "fingerprint1", expire=None),
+                call(
+                    "some event id",
+                    "fingerprint1",
+                    expire=self.fake_config.cache_expire_time,
+                ),
                 call("fingerprint1", "silence1", expire=None),
             ]
         )
@@ -408,6 +412,37 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase):
         self.fake_cache.get.assert_not_called()
         self.fake_cache.set.assert_not_called()
 
+    @patch.object(matrix_alertbot.command, "send_text_to_room")
+    async def test_ack_with_negative_duration(
+        self, fake_send_text_to_room: Mock
+    ) -> None:
+        """Tests the callback for InviteMemberEvents"""
+        # Tests that the bot attempts to join a room after being invited to it
+        command = AckAlertCommand(
+            self.fake_client,
+            self.fake_cache,
+            self.fake_alertmanager,
+            self.fake_config,
+            self.fake_room,
+            self.fake_sender,
+            self.fake_event_id,
+            self.fake_alert_event_id,
+            ("-1d",),
+        )
+
+        await command.process()
+
+        # Check that we attempted to create silences
+        self.fake_alertmanager.create_silence.assert_not_called()
+        fake_send_text_to_room.assert_called_once_with(
+            self.fake_client,
+            self.fake_room.room_id,
+            "I can't create a silence with a negative duration!",
+        )
+        self.fake_cache.__getitem__.assert_not_called()
+        self.fake_cache.get.assert_not_called()
+        self.fake_cache.set.assert_not_called()
+
     @patch.object(matrix_alertbot.command, "send_text_to_room")
     async def test_ack_with_alert_event_not_found_in_cache(
         self, fake_send_text_to_room: Mock
@@ -479,7 +514,11 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase):
         self.fake_cache.get.assert_called_once_with("fingerprint1")
         self.fake_cache.set.assert_has_calls(
             [
-                call(self.fake_event_id, "fingerprint1", expire=None),
+                call(
+                    self.fake_event_id,
+                    "fingerprint1",
+                    expire=self.fake_config.cache_expire_time,
+                ),
                 call("fingerprint1", "silence1", expire=None),
             ]
         )

tests/test_config.py

@@ -57,7 +57,7 @@ class ConfigTestCase(unittest.TestCase):
         self.assertEqual("ABCDEFGHIJ", config.device_id)
         self.assertEqual("matrix-alertbot", config.device_name)
         self.assertEqual("https://matrix.example.com", config.homeserver_url)
-        self.assertEqual("!abcdefgh:matrix.example.com", config.room_id)
+        self.assertEqual(["!abcdefgh:matrix.example.com"], config.allowed_rooms)
 
         self.assertEqual("0.0.0.0", config.address)
         self.assertEqual(8080, config.port)
@@ -95,7 +95,7 @@ class ConfigTestCase(unittest.TestCase):
         self.assertEqual("ABCDEFGHIJ", config.device_id)
         self.assertEqual("fake_device_name", config.device_name)
         self.assertEqual("https://matrix.example.com", config.homeserver_url)
-        self.assertEqual("!abcdefgh:matrix.example.com", config.room_id)
+        self.assertEqual(["!abcdefgh:matrix.example.com"], config.allowed_rooms)
 
         self.assertIsNone(config.address)
         self.assertIsNone(config.port)
@@ -199,7 +199,7 @@ class ConfigTestCase(unittest.TestCase):
     @patch("os.path.isdir")
     @patch("os.path.exists")
     @patch("os.mkdir")
-    def test_parse_config_with_missing_matrix_room(
+    def test_parse_config_with_missing_matrix_allowed_rooms(
         self, fake_mkdir: Mock, fake_path_exists: Mock, fake_path_isdir: Mock
     ) -> None:
         fake_path_isdir.return_value = False
@@ -207,7 +207,7 @@ class ConfigTestCase(unittest.TestCase):
 
         config_path = os.path.join(CONFIG_RESOURCES_DIR, "config.minimal.yml")
         config = DummyConfig(config_path)
-        del config.config_dict["matrix"]["room"]
+        del config.config_dict["matrix"]["allowed_rooms"]
 
         with self.assertRaises(RequiredConfigKeyError):
             config._parse_config_values()

tests/test_webhook.py

@@ -24,11 +24,13 @@ class WebhookApplicationTestCase(aiohttp.test_utils.AioHTTPTestCase):
         self.fake_client = Mock(spec=nio.AsyncClient)
         self.fake_cache = Mock(spec=Cache)
 
+        self.fake_room_id = "!abcdefg:example.com"
+
         self.fake_config = Mock(spec=Config)
         self.fake_config.port = aiohttp.test_utils.unused_port()
         self.fake_config.address = "localhost"
         self.fake_config.socket = "webhook.sock"
-        self.fake_config.room_id = "!abcdefg:example.com"
+        self.fake_config.allowed_rooms = [self.fake_room_id]
         self.fake_config.cache_expire_time = 0
 
         self.fake_alerts = {
@@ -64,13 +66,16 @@ class WebhookApplicationTestCase(aiohttp.test_utils.AioHTTPTestCase):
     @patch.object(matrix_alertbot.webhook, "send_text_to_room")
     async def test_post_alerts(self, fake_send_text_to_room: Mock) -> None:
         data = self.fake_alerts
-        async with self.client.request("POST", "/alerts", json=data) as response:
+        async with self.client.request(
+            "POST", f"/alerts/{self.fake_room_id}", json=data
+        ) as response:
             self.assertEqual(200, response.status)
+
         fake_send_text_to_room.assert_has_calls(
             [
                 call(
                     self.fake_client,
-                    self.fake_config.room_id,
+                    self.fake_room_id,
                     "[🔥 CRITICAL] alert1: some description1",
                     "<font color='#dc3545'><b>[🔥 CRITICAL]</b></font> "
                     "<a href='http://example.com/alert1'>alert1</a> (job1)<br/>"
@@ -79,7 +84,7 @@ class WebhookApplicationTestCase(aiohttp.test_utils.AioHTTPTestCase):
                 ),
                 call(
                     self.fake_client,
-                    self.fake_config.room_id,
+                    self.fake_room_id,
                     "[🥦 RESOLVED] alert2: some description2",
                     "<font color='#33cc33'><b>[🥦 RESOLVED]</b></font> "
                     "<a href='http://example.com/alert2'>alert2</a> (job2)<br/>"
@@ -88,45 +93,83 @@ class WebhookApplicationTestCase(aiohttp.test_utils.AioHTTPTestCase):
                 ),
             ]
         )
+        self.fake_cache.set.assert_called_once_with(
+            fake_send_text_to_room.return_value.event_id,
+            "fingerprint1",
+            expire=self.fake_config.cache_expire_time,
+        )
+
+    @patch.object(matrix_alertbot.webhook, "send_text_to_room")
+    async def test_post_alerts_in_unauthorized_room(
+        self, fake_send_text_to_room: Mock
+    ) -> None:
+        room_id = "!unauthorized_room@example.com"
+        async with self.client.request(
+            "POST", f"/alerts/{room_id}", json=self.fake_alerts
+        ) as response:
+            self.assertEqual(401, response.status)
+            error_msg = await response.text()
+
+        self.assertEqual(
+            "Cannot send alerts to room ID !unauthorized_room@example.com.", error_msg
+        )
+        fake_send_text_to_room.assert_not_called()
+        self.fake_cache.set.assert_not_called()
 
     @patch.object(matrix_alertbot.webhook, "send_text_to_room")
     async def test_post_alerts_with_empty_data(
         self, fake_send_text_to_room: Mock
     ) -> None:
-        async with self.client.request("POST", "/alerts", json={}) as response:
+        async with self.client.request(
+            "POST", f"/alerts/{self.fake_room_id}", json={}
+        ) as response:
             self.assertEqual(400, response.status)
             error_msg = await response.text()
+
         self.assertEqual("Data must contain 'alerts' key.", error_msg)
         fake_send_text_to_room.assert_not_called()
+        self.fake_cache.set.assert_not_called()
 
     @patch.object(matrix_alertbot.webhook, "send_text_to_room")
     async def test_post_empty_alerts(self, fake_send_text_to_room: Mock) -> None:
         data: Dict = {"alerts": []}
-        async with self.client.request("POST", "/alerts", json=data) as response:
+        async with self.client.request(
+            "POST", f"/alerts/{self.fake_room_id}", json=data
+        ) as response:
             self.assertEqual(400, response.status)
             error_msg = await response.text()
+
         self.assertEqual("Alerts cannot be empty.", error_msg)
         fake_send_text_to_room.assert_not_called()
+        self.fake_cache.set.assert_not_called()
 
     @patch.object(matrix_alertbot.webhook, "send_text_to_room")
     async def test_post_invalid_alerts(self, fake_send_text_to_room: Mock) -> None:
         data = {"alerts": "invalid"}
-        async with self.client.request("POST", "/alerts", json=data) as response:
+        async with self.client.request(
+            "POST", f"/alerts/{self.fake_room_id}", json=data
+        ) as response:
             self.assertEqual(400, response.status)
             error_msg = await response.text()
-            self.assertEqual("Alerts must be a list.", error_msg)
+
+        self.assertEqual("Alerts must be a list, got 'str'.", error_msg)
         fake_send_text_to_room.assert_not_called()
+        self.fake_cache.set.assert_not_called()
 
     @patch.object(matrix_alertbot.webhook, "send_text_to_room")
     async def test_post_alerts_with_empty_items(
         self, fake_send_text_to_room: Mock
     ) -> None:
         data: Dict = {"alerts": [{}]}
-        async with self.client.request("POST", "/alerts", json=data) as response:
+        async with self.client.request(
+            "POST", f"/alerts/{self.fake_room_id}", json=data
+        ) as response:
             self.assertEqual(400, response.status)
             error_msg = await response.text()
+
         self.assertEqual("Invalid alert: {}.", error_msg)
         fake_send_text_to_room.assert_not_called()
+        self.fake_cache.set.assert_not_called()
 
     @patch.object(
         matrix_alertbot.webhook,
@@ -137,19 +180,27 @@ class WebhookApplicationTestCase(aiohttp.test_utils.AioHTTPTestCase):
         self, fake_send_text_to_room: Mock
     ) -> None:
         data = self.fake_alerts
-        async with self.client.request("POST", "/alerts", json=data) as response:
+        async with self.client.request(
+            "POST", f"/alerts/{self.fake_room_id}", json=data
+        ) as response:
             self.assertEqual(500, response.status)
             error_msg = await response.text()
+
         self.assertEqual(
             "An error occured when sending alert with fingerprint 'fingerprint1' to Matrix room.",
             error_msg,
         )
         fake_send_text_to_room.assert_called_once()
+        self.fake_cache.set.assert_not_called()
 
     async def test_health(self) -> None:
         async with self.client.request("GET", "/health") as response:
             self.assertEqual(200, response.status)
 
+    async def test_metrics(self) -> None:
+        async with self.client.request("GET", "/metrics") as response:
+            self.assertEqual(200, response.status)
+
 
 class WebhookServerTestCase(unittest.IsolatedAsyncioTestCase):
     async def asyncSetUp(self) -> None:
@@ -160,7 +211,6 @@ class WebhookServerTestCase(unittest.IsolatedAsyncioTestCase):
         self.fake_config.port = aiohttp.test_utils.unused_port()
         self.fake_config.address = "localhost"
         self.fake_config.socket = "webhook.sock"
-        self.fake_config.room_id = "!abcdefg:example.com"
         self.fake_config.cache_expire_time = 0
 
     @patch.object(matrix_alertbot.webhook.web, "TCPSite", autospec=True)