From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id F046F1FF164 for ; Fri, 4 Jul 2025 13:55:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 06B6836FC3; Fri, 4 Jul 2025 13:56:16 +0200 (CEST) Message-ID: Date: Fri, 4 Jul 2025 13:56:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Thomas Lamprecht , Proxmox Backup Server development discussion References: <20250703131837.786811-1-c.ebner@proxmox.com> <20250703131837.786811-2-c.ebner@proxmox.com> <383c895d-52ba-42f3-bf97-a73c017bcec3@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <383c895d-52ba-42f3-bf97-a73c017bcec3@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.040 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy 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 Subject: Re: [pbs-devel] [PATCH proxmox v5 1/3] pbs-api-types: add types for S3 client configs and secrets X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On 7/4/25 13:37, Thomas Lamprecht wrote: > 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 >> --- >> 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: Great, thanks! > Might it be better to decouple this from the PBS repo from the beginning? Yes, indeed moving the s3 client implementation to it's own independent crate makes sense. I only opted for it being included in the proxmox-backup crate as it is (still) limited in functionality and contains some helper methods which make mostly sense in the context of PBS. But since this can be easily extended, splitting it of now helps with compile time and enforces a clear logic separation from the more PBS specific logic. > 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. Okay, will move this to an independent single crate then for the next iteration of the patches. > 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. Agreed, will add this as well for the next version. _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel