Re: [pve-devel] [PATCH storage 09/10] add FileRestore API for PBS

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On April 21, 2021 1:15 pm, Stefan Reiter wrote:
> Includes list and restore calls.
> 
> Requires VM.Backup and Datastore.Audit permissions, for the accessed
> VM/CT and containing datastore respectively.

we require Datastore.AllocateSpace + VM.Backup for the owning guest, 
or Datastore.Allocate for the storage altogether for accessing backup 
archives otherwise, maybe this should have the same logic?

> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> Requires updated pve-common, pve-http-server.
> 
>  PVE/API2/Storage/FileRestore.pm | 163 ++++++++++++++++++++++++++++++++
>  PVE/API2/Storage/Makefile       |   2 +-
>  PVE/API2/Storage/Status.pm      |   6 ++
>  3 files changed, 170 insertions(+), 1 deletion(-)
>  create mode 100644 PVE/API2/Storage/FileRestore.pm
> 
> diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
> new file mode 100644
> index 0000000..a0b5e88
> --- /dev/null
> +++ b/PVE/API2/Storage/FileRestore.pm
> @@ -0,0 +1,163 @@
> +package PVE::API2::Storage::FileRestore;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::PBSClient;
> +use PVE::Storage;
> +use PVE::Tools qw(extract_param);
> +
> +use PVE::RESTHandler;
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> +    name => 'list',
> +    path => 'list',
> +    method => 'GET',
> +    proxyto => 'node',
> +    permissions => {
> +	description => "Requires 'VM.Backup' permission on the VM being accessed, and " .
> +	    "'Datastore.Audit' on the datastore being restored from.",
> +	user => 'all', # checked explicitly
> +    },
> +    description => "List files and directories for single file restore under the given path.",
> +    protected => 1,
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    storage => get_standard_option('pve-storage-id'),
> +	    snapshot => {
> +		description => "Backup snapshot identifier.",
> +		type => 'string',
> +	    },

why not use a volume id here (instead of storage + snapshot ID), and 
then check inside whether it's a pbs backup? would allow easily 
extending this to VMA backups as well later on, completion by our usual 
volume ID helpers/selectors, ..

> +	    filepath => {
> +		description => 'base64-path to the directory or file being listed, or "/".',
> +		type => 'string',
> +	    },
> +	},
> +    },
> +    returns => {
> +	type => 'array',
> +	items => {
> +	    type => "object",
> +	    properties => {
> +		filepath => {
> +		    description => "base64 path of the current entry",
> +		    type => 'string',
> +		},
> +		type => {
> +		    description => "Entry type.",
> +		    type => 'string',
> +		},
> +		text => {
> +		    description => "Entry display text.",
> +		    type => 'string',
> +		},
> +		leaf => {
> +		    description => "If this entry is a leaf in the directory graph.",
> +		    type => 'any', # JSON::PP::Boolean gets passed through
> +		},
> +		size => {
> +		    description => "Entry file size.",
> +		    type => 'integer',
> +		    optional => 1,
> +		},
> +		mtime => {
> +		    description => "Entry last-modified time (unix timestamp).",
> +		    type => 'integer',
> +		    optional => 1,
> +		},
> +	    },
> +	},
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $user = $rpcenv->get_user();
> +
> +	my $path = extract_param($param, 'filepath') || "/";
> +	my $base64 = $path ne "/";
> +	my $snap = extract_param($param, 'snapshot');
> +	my $storeid = extract_param($param, 'storage');
> +	my $cfg = PVE::Storage::config();
> +	my $scfg = PVE::Storage::storage_config($cfg, $storeid);
> +
> +	my $volid = "$storeid:backup/$snap";
> +	my (undef, undef, $ownervm) = PVE::Storage::parse_volname($cfg, $volid);
> +	$rpcenv->check($user, "/storage/$storeid", ['Datastore.Audit']);
> +	$rpcenv->check($user, "/vms/$ownervm", ['VM.Backup']);

see comment above, this could then become 
'PVE::Storage::check_volume_access(..)
> +
> +	my $client = PVE::PBSClient->new($scfg, $storeid);
> +	my $ret = $client->file_restore_list($snap, $path, $base64);
> +
> +	return $ret;
> +    }});
> +
> +__PACKAGE__->register_method ({
> +    name => 'download',
> +    path => 'download',
> +    method => 'GET',
> +    proxyto => 'node',
> +    permissions => {
> +	description => "Requires 'VM.Backup' permission on the VM being accessed, and " .
> +	    "'Datastore.Audit' on the datastore being restored from.",
> +	user => 'all', # checked explicitly
> +    },
> +    description => "Extract a file or directory (as zip archive) from a PBS backup.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    storage => get_standard_option('pve-storage-id'),
> +	    snapshot => {
> +		description => "Backup snapshot identifier.",
> +		type => 'string',
> +	    },

same here as above

> +	    filepath => {
> +		description => 'base64-path to the directory or file being listed.',
> +		type => 'string',
> +	    },
> +	},
> +    },
> +    returns => {
> +	type => 'any', # download
> +    },
> +    protected => 1,
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $user = $rpcenv->get_user();
> +
> +	my $path = extract_param($param, 'filepath');
> +	my $snap = extract_param($param, 'snapshot');
> +	my $storeid = extract_param($param, 'storage');
> +	my $cfg = PVE::Storage::config();
> +	my $scfg = PVE::Storage::storage_config($cfg, $storeid);
> +
> +	my $volid = "$storeid:backup/$snap";
> +	my (undef, undef, $ownervm) = PVE::Storage::parse_volname($cfg, $volid);
> +	$rpcenv->check($user, "/storage/$storeid", ['Datastore.Audit']);
> +	$rpcenv->check($user, "/vms/$ownervm", ['VM.Backup']);

and here as well

> +
> +	my $client = PVE::PBSClient->new($scfg, $storeid);
> +	my $fifo = $client->file_restore_extract_prepare();
> +
> +	$rpcenv->fork_worker('pbs-download', undef, $user, sub {
> +	    $client->file_restore_extract($fifo, $snap, $path, 1);
> +	});
> +
> +	my $ret = {
> +	    download => {
> +		path => $fifo,
> +		stream => 1,
> +		'content-type' => 'application/octet-stream',
> +	    },
> +	};
> +	return $ret;
> +    }});
> +
> +1;
> diff --git a/PVE/API2/Storage/Makefile b/PVE/API2/Storage/Makefile
> index 690b437..1705080 100644
> --- a/PVE/API2/Storage/Makefile
> +++ b/PVE/API2/Storage/Makefile
> @@ -1,5 +1,5 @@
>  
> -SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm Scan.pm
> +SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm Scan.pm FileRestore.pm
>  
>  .PHONY: install
>  install:
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index d12643f..897b4a7 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -12,6 +12,7 @@ use PVE::RRD;
>  use PVE::Storage;
>  use PVE::API2::Storage::Content;
>  use PVE::API2::Storage::PruneBackups;
> +use PVE::API2::Storage::FileRestore;
>  use PVE::RESTHandler;
>  use PVE::RPCEnvironment;
>  use PVE::JSONSchema qw(get_standard_option);
> @@ -32,6 +33,11 @@ __PACKAGE__->register_method ({
>      path => '{storage}/content',
>  });
>  
> +__PACKAGE__->register_method ({
> +   subclass => "PVE::API2::Storage::FileRestore",
> +   path => '{storage}/file-restore',
> +});
> +
>  __PACKAGE__->register_method ({
>      name => 'index',
>      path => '',
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>