Icinga / icinga2

The core of our monitoring platform with a powerful configuration language and REST API.
https://icinga.com/docs/icinga2/latest
GNU General Public License v2.0
2k stars 575 forks source link

[dev.icinga.com #8067] Require at least one user for notification objects (user or as member of user_groups) #2423

Closed icinga-migration closed 9 years ago

icinga-migration commented 9 years ago

This issue has been migrated from Redmine: https://dev.icinga.com/issues/8067

Created by theunskilled on 2014-12-11 15:41:10 +00:00

Assignee: gbeutner Status: Resolved (closed on 2015-02-08 10:05:04 +00:00) Target Version: 2.3.0 Last Update: 2015-02-13 07:00:30 +00:00 (in Redmine)

Icinga Version: Icinga r2.2.1-1
Backport?: No

Required Changes

Original request

In my case I tried to send sms-notifications using an external shellscript, which also handles the users and phone numbers. So I did not use the mandatory "user_group" settings for the notifications. Icinga started and the log showed that notifications were sent, but the script wasn't executed. Adding "user_group" made it work, but is useless in this context. Should "user_group" stay mandatory or should it be set to optional?

Changesets

2015-02-07 22:04:01 +00:00 by mfriedrich db9c55835f411dd82acff2f0564207ecc95770c1

Require at least one user for notification objects (user or as member of user_groups)

fixes #8067

2015-02-08 10:03:10 +00:00 by (unknown) e456cfd99ce2e40a4c07475447b821e056b94e4f

Relax the validator for Notification::users/Notification::user_groups a bit

fixes #8067
icinga-migration commented 9 years ago

Updated by gbeutner on 2014-12-11 17:59:44 +00:00

The "user_groups" attribute isn't mandatory at all. Icinga uses both the "users" and "user_groups" attributes to figure out who to send notifications to. Can you show us the notification config you tried?

icinga-migration commented 9 years ago

Updated by theunskilled on 2014-12-15 07:46:37 +00:00

commands.conf:

object NotificationCommand "sms-notify" {
         import "sms-host-notification"
}

templates.conf:

template NotificationCommand "sms-host-notification" {

  import "plugin-notification-command"

  command = [ SysconfDir +  "/icinga2/scripts/test.sh" ]
}

notifications.conf:

apply Notification "sms-host-notification" to Host {

  states = [ Up, Down ]
  types = [ Problem, Acknowledgement, Recovery, Custom,
            FlappingStart, FlappingEnd,
            DowntimeStart, DowntimeEnd, DowntimeRemoved ]

        command = "sms-notify"
        interval = 1m
        period = "24x7"
        assign where host.vars.notify == "24x7-sms-host"
}

This is the non working configuration. After adding ' user_groups = [ "icingaadmins" ] ' to the notifications.conf, the shellscript was executed. Without, Icinga2 did not even access the file. I understand that "user_groups" and "users" are attributes to tell Icinga2 who to notify, but in my case these were not necessary, because the shellscript itself had all information needed. Icinga2 did not throw any exception or message in the logs, stating that a mandatory or needed argument was missing or empty.

icinga-migration commented 9 years ago

Updated by gbeutner on 2014-12-15 10:26:40 +00:00

I suppose we should add a config validator that ensures that there's at least one user.

icinga-migration commented 9 years ago

Updated by mfriedrich on 2014-12-15 10:29:49 +00:00

Imho that should be a runtime warning similar to failing dependencies, or we might consider allowing to send a notification without users, only triggering the command (this will cause problems with $user.xy$ macros and current interfaces (livestatus, db ido, etc)).

icinga-migration commented 9 years ago

Updated by tobiasvdk on 2015-01-11 12:06:06 +00:00

I think a notification must have an addressee (user). A config validator should ensure this as gunnarbeutner said. If the script does not need such information you can create a 'nobody' user which would prevent problems mentioned by dnsmichi.

icinga-migration commented 9 years ago

Updated by mfriedrich on 2015-02-07 22:03:50 +00:00

icinga-migration commented 9 years ago

Updated by mfriedrich on 2015-02-07 22:05:02 +00:00

Applied in changeset db9c55835f411dd82acff2f0564207ecc95770c1.

icinga-migration commented 9 years ago

Updated by gbeutner on 2015-02-08 10:00:29 +00:00

With the default config:

acheron:icinga2 gunnar$ icinga2 daemon
[2015-02-08 10:59:29 +0100] information/cli: Icinga application loader (version: v2.2.0-406-gb23d58d; debug)
[2015-02-08 10:59:29 +0100] information/cli: Loading application type: icinga/IcingaApplication
[2015-02-08 10:59:29 +0100] information/Utility: Loading library 'libicinga.dylib'
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/icinga2.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/constants.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/zones.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/share/icinga2/include/itl
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/share/icinga2/include/plugins
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/share/icinga2/include/command.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/features-enabled/checker.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/share/icinga2/include/command-icinga.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/features-enabled/mainlog.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/share/icinga2/include/command-plugins.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/share/icinga2/include/timeperiod.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/features-enabled/notification.conf
[2015-02-08 10:59:29 +0100] information/Utility: Loading library 'libnotification.dylib'
[2015-02-08 10:59:29 +0100] information/Utility: Loading library 'libmethods.dylib'
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/commands.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/downtimes.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/groups.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/hosts.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/notifications.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/satellite.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/services.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/templates.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/test.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/timeperiods.conf
[2015-02-08 10:59:29 +0100] information/ConfigCompiler: Compiling config file: /Users/gunnar/i2/etc/icinga2/conf.d/users.conf
[2015-02-08 10:59:29 +0100] information/Utility: Loading library 'libchecker.dylib'
[2015-02-08 10:59:29 +0100] information/ConfigItem: Committing config items
[2015-02-08 10:59:29 +0100] critical/config: Error: Validation failed for Object 'mail-icingaadmin' (Type: 'Notification') at /Users/gunnar/i2/etc/icinga2/conf.d/notifications.conf:11: No users/user_groups specified.
Location:
/Users/gunnar/i2/etc/icinga2/conf.d/notifications.conf(9):  */
/Users/gunnar/i2/etc/icinga2/conf.d/notifications.conf(10): 
/Users/gunnar/i2/etc/icinga2/conf.d/notifications.conf(11): apply Notification "mail-icingaadmin" to Host {
                                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/Users/gunnar/i2/etc/icinga2/conf.d/notifications.conf(12):   import "mail-host-notification"
/Users/gunnar/i2/etc/icinga2/conf.d/notifications.conf(13): 

[2015-02-08 10:59:29 +0100] critical/config: 1 error
acheron:icinga2 gunnar$ 
icinga-migration commented 9 years ago

Updated by gbeutner on 2015-02-08 10:04:29 +00:00

The problem here is that you can't safely call UserGroup::GetMembers in a validator because the UserGroup's member list hasn't been set up yet - this is only done after all objects have been properly validated.

icinga-migration commented 9 years ago

Updated by Anonymous on 2015-02-08 10:05:04 +00:00

Applied in changeset e456cfd99ce2e40a4c07475447b821e056b94e4f.

icinga-migration commented 9 years ago

Updated by mfriedrich on 2015-02-08 11:17:30 +00:00

Sorry about that, you were faster in fixing than me. A proper fix on getting the members would be OnAllConfigLoaded(), but at that stage it's better to check that directly at runtime when a notification happens and log a warning.