Re: [pbs-devel] [PATCH proxmox v5 1/3] pbs-api-types: add types for S3 client configs and secrets

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

Am 03.07.25 um 15:17 schrieb Christian Ebner:
> Adds the new config types `S3ClientConfig` and `S3ClientSecret` to
> configure datastore backends using an S3 compatible object store.
> 
> Secrets are stored as different config to never be returned on api
> calls, only allowing to set/update the values.
> 
> Use a different name (`secrets_id`) for the unique identifier in case
> of the secrets type, although the same id should be used for storing
> and lookup. By this, clashing of property names when using flattened
> types as api parameters is avoided.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  pbs-api-types/src/lib.rs |   3 +
>  pbs-api-types/src/s3.rs  | 161 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 164 insertions(+)
>  create mode 100644 pbs-api-types/src/s3.rs
> 
Seeing some good work here, I'll try to give some core design stuff
a closer look over the next days, for now just a high level comment for
this and the pbs-s3-client:

Might it be better to decouple this from the PBS repo from the beginning?

As AFAICT, this is all quite generic, and S3 is rather set in stone, or at
least externally controlled. Further, the S3 client crate seems to purely
use the new S3ClientConfig and S3ClientSecretsConfig types added here from
the PBS API types crate, so it would be quite self-contained already.

Even if I do not see any other use site of this crate on the horizon,
object storage has become quite common and especially with things like
the OCI application containers I could imagine the need for pulling stuff
from an object storage; and IMO reuse is just one of the reason for a
separate crate, even if not reused such a contained library often profits
from being a separate crate, and splitting it out later is most of the time
more work.

We could either create a proxmox-s3-client and a proxmox-s3-client-api-types
crate, or have both in a single proxmox-s3-client crate where the actual
implementation is only enabled with an impl feature, i.e. similar to the
proxmox-dns-api crate.


Besides this code organization point I'd welcome having a small example
added to the s3-client crate, while some unit tests would be also nice,
having a simple example with the core methods exposed that one can point
to an arbitrary s3 storage might help not only testing this on review but
also for the longer term, e.g. when debugging issues with some specific
providers that promise s3 compatible object store but surely have some
oddities.



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel