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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 92B5075845 for ; Thu, 22 Apr 2021 08:20:02 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8649417810 for ; Thu, 22 Apr 2021 08:20:02 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 533A317804 for ; Thu, 22 Apr 2021 08:20:00 +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 21070425E5 for ; Thu, 22 Apr 2021 08:20:00 +0200 (CEST) Date: Thu, 22 Apr 2021 08:19:51 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion , Stefan Reiter References: <20210421111539.29261-1-s.reiter@proxmox.com> <20210421111539.29261-10-s.reiter@proxmox.com> <1619010196.oa6mrn2qjn.astroid@nora.none> In-Reply-To: MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1619071785.ey2usqrbcy.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.400 Adjusted score from AWL reputation of From: address KAM_ASCII_DIVIDERS 0.8 Spam that uses ascii formatting tricks 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [prunebackups.pm, config.pm, content.pm, proxmox.com, scan.pm, status.pm, filerestore.pm] Subject: Re: [pve-devel] [PATCH storage 09/10] add FileRestore API for PBS 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: Thu, 22 Apr 2021 06:20:02 -0000 On April 21, 2021 3:38 pm, Stefan Reiter wrote: > On 21/04/2021 15:26, Fabian Gr=C3=BCnbichler wrote: >> 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. >>=20 >> 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? >>=20 >=20 > sounds reasonable >=20 >>> >>> Signed-off-by: Stefan Reiter >>> --- >>> >>> 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/FileRes= tore.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 =3D> 'list', >>> + path =3D> 'list', >>> + method =3D> 'GET', >>> + proxyto =3D> 'node', >>> + permissions =3D> { >>> + description =3D> "Requires 'VM.Backup' permission on the VM being acc= essed, and " . >>> + "'Datastore.Audit' on the datastore being restored from.", >>> + user =3D> 'all', # checked explicitly >>> + }, >>> + description =3D> "List files and directories for single file resto= re under the given path.", >>> + protected =3D> 1, >>> + parameters =3D> { >>> + additionalProperties =3D> 0, >>> + properties =3D> { >>> + node =3D> get_standard_option('pve-node'), >>> + storage =3D> get_standard_option('pve-storage-id'), >>> + snapshot =3D> { >>> + description =3D> "Backup snapshot identifier.", >>> + type =3D> 'string', >>> + }, >>=20 >> 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, .. >>=20 >=20 > I did it this way mostly because we get the 'storage' parameter here=20 > anyway - it's in the URL path, since this lives under=20 > '/nodes/{node}/storage/{storage}'. Thus the only thing missing was the=20 > snapshot. hmm, yeah. there is precedent for using a volname/volid accepting=20 parameter in PVE::API2::Storage::Content though (complete with helper to=20 resolve the storage path parameter and the 'potentially contained in=20 volid' storage parameter), so maybe that might be an option as well?=20 might make the CLI easier, and potentially also API usage (no need to=20 have the "split volume id into parts" logic client-side as well then). > Is there a format for the "latter part of a volume-id"? If there is,=20 > this would also just be a simple change later on, as it'd just replace=20 > the 'snapshot' param. no, although it might be nice to make one ;) the volume id format is=20 specified at least as far as 'STORAGE_ID:VOLUME_NAME', but the volname=20 itself is rather "freeform" (as in, '(.+)' with the details left up to=20 the plugin :-/ - see PVE::Storage::Plugin::parse_volume_id). >=20 >>> + filepath =3D> { >>> + description =3D> 'base64-path to the directory or file being listed,= or "/".', >>> + type =3D> 'string', >>> + }, >>> + }, >>> + }, >>> + returns =3D> { >>> + type =3D> 'array', >>> + items =3D> { >>> + type =3D> "object", >>> + properties =3D> { >>> + filepath =3D> { >>> + description =3D> "base64 path of the current entry", >>> + type =3D> 'string', >>> + }, >>> + type =3D> { >>> + description =3D> "Entry type.", >>> + type =3D> 'string', >>> + }, >>> + text =3D> { >>> + description =3D> "Entry display text.", >>> + type =3D> 'string', >>> + }, >>> + leaf =3D> { >>> + description =3D> "If this entry is a leaf in the directory graph= .", >>> + type =3D> 'any', # JSON::PP::Boolean gets passed through >>> + }, >>> + size =3D> { >>> + description =3D> "Entry file size.", >>> + type =3D> 'integer', >>> + optional =3D> 1, >>> + }, >>> + mtime =3D> { >>> + description =3D> "Entry last-modified time (unix timestamp).", >>> + type =3D> 'integer', >>> + optional =3D> 1, >>> + }, >>> + }, >>> + }, >>> + }, >>> + code =3D> sub { >>> + my ($param) =3D @_; >>> + >>> + my $rpcenv =3D PVE::RPCEnvironment::get(); >>> + my $user =3D $rpcenv->get_user(); >>> + >>> + my $path =3D extract_param($param, 'filepath') || "/"; >>> + my $base64 =3D $path ne "/"; >>> + my $snap =3D extract_param($param, 'snapshot'); >>> + my $storeid =3D extract_param($param, 'storage'); >>> + my $cfg =3D PVE::Storage::config(); >>> + my $scfg =3D PVE::Storage::storage_config($cfg, $storeid); >>> + >>> + my $volid =3D "$storeid:backup/$snap"; >>> + my (undef, undef, $ownervm) =3D PVE::Storage::parse_volname($cfg, $vo= lid); >>> + $rpcenv->check($user, "/storage/$storeid", ['Datastore.Audit']); >>> + $rpcenv->check($user, "/vms/$ownervm", ['VM.Backup']); >>=20 >> see comment above, this could then become >> 'PVE::Storage::check_volume_access(..) >>> + >>> + my $client =3D PVE::PBSClient->new($scfg, $storeid); >>> + my $ret =3D $client->file_restore_list($snap, $path, $base64); >>> + >>> + return $ret; >>> + }}); >>> + >>> +__PACKAGE__->register_method ({ >>> + name =3D> 'download', >>> + path =3D> 'download', >>> + method =3D> 'GET', >>> + proxyto =3D> 'node', >>> + permissions =3D> { >>> + description =3D> "Requires 'VM.Backup' permission on the VM being acc= essed, and " . >>> + "'Datastore.Audit' on the datastore being restored from.", >>> + user =3D> 'all', # checked explicitly >>> + }, >>> + description =3D> "Extract a file or directory (as zip archive) fro= m a PBS backup.", >>> + parameters =3D> { >>> + additionalProperties =3D> 0, >>> + properties =3D> { >>> + node =3D> get_standard_option('pve-node'), >>> + storage =3D> get_standard_option('pve-storage-id'), >>> + snapshot =3D> { >>> + description =3D> "Backup snapshot identifier.", >>> + type =3D> 'string', >>> + }, >>=20 >> same here as above >>=20 >>> + filepath =3D> { >>> + description =3D> 'base64-path to the directory or file being listed.= ', >>> + type =3D> 'string', >>> + }, >>> + }, >>> + }, >>> + returns =3D> { >>> + type =3D> 'any', # download >>> + }, >>> + protected =3D> 1, >>> + code =3D> sub { >>> + my ($param) =3D @_; >>> + >>> + my $rpcenv =3D PVE::RPCEnvironment::get(); >>> + my $user =3D $rpcenv->get_user(); >>> + >>> + my $path =3D extract_param($param, 'filepath'); >>> + my $snap =3D extract_param($param, 'snapshot'); >>> + my $storeid =3D extract_param($param, 'storage'); >>> + my $cfg =3D PVE::Storage::config(); >>> + my $scfg =3D PVE::Storage::storage_config($cfg, $storeid); >>> + >>> + my $volid =3D "$storeid:backup/$snap"; >>> + my (undef, undef, $ownervm) =3D PVE::Storage::parse_volname($cfg, $vo= lid); >>> + $rpcenv->check($user, "/storage/$storeid", ['Datastore.Audit']); >>> + $rpcenv->check($user, "/vms/$ownervm", ['VM.Backup']); >>=20 >> and here as well >>=20 >>> + >>> + my $client =3D PVE::PBSClient->new($scfg, $storeid); >>> + my $fifo =3D $client->file_restore_extract_prepare(); >>> + >>> + $rpcenv->fork_worker('pbs-download', undef, $user, sub { >>> + $client->file_restore_extract($fifo, $snap, $path, 1); >>> + }); >>> + >>> + my $ret =3D { >>> + download =3D> { >>> + path =3D> $fifo, >>> + stream =3D> 1, >>> + 'content-type' =3D> '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 @@ >>> =20 >>> -SOURCES=3D Content.pm Status.pm Config.pm PruneBackups.pm Scan.pm >>> +SOURCES=3D Content.pm Status.pm Config.pm PruneBackups.pm Scan.pm File= Restore.pm >>> =20 >>> .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 =3D> '{storage}/content', >>> }); >>> =20 >>> +__PACKAGE__->register_method ({ >>> + subclass =3D> "PVE::API2::Storage::FileRestore", >>> + path =3D> '{storage}/file-restore', >>> +}); >>> + >>> __PACKAGE__->register_method ({ >>> name =3D> 'index', >>> path =3D> '', >>> --=20 >>> 2.20.1 >>> >>> >>> >>> _______________________________________________ >>> pve-devel mailing list >>> pve-devel@lists.proxmox.com >>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >>> >>> >>> >>=20 >>=20 >> _______________________________________________ >> pve-devel mailing list >> pve-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >>=20 >>=20 >=20 =