From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 5A6CC91C3C for ; Mon, 27 Mar 2023 17:19:52 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 94847D4F7 for ; Mon, 27 Mar 2023 17:19:25 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 27 Mar 2023 17:19:24 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 7E65B46FF9 for ; Mon, 27 Mar 2023 17:19:23 +0200 (CEST) From: Lukas Wagner To: pve-devel@lists.proxmox.com Date: Mon, 27 Mar 2023 17:18:51 +0200 Message-Id: <20230327151857.495565-13-l.wagner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230327151857.495565-1-l.wagner@proxmox.com> References: <20230327151857.495565-1-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.168 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH pve-manager 12/18] vzdump: send notifications via new notification module X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Mar 2023 15:19:52 -0000 ... instead of using sendmail directly. If the 'mailto' property is set set for a backup, an ephemeral notification endpoint of type 'sendmail' will be added to the notification backend. Also, a fitting notification filter will be added, with the 'min-severity' property set to either 'info' or 'error', depending on whether 'notify always' or 'on failure only' is set for a backup job. This approach allows us to add the new notification backend without breaking any existing mail notifications. The notification backend only supports plain text for message bodies, thus the HTML table generation has been removed. Signed-off-by: Lukas Wagner --- PVE/VZDump.pm | 116 +++++++++++++--------------------------------- test/mail_test.pl | 22 +++++---- 2 files changed, 44 insertions(+), 94 deletions(-) diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm index a04837e7..c0971220 100644 --- a/PVE/VZDump.pm +++ b/PVE/VZDump.pm @@ -19,6 +19,7 @@ use PVE::Exception qw(raise_param_exc); use PVE::HA::Config; use PVE::HA::Env::PVE2; use PVE::JSONSchema qw(get_standard_option); +use PVE::Notification; use PVE::RPCEnvironment; use PVE::Storage; use PVE::VZDump::Common; @@ -308,7 +309,7 @@ sub sendmail { my $mailto = $opts->{mailto}; - return if !($mailto && scalar(@$mailto)); + # return if !($mailto && scalar(@$mailto)); my $cmdline = $self->{cmdline}; @@ -326,10 +327,10 @@ sub sendmail { } } - my $notify = $opts->{mailnotification} || 'always'; - return if (!$ecount && !$err && ($notify eq 'failure')); + my $failed = ($ecount || $err); + + my $stat = $failed ? 'backup failed' : 'backup successful'; - my $stat = ($ecount || $err) ? 'backup failed' : 'backup successful'; if ($err) { if ($err =~ /\n/) { $stat .= ": multiple problems"; @@ -391,90 +392,13 @@ sub sendmail { } $text_log_part .= $detail_post if defined($detail_post); - # html part - my $html = "\n"; - $html .= "

" . (escape_html($err) =~ s/\n/
/gr) . "

\n" if $err; - $html .= "\n"; - $html .= "\n"; - - my $ssize = 0; - foreach my $task (@$tasklist) { - my $vmid = $task->{vmid}; - my $name = $task->{hostname}; - - if ($task->{state} eq 'ok') { - $ssize += $task->{size}; - - $html .= sprintf ( - "\n", - $vmid, - $name, - format_time($task->{backuptime}), - format_size ($task->{size}), - escape_html ($task->{target}), - ); - } else { - $html .= sprintf ( - "\n", - $vmid, - $name, - format_time($task->{backuptime}), - escape_html ($task->{msg}), - ); - } - } - - $html .= sprintf ("", - format_time ($totaltime), format_size ($ssize)); - - $html .= "\n
VMIDNAMESTATUSTIMESIZEFILENAME
%s%sOK%s%s%s
%s%sFAILED%s%s
TOTAL%s%s


\n"; - my $html_log_part; - $html_log_part .= "Detailed backup logs:

\n"; - $html_log_part .= "
\n";
-    $html_log_part .= escape_html($cmdline) . "\n\n";
-
-    $html_log_part .= escape_html($detail_pre) . "\n" if defined($detail_pre);
-    foreach my $task (@$tasklist) {
-	my $vmid = $task->{vmid};
-	my $log = $task->{tmplog};
-	if (!$log) {
-	    $html_log_part .= "$vmid: no log available\n\n";
-	    next;
-	}
-	if (open (my $TMP, '<', "$log")) {
-	    while (my $line = <$TMP>) {
-		next if $line =~ /^status: \d+/; # not useful in mails
-		if ($line =~ m/^\S+\s\d+\s+\d+:\d+:\d+\s+(ERROR|WARN):/) {
-		    $html_log_part .= encode8bit ("$vmid: ".
-			escape_html ($line) . "");
-		} else {
-		    $html_log_part .= encode8bit ("$vmid: " . escape_html ($line));
-		}
-	    }
-	    close ($TMP);
-	} else {
-	    $html_log_part .= "$vmid: Could not open log file\n\n";
-	}
-	$html_log_part .= "\n";
-    }
-    $html_log_part .= escape_html($detail_post) if defined($detail_post);
-    $html_log_part .= "
"; - my $html_end = "\n\n"; - # end html part - - if (length($text) + length($text_log_part) + - length($html) + length($html_log_part) + - length($html_end) < MAX_MAIL_SIZE) + if (length($text) + length($text_log_part) < MAX_MAIL_SIZE) { - $html .= $html_log_part; - $html .= $html_end; $text .= $text_log_part; } else { - my $msg = "Log output was too long to be sent by mail. ". + my $msg = "Log output was too long to be sent. ". "See Task History for details!\n"; $text .= $msg; - $html .= "

$msg

"; - $html .= $html_end; } my $subject = "vzdump backup status ($hostname) : $stat"; @@ -482,7 +406,31 @@ sub sendmail { my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg'); my $mailfrom = $dcconf->{email_from} || "root"; - PVE::Tools::sendmail($mailto, $subject, $text, $html, $mailfrom, "vzdump backup tool"); + + my $notify = $opts->{mailnotification} || 'always'; + my $min_severity = $notify eq "failure" ? "error" : "info"; + + my $config = PVE::Notification::config(); + + if ($mailto && scalar(@$mailto)) { + # add ephemeral sendmail endpoint for the existing vzdump-job, + # in order to not break existing mail notifications. The changed + # config is not persisted. + # TODO: Remove in PVE x.y ? + $config->add_filter("legacy-vzdump-sendmail-filter", $min_severity); + $config->add_sendmail_endpoint( + "legacy-vzdump-sendmail", + $mailto, + $mailfrom, + "vzdump backup tool", + "legacy-vzdump-sendmail-filter"); + } + + if ($failed) { + PVE::Notification::error($subject, $text, $config); + } else { + PVE::Notification::info($subject, $text, $config); + } }; sub new { diff --git a/test/mail_test.pl b/test/mail_test.pl index d0114441..c77c1aae 100755 --- a/test/mail_test.pl +++ b/test/mail_test.pl @@ -5,7 +5,7 @@ use warnings; use lib '..'; -use Test::More tests => 5; +use Test::More tests => 3; use Test::MockModule; use PVE::VZDump; @@ -29,18 +29,22 @@ sub prepare_mail_with_status { sub prepare_long_mail { open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content # 0.5 MB * 2 parts + the overview tables gives more than 1 MB mail - print TEST_FILE "a" x (1024*1024/2); + print TEST_FILE "a" x (1024*1024); close(TEST_FILE); } -my ($result_text, $result_html); +my $result_text; -my $mock_tools_module = Test::MockModule->new('PVE::Tools'); -$mock_tools_module->mock('sendmail', sub { - my (undef, undef, $text, $html, undef, undef) = @_; +my $mock_notification_module = Test::MockModule->new('PVE::Notification'); +$mock_notification_module->mock('send_notification', sub { + my (undef, undef, $text, undef, undef) = @_; $result_text = $text; - $result_html = $html; }); +$mock_notification_module->mock('config', sub { + # Return empty config + return Proxmox::RS::Notification->new(""); +}); + my $mock_cluster_module = Test::MockModule->new('PVE::Cluster'); $mock_cluster_module->mock('cfs_read_file', sub { @@ -62,7 +66,7 @@ my $SELF = { my $task = { state => 'ok', vmid => '100', }; my $tasklist; sub prepare_test { - $result_text = $result_html = undef; + $result_text = undef; $task->{tmplog} = shift; $tasklist = [ $task ]; } @@ -77,13 +81,11 @@ sub prepare_test { prepare_mail_with_status(); PVE::VZDump::sendmail($SELF, $tasklist, 0, undef, undef, undef); unlike($result_text, $STATUS, "Status are not in text part of mails"); - unlike($result_html, $STATUS, "Status are not in HTML part of mails"); } { prepare_test($TEST_FILE_PATH); prepare_long_mail(); PVE::VZDump::sendmail($SELF, $tasklist, 0, undef, undef, undef); like($result_text, $LOG_TOO_LONG, "Text part of mails gets shortened"); - like($result_html, $LOG_TOO_LONG, "HTML part of mails gets shortened"); } unlink $TEST_FILE_PATH; -- 2.30.2