integration: Fix integration with newrelic.

Newrelic updated the payload that's sent via the webhook incoming call
causing a bug in the newrelic webhook endpoint.
This fixes the bug by updating the endpoint to respect the new format
of the payload as well as the old format. This should be updated once
the old format is EOLed.

Fixes #22338.

zerver/webhooks/newrelic/fixtures/incident_acknowledged_new.json

@@ -0,0 +1,11 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "createdAt": 1605133931151,
+    "state": "acknowledged",
+    "owner": "Alice",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/fixtures/incident_active_new.json

@@ -0,0 +1,11 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "createdAt": 1605133931151,
+    "state": "activated",
+    "owner": "",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/fixtures/incident_closed_new.json

@@ -0,0 +1,11 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "createdAt": 1605133931151,
+    "state": "closed",
+    "owner": "",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/fixtures/incident_created_new.json

@@ -0,0 +1,11 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "createdAt": 1605133931151,
+    "state": "created",
+    "owner": "",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/fixtures/incident_default_fields_new.json

@@ -0,0 +1,5 @@
+{
+    "id": 1234,
+    "createdAt": 1605133931151,
+    "state": "activated"
+}

zerver/webhooks/newrelic/fixtures/incident_malformatted_time_new.json

@@ -0,0 +1,11 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "createdAt": "1969-12-31 23:59:55",
+    "state": "acknowledged",
+    "owner": "",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/fixtures/incident_missing_state_new.json

@@ -0,0 +1,10 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "createdAt": 1605133931151,
+    "owner": "Alice",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/fixtures/incident_missing_timestamp_new.json

@@ -0,0 +1,10 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "state": "acknowledged",
+    "owner": "",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/fixtures/incident_state_not_recognized_new.json

@@ -0,0 +1,11 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "createdAt": 1605133931151,
+    "state": "hello world",
+    "owner": "Alice",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/fixtures/incident_time_too_large_new.json

@@ -0,0 +1,11 @@
+{
+    "incident_acknowledge_url": "https://alerts.newrelic.com/accounts/2941966/incidents/1234/acknowledge",
+    "id": 1234,
+    "details": "Violation description test.",
+    "alertPolicyNames": ["Test policy name"],
+    "condition_name": "Server Down",
+    "createdAt": 160513393115100000,
+    "state": "open",
+    "owner": "",
+    "issueUrl": "https://alerts.newrelic.com/accounts/2941966/incidents/1234"
+}

zerver/webhooks/newrelic/tests.py

@@ -6,7 +6,11 @@ class NewRelicHookTests(WebhookTestCase):
     URL_TEMPLATE = "/api/v1/external/newrelic?stream={stream}&api_key={api_key}"
     WEBHOOK_DIR_NAME = "newrelic"
 
-    def test_open(self) -> None:
+    # The following 9 unit tests are for the old format
+    # corresponding json fixtures were renamed to have the "_old" trailing
+    # These tests and fixtures are to be deleted when old notifications EOLed
+
+    def test_open_old(self) -> None:
         expected_topic = "Test policy name (1234)"
         expected_message = """
 [Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **opened** for condition: **Server Down** at <time:2020-11-11 22:32:11.151000+00:00>
@@ -16,42 +20,42 @@ Violation description test.
 """.strip()
 
         self.check_webhook(
-            "incident_opened",
+            "incident_opened_old",
             expected_topic,
             expected_message,
             content_type="application/json",
         )
 
-    def test_closed(self) -> None:
+    def test_closed_old(self) -> None:
         expected_topic = "Test policy name (1234)"
         expected_message = """
 [Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **closed** for condition: **Server Down**
 """.strip()
 
         self.check_webhook(
-            "incident_closed",
+            "incident_closed_old",
             expected_topic,
             expected_message,
             content_type="application/json",
         )
 
-    def test_acknowledged(self) -> None:
+    def test_acknowledged_old(self) -> None:
         expected_topic = "Test policy name (1234)"
         expected_message = """
 [Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **acknowledged** by **Alice** for condition: **Server Down**
 """.strip()
 
         self.check_webhook(
-            "incident_acknowledged",
+            "incident_acknowledged_old",
             expected_topic,
             expected_message,
             content_type="application/json",
         )
 
-    def test_not_recognized(self) -> None:
+    def test_not_recognized_old(self) -> None:
         with self.assertRaises(AssertionError) as e:
             self.check_webhook(
-                "incident_state_not_recognized",
+                "incident_state_not_recognized_old",
                 "",
                 "",
                 content_type="application/json",
@@ -61,7 +65,7 @@ Violation description test.
             e.exception.args[0],
         )
 
-    def test_missing_fields(self) -> None:
+    def test_missing_fields_old(self) -> None:
         expected_topic = "Unknown Policy (Unknown ID)"
         expected_message = """
 [Incident](https://alerts.newrelic.com) **opened** for condition: **Unknown condition** at <time:2020-11-11 22:32:11.151000+00:00>
@@ -71,16 +75,16 @@ No details.
 """.strip()
 
         self.check_webhook(
-            "incident_default_fields",
+            "incident_default_fields_old",
             expected_topic,
             expected_message,
             content_type="application/json",
         )
 
-    def test_missing_current_state(self) -> None:
+    def test_missing_current_state_old(self) -> None:
         with self.assertRaises(AssertionError) as e:
             self.check_webhook(
-                "incident_missing_current_state",
+                "incident_missing_current_state_old",
                 "",
                 "",
                 content_type="application/json",
@@ -90,10 +94,10 @@ No details.
             e.exception.args[0],
         )
 
-    def test_missing_timestamp(self) -> None:
+    def test_missing_timestamp_old(self) -> None:
         with self.assertRaises(AssertionError) as e:
             self.check_webhook(
-                "incident_missing_timestamp",
+                "incident_missing_timestamp_old",
                 "",
                 "",
                 content_type="application/json",
@@ -102,20 +106,153 @@ No details.
             "The newrelic webhook requires timestamp in milliseconds", e.exception.args[0]
         )
 
-    def test_malformatted_time(self) -> None:
+    def test_malformatted_time_old(self) -> None:
         with self.assertRaises(AssertionError) as e:
             self.check_webhook(
-                "incident_malformatted_time",
+                "incident_malformatted_time_old",
                 "",
                 "",
                 content_type="application/json",
             )
         self.assertIn("The newrelic webhook expects time in milliseconds.", e.exception.args[0])
 
-    def test_time_too_large(self) -> None:
+    def test_time_too_large_old(self) -> None:
         with self.assertRaises(AssertionError) as e:
             self.check_webhook(
-                "incident_time_too_large",
+                "incident_time_too_large_old",
+                "",
+                "",
+                content_type="application/json",
+            )
+        self.assertIn("The newrelic webhook expects time in milliseconds.", e.exception.args[0])
+
+    # The following 10 unit tests are for the new format
+    # One more test than the old format as we have 4 states instead of 3 in the old
+    # corresponding json fixtures have "_new" trailing in the name
+
+    def test_activated_new(self) -> None:
+        expected_topic = "Test policy name (1234)"
+        expected_message = """
+[Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **active** for condition: **Server Down** at <time:2020-11-11 22:32:11.151000+00:00>
+``` quote
+Violation description test.
+```
+""".strip()
+
+        self.check_webhook(
+            "incident_active_new",
+            expected_topic,
+            expected_message,
+            content_type="application/json",
+        )
+
+    def test_created_new(self) -> None:
+        expected_topic = "Test policy name (1234)"
+        expected_message = """
+[Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **created** for condition: **Server Down**
+""".strip()
+
+        self.check_webhook(
+            "incident_created_new",
+            expected_topic,
+            expected_message,
+            content_type="application/json",
+        )
+
+    def test_closed_new(self) -> None:
+        expected_topic = "Test policy name (1234)"
+        expected_message = """
+[Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **closed** for condition: **Server Down**
+""".strip()
+
+        self.check_webhook(
+            "incident_closed_new",
+            expected_topic,
+            expected_message,
+            content_type="application/json",
+        )
+
+    def test_acknowledged_new(self) -> None:
+        expected_topic = "Test policy name (1234)"
+        expected_message = """
+[Incident](https://alerts.newrelic.com/accounts/2941966/incidents/1234) **acknowledged** by **Alice** for condition: **Server Down**
+""".strip()
+
+        self.check_webhook(
+            "incident_acknowledged_new",
+            expected_topic,
+            expected_message,
+            content_type="application/json",
+        )
+
+    def test_not_recognized_new(self) -> None:
+        with self.assertRaises(AssertionError) as e:
+            self.check_webhook(
+                "incident_state_not_recognized_new",
+                "",
+                "",
+                content_type="application/json",
+            )
+        self.assertIn(
+            "The newrelic webhook requires state be in [created|activated|acknowledged|closed]",
+            e.exception.args[0],
+        )
+
+    def test_missing_fields_new(self) -> None:
+        expected_topic = "Unknown Policy (1234)"
+        expected_message = """
+[Incident](https://alerts.newrelic.com) **active** for condition: **Unknown condition** at <time:2020-11-11 22:32:11.151000+00:00>
+``` quote
+No details.
+```
+""".strip()
+
+        self.check_webhook(
+            "incident_default_fields_new",
+            expected_topic,
+            expected_message,
+            content_type="application/json",
+        )
+
+    def test_missing_state_new(self) -> None:
+        with self.assertRaises(AssertionError) as e:
+            self.check_webhook(
+                "incident_missing_state_new",
+                "",
+                "",
+                content_type="application/json",
+            )
+        self.assertIn(
+            "The newrelic webhook requires state be in [created|activated|acknowledged|closed]",
+            e.exception.args[0],
+        )
+
+    def test_missing_timestamp_new(self) -> None:
+        with self.assertRaises(AssertionError) as e:
+            self.check_webhook(
+                "incident_missing_timestamp_new",
+                "",
+                "",
+                content_type="application/json",
+            )
+        self.assertIn(
+            "The newrelic webhook requires timestamp in milliseconds", e.exception.args[0]
+        )
+
+    def test_malformatted_time_new(self) -> None:
+        with self.assertRaises(AssertionError) as e:
+            self.check_webhook(
+                "incident_malformatted_time_new",
+                "",
+                "",
+                content_type="application/json",
+            )
+        self.assertIn("The newrelic webhook expects time in milliseconds.", e.exception.args[0])
+
+    def test_time_too_large_new(self) -> None:
+        with self.assertRaises(AssertionError) as e:
+            self.check_webhook(
+                "incident_time_too_large_new",
                 "",
                 "",
                 content_type="application/json",

zerver/webhooks/newrelic/view.py

@@ -11,6 +11,12 @@ from zerver.lib.response import json_success
 from zerver.lib.webhooks.common import check_send_webhook_message, unix_milliseconds_to_timestamp
 from zerver.models import UserProfile
 
+# Newrelic planned to upgrade Alert Notification Channels to Workflows and Destinations
+# https://discuss.newrelic.com/t/plan-to-upgrade-alert-notification-channels-to-workflows-and-destinations/188205
+# This view will handle both old and new format but will keep it easy to delete the old code
+# once it is EOLed by the end of June, 2023
+
+# Once old is EOLed, delete the OPEN_TEMPLATE
 OPEN_TEMPLATE = """
 [Incident]({incident_url}) **opened** for condition: **{condition_name}** at <time:{iso_timestamp}>
 ``` quote
@@ -18,13 +24,23 @@ OPEN_TEMPLATE = """
 ```
 """.strip()
 
+ACTIVE_TEMPLATE = """
+[Incident]({incident_url}) **active** for condition: **{condition_name}** at <time:{iso_timestamp}>
+``` quote
+{details}
+```
+""".strip()
+
 DEFAULT_TEMPLATE = (
     """[Incident]({incident_url}) **{status}** {owner}for condition: **{condition_name}**""".strip()
 )
 
 TOPIC_TEMPLATE = """{policy_name} ({incident_id})""".strip()
 
-ALL_EVENT_TYPES = ["closed", "acknowledged", "open"]
+# Once old is EOLed, delete old and keep new
+OLD_EVENT_TYPES = ["closed", "acknowledged", "open"]
+NEW_EVENT_TYPES = ["created", "activated", "acknowledged", "closed"]
+ALL_EVENT_TYPES = list(set(OLD_EVENT_TYPES).union(set(NEW_EVENT_TYPES)))
 
 
 @webhook_view("NewRelic", all_event_types=ALL_EVENT_TYPES)
@@ -35,45 +51,102 @@ def api_newrelic_webhook(
     payload: Dict[str, Any] = REQ(argument_type="body"),
 ) -> HttpResponse:
 
-    info = {
-        "condition_name": payload.get("condition_name", "Unknown condition"),
-        "details": payload.get("details", "No details."),
-        "incident_url": payload.get("incident_url", "https://alerts.newrelic.com"),
-        "incident_acknowledge_url": payload.get(
-            "incident_acknowledge_url", "https://alerts.newrelic.com"
-        ),
-        "status": payload.get("current_state", "None"),
-        "iso_timestamp": "",
-        "owner": payload.get("owner", ""),
-    }
+    # Handle old format
+    # Once old is EOLed, delete if block and keep else block
+    if not payload.get("id"):
+        info = {
+            "condition_name": payload.get("condition_name", "Unknown condition"),
+            "details": payload.get("details", "No details."),
+            "incident_url": payload.get("incident_url", "https://alerts.newrelic.com"),
+            "incident_acknowledge_url": payload.get(
+                "incident_acknowledge_url", "https://alerts.newrelic.com"
+            ),
+            "status": payload.get("current_state", "None"),
+            "iso_timestamp": "",
+            "owner": payload.get("owner", ""),
+        }
 
-    unix_time = payload.get("timestamp", None)
-    if unix_time is None:
-        raise JsonableError(_("The newrelic webhook requires timestamp in milliseconds"))
+        unix_time = payload.get("timestamp", None)
+        if unix_time is None:
+            raise JsonableError(_("The newrelic webhook requires timestamp in milliseconds"))
 
-    info["iso_timestamp"] = unix_milliseconds_to_timestamp(unix_time, "newrelic")
+        info["iso_timestamp"] = unix_milliseconds_to_timestamp(unix_time, "newrelic")
 
-    # Add formatting to the owner field if owner is present
-    if info["owner"] != "":
-        info["owner"] = "by **{}** ".format(info["owner"])
+        # Add formatting to the owner field if owner is present
+        if info["owner"] != "":
+            info["owner"] = "by **{}** ".format(info["owner"])
 
-    # These are the three promised current_state values
-    if "open" in info["status"]:
-        content = OPEN_TEMPLATE.format(**info)
-    elif "acknowledged" in info["status"]:
-        content = DEFAULT_TEMPLATE.format(**info)
-    elif "closed" in info["status"]:
-        content = DEFAULT_TEMPLATE.format(**info)
+        # These are the three promised current_state values
+        if info["status"].lower() == "open":
+            content = OPEN_TEMPLATE.format(**info)
+        elif info["status"].lower() == "acknowledged":
+            content = DEFAULT_TEMPLATE.format(**info)
+        elif info["status"].lower() == "closed":
+            content = DEFAULT_TEMPLATE.format(**info)
+        else:
+            raise JsonableError(
+                _("The newrelic webhook requires current_state be in [open|acknowledged|closed]")
+            )
+
+        topic_info = {
+            "policy_name": payload.get("policy_name", "Unknown Policy"),
+            "incident_id": payload.get("incident_id", "Unknown ID"),
+        }
+        topic = TOPIC_TEMPLATE.format(**topic_info)
+
+        check_send_webhook_message(request, user_profile, topic, content, info["status"])
+        return json_success(request)
+
+    # Handle new format
     else:
-        raise JsonableError(
-            _("The newrelic webhook requires current_state be in [open|acknowledged|closed]")
-        )
+        info = {
+            "condition_name": payload.get("condition_name", "Unknown condition"),
+            "details": payload.get("details", "No details."),
+            "incident_url": payload.get("issueUrl", "https://alerts.newrelic.com"),
+            "incident_acknowledge_url": payload.get(
+                "incident_acknowledge_url", "https://alerts.newrelic.com"
+            ),
+            "status": payload.get("state", "None"),
+            "iso_timestamp": "",
+            "owner": payload.get("owner", ""),
+        }
 
-    topic_info = {
-        "policy_name": payload.get("policy_name", "Unknown Policy"),
-        "incident_id": payload.get("incident_id", "Unknown ID"),
-    }
-    topic = TOPIC_TEMPLATE.format(**topic_info)
+        unix_time = payload.get("createdAt", None)
+        if unix_time is None:
+            raise JsonableError(_("The newrelic webhook requires timestamp in milliseconds"))
 
-    check_send_webhook_message(request, user_profile, topic, content, info["status"])
-    return json_success(request)
+        info["iso_timestamp"] = unix_milliseconds_to_timestamp(unix_time, "newrelic")
+
+        # Add formatting to the owner field if owner is present
+        if info["owner"] != "":
+            info["owner"] = "by **{}** ".format(info["owner"])
+
+        # These are the three promised state values
+        if info["status"].lower() == "activated":
+            content = ACTIVE_TEMPLATE.format(**info)
+        elif info["status"].lower() == "acknowledged":
+            content = DEFAULT_TEMPLATE.format(**info)
+        elif info["status"].lower() == "closed":
+            content = DEFAULT_TEMPLATE.format(**info)
+        elif info["status"].lower() == "created":
+            content = DEFAULT_TEMPLATE.format(**info)
+        else:
+            raise JsonableError(
+                _(
+                    "The newrelic webhook requires state be in [created|activated|acknowledged|closed]"
+                )
+            )
+
+        policy_names_list = payload.get("alertPolicyNames", [])
+        if policy_names_list:
+            policy_names_str = ",".join(policy_names_list)
+        else:
+            policy_names_str = "Unknown Policy"
+        topic_info = {
+            "policy_name": policy_names_str,
+            "incident_id": payload.get("id", "Unknown ID"),
+        }
+        topic = TOPIC_TEMPLATE.format(**topic_info)
+
+        check_send_webhook_message(request, user_profile, topic, content, info["status"])
+        return json_success(request)