alertmanager icon indicating copy to clipboard operation
alertmanager copied to clipboard

Slack app support

Open pharaujo opened this issue 11 months ago β€’ 5 comments

While it's possible to configure Slack app bot tokens using a combination of http_configs authorization credentials and setting the Slack API URL to the a specific endpoint, doing so at a global level leaks the token to any other notification receiver configured, as http_configs are not specific to notifiers. Slack has also restricted webhook URLs to only be able to post to a single channel (with the legacy webhooks being marked as deprecated and not recommended), which reduces their usefulness when set at the global level.

This PR adds a way of easily setting Slack App bot tokens at the global level, as well as overriding at the individual receiver level, while keeping compatibility with existing configurations.

The decision to have a separate config field for the URL was to be able to provide a default API URL for Slack apps as well as differentiate when a webhook url is provided. Ideally we'd change the slack_api_url to be slack_webhook_url so as to avoid confusion, but that would be an unnecessary breaking change.

More context in issue #2513

pharaujo avatar Jan 17 '25 12:01 pharaujo

So we do also need BlockKit support, and I think this feature, along with BlockKit, would be perfect for a Slack V2 integration, just like we did for Microsoft Teams.

grobinson-grafana avatar Feb 02 '25 21:02 grobinson-grafana

So we do also need BlockKit support, and I think this feature, along with BlockKit, would be perfect for a Slack V2 integration, just like we did for Microsoft Teams.

I'm a bit confused. Is this feedback for me, or for other reviewers? Are you saying I should implement this under a new slack integration, or close this PR since someone else is implementing it together with Block Kit?

To be clear, I'm able to change the PR to kickstart the new slack integration and plug internals into the current integration, but I don't want to extend the scope of this PR to add such a large piece of work such as Block Kit.

What's the recommendation here?

pharaujo avatar Feb 03 '25 11:02 pharaujo

Thanks for your patience!

Thank you for reviewing!

I Took some time to go through this a little more carefully. Overall, this seems good to me. It looks like there shouldn't be any behavior changes for anybody who's already using the slack integration (because they would've needed to set slack_api_url already), which is my biggest concern.

Yeah, the way it's configured, the slack app token logic is opt in, and everything keeps working for existing users that use the legacy webhook url. The logic that's a bit more complex is to guard against breaking users that set the slack app token through http_configs, as suggested hereβ€”given the amount of reactions for that comment, I think it might be fairly widespread.

My second concern is whether the slack_app_url is necessary - is there a reason to create the new field rather than just set slack_api_url's default to https://slack.com/api/chat.postMessage? I don't think this would be a breaking change because existing users have already set this to something.

I think I addressed this in my original PR description; the decision to have a separate config field for the Slack app URL was to be able to provide a default API URL for Slack apps as well as differentiate when a webhook url is provided. I originally tried reusing the field, but it became very hairy to know whether the URL was for use with app token or for legacy URL, and I thought the added complexity wasn't worth it.

My final (and least important) concern is whether this actually works with slack. It looks like it does, but we just don't have any automated testing for that as far as I can tell. That being said, this is counter-intuitively the easiest part to fix if there's a problem πŸ˜† .

Well, I can say for certain that it works with Slack Apps, because we've been running an alertmanager fork with this patch for the last ~9 months with no issues πŸ˜„

pharaujo avatar Nov 05 '25 23:11 pharaujo

A couple of linter errors to fix.

SuperQ avatar Nov 18 '25 09:11 SuperQ

@SuperQ the linter errors should be fixed, please check again when you can πŸ˜ƒ

pharaujo avatar Nov 18 '25 17:11 pharaujo