[pmg-devel] [PATCH pmg-api 2/3] add SACustom Package and API Calls for custom SpamAssassin scores
Stoiko Ivanov
s.ivanov at proxmox.com
Wed Nov 13 15:42:24 CET 2019
On Fri, 8 Nov 2019 14:29:30 +0100
Dominik Csapak <d.csapak at proxmox.com> wrote:
> this uses our INotify interface to parse and write a custom sa config
> in /etc/mail/spamassassin/pmg-scores.cf with a shadow file in
> /var/cache/pmg-scores.cf (to track the diff)
>
> add also api calls to create a new/delete/edit/revert/apply those custom
> rules
>
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> src/Makefile | 2 +
> src/PMG/API2/Config.pm | 6 +
> src/PMG/API2/SACustom.pm | 334 +++++++++++++++++++++++++++++++++++++++
> src/PMG/SACustom.pm | 89 +++++++++++
> 4 files changed, 431 insertions(+)
> create mode 100644 src/PMG/API2/SACustom.pm
> create mode 100644 src/PMG/SACustom.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index 89658db..e3bee39 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -77,6 +77,7 @@ LIBSOURCES = \
> PMG/DKIMSign.pm \
> PMG/Quarantine.pm \
> PMG/Report.pm \
> + PMG/SACustom.pm \
> PMG/RuleDB/Group.pm \
> PMG/RuleDB/Rule.pm \
> PMG/RuleDB/Object.pm \
> @@ -130,6 +131,7 @@ LIBSOURCES = \
> PMG/API2/Cluster.pm \
> PMG/API2/ClamAV.pm \
> PMG/API2/SpamAssassin.pm \
> + PMG/API2/SACustom.pm \
> PMG/API2/Statistics.pm \
> PMG/API2/MailTracker.pm \
> PMG/API2/Backup.pm \
> diff --git a/src/PMG/API2/Config.pm b/src/PMG/API2/Config.pm
> index 43653e4..1b3743e 100644
> --- a/src/PMG/API2/Config.pm
> +++ b/src/PMG/API2/Config.pm
> @@ -24,6 +24,7 @@ use PMG::API2::MimeTypes;
> use PMG::API2::Fetchmail;
> use PMG::API2::DestinationTLSPolicy;
> use PMG::API2::DKIMSign;
> +use PMG::API2::SACustom;
>
> use base qw(PVE::RESTHandler);
>
> @@ -87,6 +88,11 @@ __PACKAGE__->register_method({
> path => 'dkim',
> });
>
> +__PACKAGE__->register_method({
> + subclass => "PMG::API2::SACustom",
> + path => 'customscores',
> +});
> +
> __PACKAGE__->register_method ({
> name => 'index',
> path => '',
> diff --git a/src/PMG/API2/SACustom.pm b/src/PMG/API2/SACustom.pm
> new file mode 100644
> index 0000000..f9ad67a
> --- /dev/null
> +++ b/src/PMG/API2/SACustom.pm
> @@ -0,0 +1,334 @@
> +package PMG::API2::SACustom;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::SafeSyslog;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::RESTHandler;
> +use PVE::INotify;
> +use PVE::Tools qw(extract_param);
> +use PVE::Exception qw(raise_param_exc);
> +
> +use PMG::RESTEnvironment;
> +use PMG::Utils;
> +use PMG::SACustom;
> +
> +use base qw(PVE::RESTHandler);
> +
> +my $score_properties = {
> + name => {
> + type => 'string',
> + description => "The name of the rule.",
> + pattern => '[a-zA-Z\_\-\.0-9]+',
> + },
> + score => {
> + type => 'number',
> + description => "The score the rule should be valued at.",
> + },
> + comment => {
> + type => 'string',
> + description => 'The Comment.',
> + optional => 1,
> + },
> +};
> +
> +sub json_config_properties {
> + my ($prop, $optional) = @_;
> +
> + foreach my $opt (keys %$score_properties) {
> + $prop->{$opt} = $score_properties->{$opt};
> + if ($optional->{$opt}) {
> + $prop->{$opt}->{optional} = 1;
> + }
> + }
> +
> + return $prop;
> +}
> +
Here $prop contains copies of the hashref in $score_properties, and the
update of the 'optional' key persists after the first invocation.
The following worked in my test for fixing this (although the readability
might be debatable):
```
map { $prop->{$_} = { %{$score_properties->{$_}}} } keys %$score_properties;
map { $prop->{$_}->{optional} = 1 } keys %$optional;
```
> +__PACKAGE__->register_method({
> + name => 'list_scores',
> + path => '',
> + method => 'GET',
> + description => "List custom scores.",
> + # protected => 1,
> + permissions => { check => [ 'admin', 'audit' ] },
> + proxyto => 'master',
> + parameters => {
> + additionalProperties => 0,
> + properties => { },
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => 'object',
> + properties => json_config_properties({
> + digest => get_standard_option('pve-config-digest'),
> + },
> + {
> + # mark all properties optional, so that we can have
> + # one entry with only digest, and all others without digest
> + name => 1,
> + score => 1,
> + comment => 1,
> + }),
> + },
> + links => [ { rel => 'child', href => "{name}" } ],
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $restenv = PMG::RESTEnvironment->get();
> +
> + my $tmp = PVE::INotify::read_file('pmg-scores.cf', 1);
> +
> + my $changes = $tmp->{changes};
> + $restenv->set_result_attrib('changes', $changes) if $changes;
> +
> + my $res = [];
> +
> + for my $rule (sort keys %{$tmp->{data}}) {
> + push @$res, $tmp->{data}->{$rule};
> + }
> +
> + my $digest = PMG::SACustom::calc_digest($tmp->{data});
> +
> + push @$res, {
> + digest => $digest,
> + };
> +
> + return $res;
> + }});
> +
> +__PACKAGE__->register_method({
> + name => 'apply_score_changes',
> + path => '',
> + method => 'PUT',
> + protected => 1,
> + description => "Apply custom score changes.",
> + proxyto => 'master',
> + permissions => { check => [ 'admin' ] },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + 'restart-daemon' => {
> + type => 'boolean',
> + description => 'If set, also restarts pmg-smtp-filter. '.
> + 'This is necessary for the changes to work.',
> + default => 0,
> + optional => 1,
> + },
> + digest => get_standard_option('pve-config-digest'),
> + },
> + },
> + returns => { type => "string" },
> + code => sub {
> + my ($param) = @_;
> +
> + my $restenv = PMG::RESTEnvironment->get();
> +
> + my $user = $restenv->get_user();
> +
> + my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> + my $digest = PMG::SACustom::calc_digest($config);
> + PVE::Tools::assert_if_modified($digest, $param->{digest})
> + if $param->{digest};
> +
> + my $realcmd = sub {
> + my $upid = shift;
> +
> + PMG::SACustom::apply_changes();
> + if ($param->{'restart-daemon'}) {
> + syslog('info', "re-starting service pmg-smtp-filter: $upid\n");
> + PMG::Utils::service_cmd('pmg-smtp-filter', 'restart');
> + }
> + };
> +
> + return $restenv->fork_worker('applycustomscores', undef, $user, $realcmd);
> + }});
> +
> +__PACKAGE__->register_method({
> + name => 'revert_score_changes',
> + path => '',
> + method => 'DELETE',
> + protected => 1,
> + description => "Revert custom score changes.",
> + proxyto => 'master',
> + permissions => { check => [ 'admin' ] },
> + parameters => {
> + additionalProperties => 0,
> + properties => { },
> + },
> + returns => { type => "null" },
> + code => sub {
> + my ($param) = @_;
> +
> + unlink PMG::SACustom::get_shadow_path();
> +
> + return undef;
> + }});
> +
> +
> +__PACKAGE__->register_method({
> + name => 'create_score',
> + path => '',
> + method => 'POST',
> + description => "Create custom SpamAssassin score",
> + protected => 1,
> + proxyto => 'master',
> + parameters => {
> + additionalProperties => 0,
> + properties => json_config_properties({
> + digest => get_standard_option('pve-config-digest'),
> + }),
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $name = extract_param($param, 'name');
> + my $score = extract_param($param, 'score');
> + my $comment = extract_param($param, 'comment');
> +
> + my $code = sub {
> + my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> + my $digest = PMG::SACustom::calc_digest($config);
> + PVE::Tools::assert_if_modified($digest, $param->{digest})
> + if $param->{digest};
> +
> + $config->{$name} = {
> + name => $name,
> + score => $score,
> + comment => $comment,
> + };
> +
> + PVE::INotify::write_file('pmg-scores.cf', $config);
> + };
> +
> + PVE::Tools::lock_file("/var/lock/pmg-scores.cf.lck", 10, $code);
> + die $@ if $@;
> +
> + return undef;
> + }});
> +
> +__PACKAGE__->register_method({
> + name => 'get_score',
> + path => '{name}',
> + method => 'GET',
> + description => "Get custom SpamAssassin score",
> + protected => 1,
> + proxyto => 'master',
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + name => {
> + type => 'string',
> + description => "The name of the rule.",
> + pattern => '[a-zA-Z\_\-\.0-9]+',
> + },
> + },
> + },
> + returns => {
> + type => 'object',
> + properties => json_config_properties(),
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $name = extract_param($param, 'name');
> + my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> + raise_param_exc({ name => "$name not found" })
> + if !$config->{$name};
> +
> + return $config->{$name};
> + }});
> +
> +__PACKAGE__->register_method({
> + name => 'edit_score',
> + path => '{name}',
> + method => 'PUT',
> + description => "Edit custom SpamAssassin score",
> + protected => 1,
> + proxyto => 'master',
> + parameters => {
> + additionalProperties => 0,
> + properties => json_config_properties({
> + digest => get_standard_option('pve-config-digest'),
> + }),
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $name = extract_param($param, 'name');
> + my $score = extract_param($param, 'score');
> + my $comment = extract_param($param, 'comment');
> +
> + my $code = sub {
> + my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> + my $digest = PMG::SACustom::calc_digest($config);
> + PVE::Tools::assert_if_modified($digest, $param->{digest})
> + if $param->{digest};
> +
> + $config->{$name} = {
> + name => $name,
> + score => $score,
> + comment => $comment,
> + };
> +
> + PVE::INotify::write_file('pmg-scores.cf', $config);
> + };
> +
> + PVE::Tools::lock_file("/var/lock/pmg-scores.cf.lck", 10, $code);
> + die $@ if $@;
> +
> + return undef;
> + }});
> +
> +__PACKAGE__->register_method({
> + name => 'delete_score',
> + path => '{name}',
> + method => 'DELETE',
> + description => "Edit custom SpamAssassin score",
> + protected => 1,
> + proxyto => 'master',
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + name => {
> + type => 'string',
> + description => "The name of the rule.",
> + pattern => '[a-zA-Z\_\-\.0-9]+',
> + },
> + digest => get_standard_option('pve-config-digest'),
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $name = extract_param($param, 'name');
> +
> + my $code = sub {
> + my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> + my $digest = PMG::SACustom::calc_digest($config);
> + PVE::Tools::assert_if_modified($digest, $param->{digest})
> + if $param->{digest};
> +
> + delete $config->{$name};
> +
> + PVE::INotify::write_file('pmg-scores.cf', $config);
> + };
> +
> + PVE::Tools::lock_file("/var/lock/pmg-scores.cf.lck", 10, $code);
> + die $@ if $@;
> +
> + return undef;
> + }});
> +
> +1;
> diff --git a/src/PMG/SACustom.pm b/src/PMG/SACustom.pm
> new file mode 100644
> index 0000000..b7a8cc9
> --- /dev/null
> +++ b/src/PMG/SACustom.pm
> @@ -0,0 +1,89 @@
> +package PMG::SACustom;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::INotify;
> +use Digest::SHA;
> +
> +my $shadow_path = "/var/cache/pmg-scores.cf";
> +my $conf_path = "/etc/mail/spamassassin/pmg-scores.cf";
> +
> +sub get_shadow_path {
> + return $shadow_path;
> +}
> +
> +sub apply_changes {
> + rename($shadow_path, $conf_path) if -f $shadow_path;
> +}
> +
> +sub calc_digest {
> + my ($data) = @_;
> +
> + my $raw = '';
> +
> + foreach my $rule (sort keys %$data) {
> + my $score = $data->{$rule}->{score};
> + my $comment = $data->{$rule}->{comment} // "";
> + $raw .= "$rule$score$comment";
> + }
> +
> + my $digest = Digest::SHA::sha1_hex($raw);
> + return $digest;
> +}
> +
> +PVE::INotify::register_file('pmg-scores.cf', $conf_path,
> + \&read_pmg_cf,
> + \&write_pmg_cf,
> + undef,
> + always_call_parser => 1,
> + shadow => $shadow_path,
> + );
> +
> +sub read_pmg_cf {
> + my ($filename, $fh) = @_;
> +
> + my $scores = {};
> +
> + my $comment = '';
> + if (defined($fh)) {
> + while (defined(my $line = <$fh>)) {
> + chomp $line;
> + next if $line =~ m/^\s*$/;
> + if ($line =~ m/^# ?(.*)\s*$/) {
> + $comment = $1;
> + next;
> + }
> + if ($line =~ m/^score\s+(\S+)\s+(\S+)\s*$/) {
> + my $rule = $1;
> + my $score = $2;
> + $scores->{$rule} = {
> + name => $rule,
> + score => $score,
> + comment => $comment,
> + };
> + $comment = '';
> + } else {
> + warn "parse error in '$filename': $line\n";
> + $comment = '';
> + }
> + }
> + }
> +
> + return $scores;
> +}
> +
> +sub write_pmg_cf {
> + my ($filename, $fh, $scores) = @_;
> +
> + foreach my $rule (sort keys %$scores) {
> + my $comment = $scores->{$rule}->{comment};
> + my $score = sprintf("%.3f", $scores->{$rule}->{score});
> + PVE::Tools::safe_print($filename, $fh, "# $comment\n")
> + if defined($comment) && $comment !~ m/^\s*$/;
> +
> + PVE::Tools::safe_print($filename, $fh, "score $rule $score\n");
hm, while we have that pattern a few times in our codebase (afair the last
one proposed by me) we could get away with only one call to safe_print:
```
my $line = '';
$line .= "# $comment\n" if defined($comment) && $comment !~ m/^\s*$/;
$line .= "score $rule $score\n"
PVE::Tools::safe_print($filename, $fh, $line);
```
(or even write the whole data to a string and use file_set_contents)
not really relevant - just caught my eye - happy with the solution
as it is as well!
> + }
> +}
> +
> +1;
More information about the pmg-devel
mailing list