Re: [PATCH pve-storage 7/7] api: add /nodes/<node>/storage/<storage>/instance-id route

["Lukas Wagner" <l.wagner@proxmox.com>]

On Tue Apr 14, 2026 at 4:38 PM CEST, Christian Ebner wrote:
> On 4/13/26 2:11 PM, Lukas Wagner wrote:
>> This API allows us to retrieve the instance ID of PBS storages. This is
>> useful to establish a mapping between PBS storage and PBS remotes in
>> PDM.
>> 
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>> 
>> Notes:
>>      Maybe we want to make this more general and just call it /id or
>>      something, in case we want to use a similar mechanism for other storage
>>      types as well?
>>      
>>      Also could make sense to cache the response.
>> 
>>   src/PVE/API2/Storage/Status.pm | 50 +++++++++++++++++++++++++++++++++-
>>   1 file changed, 49 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
>> index 8225c3a..52aa673 100644
>> --- a/src/PVE/API2/Storage/Status.pm
>> +++ b/src/PVE/API2/Storage/Status.pm
>> @@ -14,7 +14,7 @@ use PVE::JSONSchema qw(get_standard_option);
>>   use PVE::RESTHandler;
>>   use PVE::RPCEnvironment;
>>   use PVE::RRD;
>> -use PVE::Tools qw(run_command);
>> +use PVE::Tools qw(run_command extract_param);
>>   
>>   use PVE::API2::Storage::Content;
>>   use PVE::API2::Storage::FileRestore;
>> @@ -308,6 +308,7 @@ __PACKAGE__->register_method({
>>               { subdir => 'download-url' },
>>               { subdir => 'file-restore' },
>>               { subdir => 'import-metadata' },
>> +            { subdir => 'instance-id' },
>>               { subdir => 'oci-registry-pull' },
>>               { subdir => 'prunebackups' },
>>               { subdir => 'rrd' },
>> @@ -1118,4 +1119,51 @@ __PACKAGE__->register_method({
>>       },
>>   });
>>   
>> +__PACKAGE__->register_method({
>> +    name => 'instance_id',
>> +    path => '{storage}/instance-id',
>> +    method => 'GET',
>> +    description => "Return unique storage instance ID (PBS only)",
>> +    permissions => {
>> +        check => ['perm', '/storage/{storage}', ['Datastore.Audit']],
>> +    },
>> +    protected => 1,
>> +    proxyto => 'node',
>> +    parameters => {
>> +        additionalProperties => 0,
>> +        properties => {
>> +            node => get_standard_option('pve-node'),
>> +            storage => get_standard_option('pve-storage-id'),
>> +        },
>> +    },
>> +    returns => {
>> +        type => "object",
>> +        properties => {
>> +            'instance-id' => {
>> +                type => 'string',
>> +                description => 'Unique instance ID',
>> +            },

With the changes described below, I'd 
  - rename the property to 'pbs-instance-id'
  - make it optional (as a preparation for other storages that could
    have some 'identity info' that would be stored in a different key)

>> +        },
>> +    },
>> +    code => sub {
>> +        my ($param) = @_;
>> +
>> +        my $rpcenv = PVE::RPCEnvironment::get();
>> +        my $user = $rpcenv->get_user();
>> +
>> +        my $storeid = extract_param($param, 'storage');
>> +
>> +        my $cfg = PVE::Storage::config();
>> +        my $scfg = PVE::Storage::storage_config($cfg, $storeid);
>> +
>> +        raise_param_exc({ 'storage' => "Only PBS storages supported." })
>> +            if $scfg->{type} ne 'pbs';
>> +
>> +        my $client = PVE::PBSClient->new($scfg, $storeid);
>> +        my $ret = $client->get_instance_id();
>
> nit: same as on previous patches, the client part should be called 
> get_pbs_instance_id() to reflect that this stems from the server.
>
> I do agree however that this API endpoint name for PVE should be chosen 
> as generic as possible for better future extensibility.
>

I think I'm going to rename the API path to `identity`, as this seems to
be generic enough to be useful for other storages as well. Maybe the
method in PBSClient could then be called `get_server_identity`?

What do you think?

>> +
>> +        return $ret;
>> +    },
>> +});
>> +
>>   1;