[pmg-devel] [PATCH pmg-api v2 02/11] fix #2371: reload pmg-smtp-filter on config change

[Dominik Csapak]

looks good, 3 nits inline (but its fine like it is, i am
just mentioning it for a potential v3)

On 10/15/19 9:46 PM, Stoiko Ivanov wrote:
> the external services (postfix, clamav,...) are restarted if their configfile
> changes (which Template::Toolkit tells us).
> 
> By writing a current-config to '/run/pmg-smtp-filter.cfg' we can use the same
> logic to reload it on a config-change affecting it - currently hide_received
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
>   src/PMG/Config.pm       | 68 +++++++++++++++++++++++++++++++++++++++++
>   src/bin/pmg-smtp-filter |  1 +
>   2 files changed, 69 insertions(+)
> 
> diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
> index 0c907a2..1646f91 100755
> --- a/src/PMG/Config.pm
> +++ b/src/PMG/Config.pm
> @@ -1548,6 +1548,68 @@ sub rewrite_config_postfix {
>       return $changes;
>   }
>   
> +#parameters affecting services w/o config-file (pmgpolicy, pmg-smtp-filter)
> +my $pmg_service_params = {
> +    mail => { hide_received => 1 },
> +};
> +
> +my $smtp_filter_cfg = '/run/pmg-smtp-filter.cfg';
> +my $smtp_filter_cfg_lock = '/run/pmg-smtp-filter.cfg.lck';
> +
> +sub dump_smtp_filter_config {
> +    my ($self) = @_;
> +
> +    my $conf = '';
> +    my $val;
> +    foreach my $sec (sort keys %$pmg_service_params) {
> +	foreach my $key (sort keys %{$pmg_service_params->{$sec}}) {
> +	    $val = $self->{ids}->{$sec}->{$key};

caution, this will create ->{$sec} (as empty hash) even if it or $key 
does not exist
but this is probably not a problem here...

> +	    $conf .= "$sec.$key:$val\n" if defined($val);
> +	}
> +    }
> +
> +    return $conf;
> +}
> +
> +sub compare_smtp_filter_config {
> +    my ($self) = @_;
> +
> +    my $ret = 0;
> +    my $old;
> +    eval {
> +	$old = PVE::Tools::file_get_contents($smtp_filter_cfg);
> +    };
> +    my $err = $@;
> +
> +    if ($err) {

could be written as if (m $err = $@)
but its ok like it is

> +	syslog ('warning', "reloading pmg-smtp-filter: $err");
> +	$ret = 1;
> +    } else {
> +	my $new = $self->dump_smtp_filter_config();
> +	$ret = 1 if $old ne $new;
> +    }
> +
> +    $self->write_smtp_filter_config() if $ret;
> +
> +    return $ret;
> +}
> +
> +# writes the parameters relevant for pmg-smtp-filter to /run/ for comparison
> +# on config change
> +sub write_smtp_filter_config {
> +    my ($self) = @_;
> +
> +    my $code = sub {
> +	my $foo = $self->dump_smtp_filter_config();
> +
> +	PVE::Tools::file_set_contents($smtp_filter_cfg, $self->dump_smtp_filter_config());
> +    };
> +
> +    PVE::Tools::lock_file($smtp_filter_cfg_lock, undef, $code);

could be written without defining $code

lock_file(....., sub => {

});


> +
> +    die $@ if $@;
> +}
> +
>   sub rewrite_config {
>       my ($self, $rulecache, $restart_services, $force_restart) = @_;
>   
> @@ -1589,6 +1651,12 @@ sub rewrite_config {
>   	$log_restart->('clamav-freshclam');
>   	PMG::Utils::service_cmd('clamav-freshclam', 'restart');
>       }
> +
> +    if (($self->compare_smtp_filter_config() && $restart_services) ||
> +	$force_restart->{spam}) {
> +	syslog ('info', "scheduled reload for pmg-smtp-filter");
> +	PMG::Utils::reload_smtp_filter();
> +    }
>   }
>   
>   1;
> diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
> index 61eaf92..62ce9ab 100755
> --- a/src/bin/pmg-smtp-filter
> +++ b/src/bin/pmg-smtp-filter
> @@ -440,6 +440,7 @@ sub pre_loop_hook {
>       my ($backup_umask) = umask;
>   
>       my $pmg_cfg = PMG::Config->new();
> +    $pmg_cfg->write_smtp_filter_config();
>   
>       # Note: you need to restart the daemon when you change 'rbl_checks'
>       my $rbl_checks = $pmg_cfg->get('spam', 'rbl_checks');
>