[pbs-devel] [PATCH proxmox-backup 1/2] config: s3: adapt to new config struct layouts

Christian Ebner c.ebner at proxmox.com
Tue Jul 22 18:36:02 CEST 2025


In order to not return the secret key as part of the s3 endpoint
config, split the config into different struct depending on the
usecase. Either use the plain config without id and secret_key,
the struct with id and plain config or the combined variant with
all 3 fields present.

Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
 pbs-config/src/s3.rs           |  7 ++--
 pbs-datastore/src/datastore.rs | 11 ++++--
 src/api2/admin/s3.rs           |  7 ++--
 src/api2/config/datastore.rs   | 12 ++++---
 src/api2/config/s3.rs          | 65 +++++++++++++++++++++++-----------
 5 files changed, 68 insertions(+), 34 deletions(-)

diff --git a/pbs-config/src/s3.rs b/pbs-config/src/s3.rs
index 428f097bb..88c582bcb 100644
--- a/pbs-config/src/s3.rs
+++ b/pbs-config/src/s3.rs
@@ -3,11 +3,10 @@ use std::sync::LazyLock;
 
 use anyhow::Error;
 
-use proxmox_s3_client::S3ClientConfig;
+use proxmox_s3_client::{S3ClientConf, S3_CLIENT_ID_SCHEMA};
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
-use pbs_api_types::JOB_ID_SCHEMA;
 
 use pbs_buildcfg::configdir;
 
@@ -16,10 +15,10 @@ use crate::{open_backup_lockfile, replace_backup_config, BackupLockGuard};
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
 fn init() -> SectionConfig {
-    let obj_schema = S3ClientConfig::API_SCHEMA.unwrap_object_schema();
+    let obj_schema = S3ClientConf::API_SCHEMA.unwrap_all_of_schema();
     let plugin =
         SectionConfigPlugin::new("s3client".to_string(), Some(String::from("id")), obj_schema);
-    let mut config = SectionConfig::new(&JOB_ID_SCHEMA);
+    let mut config = SectionConfig::new(&S3_CLIENT_ID_SCHEMA);
     config.register_plugin(plugin);
 
     config
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index a051f0556..e8be576f7 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -14,7 +14,7 @@ use tokio::io::AsyncWriteExt;
 use tracing::{info, warn};
 
 use proxmox_human_byte::HumanByte;
-use proxmox_s3_client::{S3Client, S3ClientConfig, S3ClientOptions, S3ObjectKey, S3PathPrefix};
+use proxmox_s3_client::{S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3PathPrefix};
 use proxmox_schema::ApiType;
 
 use proxmox_sys::error::SysError;
@@ -251,9 +251,14 @@ impl DataStore {
                     .ok_or_else(|| format_err!("missing bucket for s3 backend"))?;
 
                 let (config, _config_digest) = pbs_config::s3::config()?;
-                let config: S3ClientConfig = config.lookup(S3_CFG_TYPE_ID, s3_client_id)?;
+                let config: S3ClientConf = config.lookup(S3_CFG_TYPE_ID, s3_client_id)?;
 
-                let options = S3ClientOptions::from_config(config, bucket, self.name().to_owned());
+                let options = S3ClientOptions::from_config(
+                    config.config,
+                    config.secret_key,
+                    bucket,
+                    self.name().to_owned(),
+                );
                 let s3_client = S3Client::new(options)?;
                 DatastoreBackend::S3(Arc::new(s3_client))
             }
diff --git a/src/api2/admin/s3.rs b/src/api2/admin/s3.rs
index 113ebf6fa..9a7b08c81 100644
--- a/src/api2/admin/s3.rs
+++ b/src/api2/admin/s3.rs
@@ -6,7 +6,7 @@ use serde_json::Value;
 use proxmox_http::Body;
 use proxmox_router::{list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap};
 use proxmox_s3_client::{
-    S3Client, S3ClientConfig, S3ClientOptions, S3ObjectKey, S3_BUCKET_NAME_SCHEMA,
+    S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3_BUCKET_NAME_SCHEMA,
     S3_CLIENT_ID_SCHEMA,
 };
 use proxmox_schema::*;
@@ -43,11 +43,12 @@ pub async fn check(
     _rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
     let (config, _digest) = pbs_config::s3::config()?;
-    let config: S3ClientConfig = config
+    let config: S3ClientConf = config
         .lookup(S3_CFG_TYPE_ID, &s3_client_id)
         .context("config lookup failed")?;
 
-    let options = S3ClientOptions::from_config(config, bucket, store_prefix);
+    let options =
+        S3ClientOptions::from_config(config.config, config.secret_key, bucket, store_prefix);
 
     let test_object_key =
         S3ObjectKey::try_from(".s3-client-test").context("failed to generate s3 object key")?;
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 8abed7e32..2702c7db3 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -9,7 +9,7 @@ use serde_json::Value;
 use tracing::{info, warn};
 
 use proxmox_router::{http_bail, Permission, Router, RpcEnvironment, RpcEnvironmentType};
-use proxmox_s3_client::{S3Client, S3ClientConfig, S3ClientOptions};
+use proxmox_s3_client::{S3Client, S3ClientConf, S3ClientOptions};
 use proxmox_schema::{api, param_bail, ApiType};
 use proxmox_section_config::SectionConfigData;
 use proxmox_uuid::Uuid;
@@ -147,11 +147,15 @@ pub(crate) fn do_create_datastore(
                     .ok_or_else(|| format_err!("missing required bucket"))?;
                 let (config, _config_digest) =
                     pbs_config::s3::config().context("failed to get s3 config")?;
-                let config: S3ClientConfig = 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, bucket, datastore.name.to_owned());
+                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")?;
                 // Fine to block since this runs in worker task
                 proxmox_async::runtime::block_on(s3_client.head_bucket())
diff --git a/src/api2/config/s3.rs b/src/api2/config/s3.rs
index 891c017c7..04b801028 100644
--- a/src/api2/config/s3.rs
+++ b/src/api2/config/s3.rs
@@ -4,7 +4,10 @@ use hex::FromHex;
 use serde_json::Value;
 
 use proxmox_router::{http_bail, Permission, Router, RpcEnvironment};
-use proxmox_s3_client::{S3ClientConfig, S3ClientConfigUpdater};
+use proxmox_s3_client::{
+    S3ClientConf, S3ClientConfig, S3ClientConfigUpdater, S3ClientConfigWithoutSecret,
+    S3_CLIENT_ID_SCHEMA,
+};
 use proxmox_schema::{api, param_bail, ApiType};
 
 use pbs_api_types::{
@@ -20,7 +23,7 @@ use pbs_config::s3::{self, S3_CFG_TYPE_ID};
     returns: {
         description: "List configured s3 clients.",
         type: Array,
-        items: { type: S3ClientConfig },
+        items: { type: S3ClientConfigWithoutSecret },
     },
     access: {
         permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
@@ -30,7 +33,7 @@ use pbs_config::s3::{self, S3_CFG_TYPE_ID};
 pub fn list_s3_client_config(
     _param: Value,
     rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<S3ClientConfig>, Error> {
+) -> Result<Vec<S3ClientConfigWithoutSecret>, Error> {
     let (config, digest) = s3::config()?;
     let list = config.convert_to_typed_array(S3_CFG_TYPE_ID)?;
     rpcenv["digest"] = hex::encode(digest).into();
@@ -42,10 +45,17 @@ pub fn list_s3_client_config(
     protected: true,
     input: {
         properties: {
+            id: {
+                schema: S3_CLIENT_ID_SCHEMA,
+            },
             config: {
                 type: S3ClientConfig,
                 flatten: true,
             },
+            "secret-key": {
+                type: String,
+                description: "S3 secret key",
+            },
         },
     },
     access: {
@@ -54,15 +64,23 @@ pub fn list_s3_client_config(
 )]
 /// Create a new s3 client configuration.
 pub fn create_s3_client_config(
+    id: String,
     config: S3ClientConfig,
+    secret_key: String,
     _rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<(), Error> {
     let _lock = s3::lock_config()?;
     let (mut section_config, _digest) = s3::config()?;
-    if section_config.sections.contains_key(&config.id) {
-        param_bail!("id", "s3 client config '{}' already exists.", config.id);
+    if section_config.sections.contains_key(&id) {
+        param_bail!("id", "s3 client config '{}' already exists.", &id);
     }
 
+    let config = S3ClientConf {
+        id: id.clone(),
+        config,
+        secret_key,
+    };
+
     section_config.set_data(&config.id, S3_CFG_TYPE_ID, &config)?;
     s3::save_config(&section_config)?;
 
@@ -77,7 +95,7 @@ pub fn create_s3_client_config(
             },
         },
     },
-    returns: { type: S3ClientConfig },
+    returns: { type: S3ClientConfigWithoutSecret },
     access: {
         permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
     },
@@ -86,9 +104,9 @@ pub fn create_s3_client_config(
 pub fn read_s3_client_config(
     id: String,
     rpcenv: &mut dyn RpcEnvironment,
-) -> Result<S3ClientConfig, Error> {
+) -> Result<S3ClientConfigWithoutSecret, Error> {
     let (config, digest) = s3::config()?;
-    let s3_client_config: S3ClientConfig = config.lookup(S3_CFG_TYPE_ID, &id)?;
+    let s3_client_config: S3ClientConfigWithoutSecret = config.lookup(S3_CFG_TYPE_ID, &id)?;
     rpcenv["digest"] = hex::encode(digest).into();
 
     Ok(s3_client_config)
@@ -120,6 +138,11 @@ pub enum DeletableProperty {
                 type: S3ClientConfigUpdater,
                 flatten: true,
             },
+            "secret-key": {
+                type: String,
+                description: "S3 client secret key.",
+                optional: true,
+            },
             delete: {
                 description: "List of properties to delete.",
                 type: Array,
@@ -143,6 +166,7 @@ pub enum DeletableProperty {
 pub fn update_s3_client_config(
     id: String,
     update: S3ClientConfigUpdater,
+    secret_key: Option<String>,
     delete: Option<Vec<DeletableProperty>>,
     digest: Option<String>,
     _rpcenv: &mut dyn RpcEnvironment,
@@ -156,46 +180,47 @@ pub fn update_s3_client_config(
         crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
     }
 
-    let mut data: S3ClientConfig = config.lookup(S3_CFG_TYPE_ID, &id)?;
+    let mut data: S3ClientConf = config.lookup(S3_CFG_TYPE_ID, &id)?;
 
     if let Some(delete) = delete {
         for delete_prop in delete {
             match delete_prop {
                 DeletableProperty::Port => {
-                    data.port = None;
+                    data.config.port = None;
                 }
                 DeletableProperty::Region => {
-                    data.region = None;
+                    data.config.region = None;
                 }
                 DeletableProperty::Fingerprint => {
-                    data.fingerprint = None;
+                    data.config.fingerprint = None;
                 }
                 DeletableProperty::PathStyle => {
-                    data.path_style = None;
+                    data.config.path_style = None;
                 }
             }
         }
     }
 
     if let Some(endpoint) = update.endpoint {
-        data.endpoint = endpoint;
+        data.config.endpoint = endpoint;
     }
     if let Some(port) = update.port {
-        data.port = Some(port);
+        data.config.port = Some(port);
     }
     if let Some(region) = update.region {
-        data.region = Some(region);
+        data.config.region = Some(region);
     }
     if let Some(access_key) = update.access_key {
-        data.access_key = access_key;
+        data.config.access_key = access_key;
     }
     if let Some(fingerprint) = update.fingerprint {
-        data.fingerprint = Some(fingerprint);
+        data.config.fingerprint = Some(fingerprint);
     }
     if let Some(path_style) = update.path_style {
-        data.path_style = Some(path_style);
+        data.config.path_style = Some(path_style);
     }
-    if let Some(secret_key) = update.secret_key {
+
+    if let Some(secret_key) = secret_key {
         data.secret_key = secret_key;
     }
 
-- 
2.47.2





More information about the pbs-devel mailing list