* [pve-devel] [PATCH v2 common firewall] Optonal `since` and `until` firewall log filtering
@ 2023-01-11 13:32 Christian Ebner
2023-01-11 13:32 ` [pve-devel] [PATCH v2 firewall 1/1] api: Add optional parameters `since` and `until` for timestamp filter Christian Ebner
2023-01-11 13:32 ` [pve-devel] [PATCH v2 common 1/1] tools: Add callback based filtering for logfile dump Christian Ebner
0 siblings, 2 replies; 7+ messages in thread
From: Christian Ebner @ 2023-01-11 13:32 UTC (permalink / raw)
To: pve-devel
This patch series introduces 2 optional api parameters `since` and `until` to
firewall log endpoints, in order to make them filterable.
Filtering of the firewall logs is performed by a callback function.
If the `include_rotated_logs` flag is set, also rotated logfiles are included.
---
Changes since RFC version:
- common: Use callback function filter instead of `since` `until` params
- common: code reuse for `dump_logfile` and `dump_fw_logfile`
- firewall: Style fixes and use of callback function
Changes since v1:
- common: Store parameters needed for multiple `dump_logfile_by_filehandle`
invocations in state hash.
- common: Introduce `final` parameter to signal last invocation to
`dump_logfile_by_filehandle`.
- firewall: Moved `dump_fw_logfile` to firewall helper functions
- firewall: Refactor of finding rotated logfiles by use of `dir_glob_foreach` and fixed regex.
- firewall: Avoid error if opening logfile failes with ENOENT
- Whitespace cleanup
common:
Christian Ebner (1):
tools: Add callback based filtering for logfile dump
src/PVE/Tools.pm | 59 +++++++++++++++++++++++++++++++-----------------
1 file changed, 38 insertions(+), 21 deletions(-)
firewall:
Christian Ebner (1):
api: Add optional parameters `since` and `until` for timestamp filter
src/PVE/API2/Firewall/Host.pm | 34 +++++++++++++++++++-
src/PVE/API2/Firewall/VM.pm | 40 +++++++++++++++++++++---
src/PVE/Firewall/Helpers.pm | 59 +++++++++++++++++++++++++++++++++++
3 files changed, 128 insertions(+), 5 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH v2 firewall 1/1] api: Add optional parameters `since` and `until` for timestamp filter 2023-01-11 13:32 [pve-devel] [PATCH v2 common firewall] Optonal `since` and `until` firewall log filtering Christian Ebner @ 2023-01-11 13:32 ` Christian Ebner 2023-01-18 10:33 ` Wolfgang Bumiller 2023-01-11 13:32 ` [pve-devel] [PATCH v2 common 1/1] tools: Add callback based filtering for logfile dump Christian Ebner 1 sibling, 1 reply; 7+ messages in thread From: Christian Ebner @ 2023-01-11 13:32 UTC (permalink / raw) To: pve-devel The optional unix epoch timestamps parameters `since` and `until` are introduced in order to filter firewall logs files. If one of these flags is set, also rotated logfiles are included. This is handled in the `dump_fw_logfile` helper function. Filtering is now performed based on a callback function passed to `dump_fw_logfile`. This patch depends on the corresponding patch in the pve-common repository. Signed-off-by: Christian Ebner <c.ebner@proxmox.com> --- src/PVE/API2/Firewall/Host.pm | 34 +++++++++++++++++++- src/PVE/API2/Firewall/VM.pm | 40 +++++++++++++++++++++--- src/PVE/Firewall/Helpers.pm | 59 +++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 5 deletions(-) diff --git a/src/PVE/API2/Firewall/Host.pm b/src/PVE/API2/Firewall/Host.pm index dfeccd0..02f090e 100644 --- a/src/PVE/API2/Firewall/Host.pm +++ b/src/PVE/API2/Firewall/Host.pm @@ -11,6 +11,7 @@ use PVE::Firewall; use PVE::API2::Firewall::Rules; +use Date::Parse qw(str2time); use base qw(PVE::RESTHandler); __PACKAGE__->register_method ({ @@ -172,6 +173,18 @@ __PACKAGE__->register_method({ minimum => 0, optional => 1, }, + since => { + type => 'integer', + minimum => 0, + description => "Display log since this UNIX epoch.", + optional => 1, + }, + until => { + type => 'integer', + minimum => 0, + description => "Display log until this UNIX epoch.", + optional => 1, + }, }, }, returns => { @@ -196,8 +209,27 @@ __PACKAGE__->register_method({ my $rpcenv = PVE::RPCEnvironment::get(); my $user = $rpcenv->get_user(); my $node = $param->{node}; + my $filename = "/var/log/pve-firewall.log"; + my ($start, $limit, $since, $until) = + $param->@{qw(start limit since until)}; + + my $filter = sub { + my ($line) = @_; + + if ($since || $until) { + my @words = split / /, $line; + my $timestamp = str2time($words[3], $words[4]); + return undef if $since && $timestamp < $since; + return undef if $until && $timestamp > $until; + } + + return $line; + }; + + my $include_rotated_logs = defined($since) || defined($until); - my ($count, $lines) = PVE::Tools::dump_logfile("/var/log/pve-firewall.log", $param->{start}, $param->{limit}); + my ($count, $lines) = PVE::Firewall::Helpers::dump_fw_logfile( + $filename, $start, $limit, $filter, $include_rotated_logs); $rpcenv->set_result_attrib('total', $count); diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm index 48b8c5f..53fc581 100644 --- a/src/PVE/API2/Firewall/VM.pm +++ b/src/PVE/API2/Firewall/VM.pm @@ -11,6 +11,7 @@ use PVE::API2::Firewall::Rules; use PVE::API2::Firewall::Aliases; +use Date::Parse qw(str2time); use base qw(PVE::RESTHandler); my $option_properties = $PVE::Firewall::vm_option_properties; @@ -176,6 +177,18 @@ sub register_handlers { minimum => 0, optional => 1, }, + since => { + type => 'integer', + minimum => 0, + description => "Display log since this UNIX epoch.", + optional => 1, + }, + until => { + type => 'integer', + minimum => 0, + description => "Display log until this UNIX epoch.", + optional => 1, + }, }, }, returns => { @@ -199,11 +212,30 @@ sub register_handlers { my $rpcenv = PVE::RPCEnvironment::get(); my $user = $rpcenv->get_user(); - my $vmid = $param->{vmid}; + my $filename = "/var/log/pve-firewall.log"; + my ($start, $limit, $vmid, $since, $until) = + $param->@{qw(start limit vmid since until)}; + + my $filter = sub { + my ($line) = @_; + my $reg = "^$vmid "; + + return undef if $line !~ m/$reg/; + + if ($since || $until) { + my @words = split / /, $line; + my $timestamp = str2time($words[3], $words[4]); + return undef if $since && $timestamp < $since; + return undef if $until && $timestamp > $until; + } + + return $line; + }; + + my $include_rotated_logs = defined($since) || defined($until); - my ($count, $lines) = PVE::Tools::dump_logfile("/var/log/pve-firewall.log", - $param->{start}, $param->{limit}, - "^$vmid "); + my ($count, $lines) = PVE::Firewall::Helpers::dump_fw_logfile( + $filename, $start, $limit, $filter, $include_rotated_logs); $rpcenv->set_result_attrib('total', $count); diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm index 154fca5..697176b 100644 --- a/src/PVE/Firewall/Helpers.pm +++ b/src/PVE/Firewall/Helpers.pm @@ -3,6 +3,9 @@ package PVE::Firewall::Helpers; use strict; use warnings; +use Errno qw(ENOENT); +use File::Basename qw(fileparse); +use IO::Zlib; use PVE::Cluster; use PVE::Tools qw(file_get_contents file_set_contents); @@ -52,4 +55,60 @@ sub clone_vmfw_conf { }); } +sub dump_fw_logfile { + my ($filename, $start, $limit, $filter, $include_rotated_logs) = @_; + + if (!$include_rotated_logs) { + return PVE::Tools::dump_logfile($filename, $start, $limit, $filter); + } + + my %state = ( + 'count' => 0, + 'lines' => [], + 'start' => $start, + 'limit' => $limit, + ); + + # Take into consideration also rotated logs + my ($basename, $logdir, $type) = fileparse($filename); + my $regex = "^$basename(\\.[\\d]+(\\.gz)?)?\$"; + my @files = (); + + PVE::Tools::dir_glob_foreach($logdir, $regex, sub { + my ( $file ) = @_; + push @files, $file; + }); + + @files = reverse sort @files; + + my $filecount = 0; + for my $filename (@files) { + $filecount++; + $state{'final'} = $filecount == $#files; + + my $fh; + if ($filename =~ /\.gz$/) { + $fh = IO::Zlib->new($logdir.$filename, "r"); + } else { + $fh = IO::File->new($logdir.$filename, "r"); + } + + if (!$fh) { + # If file vanished since reading dir entries, ignore + continue if $!{ENOENT}; + + my $lines = $state{'lines'}; + my $count = ++$state{'count'}; + push @$lines, ($count, { n => $count, t => "unable to open file - $!"}); + last; + } + + PVE::Tools::dump_logfile_by_filehandle($fh, $filter, \%state); + + close($fh); + } + + return ($state{'count'}, $state{'lines'}); +} + 1; -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH v2 firewall 1/1] api: Add optional parameters `since` and `until` for timestamp filter 2023-01-11 13:32 ` [pve-devel] [PATCH v2 firewall 1/1] api: Add optional parameters `since` and `until` for timestamp filter Christian Ebner @ 2023-01-18 10:33 ` Wolfgang Bumiller 2023-01-18 15:10 ` Christian Ebner 0 siblings, 1 reply; 7+ messages in thread From: Wolfgang Bumiller @ 2023-01-18 10:33 UTC (permalink / raw) To: Christian Ebner; +Cc: pve-devel On Wed, Jan 11, 2023 at 02:32:19PM +0100, Christian Ebner wrote: > The optional unix epoch timestamps parameters `since` and `until` are introduced > in order to filter firewall logs files. If one of these flags is set, also > rotated logfiles are included. This is handled in the `dump_fw_logfile` helper > function. Filtering is now performed based on a callback function passed to > `dump_fw_logfile`. > > This patch depends on the corresponding patch in the pve-common repository. > > Signed-off-by: Christian Ebner <c.ebner@proxmox.com> > --- > src/PVE/API2/Firewall/Host.pm | 34 +++++++++++++++++++- > src/PVE/API2/Firewall/VM.pm | 40 +++++++++++++++++++++--- > src/PVE/Firewall/Helpers.pm | 59 +++++++++++++++++++++++++++++++++++ > 3 files changed, 128 insertions(+), 5 deletions(-) > > diff --git a/src/PVE/API2/Firewall/Host.pm b/src/PVE/API2/Firewall/Host.pm > index dfeccd0..02f090e 100644 > --- a/src/PVE/API2/Firewall/Host.pm > +++ b/src/PVE/API2/Firewall/Host.pm > @@ -11,6 +11,7 @@ use PVE::Firewall; > use PVE::API2::Firewall::Rules; > > > +use Date::Parse qw(str2time); > use base qw(PVE::RESTHandler); > > __PACKAGE__->register_method ({ > @@ -172,6 +173,18 @@ __PACKAGE__->register_method({ > minimum => 0, > optional => 1, > }, > + since => { > + type => 'integer', > + minimum => 0, > + description => "Display log since this UNIX epoch.", > + optional => 1, > + }, > + until => { > + type => 'integer', > + minimum => 0, > + description => "Display log until this UNIX epoch.", > + optional => 1, > + }, > }, > }, > returns => { > @@ -196,8 +209,27 @@ __PACKAGE__->register_method({ > my $rpcenv = PVE::RPCEnvironment::get(); > my $user = $rpcenv->get_user(); > my $node = $param->{node}; > + my $filename = "/var/log/pve-firewall.log"; > + my ($start, $limit, $since, $until) = > + $param->@{qw(start limit since until)}; > + > + my $filter = sub { I think this filter could be implied by the `dump_fw_logfile` sub. > + my ($line) = @_; > + > + if ($since || $until) { > + my @words = split / /, $line; > + my $timestamp = str2time($words[3], $words[4]); > + return undef if $since && $timestamp < $since; > + return undef if $until && $timestamp > $until; > + } > + > + return $line; > + }; > + > + my $include_rotated_logs = defined($since) || defined($until); > > - my ($count, $lines) = PVE::Tools::dump_logfile("/var/log/pve-firewall.log", $param->{start}, $param->{limit}); > + my ($count, $lines) = PVE::Firewall::Helpers::dump_fw_logfile( > + $filename, $start, $limit, $filter, $include_rotated_logs); > > $rpcenv->set_result_attrib('total', $count); > > diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm > index 48b8c5f..53fc581 100644 > --- a/src/PVE/API2/Firewall/VM.pm > +++ b/src/PVE/API2/Firewall/VM.pm > @@ -11,6 +11,7 @@ use PVE::API2::Firewall::Rules; > use PVE::API2::Firewall::Aliases; > > > +use Date::Parse qw(str2time); > use base qw(PVE::RESTHandler); > > my $option_properties = $PVE::Firewall::vm_option_properties; > @@ -176,6 +177,18 @@ sub register_handlers { > minimum => 0, > optional => 1, > }, > + since => { > + type => 'integer', > + minimum => 0, > + description => "Display log since this UNIX epoch.", > + optional => 1, > + }, > + until => { > + type => 'integer', > + minimum => 0, > + description => "Display log until this UNIX epoch.", > + optional => 1, > + }, > }, > }, > returns => { > @@ -199,11 +212,30 @@ sub register_handlers { > > my $rpcenv = PVE::RPCEnvironment::get(); > my $user = $rpcenv->get_user(); > - my $vmid = $param->{vmid}; > + my $filename = "/var/log/pve-firewall.log"; > + my ($start, $limit, $vmid, $since, $until) = > + $param->@{qw(start limit vmid since until)}; > + > + my $filter = sub { > + my ($line) = @_; > + my $reg = "^$vmid "; > + > + return undef if $line !~ m/$reg/; And the `dump_fw_logfile`'s $filter parameter would just be this extra filter above? > + > + if ($since || $until) { > + my @words = split / /, $line; > + my $timestamp = str2time($words[3], $words[4]); > + return undef if $since && $timestamp < $since; > + return undef if $until && $timestamp > $until; > + } > + > + return $line; > + }; > + > + my $include_rotated_logs = defined($since) || defined($until); > > - my ($count, $lines) = PVE::Tools::dump_logfile("/var/log/pve-firewall.log", > - $param->{start}, $param->{limit}, > - "^$vmid "); > + my ($count, $lines) = PVE::Firewall::Helpers::dump_fw_logfile( > + $filename, $start, $limit, $filter, $include_rotated_logs); > > $rpcenv->set_result_attrib('total', $count); > > diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm > index 154fca5..697176b 100644 > --- a/src/PVE/Firewall/Helpers.pm > +++ b/src/PVE/Firewall/Helpers.pm > @@ -3,6 +3,9 @@ package PVE::Firewall::Helpers; > use strict; > use warnings; > > +use Errno qw(ENOENT); > +use File::Basename qw(fileparse); > +use IO::Zlib; > use PVE::Cluster; > use PVE::Tools qw(file_get_contents file_set_contents); > > @@ -52,4 +55,60 @@ sub clone_vmfw_conf { > }); > } > > +sub dump_fw_logfile { > + my ($filename, $start, $limit, $filter, $include_rotated_logs) = @_; > + > + if (!$include_rotated_logs) { > + return PVE::Tools::dump_logfile($filename, $start, $limit, $filter); > + } > + > + my %state = ( > + 'count' => 0, > + 'lines' => [], > + 'start' => $start, > + 'limit' => $limit, > + ); > + > + # Take into consideration also rotated logs > + my ($basename, $logdir, $type) = fileparse($filename); > + my $regex = "^$basename(\\.[\\d]+(\\.gz)?)?\$"; Let's please use `qr//` syntax for this regex and put `\Q` and `\E` around `$basename`. This way it's much harder to confuse literal backslashes inside the regex with literal backslashes inside the double quoted string being escape-characters within the regex etc. ;-) > + my @files = (); > + > + PVE::Tools::dir_glob_foreach($logdir, $regex, sub { > + my ( $file ) = @_; > + push @files, $file; > + }); > + > + @files = reverse sort @files; > + > + my $filecount = 0; > + for my $filename (@files) { > + $filecount++; > + $state{'final'} = $filecount == $#files; > + > + my $fh; > + if ($filename =~ /\.gz$/) { > + $fh = IO::Zlib->new($logdir.$filename, "r"); > + } else { > + $fh = IO::File->new($logdir.$filename, "r"); > + } > + > + if (!$fh) { > + # If file vanished since reading dir entries, ignore > + continue if $!{ENOENT}; > + > + my $lines = $state{'lines'}; > + my $count = ++$state{'count'}; > + push @$lines, ($count, { n => $count, t => "unable to open file - $!"}); > + last; > + } > + > + PVE::Tools::dump_logfile_by_filehandle($fh, $filter, \%state); > + > + close($fh); > + } > + > + return ($state{'count'}, $state{'lines'}); > +} > + > 1; > -- > 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH v2 firewall 1/1] api: Add optional parameters `since` and `until` for timestamp filter 2023-01-18 10:33 ` Wolfgang Bumiller @ 2023-01-18 15:10 ` Christian Ebner 2023-01-18 15:46 ` Wolfgang Bumiller 0 siblings, 1 reply; 7+ messages in thread From: Christian Ebner @ 2023-01-18 15:10 UTC (permalink / raw) To: Wolfgang Bumiller; +Cc: pve-devel > On 18.01.2023 11:33 CET Wolfgang Bumiller <w.bumiller@proxmox.com> wrote: > > > On Wed, Jan 11, 2023 at 02:32:19PM +0100, Christian Ebner wrote: > > The optional unix epoch timestamps parameters `since` and `until` are introduced > > in order to filter firewall logs files. If one of these flags is set, also > > rotated logfiles are included. This is handled in the `dump_fw_logfile` helper > > function. Filtering is now performed based on a callback function passed to > > `dump_fw_logfile`. > > > > This patch depends on the corresponding patch in the pve-common repository. > > > > Signed-off-by: Christian Ebner <c.ebner@proxmox.com> > > --- > > src/PVE/API2/Firewall/Host.pm | 34 +++++++++++++++++++- > > src/PVE/API2/Firewall/VM.pm | 40 +++++++++++++++++++++--- > > src/PVE/Firewall/Helpers.pm | 59 +++++++++++++++++++++++++++++++++++ > > 3 files changed, 128 insertions(+), 5 deletions(-) > > > > diff --git a/src/PVE/API2/Firewall/Host.pm b/src/PVE/API2/Firewall/Host.pm > > index dfeccd0..02f090e 100644 > > --- a/src/PVE/API2/Firewall/Host.pm > > +++ b/src/PVE/API2/Firewall/Host.pm > > @@ -11,6 +11,7 @@ use PVE::Firewall; > > use PVE::API2::Firewall::Rules; > > > > > > +use Date::Parse qw(str2time); > > use base qw(PVE::RESTHandler); > > > > __PACKAGE__->register_method ({ > > @@ -172,6 +173,18 @@ __PACKAGE__->register_method({ > > minimum => 0, > > optional => 1, > > }, > > + since => { > > + type => 'integer', > > + minimum => 0, > > + description => "Display log since this UNIX epoch.", > > + optional => 1, > > + }, > > + until => { > > + type => 'integer', > > + minimum => 0, > > + description => "Display log until this UNIX epoch.", > > + optional => 1, > > + }, > > }, > > }, > > returns => { > > @@ -196,8 +209,27 @@ __PACKAGE__->register_method({ > > my $rpcenv = PVE::RPCEnvironment::get(); > > my $user = $rpcenv->get_user(); > > my $node = $param->{node}; > > + my $filename = "/var/log/pve-firewall.log"; > > + my ($start, $limit, $since, $until) = > > + $param->@{qw(start limit since until)}; > > + > > + my $filter = sub { > > I think this filter could be implied by the `dump_fw_logfile` sub. In which case I would need to re-introduce the `since` and `until` parameters to `dump_fw_logfile`, which was the idea to get rid of by passing the whole callback function. Or am I missing the point here? Maybe put it all together with `start` and `limit` in a `param` hash? > > > + my ($line) = @_; > > + > > + if ($since || $until) { > > + my @words = split / /, $line; > > + my $timestamp = str2time($words[3], $words[4]); > > + return undef if $since && $timestamp < $since; > > + return undef if $until && $timestamp > $until; > > + } > > + > > + return $line; > > + }; > > + > > + my $include_rotated_logs = defined($since) || defined($until); > > > > - my ($count, $lines) = PVE::Tools::dump_logfile("/var/log/pve-firewall.log", $param->{start}, $param->{limit}); > > + my ($count, $lines) = PVE::Firewall::Helpers::dump_fw_logfile( > > + $filename, $start, $limit, $filter, $include_rotated_logs); > > > > $rpcenv->set_result_attrib('total', $count); > > > > diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm > > index 48b8c5f..53fc581 100644 > > --- a/src/PVE/API2/Firewall/VM.pm > > +++ b/src/PVE/API2/Firewall/VM.pm > > @@ -11,6 +11,7 @@ use PVE::API2::Firewall::Rules; > > use PVE::API2::Firewall::Aliases; > > > > > > +use Date::Parse qw(str2time); > > use base qw(PVE::RESTHandler); > > > > my $option_properties = $PVE::Firewall::vm_option_properties; > > @@ -176,6 +177,18 @@ sub register_handlers { > > minimum => 0, > > optional => 1, > > }, > > + since => { > > + type => 'integer', > > + minimum => 0, > > + description => "Display log since this UNIX epoch.", > > + optional => 1, > > + }, > > + until => { > > + type => 'integer', > > + minimum => 0, > > + description => "Display log until this UNIX epoch.", > > + optional => 1, > > + }, > > }, > > }, > > returns => { > > @@ -199,11 +212,30 @@ sub register_handlers { > > > > my $rpcenv = PVE::RPCEnvironment::get(); > > my $user = $rpcenv->get_user(); > > - my $vmid = $param->{vmid}; > > + my $filename = "/var/log/pve-firewall.log"; > > + my ($start, $limit, $vmid, $since, $until) = > > + $param->@{qw(start limit vmid since until)}; > > + > > + my $filter = sub { > > + my ($line) = @_; > > + my $reg = "^$vmid "; > > + > > + return undef if $line !~ m/$reg/; > > And the `dump_fw_logfile`'s $filter parameter would just be this extra > filter above? > > > + > > + if ($since || $until) { > > + my @words = split / /, $line; > > + my $timestamp = str2time($words[3], $words[4]); > > + return undef if $since && $timestamp < $since; > > + return undef if $until && $timestamp > $until; > > + } > > + > > + return $line; > > + }; > > + > > + my $include_rotated_logs = defined($since) || defined($until); > > > > - my ($count, $lines) = PVE::Tools::dump_logfile("/var/log/pve-firewall.log", > > - $param->{start}, $param->{limit}, > > - "^$vmid "); > > + my ($count, $lines) = PVE::Firewall::Helpers::dump_fw_logfile( > > + $filename, $start, $limit, $filter, $include_rotated_logs); > > > > $rpcenv->set_result_attrib('total', $count); > > > > diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm > > index 154fca5..697176b 100644 > > --- a/src/PVE/Firewall/Helpers.pm > > +++ b/src/PVE/Firewall/Helpers.pm > > @@ -3,6 +3,9 @@ package PVE::Firewall::Helpers; > > use strict; > > use warnings; > > > > +use Errno qw(ENOENT); > > +use File::Basename qw(fileparse); > > +use IO::Zlib; > > use PVE::Cluster; > > use PVE::Tools qw(file_get_contents file_set_contents); > > > > @@ -52,4 +55,60 @@ sub clone_vmfw_conf { > > }); > > } > > > > +sub dump_fw_logfile { > > + my ($filename, $start, $limit, $filter, $include_rotated_logs) = @_; > > + > > + if (!$include_rotated_logs) { > > + return PVE::Tools::dump_logfile($filename, $start, $limit, $filter); > > + } > > + > > + my %state = ( > > + 'count' => 0, > > + 'lines' => [], > > + 'start' => $start, > > + 'limit' => $limit, > > + ); > > + > > + # Take into consideration also rotated logs > > + my ($basename, $logdir, $type) = fileparse($filename); > > + my $regex = "^$basename(\\.[\\d]+(\\.gz)?)?\$"; > > Let's please use `qr//` syntax for this regex and put `\Q` and `\E` > around `$basename`. > > This way it's much harder to confuse literal backslashes inside the > regex with literal backslashes inside the double quoted string being > escape-characters within the regex etc. ;-) Yes, makes sense :-) > > > + my @files = (); > > + > > + PVE::Tools::dir_glob_foreach($logdir, $regex, sub { > > + my ( $file ) = @_; > > + push @files, $file; > > + }); > > + > > + @files = reverse sort @files; > > + > > + my $filecount = 0; > > + for my $filename (@files) { > > + $filecount++; > > + $state{'final'} = $filecount == $#files; > > + > > + my $fh; > > + if ($filename =~ /\.gz$/) { > > + $fh = IO::Zlib->new($logdir.$filename, "r"); > > + } else { > > + $fh = IO::File->new($logdir.$filename, "r"); > > + } > > + > > + if (!$fh) { > > + # If file vanished since reading dir entries, ignore > > + continue if $!{ENOENT}; > > + > > + my $lines = $state{'lines'}; > > + my $count = ++$state{'count'}; > > + push @$lines, ($count, { n => $count, t => "unable to open file - $!"}); > > + last; > > + } > > + > > + PVE::Tools::dump_logfile_by_filehandle($fh, $filter, \%state); > > + > > + close($fh); > > + } > > + > > + return ($state{'count'}, $state{'lines'}); > > +} > > + > > 1; > > -- > > 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH v2 firewall 1/1] api: Add optional parameters `since` and `until` for timestamp filter 2023-01-18 15:10 ` Christian Ebner @ 2023-01-18 15:46 ` Wolfgang Bumiller 0 siblings, 0 replies; 7+ messages in thread From: Wolfgang Bumiller @ 2023-01-18 15:46 UTC (permalink / raw) To: Christian Ebner; +Cc: pve-devel On Wed, Jan 18, 2023 at 04:10:28PM +0100, Christian Ebner wrote: > > On 18.01.2023 11:33 CET Wolfgang Bumiller <w.bumiller@proxmox.com> wrote: > > > > > > On Wed, Jan 11, 2023 at 02:32:19PM +0100, Christian Ebner wrote: > > > The optional unix epoch timestamps parameters `since` and `until` are introduced > > > in order to filter firewall logs files. If one of these flags is set, also > > > rotated logfiles are included. This is handled in the `dump_fw_logfile` helper > > > function. Filtering is now performed based on a callback function passed to > > > `dump_fw_logfile`. > > > > > > This patch depends on the corresponding patch in the pve-common repository. > > > > > > Signed-off-by: Christian Ebner <c.ebner@proxmox.com> > > > --- > > > src/PVE/API2/Firewall/Host.pm | 34 +++++++++++++++++++- > > > src/PVE/API2/Firewall/VM.pm | 40 +++++++++++++++++++++--- > > > src/PVE/Firewall/Helpers.pm | 59 +++++++++++++++++++++++++++++++++++ > > > 3 files changed, 128 insertions(+), 5 deletions(-) > > > > > > diff --git a/src/PVE/API2/Firewall/Host.pm b/src/PVE/API2/Firewall/Host.pm > > > index dfeccd0..02f090e 100644 > > > --- a/src/PVE/API2/Firewall/Host.pm > > > +++ b/src/PVE/API2/Firewall/Host.pm > > > @@ -11,6 +11,7 @@ use PVE::Firewall; > > > use PVE::API2::Firewall::Rules; > > > > > > > > > +use Date::Parse qw(str2time); > > > use base qw(PVE::RESTHandler); > > > > > > __PACKAGE__->register_method ({ > > > @@ -172,6 +173,18 @@ __PACKAGE__->register_method({ > > > minimum => 0, > > > optional => 1, > > > }, > > > + since => { > > > + type => 'integer', > > > + minimum => 0, > > > + description => "Display log since this UNIX epoch.", > > > + optional => 1, > > > + }, > > > + until => { > > > + type => 'integer', > > > + minimum => 0, > > > + description => "Display log until this UNIX epoch.", > > > + optional => 1, > > > + }, > > > }, > > > }, > > > returns => { > > > @@ -196,8 +209,27 @@ __PACKAGE__->register_method({ > > > my $rpcenv = PVE::RPCEnvironment::get(); > > > my $user = $rpcenv->get_user(); > > > my $node = $param->{node}; > > > + my $filename = "/var/log/pve-firewall.log"; > > > + my ($start, $limit, $since, $until) = > > > + $param->@{qw(start limit since until)}; > > > + > > > + my $filter = sub { > > > > I think this filter could be implied by the `dump_fw_logfile` sub. > > In which case I would need to re-introduce the `since` and `until` parameters to `dump_fw_logfile`, which was the idea to get rid of by passing the whole callback function. Or am I missing the point here? Maybe put it all together with `start` and `limit` in a `param` hash? Well I mostly just wanted it out of `PVE::Tools` since it also parses fw log specific lines, but now that part is inside the firewall package, and it's just duplicate code. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH v2 common 1/1] tools: Add callback based filtering for logfile dump 2023-01-11 13:32 [pve-devel] [PATCH v2 common firewall] Optonal `since` and `until` firewall log filtering Christian Ebner 2023-01-11 13:32 ` [pve-devel] [PATCH v2 firewall 1/1] api: Add optional parameters `since` and `until` for timestamp filter Christian Ebner @ 2023-01-11 13:32 ` Christian Ebner 2023-01-18 10:28 ` [pve-devel] applied: " Wolfgang Bumiller 1 sibling, 1 reply; 7+ messages in thread From: Christian Ebner @ 2023-01-11 13:32 UTC (permalink / raw) To: pve-devel This patch introduces callback based filtering functionality for logfile dumps. Further, the `dump_logfile` function is split into a reusable part for dumps generated based on a filehandle. The state parameter can be used to keep the state for multiple consecutive function invocations. Signed-off-by: Christian Ebner <c.ebner@proxmox.com> --- src/PVE/Tools.pm | 59 +++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm index cdbee6d..d933503 100644 --- a/src/PVE/Tools.pm +++ b/src/PVE/Tools.pm @@ -1265,29 +1265,25 @@ sub split_args { return $str ? [ Text::ParseWords::shellwords($str) ] : []; } -sub dump_logfile { - my ($filename, $start, $limit, $filter) = @_; - - my $lines = []; - my $count = 0; - - my $fh = IO::File->new($filename, "r"); - if (!$fh) { - $count++; - push @$lines, { n => $count, t => "unable to open file - $!"}; - return ($count, $lines); - } +sub dump_logfile_by_filehandle { + my ($fh, $filter, $state) = @_; - $start = $start // 0; - $limit = $limit // 50; + my $count = ($state->{count} //= 0); + my $lines = ($state->{lines} //= []); + my $start = ($state->{start} //= 0); + my $limit = ($state->{limit} //= 50); + my $final = ($state->{final} //= 1); + my $read_until_end = ($state->{read_until_end} //= $limit == 0); - my $read_until_end = $limit == 0; my $line; - if ($filter) { # duplicate code, so that we do not slow down normal path while (defined($line = <$fh>)) { - next if $line !~ m/$filter/; + if (ref($filter) eq 'CODE') { + next if !$filter->($line); + } else { + next if $line !~ m/$filter/; + } next if $count++ < $start; if (!$read_until_end) { next if $limit <= 0; @@ -1308,16 +1304,37 @@ sub dump_logfile { } } - close($fh); - # HACK: ExtJS store.guaranteeRange() does not like empty array # so we add a line - if (!$count) { + if (!$count && $final) { $count++; push @$lines, { n => $count, t => "no content"}; } - return ($count, $lines); + $state->{count} = $count; + $state->{limit} = $limit; +} + +sub dump_logfile { + my ($filename, $start, $limit, $filter) = @_; + + my $fh = IO::File->new($filename, "r"); + if (!$fh) { + return (1, { n => 1, t => "unable to open file - $!"}); + } + + my %state = ( + 'count' => 0, + 'lines' => [], + 'start' => $start, + 'limit' => $limit, + ); + + dump_logfile_by_filehandle($fh, $filter, \%state); + + close($fh); + + return ($state{'count'}, $state{'lines'}); } sub dump_journal { -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] applied: [PATCH v2 common 1/1] tools: Add callback based filtering for logfile dump 2023-01-11 13:32 ` [pve-devel] [PATCH v2 common 1/1] tools: Add callback based filtering for logfile dump Christian Ebner @ 2023-01-18 10:28 ` Wolfgang Bumiller 0 siblings, 0 replies; 7+ messages in thread From: Wolfgang Bumiller @ 2023-01-18 10:28 UTC (permalink / raw) To: Christian Ebner; +Cc: pve-devel applied this one, some nits in the other one ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-01-18 15:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-11 13:32 [pve-devel] [PATCH v2 common firewall] Optonal `since` and `until` firewall log filtering Christian Ebner 2023-01-11 13:32 ` [pve-devel] [PATCH v2 firewall 1/1] api: Add optional parameters `since` and `until` for timestamp filter Christian Ebner 2023-01-18 10:33 ` Wolfgang Bumiller 2023-01-18 15:10 ` Christian Ebner 2023-01-18 15:46 ` Wolfgang Bumiller 2023-01-11 13:32 ` [pve-devel] [PATCH v2 common 1/1] tools: Add callback based filtering for logfile dump Christian Ebner 2023-01-18 10:28 ` [pve-devel] applied: " Wolfgang Bumiller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.