Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wip] Inventory Linux and Windows crontasks #900

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/FusionInventory/Agent/Inventory.pm
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ my %fields = (
VOLUME_GROUPS => [ qw/VG_NAME PV_COUNT LV_COUNT ATTR SIZE FREE VG_UUID
VG_EXTENT_SIZE/ ],
VERSIONPROVIDER => [ qw/NAME VERSION COMMENTS PERL_EXE PERL_VERSION PERL_ARGS
PROGRAM PERL_CONFIG PERL_INC PERL_MODULE/ ]
PROGRAM PERL_CONFIG PERL_INC PERL_MODULE/ ],
CRONTASKS => [ qw/NAME DESCRIPTION COMMAND EXECUTION_MONTH
EXECUTION_DAY EXECUTION_HOUR EXECUTION_MINUTE
EXECUTION_WEEKDAY USER_EXECUTION STORAGE
USER_STORAGE STATUS/ ]
);

my %checks = (
Expand Down
175 changes: 175 additions & 0 deletions lib/FusionInventory/Agent/Task/Inventory/Generic/Crontasks.pm
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
package FusionInventory::Agent::Task::Inventory::Generic::Crontasks;

use strict;
use warnings;

use parent 'FusionInventory::Agent::Task::Inventory::Module';

use English qw(-no_match_vars);

use File::Find;

use FusionInventory::Agent::Tools;

sub isEnabled {
my (%params) = @_;

# return if $params{no_category}->{crontasks};

# Not working under win32
return 0 if $OSNAME eq 'MSWin32';

return
canRun('crontab');
}

sub doInventory {
my (%params) = @_;

my $inventory = $params{inventory};
my $logger = $params{logger};

my %tasks;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused, remove it

Suggested change
my %tasks;


foreach my $task (_getTasks(logger => $logger)) {
$inventory->addEntry(
section => 'CRONTASKS',
entry => $task
);
}
foreach my $task (_getUsersTasks(logger => $logger)) {
$inventory->addEntry(
section => 'CRONTASKS',
entry => $task
);
}
}

sub _getTasks {

my @files;
my @folders;
push @files, '/etc/crontab';
push @folders, '/etc/cron.d';
if (-d '/etc/cron.daily') {
push @folders, '/etc/cron.daily';
}
if (-d '/etc/cron.hourly') {
push @folders, '/etc/cron.hourly';
}
if (-d '/etc/cron.monthly') {
push @folders, '/etc/cron.monthly';
}
if (-d '/etc/cron.weekly') {
push @folders, '/etc/cron.weekly';
}
Comment on lines +50 to +65
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should better initialize arrays here with something like.

Suggested change
my @files;
my @folders;
push @files, '/etc/crontab';
push @folders, '/etc/cron.d';
if (-d '/etc/cron.daily') {
push @folders, '/etc/cron.daily';
}
if (-d '/etc/cron.hourly') {
push @folders, '/etc/cron.hourly';
}
if (-d '/etc/cron.monthly') {
push @folders, '/etc/cron.monthly';
}
if (-d '/etc/cron.weekly') {
push @folders, '/etc/cron.weekly';
}
my @files = qw(/etc/crontab);
my @folders = grep { -d $_ } qw(
/etc/cron.d
/etc/cron.hourly
/etc/cron.daily
/etc/cron.weekly
/etc/cron.monthly
);

As in my suggestion, you should even test /etc/cron.d, this is just safe and will avoid an error if the folder finally doesn't exist.

Copy link

@ghost ghost May 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should better initialize arrays here with something like.
As in my suggestion, you should even test /etc/cron.d, this is just safe and will avoid an error if the folder finally doesn't exist.

Nothing to add to the previous suggestion, except for the paths of the Users crontabs -- for RHEL and Debian based distribs.

Suggested change
my @files;
my @folders;
push @files, '/etc/crontab';
push @folders, '/etc/cron.d';
if (-d '/etc/cron.daily') {
push @folders, '/etc/cron.daily';
}
if (-d '/etc/cron.hourly') {
push @folders, '/etc/cron.hourly';
}
if (-d '/etc/cron.monthly') {
push @folders, '/etc/cron.monthly';
}
if (-d '/etc/cron.weekly') {
push @folders, '/etc/cron.weekly';
}
my @files = qw(/etc/crontab);
my @folders = grep { -d $_ } qw(
/etc/cron.d
/etc/cron.hourly
/etc/cron.daily
/etc/cron.weekly
/etc/cron.monthly
/var/spool/cron
/var/spool/cron/crontabs
);


my $searchFiles = sub {
return if (($_ eq '.') || ($_ eq '..'));
if (-d && $_ eq 'fp') {
$File::Find::prune = 1;
return;
}
return if (-d);
push @files, $File::Find::name;
};

find($searchFiles, @folders);

my @tasks;
foreach my $filename (@files) {

my (%params) = (
file => '/etc/crontab',
@_
);
Comment on lines +82 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems not used to me

Suggested change
my (%params) = (
file => '/etc/crontab',
@_
);


my $handle = getFileHandle((
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should better use getAllLines() API here and then won't have to chomp lines like suggested behind, but then you'll have to replace your while (my $line...) { loop with a foreach my $line (@lines) {

file => $filename,
@_
));
continue unless $handle;
Comment on lines +87 to +91
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use continue directive but next even after an or like in:

Suggested change
my $handle = getFileHandle((
file => $filename,
@_
));
continue unless $handle;
my $handle = getFileHandle(
file => $filename,
@_
)
or next;

or just:

Suggested change
my $handle = getFileHandle((
file => $filename,
@_
));
continue unless $handle;
my $handle = getFileHandle((
file => $filename,
@_
));
next unless $handle;


my $description = '';
my $pathFound = 0;
while (my $line = <$handle>) {
if ($line =~ /^PATH/) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably miss a chomp $line; here even if that won't probably change anything.

Suggested change
if ($line =~ /^PATH/) {
chomp $line;
if ($line =~ /^PATH/) {

$pathFound = 1;
next;
}
next if !$pathFound;
Comment on lines +96 to +100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is PATH really a constraint in crontab files ? When I read my man 5 crontab page, I see no ref for such a constraint.
I guess you may miss some tables then.

I see another problem here, you're parsing also files under /etc/cron.{daily,hourly,weekly,monthly}... These files are not in crontab format but are generally scripts.
So something is wrong with your "find" logic IMHO.

if ($line =~ /^[#\s*|]$/) {
$description = '';
next;
}
if ($line =~ /^#\s*\w/) {
$description .= substr($line, 1);
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use regexp match here:

Suggested change
if ($line =~ /^#\s*\w/) {
$description .= substr($line, 1);
if ($line =~ /^#\s*(\w.*)$/) {
$description .= $1;

} else {
my @args = split(/\s+/, $line, 7);
push @tasks, {
Comment on lines +108 to +109
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you assume @args is 7 elements long, this will probably be the case most time but in case of wrong crontab line entry you'll have a task without command, so you should just keep it

Suggested change
my @args = split(/\s+/, $line, 7);
push @tasks, {
my @args = split(/\s+/, $line, 7);
next unless @args >= 7;
push @tasks, {

NAME => $args[6],
DESCRIPTION => trimWhitespace($description),
COMMAND => $args[6],
EXECUTION_MONTH => $args[3],
EXECUTION_DAY => $args[2],
EXECUTION_HOUR => $args[1],
EXECUTION_MINUTE => $args[0],
EXECUTION_WEEKDAY => $args[4],
USER_EXECUTION => $args[5],
STORAGE => $filename,
USER_STORAGE => 'system',
STATUS => 1
};
}
}
close $handle;
}
return @tasks;
}

sub _getUsersTasks {
my (%params) = @_;

my $logger = $params{logger};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strip spaces here:

Suggested change
my $logger = $params{logger};
my $logger = $params{logger};


my @tasks;
foreach my $user (FusionInventory::Agent::Task::Inventory::Generic::Users::_getLocalUsers(logger => $logger)) {

my $handle = getFileHandle((
command => 'crontab -u '.$user->{LOGIN}.' -l',
@_
));
continue unless $handle;
Comment on lines +138 to +142
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above


my $description = '';
while (my $line = <$handle>) {
$line =~ s/\R//g;
if ($line =~ /^[#\s*|]$/) {
$description = '';
next;
}
if ($line =~ /^#\s*\w/) {
$description .= substr($line, 1);
} else {
my @args = split(/\s+/, $line, 6);
push @tasks, {
NAME => $args[5],
DESCRIPTION => trimWhitespace($description),
COMMAND => $args[5],
EXECUTION_MONTH => $args[3],
EXECUTION_DAY => $args[2],
EXECUTION_HOUR => $args[1],
EXECUTION_MINUTE => $args[0],
EXECUTION_WEEKDAY => $args[4],
USER_EXECUTION => $user->{LOGIN},
STORAGE => $user->{LOGIN},
USER_STORAGE => 'user',
STATUS => 1
};
}
}
close $handle;
Comment on lines +144 to +171
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as above for _getTasks()

}
return @tasks;
}
1;
Loading