[pbs-devel] [PATCH proxmox-backup v8 03/45] api: config: implement endpoints to manipulate and list s3 configs

[Lukas Wagner]

Reviewed-by: Lukas Wagner <l.wagner at proxmox.com>


On  2025-07-15 14:52, Christian Ebner wrote:
> +/// Update an s3 client configuration.
> +#[allow(clippy::too_many_arguments)]
> +pub fn update_s3_client_config(
> +    id: String,
> +    update: S3ClientConfigUpdater,
> +    update_secrets: S3ClientSecretsConfigUpdater,
> +    delete: Option<Vec<DeletableProperty>>,
> +    digest: Option<String>,
> +    _rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<(), Error> {
> +    let _lock = s3::lock_config()?;
> +    let (mut config, expected_digest) = s3::config()?;
> +    let (mut secrets, secrets_digest) = s3::secrets_config()?;
> +    let expected_digest = digest_with_secrets(&expected_digest, &secrets_digest);
> +
> +    // Secrets are not included in digest concurrent changes therefore not detected.
> +    if let Some(ref digest) = digest {
> +        let digest = <[u8; 32]>::from_hex(digest)?;
> +        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> +    }
> +
> +    let mut data: S3ClientConfig = config.lookup("s3client", &id)?;
> +
> +    if let Some(delete) = delete {
> +        for delete_prop in delete {
> +            match delete_prop {
> +                DeletableProperty::Port => {
> +                    data.port = None;
> +                }
> +                DeletableProperty::Region => {
> +                    data.region = None;
> +                }
> +                DeletableProperty::Fingerprint => {
> +                    data.fingerprint = None;
> +                }
> +                DeletableProperty::PathStyle => {
> +                    data.path_style = None;
> +                }
> +            }
> +        }
> +    }

Some time ago I've found that it is quite useful to
destructure the updater like I did in proxmox-notify [1].
This ensures that you don't forget to update the
API handler after adding a new field to the config struct.
Not a must, just a suggestion, since I like this pattern quite a bit :)

[1] https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox-notify/src/api/webhook.rs;h=9d904d0bf57f9f789bb6723e1d8ca710fcf0cb96;hb=HEAD#l175

> +
> +    if let Some(endpoint) = update.endpoint {
> +        data.endpoint = endpoint;
> +    }
> +    if let Some(port) = update.port {
> +        data.port = Some(port);
> +    }
> +    if let Some(region) = update.region {
> +        data.region = Some(region);
> +    }
> +    if let Some(access_key) = update.access_key {
> +        data.access_key = access_key;
> +    }
> +    if let Some(fingerprint) = update.fingerprint {
> +        data.fingerprint = Some(fingerprint);
> +    }
> +    if let Some(path_style) = update.path_style {
> +        data.path_style = Some(path_style);
> +    }
> +
> +    let mut secrets_data: S3ClientSecretsConfig = secrets.lookup("s3secrets", &id)?;
> +    if let Some(secret_key) = update_secrets.secret_key {
> +        secrets_data.secret_key = secret_key;
> +    }
> +
> +    config.set_data(&id, "s3client", &data)?;
> +    secrets.set_data(&id, "s3secrets", &secrets_data)?;
> +    s3::save_config(&config, &secrets)?;
> +
> +    Ok(())
> +}


-- 
- Lukas