fix(notifications): update Shoutrrr MS Teams integration - #1718
Conversation
- Replace `ConfigFromWebhookURL` with direct `teams.Config` construction using the full webhook URL as host - Remove `data` field and `errConfigWebhookFailed` error that are no longer needed - Update test to use a Power Automate workflow URL instead of an Office 365 webhook URL - Adjust expected test output to match the new `teams:?color=...&host=...` URL format - Update MSTeams docs for Power Automate webhook URLs - Fix MSTeams docs with correct configuration name
- Validate that MSTeams webhook URLs use HTTPS scheme and contain a host
- Remove `--notification-msteams-data` flag and `WATCHTOWER_NOTIFICATION_MSTEAMS_USE_LOG_DATA` env var - Strip MSTeams log data documentation and example references - Simplify MSTeams notifier initialization by removing unused withData logic
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR simplifies Microsoft Teams notification configuration by removing the data extraction flag, renaming the webhook environment variable to ChangesMSTeams notification configuration update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1718 +/- ##
==========================================
- Coverage 74.59% 74.23% -0.36%
==========================================
Files 60 61 +1
Lines 10052 10188 +136
==========================================
+ Hits 7498 7563 +65
- Misses 2286 2347 +61
- Partials 268 278 +10
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/notifications/overview/index.md (1)
827-831: ⚡ Quick winDocument the HTTPS/absolute URL requirement explicitly.
Please state that
--notification-msteams-hookmust be an absolutehttps://URL (with host), matching runtime validation inpkg/notifications/msteams.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/notifications/overview/index.md` around lines 827 - 831, Update the MSTeams Hook URL docs block in docs/notifications/overview/index.md to explicitly require that the --notification-msteams-hook flag value is an absolute https:// URL including the host (no relative URLs, must use HTTPS), and note that this matches the runtime validation implemented in pkg/notifications/msteams.go; mention the exact flag name (--notification-msteams-hook) and the HTTPS/absolute requirement so users know the expected format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/notifications/msteams.go`:
- Around line 76-82: The code is incorrectly passing the entire Power Automate
webhook URL into teams.Config.Host (n.webHookURL) — change
pkg/notifications/msteams.go so you either (A) extract only the legacy Teams
webhook hostname (e.g., organization.webhook.office.com) and set that into
teams.Config.Host before calling config.GetURL(), or (B) when the provided
webhook is a full Power Automate/workflow URL, do not use teams.Config at all
and instead construct a shoutrrr generic service URL
(generic://<full-webhook>?contenttype=text/plain) and use the generic sender;
update the logic around teams.Config, n.webHookURL, and config.GetURL()
accordingly and adjust tests in notifier_test.go to expect the generic URL for
Power Automate-style hooks.
---
Nitpick comments:
In `@docs/notifications/overview/index.md`:
- Around line 827-831: Update the MSTeams Hook URL docs block in
docs/notifications/overview/index.md to explicitly require that the
--notification-msteams-hook flag value is an absolute https:// URL including the
host (no relative URLs, must use HTTPS), and note that this matches the runtime
validation implemented in pkg/notifications/msteams.go; mention the exact flag
name (--notification-msteams-hook) and the HTTPS/absolute requirement so users
know the expected format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84d186d2-bd94-40db-b3d1-9f19e489f6fb
📒 Files selected for processing (5)
docs/configuration/arguments/index.mddocs/notifications/overview/index.mdinternal/flags/flags.gopkg/notifications/msteams.gopkg/notifications/notifier_test.go
💤 Files with no reviewable changes (1)
- internal/flags/flags.go
This PR updates the Microsoft Teams notification integration to work with the updated Shoutrrr MS Teams service SDK, which removed
ConfigFromWebhookURL()and changed the webhook format from legacyoutlook.office.com/webhookb2/...URLs to Power Automate workflow URLs. It also removes the dead--notification-msteams-dataflag that was registered but never consumed.Problems
A recent change to Microsoft Teams webhook format was recently implemented in Shoutrrr's MS Teams service. This change removed
ConfigFromWebhookURL()and changed the webhook URL format, causing a compiler error (undefined: teams.ConfigFromWebhookURL). The test suite also used the old webhook format and expected the oldteams://path?...URL serialization. Additionally, the--notification-msteams-dataflag was dead code — registered in flags.go and parsed in msteams.go but never stored or used in any behavior.Solution
Updated the Teams notifier to construct
teams.Configdirectly with the full Power Automate workflow URL asConfig.Host, which is the new SDK pattern. Removed the dead--notification-msteams-dataflag registration, parsing, and all documentation references. Updated tests and docs to use the current webhook URL format.Changes
pkg/notifications/msteams.go— replacedteams.ConfigFromWebhookURL()with directteams.Config{Host: webhookURL, Color: ColorHex}construction; removeddatafield,errConfigWebhookFailedsentinel, and dead flag parsingpkg/notifications/notifier_test.go— updated test to use Power Automate workflow URL and newteams:?color=...&host=...URL formatinternal/flags/flags.go— removed--notification-msteams-dataflag registrationdocs/notifications/overview/index.md— updated all 3 code examples with new webhook URL, removed MSTeams Use Log Data config section, fixed env var referencesdocs/configuration/arguments/index.md— fixed env var nameWATCHTOWER_NOTIFICATION_MSTEAMS_HOOK→WATCHTOWER_NOTIFICATION_MSTEAMS_HOOK_URLSummary by CodeRabbit
Documentation
Refactor