[pve-devel] [PATCH manager v8 02/13] api: jobs: vzdump: pass job 'job-id' parameter

[Lukas Wagner]

This allows us to access us the backup job id in the send_notification
function, where we can set it as metadata for the notification.
The 'job-id' parameter can only be used by 'root at pam' to prevent
abuse. This has the side effect that manually triggered backup jobs
cannot have the 'job-id' parameter at the moment. To mitigate that,
manually triggered backup jobs could be changed so that they
are not performed by a direct API call by the UI, but by requesting
pvescheduler to execute the job in the near future (similar to how
manually triggered replication jobs work).

Signed-off-by: Lukas Wagner <l.wagner at proxmox.com>
---
 PVE/API2/Backup.pm |  2 +-
 PVE/API2/VZDump.pm | 13 +++++++++++--
 PVE/Jobs/VZDump.pm |  4 +++-
 PVE/VZDump.pm      |  6 +++---
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 88140323..48598b8f 100644
--- a/PVE/API2/Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -45,7 +45,7 @@ sub assert_param_permission_common {
     my ($rpcenv, $user, $param, $is_delete) = @_;
     return if $user eq 'root at pam'; # always OK
 
-    for my $key (qw(tmpdir dumpdir script)) {
+    for my $key (qw(tmpdir dumpdir script job-id)) {
 	raise_param_exc({ $key => "Only root may set this option."}) if exists $param->{$key};
     }
 
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 7f92e7ec..15c9b0dc 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -42,8 +42,8 @@ __PACKAGE__->register_method ({
     permissions => {
 	description => "The user needs 'VM.Backup' permissions on any VM, and "
 	    ."'Datastore.AllocateSpace' on the backup storage (and fleecing storage when fleecing "
-	    ."is used). The 'tmpdir', 'dumpdir' and 'script' parameters are restricted to the "
-	    ."'root\@pam' user. The 'maxfiles' and 'prune-backups' settings require "
+	    ."is used). The 'tmpdir', 'dumpdir', 'script' and 'job-id' parameters are restricted "
+	    ."to the 'root\@pam' user. The 'maxfiles' and 'prune-backups' settings require "
 	    ."'Datastore.Allocate' on the backup storage. The 'bwlimit', 'performance' and "
 	    ."'ionice' parameters require 'Sys.Modify' on '/'.",
 	user => 'all',
@@ -53,6 +53,15 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => PVE::VZDump::Common::json_config_properties({
+	    'job-id' => {
+		description => "The ID of the backup job. If set, the 'backup-job' metadata field"
+		    . " of the backup notification will be set to this value. Only root\@pam"
+		    . " can set this parameter.",
+		type => 'string',
+		format => 'pve-configid',
+		maxLength => 256,
+		optional => 1,
+	    },
 	    stdout => {
 		type => 'boolean',
 		description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index b8e57945..2dad3f55 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
 use base qw(PVE::VZDump::JobBase);
 
 sub run {
-    my ($class, $conf) = @_;
+    my ($class, $conf, $job_id) = @_;
 
     my $props = $class->properties();
     # remove all non vzdump related options
@@ -20,6 +20,8 @@ sub run {
 	delete $conf->{$opt} if !defined($props->{$opt});
     }
 
+    $conf->{'job-id'} = $job_id;
+
     # Required as string parameters # FIXME why?! we could just check ref()
     for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
 	if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 8dbcc4a9..f1a6b220 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -483,6 +483,7 @@ sub send_notification {
     my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
 
     my $opts = $self->{opts};
+    my $job_id = $opts->{'job-id'};
     my $mailto = $opts->{mailto};
     my $cmdline = $self->{cmdline};
     my $policy = $opts->{mailnotification} // 'always';
@@ -528,13 +529,12 @@ sub send_notification {
     };
 
     my $fields = {
-	# TODO: There is no straight-forward way yet to get the
-	# backup job id here... (I think pvescheduler would need
-	# to pass that to the vzdump call?)
 	type => "vzdump",
 	# Hostname (without domain part)
 	hostname => PVE::INotify::nodename(),
     };
+    # Add backup-job metadata field in case this is a backup job.
+    $fields->{'job-id'} = $job_id if $job_id;
 
     my $severity = $failed ? "error" : "info";
     my $email_configured = $mailto && scalar(@$mailto);
-- 
2.39.2