NMS-20113: Replace the Slack and Mattermost notification channels with a generic webhook - #8734
Open
marshallmassengill wants to merge 4 commits into
Open
NMS-20113: Replace the Slack and Mattermost notification channels with a generic webhook#8734marshallmassengill wants to merge 4 commits into
marshallmassengill wants to merge 4 commits into
Conversation
marshallmassengill
requested review from
cgorantla,
Copilot,
dino2gnt,
indigo423 and
joseanesONMS
August 3, 2026 15:44
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the dedicated Slack and Mattermost notification strategies in notifd with a single WebhookNotificationStrategy that can send arbitrary HTTP requests (method, headers, body templating), and updates tests and documentation accordingly.
Changes:
- Introduces
WebhookNotificationStrategywith body templating, optional JSON escaping/validation, configurable headers, timeouts, proxy usage, and success matching. - Replaces legacy Slack/Mattermost strategy implementations and their integration/unit tests with webhook-focused tests and a new test servlet/webapp.
- Updates release notes and operations docs to document webhook-based notifications and migration guidance; adds an explicit
jackson-databinddependency.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| opennms-services/src/test/resources/WebhookNotificationStrategyTest/WEB-INF/web.xml | Updates the embedded test webapp to expose a webhook endpoint and servlet. |
| opennms-services/src/test/java/org/opennms/netmgt/notifd/WebhookNotificationStrategyTestServlet.java | Adds a generic servlet for capturing webhook requests during ITs. |
| opennms-services/src/test/java/org/opennms/netmgt/notifd/WebhookNotificationStrategyTest.java | Adds unit tests for templating, escaping, JSON validation, headers, and success evaluation. |
| opennms-services/src/test/java/org/opennms/netmgt/notifd/WebhookNotificationStrategyIT.java | Adds integration tests exercising end-to-end HTTP behavior against the embedded servlet. |
| opennms-services/src/test/java/org/opennms/netmgt/notifd/SlackNotificationStrategyIT.java | Removes legacy Slack strategy integration test. |
| opennms-services/src/test/java/org/opennms/netmgt/notifd/SlackCompatibleNotificationStrategyTest.java | Removes legacy shared Slack/Mattermost compatibility tests. |
| opennms-services/src/test/java/org/opennms/netmgt/notifd/MattermostNotificationStrategyTestServlet.java | Removes legacy Mattermost test servlet. |
| opennms-services/src/test/java/org/opennms/netmgt/notifd/MattermostNotificationStrategyIT.java | Removes legacy Mattermost integration test. |
| opennms-services/src/main/java/org/opennms/netmgt/notifd/WebhookNotificationStrategy.java | Adds the new generic webhook notification strategy implementation. |
| opennms-services/src/main/java/org/opennms/netmgt/notifd/SlackNotificationStrategy.java | Removes legacy Slack-specific strategy. |
| opennms-services/src/main/java/org/opennms/netmgt/notifd/MattermostNotificationStrategy.java | Removes legacy Mattermost-specific strategy. |
| opennms-services/src/main/java/org/opennms/netmgt/notifd/AbstractSlackCompatibleNotificationStrategy.java | Removes legacy shared base strategy for Slack/Mattermost. |
| opennms-services/pom.xml | Adds explicit jackson-databind dependency used by the new strategy. |
| docs/modules/releasenotes/pages/whatsnew.adoc | Documents the removal of Slack/Mattermost strategies and migration to webhook strategy. |
| docs/modules/releasenotes/pages/changelog.adoc | Adds changelog entry for NMS-20113. |
| docs/modules/operation/pages/quick-start/beyond-qs.adoc | Updates quick-start guidance to reference webhook-based notification routing. |
| docs/modules/operation/pages/deep-dive/notifications/strategies/webhook.adoc | Adds comprehensive webhook strategy documentation and migration mapping. |
| docs/modules/operation/pages/deep-dive/notifications/strategies/slack.adoc | Removes Slack strategy documentation. |
| docs/modules/operation/pages/deep-dive/notifications/strategies/mattermost.adoc | Removes Mattermost strategy documentation. |
| docs/modules/operation/pages/deep-dive/notifications/introduction.adoc | Updates notification methods list to describe webhook destinations. |
| docs/modules/operation/pages/deep-dive/notifications/bonus-strategies.adoc | Updates bonus strategies overview to reflect webhook approach. |
| docs/modules/operation/nav.adoc | Updates navigation to link to webhook strategy docs instead of Slack/Mattermost pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…h a generic webhook The Slack and Mattermost strategies shared a base class that built one hardcoded JSON shape, so no other receiver could be expressed, and it treated only a literal "ok" response body as success, which meant a receiver answering 204 with no body was always logged as failed. Adds WebhookNotificationStrategy, which takes the URL, method, content type, headers, and body template from the notification command. Values substituted into a JSON body are escaped, with a |raw modifier for fragments that are themselves JSON, and the rendered body is parsed before it is sent so a template mistake is reported locally instead of as an opaque 400. Any 2xx counts as success, narrowed by an optional -success-match for receivers that report failure behind a 200. Template tokens deliberately exclude colons so they cannot collide with the metadata DSL, which notifd resolves against notification parameters before the command runs. Removes the two strategies, their shared base class, and their tests, and replaces the two documentation pages with a single webhook page carrying Slack, Mattermost, Microsoft Teams, Discord, and non-chat examples plus a migration table.
Log only the scheme and host of the webhook URL. Slack, Discord and
Teams URLs carry their credential in the path or query, so the previous
error log wrote a secret to notifd.log on any connection failure.
Reject blank bodies and trailing content when validating a JSON payload.
ObjectMapper.readTree maps blank input to a MissingNode and stops at the
first value, so {"text": "x"} oops was passing the check the strategy
advertises and failing at the receiver instead.
Also warn on an unknown template modifier rather than emitting the token
verbatim, ignore a negative timeout, and document that substitution
applies to the body alone and that a header must be declared as an
argument for its system property to take effect.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
marshallmassengill
force-pushed
the
mm/NMS-20113-smoke
branch
from
August 3, 2026 17:37
3b3ed9f to
7ca5894
Compare
AsciiDoc honors a backslash escape before a brace only when the braces
hold a valid attribute name, so ${...}, ${customBlocks|raw} and
${scv:...} kept their backslash and rendered as $\{...} on the page.
None of the three needs escaping, because none of them parses as an
attribute reference in the first place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the Slack and Mattermost notification strategies with a single WebhookNotificationStrategy that takes the URL, method, content type, headers, and body from the notification command, so it can also target Microsoft Teams, Discord (for all Gen Z users I guess), and non-chat endpoints.
Highlights:
-useSystemProxy now defaults to false, matching HttpNotificationStrategy; the removed strategies defaulted it to true.
I'm going to need to resolve the conflicts with the whatsnew and such based on the other notifd update PRs.
Improvements assisted by Anthropic Claude Opus 5.
External References