Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: allow notifying multiple Slack channels #41

Closed
wants to merge 1 commit into from
Closed

feat!: allow notifying multiple Slack channels #41

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 11, 2022

BREAKING CHANGE: vars.slack_notifications_channel has been renamed to vars.slack_notifications_channels and accepts a list instead of a string.

Sending notifications to multiple channels is done by calling the webhook multiple times. Slack does not allow notifying multiple channels with a single webhook call as far as I know.

BREAKING CHANGE: vars.slack_notifications_channel has been renamed to
vars.slack_notifications_channels and accepts a list instead of a
string.

Sending notifications to multiple channels is done by calling the
webhook multiple times. Slack does not allow notifying multiple channels
with a single webhook call as far as I know.
@nisabek
Copy link
Owner

nisabek commented Oct 11, 2022

Hi there, thanks for contributing... I'm really not sure about introducing a breaking change. Maybe we keep both?
Also, since you seem to be using this actively.. can you help people here ? If that's really a bug, we shall fix that first, before introducing multi-channel - WDYT?

@ghost
Copy link
Author

ghost commented Oct 13, 2022

Hi Nune, thanks for the feedback. This is a change I had to make for the company I work for. We have an internal ticket open regarding various improvements to our monitoring setup. Syncing our fork of icinga2-slack-notifications and upstreaming this pull request is one of them. It could take some time before we get around to this. We'll try to accommodate your wish to remain backwards-compatible, and also make some documentation changes that I forgot in my initial commit. You can just leave this PR open for now.

Regarding the issue you linked to: I will realistically only work on that if my company experiences the same problem and considers it something to prioritise, otherwise I just can't make time available. Sorry.

@ghost ghost marked this pull request as draft October 13, 2022 07:52
@nisabek
Copy link
Owner

nisabek commented Oct 13, 2022

Thanks, I understand...
my problem is that I don't work with icinga actively anymore, and don't even have a proper test setup, so I can test and help others. So actively looking for somebody else to maintain the repo. Anyhow, will be looking forward to the finalized PR version and good luck to your company :)

Cheers,
Nune

@Darkentik
Copy link

Darkentik commented Aug 25, 2023

Hi, @imre-uncinc can you please provide some docs for your improvement?
In our company we are facing the same issue and i want to try your code changes in our staging environment.
For now it is not so easy for me how to setup the vars for multiple slack channel parameters.
We have the following setup in slack:
alert-channel-01 -> webhook-01
alert-channel-02 -> webhook-02
alert-channel-03 -> webhook-03

But your code did only expand a list for slack channels using the same webhook right?
This code change will not work for us.
I prefer some code changes like defining multiple disk check like so:

HOST or SERVICE variable definition:

vars.slack_alerts["alert for windows admins"] = {
  vars.slack_notifications_webhook_url = "https://hooks.slack.com/services/foobar01"
  vars.slack_notifications_icinga2_base_url = "https://sysmonweb.company.tld" 
}

vars.slack_alerts["alert for linux admins"] = {
  vars.slack_notifications_webhook_url = "https://hooks.slack.com/services/barfoo02"
  vars.slack_notifications_icinga2_base_url = "https://sysmonweb.company.tld" 
}

APPLY FOR rule:

apply Notification for (alert => config in host.vars.slack_alerts) {
  import "slack-notifications-default-configuration-hosts"

  vars += config

  assign where host.vars.custom_var == "custom_value"
}

This would be nice if we can work with slack like working with builtin "check_disk" command to keep the same syntax.

Thanks a lot! :)

@ghost
Copy link
Author

ghost commented Aug 28, 2023

Hi @Darkentik, it is unlikely that I will complete this MR, it didn't have any priority over the last year and I'm leaving the company next month. Maybe one of my coworkers will pick it up, maybe not.

Regarding your question: our use case was to support sending notifications to multiple Slack channels using the same webhook. I have not tested this with multiple webhooks and probably never will.

Feel free to expand on my MR to build in support for multiple Slack webhooks!

@Darkentik
Copy link

Darkentik commented Aug 28, 2023

Hey, not so good to hear that you leave the focus. I wish you a good time with the new job. :)

So for all others i think, that no one need any change at this plugin.
Not for multiple channels nor for multiple webhooks.
Just use the Icinga2 magic with inheritance and multiple notifications.
The key is that you declare multiple apply rules for a slack notification and each with different vars fitting your needs.

The var host.vars.slack_notifications == "enabled" is declared via linux host template in our setup.

template Host "linux-host-default" {
  import "generic-host"

  vars.os = "Linux"
...snip...
  vars.slack_notifications = "enabled"
...snip...
}

I have tested at weekend with my testserver to have 2 slack notifications each with different custom var for the webhook_url and the notifications now come to 2 slack channels each with a unique webhook.

Here is the setup:
default slack notification apply rule with assign where:

apply Notification "slack-notifications-notification-hosts" to Host {
  import "slack-notifications-user-configuration-hosts"

  vars.slack_notifications_channel = host.vars.slack_notifications_channel? host.vars.slack_notifications_channel : "#monitoring_alerts"
  
  assign where host.vars.slack_notifications == "enabled" || host.vars.slack_notifications_channel
}

My custom slack notification apply rule with assign where:

$ cat /etc/icinga2/zones.d/global-templates/notifications/slack-notifications/relaxdays-slack-notifications-test.conf 
apply Notification "relaxdays-slack-notifications-test" to Host {
  import "slack-notifications-user-configuration-hosts"

  vars.slack_notifications_webhook_url = "https://hooks.slack.com/services/foobar12345"
  
  assign where host.vars.slack_notifications == "enabled" && host.name == "davidtest01-a.test.mycompany.local"
}

Repository owner closed this by deleting the head repository Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants