RMST-421: Optional webhook per channel

kinto-slack/README.rst

@@ -29,7 +29,6 @@ The metadata on the collection (or the bucket) must look like this:
     {
       "kinto-slack": {
         "hooks": [{
-          "channel": "#general",
           "template": "Something happened!"
         }]
       }
@@ -45,15 +44,14 @@ Selection
 ---------
 
 It is possible to define several *hooks* and filter on conditions. For example,
-to notify ``#team-component`` whenever a review is requested on any collection:
+to notify whenever a review is requested on any collection:
 
 .. code-block:: json
 
     {
       "kinto-slack": {
         "hooks": [{
           "event": "kinto_remote_settings.signer.events.ReviewRequested",
-          "channel": "#team-component",
           "template": "{user_id} requested review of {collection_id} ({root_url}{uri})"
         }]
       }
@@ -106,3 +104,35 @@ The template string can contain the following placeholders:
 For example:
 
 ``{user_id} has {action}d {resource_name} {id} in {bucket_id}/{collection_id}.``
+
+
+Slack Channel Routing
+---------------------
+
+Slack Webhooks URLs are bound to a specific Slack channel. By default, all
+notifications will be sent to the Slack channel configured globally in the
+``kinto.slack.webhook_url`` setting.
+
+In order to send a Slack notification to a specific channel, start with adding the `channel` field
+to the collection metadata:
+
+.. code-block:: json
+
+    {
+      "kinto-slack": {
+          "hooks": [{
+            ...
+            "channel": "#fxmonitor-alerts"
+          }]
+      }
+    }
+
+The Webhook URL for this channel can now be configured via ``.ini`` config:
+
+.. code-block:: ini
+
+    kinto.slack.fxmonitor-alerts.webhook_url = https://...
+
+or the ``KINTO_SLACK_FXMONITOR_ALERTS_WEBHOOK_URL`` environment variable (safer).
+
+In order to obtain the Webhook URL, `see official Slack docs <https://docs.slack.dev/messaging/sending-messages-using-incoming-webhooks/>`_.

kinto-slack/src/kinto_slack/__init__.py

@@ -64,7 +64,7 @@ def get_messages(storage, context):
 
         messages.append(
             {
-                "channel": hook["channel"],
+                "channel": hook.get("channel"),
                 "text": hook["template"].format_map(defaultdict(str, context)),
             }
         )
@@ -107,22 +107,42 @@ def send_notification(event):
         return
 
     settings = event.request.registry.settings
-    webhook_url = settings.get("slack.webhook_url")
+    # Get default webhook URL from .ini
+    default_webhook_url = settings.get("slack.webhook_url")
+    # Try to read it from env.
+    default_webhook_url = read_env("kinto.slack.webhook_url", default_webhook_url)
+    # Used for testing and debugging (similar to kinto-emailer).
     debug_dir = settings.get("slack.debug_dir")
 
-    if not webhook_url and not debug_dir:
-        logger.warning("slack.webhook_url is not configured")
-        return
-
     for msg in messages:
+        channel_webhook_url = None
+        if msg["channel"]:
+            slack_channel = msg["channel"].replace("#", "")
+            # Channel webhook URL from .ini
+            settings_key = f"slack.{slack_channel}.webhook_url"
+            channel_webhook_url = settings.get(settings_key)
+            # Try to read it from env. See `read_env()`.
+            # The env var must be `KINTO_SLACK_{CHANNEL}_WEBHOOK_URL
+            # eg. KINTO_SLACK_FXMONITOR_ALERTS_WEBHOOK_URL
+            channel_webhook_url = read_env(f"kinto.{settings_key}", channel_webhook_url)
+            if not channel_webhook_url and default_webhook_url:
+                logger.warning(
+                    f"{settings_key} not found in config or environment. Fallback to default"
+                )
+        if not channel_webhook_url:
+            channel_webhook_url = default_webhook_url
+            if not channel_webhook_url and not debug_dir:
+                logger.warning("No Slack Webhook URL or debug dir configured.")
+
         if debug_dir:
             os.makedirs(debug_dir, exist_ok=True)
             filename = os.path.join(debug_dir, f"{time.time_ns()}.json")
             with open(filename, "w") as f:
                 json.dump(msg, f)
-        if webhook_url:
+
+        if channel_webhook_url:
             try:
-                resp = requests.post(webhook_url, json=msg, timeout=5)
+                resp = requests.post(channel_webhook_url, json=msg, timeout=5)
                 resp.raise_for_status()
             except Exception:
                 logger.exception("Could not send Slack notification")
@@ -135,12 +155,6 @@ def _validate_slack_settings(event):
         obj = impacted.get("new", {})
         hooks = obj.get("kinto-slack", {}).get("hooks", [])
         for hook in hooks:
-            if "channel" not in hook:
-                raise_invalid(
-                    event.request,
-                    name="kinto-slack",
-                    description="Hook is missing 'channel'",
-                )
             if "template" not in hook:
                 raise_invalid(
                     event.request,
@@ -151,9 +165,6 @@ def _validate_slack_settings(event):
 
 def includeme(config):
     settings = config.get_settings()
-    webhook_url = settings.get("slack.webhook_url")
-    webhook_url = read_env("kinto.slack.webhook_url", webhook_url)
-    config.add_settings({"slack.webhook_url": webhook_url})
 
     tmp_dir = settings.get("slack.debug_dir")
     tmp_dir = read_env("kinto.slack.debug_dir", tmp_dir)

kinto-slack/tests/test_kinto_slack.py

@@ -1,7 +1,7 @@
 import unittest
 from unittest import mock
 
-from kinto_slack import _validate_slack_settings, get_messages
+from kinto_slack import _validate_slack_settings, get_messages, send_notification
 
 
 COLLECTION_METADATA = {
@@ -147,6 +147,21 @@ def test_falls_back_to_bucket_metadata(self):
         (msg,) = get_messages(self.storage, CONTEXT)
         assert msg["channel"] == "#security-alerts"
 
+    def test_channel_is_optional(self):
+        self.storage.get.return_value = {
+            "kinto-slack": {
+                "hooks": [
+                    {
+                        "resource_name": "record",
+                        "action": "create",
+                        "template": "no channel",
+                    }
+                ]
+            }
+        }
+        (msg,) = get_messages(self.storage, CONTEXT)
+        assert msg["channel"] is None
+
     def test_no_hooks_returns_empty(self):
         self.storage.get.return_value = {}
         assert get_messages(self.storage, CONTEXT) == []
@@ -178,12 +193,11 @@ def test_valid_hook_does_not_raise(self):
         )
         _validate_slack_settings(event)  # no exception
 
-    def test_missing_channel_raises(self):
+    def test_missing_channel_does_not_raise(self):
         event = self._make_event([{"template": "msg"}])
         with mock.patch("kinto_slack.raise_invalid") as patched:
             _validate_slack_settings(event)
-        patched.assert_called_once()
-        assert "channel" in patched.call_args.kwargs["description"]
+        patched.assert_not_called()
 
     def test_missing_template_raises(self):
         event = self._make_event([{"channel": "#alerts"}])
@@ -198,3 +212,70 @@ def test_delete_action_skips_validation(self):
             {"old": {"kinto-slack": {"hooks": [{"bad": "hook"}]}}}
         ]
         _validate_slack_settings(event)  # no exception
+
+
+class SendNotificationTest(unittest.TestCase):
+    def setUp(self) -> None:
+        patch = mock.patch("kinto_slack.requests.post")
+        self.mocked_post = patch.start()
+        self.addCleanup(patch.stop)
+        return super().setUp()
+
+    def _make_event(self, messages, settings):
+        event = mock.MagicMock()
+        event.request._kinto_slack_messages = messages
+        event.request.registry.settings = settings
+        return event
+
+    def test_no_messages_does_nothing(self):
+        event = self._make_event([], {"slack.webhook_url": "https://default"})
+        send_notification(event)
+        self.mocked_post.assert_not_called()
+
+    def test_uses_channel_specific_webhook_url(self):
+        event = self._make_event(
+            [{"channel": "#fxmonitor-alerts", "text": "hi"}],
+            {
+                "slack.webhook_url": "https://default",
+                "slack.fxmonitor-alerts.webhook_url": "https://channel",
+            },
+        )
+        send_notification(event)
+        self.mocked_post.assert_called_once()
+        assert self.mocked_post.call_args.args[0] == "https://channel"
+
+    def test_falls_back_to_default_webhook_url(self):
+        event = self._make_event(
+            [{"channel": "#unknown", "text": "hi"}],
+            {"slack.webhook_url": "https://default"},
+        )
+        send_notification(event)
+        self.mocked_post.assert_called_once()
+        assert self.mocked_post.call_args.args[0] == "https://default"
+
+    def test_no_channel_uses_default_webhook_url(self):
+        event = self._make_event(
+            [{"channel": None, "text": "hi"}],
+            {"slack.webhook_url": "https://default"},
+        )
+        send_notification(event)
+        self.mocked_post.assert_called_once()
+        assert self.mocked_post.call_args.args[0] == "https://default"
+
+    def test_no_webhook_url_configured_does_not_post(self):
+        event = self._make_event([{"channel": "#alerts", "text": "hi"}], {})
+        send_notification(event)
+        self.mocked_post.assert_not_called()
+
+    def test_reads_channel_webhook_url_from_env(self):
+        event = self._make_event(
+            [{"channel": "#fxmonitor-alerts", "text": "hi"}],
+            {"slack.webhook_url": "https://default"},
+        )
+        with mock.patch.dict(
+            "os.environ",
+            {"KINTO_SLACK_FXMONITOR_ALERTS_WEBHOOK_URL": "https://from-env"},
+        ):
+            send_notification(event)
+        self.mocked_post.assert_called_once()
+        assert self.mocked_post.call_args.args[0] == "https://from-env"