From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <f.gruenbichler@proxmox.com>
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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Stefan Reiter <s.reiter@proxmox.com>
References: <20210421111539.29261-1-s.reiter@proxmox.com>
<20210421111539.29261-10-s.reiter@proxmox.com>
<1619010196.oa6mrn2qjn.astroid@nora.none>
<fd73a3a0-4d16-7ab9-ed7c-551d7bd6e295@proxmox.com>
In-Reply-To: <fd73a3a0-4d16-7ab9-ed7c-551d7bd6e295@proxmox.com>
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 <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=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 <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/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
=