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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 22.04.26 um 09:47 schrieb Lukas Wagner:
> So in summary, as of now, we have
>   - the storage API route is called `identity` and the response contains
>     `pbs-instance-id`
>   - the PBSClient method is called `get_server_identity`, returning a
>     hash that contains a `pbs-instance-id`
>   - in PBS, the route is `/nodes/localhost/server-identity` and the
>     response contains a `pbs-instance-id` field
>   - the PBS client binary has a `server-identity` sub-command
> 
> In your reply you stated a preference for `instance-id`, but also I did
> not understand it as concrete instructions for the next version. With my
> (hopefully) more elaborated reasoning above, is there any one of those
> that you'd like me change before this gets applied (and if so, what
> exactly should they be named?) -- just to avoid any misunderstandings
> and churn for the next version of this series.

OK, to close this out with something concrete instead of all to much
naming bike-shed back-and-forth:

First, and largely separate from the naming: FWICT it really wouldn't
be much extra work to wire up a proper plugin method in pve-storage
right away. Bump APIVER + APIAGE in PVE::Storage, add a get_identity()
(or similar) plugin method with a default impl that dies or returns
undef, and have PBSPlugin override it.
Here we then just call `$plugin->get_identity(...)` and the
`if $scfg->{type} ne 'pbs'` special-case can be dropped.
You wouldn't have to implement the method on any non-PBS plugin for
now though; the main point is that the API surface isn't hard-wired
to `pbs` at the API endpoint level any more, so opting another type in
later mostly needs implementing the method in the respective plugin.

With that in place IMO the response properties should be generic too,
otherwise the plugin-method is a bit odd combined with using
`pbs-instance-id` here, one option might be a sort of oneOf schema,
but I'd actually rather go with an sorta "adjacently-tagged" (like
serde would call it) `{ type: "<plugin>", id: "<string>" }` from the
endpoint.
PDM (or other clients) can then reconstruct and, if needed, also
correctly verify the full identifier itself.

The `pbs-instance-id` field name can stay PBS-side (ServerIdentity
struct + PBS API return), where it's PBS-specific by construction, it
just shouldn't sneak into the generic pve-storage endpoint (but in case
you strongly favor that: I'd not be totally opposed to oneOf).

While this has IMO no objective true answer, the consolidated naming
I'd use, to make my reply a bit less ambiguous:

- PBS API: `/nodes/<node>/identity` - drop `server-`, reasoning: we're
  on the server, the qualifier is redundant there.
- PBS client CLI + Perl PBSClient: keep `server-identity` /
  `get_server_identity`, client-side the qualifier is worth it to
  correctly relay what this identifies.
- pve-storage API: keep bare `identity`, adding server isn't true for
  all storage types after all and I do not find instance-id that much
  better to really request it, especially given that others found the
  "identity" variant better I'm fine with it.
- pve-storage response: `{ type, id }` via the new plugin method.
- PBS-internal `ServerIdentity { pbs_instance_id }`: unchanged.

btw. CC'ing Max, in case this overlaps with his storage UI/schema work -
definitively not a blocker here, just so it's on his radar.