Monitoring::Plugin https://www.monitoring-plugins.org/repositories/monitoring-plugin-perl/atom?h=v0.40 2018-07-25T20:16:18Z 2018-07-25T20:16:18Z Sven Nierlein sven@nierlein.de 2018-07-25T20:16:18Z urn:sha1:f11af14b6d68fd752cf74cd1ebb397688abbed7e 2018-05-24T01:01:21Z Tom Ryder tom@sanctum.geek.nz 2018-05-24T01:01:21Z urn:sha1:301a3564e6b7cf716b75440c5333e688ea814873 The Config::Tiny parent class for Monitoring::Plugin::Config does not necessarily raise an exception in $@/$EVAL_ERROR on a failed call to its ->read() method. Indeed, it does not in most cases, at least in the most recent Config::Tiny. If a file does not exist or for whatever other reason cannot actually be read--such as permissions problems--Monitoring::Plugin ends up reporting a "Missing config section" to the caller, which is misleading. This information is available from the ->errstr() method of the base class, however, and an inspection of the code for the ->read() method in the parent class suggests the correct approach is to look for a return of `undef` from the ->read() method, and use the return value of ->errstr() as the reported error for ->_die(). This commit adds such a check. To reproduce, given the following plugin `mptest`: use strict; use warnings; use Monitoring::Plugin qw(%ERRORS); my $mp = Monitoring::Plugin->new( usage => '', ); $mp->getopts; $mp->plugin_exit($ERRORS{OK}, 'Done!'); Running it without options of course works, like so: $ perl mptest MPTEST OK - Done! However, prior to this patch, if we specify --extra-opts with a nonexistent filename, we get a misleading error: $ perl mptest --extra-opts=@/nonexistent Invalid section 'mptest' in config file '/nonexistent' With this patch included, the error is more accurate and helpful: $ perl mptest --extra-opts=@/nonexistent Failed to open file '/nonexistent' for reading: No such file or directory 2018-03-04T20:59:29Z Tom Ryder tom@sanctum.geek.nz 2018-03-04T20:56:22Z urn:sha1:77de65e149c519f551e03e11c9c26605c9866b72 From a maintainer, Sven Nierlein: > I'd say we can remove the "experimental" tag there. It had been there > for years now and i see no reason why we would change them. Partly resolves #14. 2017-12-16T01:09:16Z Tom Ryder tom@sanctum.geek.nz 2017-12-16T01:09:16Z urn:sha1:da6fe80e84a20aaf405a89bd0203d9ce1abc097d Context with the lhs of the == operator forces this into scalar context anyway, making this call redundant. 2017-12-16T00:34:38Z Tom Ryder tom@sanctum.geek.nz 2017-12-16T00:34:38Z urn:sha1:6518f384cc20561ddc0df32414a714164dd2eb95 Since Params::Validate::validate() doesn't seem to actually mess with this specification, may as well pass a reference rather than bother copying the whole thing. 2017-12-16T00:30:52Z Tom Ryder tom@sanctum.geek.nz 2017-12-16T00:30:52Z urn:sha1:afa636bc749f76aed77380d39269bceecdd879d2 Operator precedence allows leaving these out. 2017-12-16T00:25:34Z Tom Ryder tom@sanctum.geek.nz 2017-12-16T00:25:34Z urn:sha1:c3f2a7cd7313868166ba205e9eff2b8be6a36ea4 Define the expected order of parameters by key in an array, and then apply hash slices to validate and assign the arguments including their required flags in one swoop, again hinging on the parameter definitions established in c1046ba. 2017-12-16T00:08:44Z Tom Ryder tom@sanctum.geek.nz 2017-12-16T00:08:44Z urn:sha1:40d5fbea30c310004233ef4cd3e570c351c54233 Use the parameter label definitions from commit c1046ba to simplify the block of code that checks whether the first argument passed to arg() is one of the known param keys, rather than using a regular expression with alternating expression. This is more compact, may be marginally faster, and will make adding new parameters to this method more straightforward in future, avoiding the situation corrected in commit 4aa2aee. 2017-12-16T00:02:34Z Tom Ryder tom@sanctum.geek.nz 2017-12-16T00:02:34Z urn:sha1:c1046bab2e6681f5e4714178d6cc1cd16f067382 This shift and its comment makes what the values of the hashref passed to the validate() methods mean clearer, and also allows the use of the keys as a means of determining whether arg() was passed its definition in the array or hash format in a separate commit. 2017-11-15T10:42:10Z Tom Ryder tom@sanctum.geek.nz 2017-10-24T21:03:18Z urn:sha1:4aa2aee51b54cb8988d87a01d1a3b9a0d90bb8bf If the `label` key is given as a named argument for argument specification, if it turns out to be the first element returned in the argument list subject to hash randomization, then this test for named arguments fails due to the omission of the label from this alternating group in the pattern. When this happens, the code for an unnamed list of items was run instead, leading to a very confusing area happening randomly: 8 parameters were passed to Monitoring::Plugin::Getopt::arg but 2 - 5 were expected at PERL/lib/perl5/Monitoring/Plugin/Getopt.pm line 397. Monitoring::Plugin::Getopt::arg(undef, "label", "HOSTNAME", "required", 1, "help", "Hostname of device to check", "spec", ...) called at PERL/lib/perl5/Monitoring/Plugin.pm line 161 Monitoring::Plugin::add_arg(Monitoring::Plugin=HASH(0x1f90fd8), "label", "HOSTNAME", "required", 1, "help", "Hostname of device to check", "spec", ...) called at libexec/check_example line 144 If you specified all five keys for your argument, then this happens (roughly) one-fifth of the time.