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 0F4161FF187 for ; Mon, 28 Jul 2025 14:40:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2E25A32275; Mon, 28 Jul 2025 14:41:58 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Mon, 28 Jul 2025 14:40:05 +0200 Message-ID: <20250728124006.868999-7-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20250728124006.868999-1-c.ebner@proxmox.com> References: <20250728124006.868999-1-c.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1753706473720 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 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: [pbs-devel] [PATCH proxmox-backup 6/7] datastore: add helper to get s3 client from datastore config 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" Refactors the currently duplicate code to get the s3 client based on a datastore config into a dedicated helper associated function of the datastore. This can then further be used to get the s3 client for datastore deletion when all data are requested to be removed. Signed-off-by: Christian Ebner --- pbs-datastore/src/datastore.rs | 34 ++++++++++ src/api2/config/datastore.rs | 119 ++++++++++----------------------- 2 files changed, 71 insertions(+), 82 deletions(-) diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index a6849ecf4..3bc3aab0d 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -2382,4 +2382,38 @@ impl DataStore { Ok(()) } + + pub fn s3_client_and_backend_from_datastore_config( + datastore_config: &DataStoreConfig, + ) -> Result<(DatastoreBackendType, Option), Error> { + let backend_config: DatastoreBackendConfig = + datastore_config.backend.as_deref().unwrap_or("").parse()?; + let backend_type = backend_config.ty.unwrap_or_default(); + + if backend_type != DatastoreBackendType::S3 { + return Ok((backend_type, None)); + } + + let s3_client_id = backend_config + .client + .as_ref() + .ok_or_else(|| format_err!("missing required client"))?; + let bucket = backend_config + .bucket + .clone() + .ok_or_else(|| format_err!("missing required bucket"))?; + let (config, _config_digest) = + pbs_config::s3::config().context("failed to get s3 config")?; + let client_config: S3ClientConf = config + .lookup(S3_CFG_TYPE_ID, s3_client_id) + .with_context(|| format!("no '{s3_client_id}' in config"))?; + let options = S3ClientOptions::from_config( + client_config.config, + client_config.secret_key, + bucket, + datastore_config.name.to_owned(), + ); + let s3_client = S3Client::new(options).context("failed to create s3 client")?; + Ok((backend_type, Some(s3_client))) + } } diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index d24323152..1b56fd276 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -2,25 +2,23 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use ::serde::{Deserialize, Serialize}; -use anyhow::{bail, format_err, Context, Error}; +use anyhow::{bail, Context, Error}; use hex::FromHex; use http_body_util::BodyExt; use serde_json::Value; use tracing::{info, warn}; use proxmox_router::{http_bail, Permission, Router, RpcEnvironment, RpcEnvironmentType}; -use proxmox_s3_client::{S3Client, S3ClientConf, S3ClientOptions}; use proxmox_schema::{api, param_bail, ApiType}; use proxmox_section_config::SectionConfigData; use proxmox_uuid::Uuid; use pbs_api_types::{ - Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreBackendConfig, DatastoreBackendType, - DatastoreNotify, DatastoreTuning, KeepOptions, MaintenanceMode, Operation, PruneJobConfig, - PruneJobOptions, DATASTORE_SCHEMA, PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, - PRIV_DATASTORE_MODIFY, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA, + Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreBackendType, DatastoreNotify, + DatastoreTuning, KeepOptions, MaintenanceMode, Operation, PruneJobConfig, PruneJobOptions, + DATASTORE_SCHEMA, PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, + PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA, }; -use pbs_config::s3::S3_CFG_TYPE_ID; use pbs_config::BackupLockGuard; use pbs_datastore::chunk_store::ChunkStore; @@ -34,7 +32,7 @@ use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_bac use crate::api2::config::verify::delete_verification_job; use pbs_config::CachedUserInfo; -use pbs_datastore::{get_datastore_mount_status, DatastoreBackend}; +use pbs_datastore::{get_datastore_mount_status, DataStore, DatastoreBackend}; use proxmox_rest_server::WorkerTask; use proxmox_s3_client::S3ObjectKey; @@ -131,54 +129,34 @@ pub(crate) fn do_create_datastore( .parse_property_string(datastore.tuning.as_deref().unwrap_or(""))?, )?; - let backend_config: DatastoreBackendConfig = - datastore.backend.as_deref().unwrap_or("").parse()?; - let backend_type = backend_config.ty.unwrap_or_default(); - let backend_s3_client = if backend_type == DatastoreBackendType::S3 { - let s3_client_id = backend_config - .client - .as_ref() - .ok_or_else(|| format_err!("missing required client"))?; - let bucket = backend_config - .bucket - .clone() - .ok_or_else(|| format_err!("missing required bucket"))?; - let (config, _config_digest) = - pbs_config::s3::config().context("failed to get s3 config")?; - let config: S3ClientConf = config - .lookup(S3_CFG_TYPE_ID, s3_client_id) - .with_context(|| format!("no '{s3_client_id}' in config"))?; - let options = S3ClientOptions::from_config( - config.config, - config.secret_key, - bucket, - datastore.name.to_owned(), - ); - let s3_client = S3Client::new(options).context("failed to create s3 client")?; - - if !overwrite_in_use { - let object_key = S3ObjectKey::try_from(S3_DATASTORE_IN_USE_MARKER) - .context("failed to generate s3 object key")?; - if let Some(response) = - proxmox_async::runtime::block_on(s3_client.get_object(object_key.clone())) - .context("failed to get in-use marker from bucket")? - { - let content = proxmox_async::runtime::block_on(response.content.collect()) - .unwrap_or_default(); - let content = String::from_utf8(content.to_bytes().to_vec()).unwrap_or_default(); - let in_use: InUseContent = serde_json::from_str(&content).unwrap_or_default(); - if let Some(hostname) = in_use.hostname { - bail!("Bucket already contains datastore in use by host {hostname}"); - } else { - bail!("Bucket already contains datastore in use"); + let (backend_type, backend_s3_client) = + match DataStore::s3_client_and_backend_from_datastore_config(&datastore)? { + (backend_type, Some(s3_client)) => { + if !overwrite_in_use { + let object_key = S3ObjectKey::try_from(S3_DATASTORE_IN_USE_MARKER) + .context("failed to generate s3 object key")?; + if let Some(response) = + proxmox_async::runtime::block_on(s3_client.get_object(object_key.clone())) + .context("failed to get in-use marker from bucket")? + { + let content = proxmox_async::runtime::block_on(response.content.collect()) + .unwrap_or_default(); + let content = + String::from_utf8(content.to_bytes().to_vec()).unwrap_or_default(); + let in_use: InUseContent = + serde_json::from_str(&content).unwrap_or_default(); + if let Some(hostname) = in_use.hostname { + bail!("Bucket already contains datastore in use by host {hostname}"); + } else { + bail!("Bucket already contains datastore in use"); + } + } } - } - } - Some(Arc::new(s3_client)) - } else { - None - }; + (backend_type, Some(Arc::new(s3_client))) + } + (backend_type, None) => (backend_type, None), + }; let unmount_guard = if datastore.backing_device.is_some() { do_mount_device(datastore.clone())?; @@ -359,34 +337,11 @@ pub fn create_datastore( let store_name = config.name.to_string(); - let backend_config: DatastoreBackendConfig = config.backend.as_deref().unwrap_or("").parse()?; - match backend_config.ty.unwrap_or_default() { - DatastoreBackendType::Filesystem => (), - DatastoreBackendType::S3 => { - let s3_client_id = backend_config - .client - .as_ref() - .ok_or_else(|| format_err!("missing required client"))?; - let bucket = backend_config - .bucket - .clone() - .ok_or_else(|| format_err!("missing required bucket"))?; - let (config, _config_digest) = - pbs_config::s3::config().context("failed to get s3 config")?; - let config: S3ClientConf = config - .lookup(S3_CFG_TYPE_ID, s3_client_id) - .with_context(|| format!("no '{s3_client_id}' in config"))?; - let options = S3ClientOptions::from_config( - config.config, - config.secret_key, - bucket, - store_name.clone(), - ); - let s3_client = S3Client::new(options).context("failed to create s3 client")?; - // Fine to block since this runs in worker task - proxmox_async::runtime::block_on(s3_client.head_bucket()) - .context("failed to access bucket")?; - } + if let (_backend, Some(s3_client)) = + DataStore::s3_client_and_backend_from_datastore_config(&config)? + { + proxmox_async::runtime::block_on(s3_client.head_bucket()) + .context("failed to access bucket")?; } WorkerTask::new_thread( -- 2.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel