Using a config file in the manner that you are is a common way to achieve extensibility. Although I agree with the suggestion that you impose a more readable order on the lines, your basic approach is very reasonable.

As far as security goes, you should remember that not every script needs to be written like Fort Knox depends on it. It's a little difficult to explain what's appropriate and what isn't because it all depends so heavily on your particular situation and environment. In a lot of cases, file permissions are pretty much all the security that you need. Sometimes you need to allow users to run a script as a different user and need to take some more precautions. Sometimes you need to allow people who aren't users to run a script (such as a CGI script executed by a webserver) and that might require more or different precautions. It is pretty unlikely that this script would require any security other than properly set permissions.

Just as extensibility and security are important, so is robustness. One suggestion that I do have is to examine how you are checking for a process's existence and consider how such a check might fail. As an example of what I mean, if your process check was for /sshd and someone happened to be running vi /etc/sshd_config it might look like sshd was alive even when it wasn't.

-sauoq
"My two cents aren't worth a dime.";

This is a clever approach.

That said, why are you so concerned about extending your program's behaviour without modifying the code? (That's a question, not a criticism.) It seems to me that being able to extend a program without adding code is most useful when the program takes effort to re-package (for instance, the familiar compile-link step with C), or when the code itself is difficult to modify. Perl avoids the first issue -- just edit and go. The second will become a problem no matter how you're extending the program.

If you have a block that looks like:

&check_proc('ldap -p 389', 'LDAP is not running', 'medium');
&check_proc('sendmail: accepting',
            'sendmail is not accepting new connections',
            'major');
&check_proc('/xntpd', 'NTP daemon is not running', 'minor');
&check_proc('apache/bin/httpd',
            'Apache web server is not running',
            'minor');
[download]

it doesn't take much more effort to add another call to check_proc than it does to modify a config file, and if you ever want to do something different (like monitor system load), you're set: you won't have to add syntax to the config file for a different kind of check, just add another sub. Never mind that you've eliminated a goodly chunk of code (the config file parser) and another feature (reading config files), which won't exactly make your code more difficult to maintain.

The only disadvantages I can see are:

1. Depending on how your monitoring script is running, it may be more difficult to refresh the code than a config file.
2. It'll probably be more difficult for other people to change the code than the config file, especially if the code has appropriately strict permissions.

but these may not be of concern to you.

Keep the code loosely coupled, write solid unit tests, and code becomes easy to change. (At least, that's what the eXtreme Programming zealots would have you believe... and in my experience, they're on to something.)

Good luck, and let us know how it turns out. I wouldn't mind seeing this appear in the Code Catacombs.

--
F o x t r o t U n i f o r m
Found a typo in this node? /msg me
The hell with paco, vote for Erudil!

Looks good.

I can add a few suggestions. You can reduce the number of lines to search by refining the ps call. It appears that you are checking daemon status, so `ps x` to show only processes with no controlling tty will cut the search space a lot.

You could improve the process check loop like this:

    @regexen = @process;
    for (@regexen) {
        quotemeta;
        $_ = qr/$_/;
    }
    for ( my $i = 0; $i <= $#alert; $i++ ) {
        unless(grep /$regexen[$i]/ , @ptable) {
            $alerts{"$alert[$i]"} .= "\n$message[$i]";
        }
    }
[download]

The quotemeta in the regex will save some hard to diagnose errors. and precompiling will likely save time since they are used to scan all the process entries.

A third improvement would be to reformulate your search to stop scanning when a regex matches, and to not check that regex again afterwards. See merlyn's reply to Ovid's (OT) Interview questions -- your response?, question 1.

After Compline,
Zaxo

Yes, this is called "data driven programming" and is generally agreed on as a Good Thing. :-)

There is room for improvement though. You are using functions that take no parameters, a single time each. There's no need to put that code in functions.

As suggested, a refined ps call will help a lot. If you have a GNU ps available, try ps xo command.

A suggestion for your configuration needs: use Config::General, makes for much nicer files, something like:

Enabled 1

<Process ldap>
    Name "ldapd -p 389"
    Message "LDAP is not running"
    Level medium
</Process>

<Process mta>
    Name "sendmail: accepting"
    Message "Sendmail daemon is not accepting connections"
    Level critical
</Process>

<Process ntp>
    Name /xntpd
    Message "NTP daemon is not running"
    Level minor
</Process>

<Process httpd>
    Name apache/bin/httpd
    Message "Apache web server is not running"
    Level minor
</Process>
[download]

#!/usr/bin/perl -w
use strict;
use Config::General;

delete @ENV{qw(IFS PATH CDPATH ENV BASH_ENV)};

my %config = Config::General->new(
    -ConfigFile => ".processcheckrc",
    -LowerCaseNames => 1,
)->getall;

exit unless $config{enabled};

my $ptable = `/usr/bin/ps xo command`;

my %alert;

while(my($name,$option) = each %{$config{process}}) {
    my $re = quotemeta $option->{name};
    push @{ $alert{ $option->{level} } }, $option->{message}
        unless $ptable =~ /$re/;
}

print "REPORT TAKEN AT " . localtime . "\n";

print map "$_\n", "\U$_\E ALERTS", @{%alert{$_}}
    for sort keys %alert;
[download]

Makeshifts last the longest.

I'd order it as below, but it is a matter of taste. I think it secure enough - it won't do anything the user himself can't do.

PROCESS = ...
MESSAGE = ...
ALERT = ...

PROCESS = ...
MESSAGE = ...
ALERT = ...
[download]

I got here at July 12th, 2002 :)

My actual config file looks a tad bit different, and I agree with you. I was just trying to give an example and I forgot to include what it would take to add a new check - that was my intention, so here it is:

PROCESS = /sshd
MESSAGE = The SSH Daemon has terminated
ALERT = major
[download]

In just a few key strokes, the program now performs one more check and no one even had to open up the code. Of course, I realize that if you want to define a new "type" of check that it has to be coded. One of the other side effects of doing this is having multiple configuration files. Instead of hard coding to "default.ini" - you could use getargs to specify an alternate - as in changing disk space threshold. Each admin could tailor the file to their liking.

for ( my $i = 0; $i <= $#alert; $i++ ) {
  unless(grep /$process[$i]/ , @ptable) {
   $alerts{"$alert[$i]"­} .= "\n$message[$i]";
  }
}
[download]

for (0..@alert) {
  next if (grep /$process[$_]/, @ptable);
  $alerts{$alert[$_]} .= "\n$message[$_]";
}
[download]

Note: scalar @a != $#a

scalar @a is the number of elements

$#a is the last element index (scalar @a - 1)

Bah! I am many things, but a careful coder isn't one of them nearly as often as it should be. You are, of course, right, fglock.

---

Confession: It does an Immortal Body good.