naemon / naemon-core

Networks, Applications and Event Monitor
http://www.naemon.io/
GNU General Public License v2.0
154 stars 63 forks source link

Revomve enable_environment_macros is a bad idea! #100

Closed nook24 closed 8 years ago

nook24 commented 9 years ago

Hi guys, i installed current Naemon Core 1.0.0 today and enable_environment_macros seems to be removed. The parameter in naemon.cfg still exists but if i enable it my envoronment is still empty. In the docs is a hint like this: command_line NAGIOS_HOSTNAME="$HOSTNAME$" /

The problem is Naemon uses execvp() to execute my notification script and execvp is not able to handle export of variables.

Now i need to use some ugly stuf like /bin/bash -c 'NAGIOS_HOSTNAME="$HOSTNAME$"' wich will fail if $CONTACTALIAS$ is some like Bob's Contact, because $CONTACTALIAS$ is not in the list of striped macros.

Was enable_environment_macros really removed or is it just broken?

All these problems are gone with env macros, and ther was a way to turn it on or off. No Problem with ' " | $ or something else

//Edit: Or may be create a new config variable were the user can define, wich macros sould be exported to the env. Than Naemon dont net to set 200 macros where the user only uses two of them.

mmarodin commented 9 years ago

I've the same problem, using pnp4n_send_service_mail.pl script, to include pnp4nagios graphs into email notification.

Seems that Naemon doesn't export variables:

==> /var/lib/naemon/naemon.debug <== ... [1425397122.833599] [032.2] [pid=20539] Raw notification command: /opt/scripts/nagios/pnp4n_send_service_mail.pl -p "Mycompany" -c "$CONTACTADDRESS1$" -f graph -u -l en [1425397122.833608] [032.2] [pid=20539] Processed notification command: /opt/scripts/nagios/pnp4n_send_service_mail.pl -p "Mycompany" -c "" -f graph -u -l en ...

==> /var/log/naemon/naemon.log <== ... [1425397122] Warning: Notification command for contact 'mmarodin' about service 'google;www.google.it' exited with exit code 255. stdout: 'Error: no recipients have been provided Usage: /opt/scripts/nagios/pnp4n_send_service_mail.pl [-v] [-V] [-h] [-t] [-H ] [-p ] [-r or -g ] [-c ] [-b ] [-f <text|html|multi|graph>] [-u] [-l <en|jp|fr|de|(or other languages if added)] ', stderr: '(empty)'

...

Using Nagios 3.5.1 this notification script is working :) Using the pnp4n_send_service_mail.pl test arguments the mail message will be sent correctly.

There is a way to export variables to external commands?

I've also tried with this syntax but it did not work (like this article http://www.naemon.org/documentation/usersguide/macros.html):

define command { command_name my_old_notification_script command_line NAGIOS_CONTACTEMAIL="$CONTACTEMAIL$" /opt/scripts/nagios/pnp4n_send_service_mail.pl ...

}

Let me known, thanks. Morgan

mboden commented 9 years ago

Hello,

I got help from Max Sikström so I got the example in the documentation to work by adding $OVE at the end of the commandline. OVE is nothing in my environment but the $OVE causes naemon to not use execvp but instead run it through a shell. See Max email for more info.

https://www.monitoring-lists.org/archive/naemon-users/2015-March/000073.html

I also second that I would really like to have this feature back.

enable_environment_macros

Regards Magnus

mmarodin commented 9 years ago

This solution also works for me, thanks! :)

ageric commented 9 years ago

Hmm... I could make sure that "LALA=$SOMEMACRO$ /path/to/command arg1 arg2 arg3" works, but bringing back the whole environment macros is very unlikely to happen, for a couple of reasons.

So re-enabling the feature adds quite a bit of complexity at pretty limited value, and with a huge tradeoff in performance.

VladimirBilik commented 9 years ago

What about some different way how to feed plugin with NAEMON status variables? For example from STDIN? Yes, I know that it would break compatibility, so it is not so good idea.

pengi commented 9 years ago

Environment variables is good in theory, but in practice it has lot of drawbacks.

The most appearent drawback is however performance. Since environment varaibles needs to be set up before the command starts, all variables needs to be resolved before the command is executed, but it's the command itself that needs to know what it uses.

There are several macros that contains information that needs to be generated, like list of host groups, service groups and such. Resolving all of those takes time, and needs access to long list.

There is generally no problem resolving one or two of those for some commands, but resolving those for everything every type is a potential performance issue.

Macro resolution is implemented quite badly at the moment, so it might not be that huge performance issue. But improving an interface that is bad by design is just counter-productive.

Feeding information to plugins from stdin has the same issue as environment variables; it needs to be prepared before the plugin is executed.

I would rather see some other design that would both fulfill the need of passing variables without escaping problems to command, keep it plugin-compatible, and being able to scale. I would also like to see some kind of logic on top of the macros, where macros can be escaped, converted, filtered on user request.

nook24 commented 9 years ago

I guess to rebuild the macros that there is more logic like escape or convert will slow down naemon as well?

I use the env macros for OCSP and OCHP and for my notification scrips. This is a full list of macros i use: https://gist.github.com/nook24/0ec7a33bdcfb78bb605e

As far as i know naemon don't need to calculate something to fill this macros. To remove the service group members and other stuff should not create to big issues, its more efficient if a plugin that really need this information it query MySQL or livestatus. 99% of all other plugins will not use this information at all.

May be it is not the worst idea to add new config variables, wich macros naemon should set to the env and on wich action (for example on ocsp and notifications)

If we use the $OVE trick, are there any known escape issues?

An other idea is to use a event broker to send notifications but this is so hacky!

mboden commented 9 years ago

I think I like the ide of livestatus to query for macros. But as far as I know it is not possible to query the macros through livestatus today, only if you use the macros in a host note or something like that. It would be good if it was possible to just get all macros including custom macros from the host object through livestatus.

Regards Magnus

nook24 commented 9 years ago

My idea behind the livestatus method is, that naemon don't need to run heavy calculations to create the macros. Like all hosts of a host group. Livestatus (or other event brokers) can not hold env macros for your plugin.

@mboden: I don't use livestatus on my systems so i create a little SQL query to select all hosts of all hostgroups where the given host is a member of: https://gist.github.com/nook24/a80538a7ee0e1512d354 Check my GitHub repository if your naemon runs without a database and sorry for the bad query to late for nice statements... How ever this is a typical calculation that 99% of all plugins don't require, so in my opinion "outsourcing" stuff like this to MySQL make sense.

I guess there are a lot of useless macros like: Host Group Macros, Service Group Macros, Contact Group Macros, Summary Macros (sounds very heavy), File Macros and Misc Macros If we strip down the rest of the macros to only the most used macros, i think there should not be any performance or kernel issues at all.

VladimirBilik commented 9 years ago

Is it possible to have
$HOSTACKAUTHOR$ $HOSTACKAUTHORNAME$ $HOSTACKCOMMENT$ $SERVICEACKAUTHOR$ $SERVICEACKAUTHORNAME$ $SERVICEACKCOMMENT$ $CONTACTEMAIL$ $CONTACTPAGER$ $CONTACTADDRESSn$ $NOTIFICATIONTYPE$ $NOTIFICATIONISESCALATED$ $NOTIFICATIONAUTHOR$ $NOTIFICATIONCOMMENT$ and other $NOTIFICATION* with livestatus? I'm just dealing with notification plugins and I need to get informations contained in these macros. Thanks for info

ageric commented 9 years ago

@nook24 : Even if the macros you listed don't need calculations (I didn't check), that doesn't mean that Naemon knows which macros you want to use. With the earlier environment-macro implementation, we always had to calculate ALL macros for every check, eventhandler, notification and ocsp/ochp command. Calculating all macros takes about 0.05 second on average. That may not sound like much, but if you consider a system that runs 500 checks per second (which isn't very much), and they all run a global ocsp/ochp command after every check, that means we have to do 50 seconds worth of calculations for every second of runtime we have. Obviously, that doesn't work.

@VladimirBilik : You can't get the NOTIFICATION* macros from livestatus, as they're per-notification only. You can easily pass them as arguments on the commandline though, which is what I recommend.

nook24 commented 9 years ago

@ageric: I created now this command on my system: https://gist.github.com/nook24/95f0a2f0d548ccf1fb62 this works well for notifications may be but not for OCSP/OCHP:

NAEMON_SERVICEOUTPUT=Im a check from hell 日本 भारत 中國 (naemon 1.0.0) NAGIOS_SERVICEOUTPUT=I'm a check from "hell" 日本 भारत 中國 (nagios 4.0.8)

As you can see, naemon touch the output wich is Ok for notifications but bad for the OCSP which like to work with the real output of the plugin.

So if you strip down the macros to HOSTOUTPUT, SERVICEOUTPUT and SERVICEPERFDATA for notifications and OCHP/OCSP i can not belive that this will have any impact to the system performance.

pengi commented 9 years ago

The real reason isn't, as far as I can see, that naemon doesn't handle enviornment variables, but that it can't correctly shell escape parameters.

What should be done is actually to make neamon to correctly escape the shell arguments, not just strip some might-be-nasty-characters as specified in the configuration.

I would really prefer to write for example $USER1$/my_command $HOSTOUTPUT|shellescape$ should be correctly escaped as an argument to my_command.

nook24 commented 9 years ago

@pengi: sound good for me, to escape the shell arguments. Is there a platform where you discuss about new developments of naemon core? The problem is now, environment variables are gone and many people have different issues now because there is no solid workaround. I developed a neb module, so it would be good to know if things change, that developers can maintain their addons.

Is there a way we can help you to implement this escape feature? May be it's easier to escape every parameter, without parsing a special suffix like |shellescape$ this should speed up development to get the issue fixed.

The C code of PHPs escapeshellarg() looks not to hard: https://github.com/php/php-src/blob/master/ext/standard/exec.c#L340

mboden commented 9 years ago

Hello,

I rewrote some of my plugins to get macros and custom variables through livestatus which works great.

This requires that you know your own hostname though so I think at least $HOSTNAME$ should be made available as a environment variable so I don't have to pass both -H $HOSTNAME$ and -I $HOSTADDRESS$ to the plugins.

Regards Magnus

nook24 commented 9 years ago

Any news on how to solve the escaping issue?

catharsis commented 8 years ago

This issue seems to have sidetracked and gone stagnant (it now seems to be related to escaping of command lines ..?). enable_environment_macros is not coming back in the foreseeable future so I'm closing this.

If anyone cares to formulate an issue on what the actual, experienced problem is, feel free to report this as a new issue.