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

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

Am 23.04.26 um 11:24 schrieb Lukas Wagner:
> One last point, but maybe I'm overthinking this:
> 
> To me, the { type: "<plugin>", id: "<string" } approach kind of implies that
> this ID would uniquely identify the PBS instance *as well as* the
> datastore/namespace, which it does not at the moment.
> 
> Would you think that it would make sense for PBS storage plugin to
> maybe combine the datastore, namespace and the
> pbs-instance-id into a single identifier here?
> 
> Maybe something like, in pseudo-regex
> 
> <datastore>(/<namespace>)?@<pbs-instance-id>
> 
> e.g. store/some/name/space@dade89fe26104bc3858ae719c23594a7
> 
> This would then actually uniquely identify the PBS storage (as in, two
> storages with the same ID should always point to the same content),
> while still allowing PDM to parse the identifier to retrieve the
> pbs-instance-id, if needed.

Fair point but I'd keep `id` scoped to the server instance though,
because:

- The pve-storage entry already pins datastore + namespace via
  storage.cfg, so a caller that knows which pve-storage it queried has
  those two from local context. Callers doing content-level matching
  anyway start from a specific (datastore, namespace) target, so once
  the instance ids line up the rest just falls out of the storage (or
  e.g. PBS remote) config. Baking datastore + namespace into `id` mostly
  duplicates what the caller already has and I'm not really a of such
  packed strings (reminds me of PBS' backup repo syntax I recently split
  up for better UX, albeit here it at least wouldn't be handled by the
  user directly)
- Content-location identity as a general plugin contract is also
  awkward: not every storage type decomposes into a "server + container
  + sub-path" shape.

So the contract should IMO rather stay "`id` identifies the server/service
instance", which matches PDM's verification use. If auxiliary fields turn
out to be useful later, `{ type, id, ... }` extends additively without
breaking anyone, and I'd have nothing against adding datastore and
namespace as extra (nested) fields there if you strongly prefer that and
it really helps, but I would prefer to not "cram" it in the (instance) id.