Proposal for new Feature : Immutable Macro Variables

[William Leibzon]

On Thu, Jan 12, 2012 at 12:55 AM, Andreas Ericsson <ae at op5.se> wrote:
> On 01/12/2012 05:49 AM, William Leibzon wrote:
>> Hi everyone,
>>
>> I'd like to have a community discussion on a possible new feature for Nagios.
>>
>> To start of, I have written a number of plugins that reuse data from
>> their previous run, for optimization reasons (one time find correct
>> SNMP OID to pull data based on regex and just pull from that OID
>> after) and to calculate percent of change from counters values. The
>> standard way plugins deal with this is by creating temporary files or
>> using database. I'm not big fan of those approaches - large number of
>> such files is a maintenance issue and its a a performance disadvantage
>> to write/read those files, database is an external service and you
>> want to avoid relying on that and opening/closing connection for every
>> plugin run. Instead the plugins I write cache this data using nagios
>> itself. My plugins that do that are check_snmp_netint.pl,
>> check_sasraid_megaraid.pl and at least 2 others you can find at
>> http://william.leibzon.org/nagios/. Right now all these plugins output
>> these "cache" data as part of part of Nagios performance data which is
>> then fed back to plugin at each run. This all works ok except for some
>> graphing programs (pnp4nagios) that parse performance data and get
>> confused about non-numeric values my plugins may output, but I have
>> patches for these issues too. The thing is it all seems like hack. At
>> the same time its a good feature to be able to use nagios itself to
>> store small amount of data for use by plugins.
>>
>> So I have a proposal on how to move forward with this. I propose that
>> we have a way for plugins to return data back to nagios that would be
>> stored as Nagios MACRO variables. The idea is basically to add another
>> line of data that nagios plugins would output in addition to status
>> line and performance data. Something like:
>>
>> eth0:UP (0.0Mbps/0.0Mbps/0.0/0.0/0.0/0.0):(1 UP): OK |
>> 'eth0_in_prct'=0%;50;100;0;100 'eth0_out_prct'=0%;50;100;0;100
>> 'eth0_in_octet'=402666722c 'eth0_out_octet'=1053106629c
>> 'eth0_in_error'=0c 'eth0_in_discard'=0c 'eth0_out_error'=0c
>> 'eth0_out_discard'=0c || cache_descr_ids=2 cache_descr_names=eth0
>> cache_descr_time=1326310364 eth0_in_octet.1=400843541
>> eth0_out_octet.1=1052938634 eth0_in_error.1=0 eth0_out_error.1=0
>> eth0_in_discard.1=0 eth0_out_discard.1=0 time.1=1326340964
>>
>> I above I separated new type of data with ||. It can be something else
>> or just one |.  I'm quite open to suggestions on how to best do this.
>>
>
> I like the general idea so far, but do read on.
>
>> In any case the idea here is that Nagios would not just put everything
>> after || into some new variable like EXTRASERVICEDATA but would parse
>> the line looking for name=data pairs. These would be interpreted as
>> new values for MACRO variables previously defined in Nagios config. In
>> order to have proper security any variable that can be modified in
>> this way would have to be specifically listed as such so I propose to
>> add a new "immutable_macros" config line. Here is an example of what I
>> have in mind:
>>
>> define service {
>>     ...
>>     immutable_macros cache_descr_ids,  cache_descr_names,
>> cache_descr_time, eth0_in_octet.p1, eth0_out_octet.1, eth0_in_error.1,
>> eth0_out_discard.1, eth0_in_discard.1, time.1
>>     _cache_descr_ids = ''
>>     _cache_descr_names = ''
>>     _cache_descr_time = 0
>>    ...
>> }
>>
>
> This I don't like. The $EXTRASERVICEDATA$ thing is much more flexible
> and requires a lot less configuration. It also makes it possible to
> just drop in replacement plugins that use other variables, whereas
> your suggestion requires that the user doing the configuration knows
> which extra variables the plugin is supposed to output. That's bad
> and makes this feature a lot less useful.

It probably very easily to do both i.e. store what was returned after
|| as "EXTRASERVICEDATA" and allow plugins to parse it directly if
they like, but also allow to modify custom variables if people want
this feature.

> Besides; Calling the macros "immutable" makes absolutely no sense,
> since any macro that isn't "mutable" can't ever be changed. There's
> already custom variables available for that.

Ok. I think its a naming issue. I should just drop name "macro" and
call it IMMUTABLE VARIABLE.

The idea is basically that Nagios 3.0 custom variables defined with
_NAME=something and made available with $_NAME$ be changeable by
plugins in real-time. And immutable config line specifies which ones
can be changed.

The security issue here is different here and has nothing to do with
usual exec worries. Its more about rogue plugins (not necessarily
rogue on purpose, some people write plugins in weird ways). I don't
want nagios variables to just be changeable by any plugin, I want
sysadmin to know that this plugin is in fact going to change this
variable and specify it on the config line. For similar reasons I'd
prefer not to have this line available on host-config basis because
then it would mean that any plugin for this host can change the
variable and there maybe some unusual plugin admin admin added later.
At the same time allowing HOST-wide custom variables to be modified by
the plugin opens interesting ways in how plugins can now cooperate and
share data. I think people will in fact use it.

>> Both host and service macros would be allowed to be immutable (but it
>> would have to be specified only in service definition, for security)
>> which would allow a new features where one plugin can pass data to
>> another plugin by means of a macro that both share from a host. This
>> may get quite useful i.e. you may already be retrieving OS
>> version/string from a box with one plugin (maybe by remote with SNMP
>> and not just 'uname') which would be used by several plugins to
>> determine what data is retrieved and how. There are really lots of
>> opportunities for improvement in this area.
>
> Too complex. Keep it simple and keep it small. What sort of security
> you're referring to I have absolutely no idea. The check engine in
> Nagios is currently undergoing a big revamp, so we'll be using execve()
> directly instead of running things through the shell. That means we
> no longer have to worry about shell characters and whatnot.

See above. Completely different issue.

BTW, I heard (from you on the conference :) you're planning to rewrite
the code which parses list of variables before command execution. I
guess you're planning per-command linked list with only variables that
are used there rather then going through it all as a hash list. That
is fine, I think it should be no problem for what I propose.

And I don't know, it may even be better if I work with newest dev
code. It seems like what I want would be better as a feature of nagios
4.0 rather then nagios 3.0, since I'm extending plugin output format
(and these change only for major version, i.e. nagios 3.0 introduced
multi-line output).

>> Functions can also be added to libraries (i.e. Nagios::Plugin which I
>> personally don't use) to make it easy to store data as nagios macros
>> variables and retrieve it back when plugin runs. These are kind-of
>> like web session variables really.
>>
>> I'm proficient with Nagios core code and willing to write this all
>> myself and submit it as a patch to Nagios. But this is a serious
>> enough feature enhancement that it requires community discussion.
>> Specifically big here is a proposal to add 3rd type of data output for
>> plugins since for very long time plugins have been expected to return
>> "status data | perf data". I'm sure any plugin that supports this new
>> feature would have to have an option to enable it so it does not screw
>> up its use on older systems. Performance parsing addons would probably
>> not be affected because they would just get output before || - and I'm
>> sure will make them very happy to see only real data in perfparse
>> line.
>>
>
> Go ahead and hack it up. For now though, you'll have to do it with a
> separate hash-table so as to not upset the object abi, and you'll have
> to do it the way I outlined it above for me to accept the patch. Adding
> code to keep track of which variables are ok for which service makes no
> sense when much easier code does the trick just as well.

It would not quite like that. Really which "is ok" would be decided by
my code at the time plugin data results are received and processed by
nagios. I would use config line and check which of the listed
immutable variables are present in the extra service data. In fact one
of the reasons for extra line is to make processing faster so it would
not go through entire list of variables but only very small list set
on for that service. When command is being ready to be executed,
everything there is ok.

William

------------------------------------------------------------------------------
RSA(R) Conference 2012
Mar 27 - Feb 2
Save $400 by Jan. 27
Register now!
http://p.sf.net/sfu/rsa-sfdev2dev2