Re: [pbs-devel] [PATCH proxmox-backup v8 05/45] api/cli: add endpoint and command to check s3 client connection

[Christian Ebner <c.ebner@proxmox.com>]

On 7/18/25 9:42 AM, Lukas Wagner wrote:
> With the magic string replaced by constants:
> 
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
> 
> 
> On  2025-07-15 14:52, Christian Ebner wrote:
>> Adds a dedicated api endpoint and a proxmox-backup-manager command to
>> check if the configured S3 client can reach the bucket.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> changes since version 7:
>> - no changes
>>
>>   src/api2/admin/mod.rs                 |  2 +
>>   src/api2/admin/s3.rs                  | 80 +++++++++++++++++++++++++++
>>   src/bin/proxmox-backup-manager.rs     |  1 +
>>   src/bin/proxmox_backup_manager/mod.rs |  2 +
>>   src/bin/proxmox_backup_manager/s3.rs  | 46 +++++++++++++++
>>   5 files changed, 131 insertions(+)
>>   create mode 100644 src/api2/admin/s3.rs
>>   create mode 100644 src/bin/proxmox_backup_manager/s3.rs
>>
>> diff --git a/src/api2/admin/mod.rs b/src/api2/admin/mod.rs
>> index a1c49f8e2..7694de4b9 100644
>> --- a/src/api2/admin/mod.rs
>> +++ b/src/api2/admin/mod.rs
>> @@ -9,6 +9,7 @@ pub mod gc;
>>   pub mod metrics;
>>   pub mod namespace;
>>   pub mod prune;
>> +pub mod s3;
>>   pub mod sync;
>>   pub mod traffic_control;
>>   pub mod verify;
>> @@ -19,6 +20,7 @@ const SUBDIRS: SubdirMap = &sorted!([
>>       ("metrics", &metrics::ROUTER),
>>       ("prune", &prune::ROUTER),
>>       ("gc", &gc::ROUTER),
>> +    ("s3", &s3::ROUTER),
>>       ("sync", &sync::ROUTER),
>>       ("traffic-control", &traffic_control::ROUTER),
>>       ("verify", &verify::ROUTER),
>> diff --git a/src/api2/admin/s3.rs b/src/api2/admin/s3.rs
>> new file mode 100644
>> index 000000000..d20031707
>> --- /dev/null
>> +++ b/src/api2/admin/s3.rs
>> @@ -0,0 +1,80 @@
>> +//! S3 bucket operations
>> +
>> +use anyhow::{Context, Error};
>> +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, S3ClientSecretsConfig, S3_BUCKET_NAME_SCHEMA,
>> +    S3_CLIENT_ID_SCHEMA,
>> +};
>> +use proxmox_schema::*;
>> +use proxmox_sortable_macro::sortable;
>> +
>> +use pbs_api_types::PRIV_SYS_MODIFY;
>> +
>> +#[api(
>> +    input: {
>> +        properties: {
>> +            "s3-client-id": {
>> +                schema: S3_CLIENT_ID_SCHEMA,
>> +            },
>> +            bucket: {
>> +                schema: S3_BUCKET_NAME_SCHEMA,
>> +            },
>> +            "store-prefix": {
>> +                type: String,
>> +                description: "Store prefix within bucket for S3 object keys (commonly datastore name)",
>> +            },
>> +        },
>> +    },
>> +    access: {
>> +        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
>> +    },
>> +)]
>> +/// Perform basic sanity check for given s3 client configuration
>> +pub async fn check(
>> +    s3_client_id: String,
>> +    bucket: String,
>> +    store_prefix: String,
>> +    _rpcenv: &mut dyn RpcEnvironment,
>> +) -> Result<Value, Error> {
>> +    let (config, _digest) = pbs_config::s3::config()?;
>> +    let config: S3ClientConfig = config
>> +        .lookup("s3client", &s3_client_id)
>> +        .context("config lookup failed")?;
>> +    let (secrets, _secrets_digest) = pbs_config::s3::secrets_config()?;
>> +    let secrets: S3ClientSecretsConfig = secrets
>> +        .lookup("s3secrets", &s3_client_id)
>> +        .context("secrets lookup failed")?;
> 
> Same thing here with regards to the section config type strings.

Adapted both to the new constants as well

>> +
>> +    let options = S3ClientOptions::from_config(config, secrets, bucket, store_prefix);
>> +
>> +    let test_object_key = ".s3-client-test";
>> +    let client = S3Client::new(options).context("client creation failed")?;
>> +    client.head_bucket().await.context("head object failed")?;
>> +    client
>> +        .put_object(test_object_key.into(), Body::empty(), true)
>> +        .await
>> +        .context("put object failed")?;
>> +    client
>> +        .get_object(test_object_key.into())
>> +        .await
>> +        .context("get object failed")?;
>> +    client
>> +        .delete_object(test_object_key.into())
>> +        .await
>> +        .context("delete object failed")?;
>> +
>> +    Ok(Value::Null)
>> +}
>> +
>> +#[sortable]
>> +const S3_OPERATION_SUBDIRS: SubdirMap = &[("check", &Router::new().get(&API_METHOD_CHECK))];
>> +
>> +const S3_OPERATION_ROUTER: Router = Router::new()
>> +    .get(&list_subdirs_api_method!(S3_OPERATION_SUBDIRS))
>> +    .subdirs(S3_OPERATION_SUBDIRS);
>> +
>> +pub const ROUTER: Router = Router::new().match_all("s3-client-id", &S3_OPERATION_ROUTER);
>> diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
>> index d4363e717..68d87c676 100644
>> --- a/src/bin/proxmox-backup-manager.rs
>> +++ b/src/bin/proxmox-backup-manager.rs
>> @@ -677,6 +677,7 @@ async fn run() -> Result<(), Error> {
>>           .insert("garbage-collection", garbage_collection_commands())
>>           .insert("acme", acme_mgmt_cli())
>>           .insert("cert", cert_mgmt_cli())
>> +        .insert("s3", s3_commands())
>>           .insert("subscription", subscription_commands())
>>           .insert("sync-job", sync_job_commands())
>>           .insert("verify-job", verify_job_commands())
>> diff --git a/src/bin/proxmox_backup_manager/mod.rs b/src/bin/proxmox_backup_manager/mod.rs
>> index 9b5c73e9a..312a6db6b 100644
>> --- a/src/bin/proxmox_backup_manager/mod.rs
>> +++ b/src/bin/proxmox_backup_manager/mod.rs
>> @@ -26,6 +26,8 @@ mod prune;
>>   pub use prune::*;
>>   mod remote;
>>   pub use remote::*;
>> +mod s3;
>> +pub use s3::*;
>>   mod subscription;
>>   pub use subscription::*;
>>   mod sync;
>> diff --git a/src/bin/proxmox_backup_manager/s3.rs b/src/bin/proxmox_backup_manager/s3.rs
>> new file mode 100644
>> index 000000000..9bb89ff55
>> --- /dev/null
>> +++ b/src/bin/proxmox_backup_manager/s3.rs
>> @@ -0,0 +1,46 @@
>> +use proxmox_router::{cli::*, RpcEnvironment};
>> +use proxmox_s3_client::{S3_BUCKET_NAME_SCHEMA, S3_CLIENT_ID_SCHEMA};
>> +use proxmox_schema::api;
>> +
>> +use proxmox_backup::api2;
>> +
>> +use anyhow::Error;
>> +use serde_json::Value;
>> +
>> +#[api(
>> +    input: {
>> +        properties: {
>> +            "s3-client-id": {
>> +                schema: S3_CLIENT_ID_SCHEMA,
>> +            },
>> +            bucket: {
>> +                schema: S3_BUCKET_NAME_SCHEMA,
>> +            },
>> +            "store-prefix": {
>> +                type: String,
>> +                description: "Store prefix within bucket for S3 object keys (commonly datastore name)",
>> +            },
>> +        },
>> +    },
>> +)]
>> +/// Perform basic sanity checks for given S3 client configuration
>> +async fn check(
>> +    s3_client_id: String,
>> +    bucket: String,
>> +    store_prefix: String,
>> +    rpcenv: &mut dyn RpcEnvironment,
>> +) -> Result<Value, Error> {
>> +    api2::admin::s3::check(s3_client_id, bucket, store_prefix, rpcenv).await?;
>> +    Ok(Value::Null)
>> +}
>> +
>> +pub fn s3_commands() -> CommandLineInterface {
>> +    let cmd_def = CliCommandMap::new().insert(
>> +        "check",
>> +        CliCommand::new(&API_METHOD_CHECK)
>> +            .arg_param(&["s3-client-id", "bucket"])
>> +            .completion_cb("s3-client-id", pbs_config::s3::complete_s3_client_id),
>> +    );
>> +
>> +    cmd_def.into()
>> +}
> 



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