Re: [PATCH] client: support individual repository component parameters

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On March 23, 2026 10:11 pm, Thomas Lamprecht wrote:
> The compact repository URL format ([[auth-id@]server[:port]:]datastore)
> can be cumbersome to remember the exact syntax and work with.
> It also makes it awkward to change a single aspect of the connection,
> like switching to a different datastore, without rewriting the whole
> string.
> 
> So add the "expanded" --server, --port, --datastore, and --auth-id as
> separate CLI parameters, mutually exclusive with --repository. Both
> forms resolve to the same BackupRepository internally.
> 
> Add a BackupRepositoryArgs that holds both the atoms and the combined
> URL and replace the existing "repository" parameter in all client
> tools, i.e. proxmox-backup-client, proxmox-file-restore, and
> proxmox-backup-debug. This struct gets flattened into the cli (api)
> schemas such that it's fully backward compatible.
> 
> Also add recognition for corresponding environment variables
> (PBS_SERVER, PBS_PORT, PBS_DATASTORE, PBS_AUTH_ID) serve as fallback
> when neither --repository/PBS_REPOSITORY nor CLI component options are
> given.
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
> 
> Disclaimer: only lightly tested! but as this is somewhat of a pain point
> I run myself into when adding a new Debian host to backup to a PBS via
> the client, I finally wanted to have this improved and polished the POC
> I have lying around for some time now already.
> 
>  docs/backup-client.rst                 |  60 ++++++++++
>  pbs-client/src/backup_repo.rs          | 117 +++++++++++++++++-
>  pbs-client/src/tools/mod.rs            | 159 +++++++++++++++++++------
>  proxmox-backup-client/src/benchmark.rs |   8 +-
>  proxmox-backup-client/src/catalog.rs   |  16 +--
>  proxmox-backup-client/src/group.rs     |   8 +-
>  proxmox-backup-client/src/main.rs      |  68 +++++------
>  proxmox-backup-client/src/mount.rs     |  13 +-
>  proxmox-backup-client/src/namespace.rs |  20 ++--
>  proxmox-backup-client/src/snapshot.rs  |  52 ++++----
>  proxmox-backup-client/src/task.rs      |  20 ++--
>  proxmox-file-restore/src/main.rs       |  15 ++-
>  src/bin/proxmox_backup_debug/diff.rs   |   9 +-
>  13 files changed, 418 insertions(+), 147 deletions(-)
> 
> diff --git a/docs/backup-client.rst b/docs/backup-client.rst
> index 40962f0e2..4d0467eb1 100644
> --- a/docs/backup-client.rst
> +++ b/docs/backup-client.rst
> @@ -28,6 +28,50 @@ brackets (for example, `[fe80::01]`).
>  You can pass the repository with the ``--repository`` command-line option, or
>  by setting the ``PBS_REPOSITORY`` environment variable.
>  
> +Alternatively, you can specify the repository components as separate
> +command-line options:
> +
> +``--server <host>``
> +  Backup server address (hostname or IP address). Defaults to ``localhost``.
> +
> +``--port <number>``
> +  Backup server port. Defaults to ``8007``.
> +
> +``--datastore <name>``
> +  Name of the target datastore. Required when using component options instead
> +  of ``--repository``.
> +
> +``--auth-id <user@realm[!token]>``
> +  Authentication identity, either a user (``user@realm``) or an API token
> +  (``user@realm!tokenname``). Defaults to ``root@pam``.
> +
> +These options are mutually exclusive with ``--repository``. Both forms
> +resolve to the same internal representation, so cached login tickets and
> +other session state are shared between them. For example, logging in with
> +``--repository`` and then running a backup with ``--server``/``--datastore``
> +(or vice versa) reuses the same ticket, as long as the server address and
> +user match.
> +
> +The component options make it easy to change individual parts of the
> +connection, for example switching to a different datastore or server without
> +having to rewrite the entire repository string. They also simplify usage
> +with API tokens, which require escaping the ``@`` separator in the compact
> +form:

that last sentence I don't understand, the `@` is part of both a user
and an API token authid, it's the `!` that is only there for tokens?

I've never had to escape the `@` in `--repository`..

> +
> +.. code-block:: console
> +
> +  # Using component options
> +  # proxmox-backup-client backup root.pxar:/ \
> +      --auth-id 'user@pbs!backup' --server pbs.example.com --datastore store1
> +
> +  # Equivalent compact form (the \@ disambiguates the user@realm
> +  # separator from the user-to-host separator in the URL format)
> +  # proxmox-backup-client backup root.pxar:/ \
> +      --repository 'user\@pbs!backup@pbs.example.com:store1'
> +
> +.. Note:: Remember to quote API token identifiers on the shell, since the
> +   exclamation mark (``!``) is a special character in most shells.
> +
>  The web interface provides copyable repository text in the datastore summary
>  with the `Show Connection Information` button.
>  
> @@ -70,6 +114,22 @@ Environment Variables
>  ``PBS_REPOSITORY``
>    The default backup repository.
>  
> +``PBS_SERVER``
> +  Default backup server address. Used as a fallback when neither
> +  ``--repository`` / ``PBS_REPOSITORY`` nor ``--server`` is given.
> +  Requires ``PBS_DATASTORE`` to be set as well.
> +
> +``PBS_PORT``
> +  Default backup server port. Defaults to ``8007`` if unset.
> +
> +``PBS_DATASTORE``
> +  Default datastore name. Used as a fallback when neither ``--repository`` /
> +  ``PBS_REPOSITORY`` nor ``--datastore`` is given.
> +
> +``PBS_AUTH_ID``
> +  Default authentication identity (``user@realm`` or
> +  ``user@realm!tokenname``). Defaults to ``root@pam`` if unset.
> +

we also have a request open for PBS_NAMESPACE, with similar rationale as
your changes here:

https://bugzilla.proxmox.com/show_bug.cgi?id=5340

>  ``PBS_PASSWORD``
>    When set, this value is used as the password for the backup server.
>    You can also set this to an API token secret.
> diff --git a/pbs-client/src/backup_repo.rs b/pbs-client/src/backup_repo.rs
> index 458f89dc2..8d2d356f4 100644
> --- a/pbs-client/src/backup_repo.rs
> +++ b/pbs-client/src/backup_repo.rs
> @@ -1,8 +1,121 @@
>  use std::fmt;
>  
> -use anyhow::{format_err, Error};
> +use anyhow::{bail, format_err, Error};
> +use serde::{Deserialize, Serialize};
>  
> -use pbs_api_types::{Authid, Userid, BACKUP_REPO_URL_REGEX, IP_V6_REGEX};
> +use proxmox_schema::api;
> +use proxmox_schema::*;
> +
> +use pbs_api_types::{
> +    Authid, Userid, BACKUP_REPO_URL, BACKUP_REPO_URL_REGEX, DATASTORE_SCHEMA, IP_V6_REGEX,
> +};
> +
> +pub const REPO_URL_SCHEMA: Schema =
> +    StringSchema::new("Repository URL: [[auth-id@]server[:port]:]datastore")
> +        .format(&BACKUP_REPO_URL)
> +        .max_length(256)
> +        .schema();
> +
> +pub const BACKUP_REPO_SERVER_SCHEMA: Schema =
> +    StringSchema::new("Backup server address (hostname or IP). Default: localhost")
> +        .format(&api_types::DNS_NAME_OR_IP_FORMAT)
> +        .max_length(256)
> +        .schema();
> +
> +pub const BACKUP_REPO_PORT_SCHEMA: Schema = IntegerSchema::new("Backup server port. Default: 8007")
> +    .minimum(1)
> +    .maximum(65535)
> +    .default(8007)
> +    .schema();
> +
> +#[api(
> +    properties: {
> +        repository: {
> +            schema: REPO_URL_SCHEMA,
> +            optional: true,
> +        },
> +        server: {
> +            schema: BACKUP_REPO_SERVER_SCHEMA,
> +            optional: true,
> +        },
> +        port: {
> +            schema: BACKUP_REPO_PORT_SCHEMA,
> +            optional: true,
> +        },
> +        datastore: {
> +            schema: DATASTORE_SCHEMA,
> +            optional: true,
> +        },
> +        "auth-id": {
> +            type: Authid,
> +            optional: true,
> +        },
> +    },
> +)]
> +#[derive(Default, Serialize, Deserialize)]
> +#[serde(rename_all = "kebab-case")]
> +/// Backup repository location, specified either as a repository URL or as individual components
> +/// (server, port, datastore, auth-id).
> +pub struct BackupRepositoryArgs {
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub repository: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub server: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub port: Option<u16>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub datastore: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub auth_id: Option<Authid>,
> +}
> +
> +impl BackupRepositoryArgs {
> +    /// Returns `true` if any atom parameter (server, port, datastore, or auth-id) is set.
> +    pub fn has_atoms(&self) -> bool {
> +        self.server.is_some()
> +            || self.port.is_some()
> +            || self.datastore.is_some()
> +            || self.auth_id.is_some()
> +    }
> +}
> +
> +impl TryFrom<BackupRepositoryArgs> for BackupRepository {
> +    type Error = anyhow::Error;
> +
> +    /// Convert explicit CLI arguments into a [`BackupRepository`].
> +    ///
> +    /// * If `repository` and any atom are both set, returns an error.
> +    /// * If atoms are present, builds the repository from them (requires `datastore`).
> +    /// * If only `repository` is set, parses the repo URL.
> +    /// * If nothing is set, returns an error - callers must fall back to environment variables /
> +    ///   credentials themselves.
> +    fn try_from(args: BackupRepositoryArgs) -> Result<Self, Self::Error> {
> +        let has_url = args.repository.is_some();
> +        let has_atoms = args.has_atoms();
> +
> +        if has_url && has_atoms {
> +            bail!("--repository and --server/--port/--datastore/--auth-id are mutually exclusive");
> +        }
> +
> +        if has_atoms {
> +            let store = args.datastore.ok_or_else(|| {
> +                format_err!("--datastore is required when not using --repository")
> +            })?;
> +            return Ok(BackupRepository::new(
> +                args.auth_id,
> +                args.server,
> +                args.port,
> +                store,
> +            ));
> +        }
> +
> +        if let Some(url) = args.repository {
> +            return url.parse();
> +        }
> +
> +        anyhow::bail!("no repository specified")
> +    }
> +}
>  
>  /// Reference remote backup locations
>  ///
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index 7a496d14c..ad83d8cab 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -17,12 +17,14 @@ use proxmox_router::cli::{complete_file_name, shellword_split};
>  use proxmox_schema::*;
>  use proxmox_sys::fs::file_get_json;
>  
> -use pbs_api_types::{
> -    Authid, BackupArchiveName, BackupNamespace, RateLimitConfig, UserWithTokens, BACKUP_REPO_URL,
> -};
> +use pbs_api_types::{Authid, BackupArchiveName, BackupNamespace, RateLimitConfig, UserWithTokens};
>  use pbs_datastore::BackupManifest;
>  
> -use crate::{BackupRepository, HttpClient, HttpClientOptions};
> +use crate::{BackupRepository, BackupRepositoryArgs, HttpClient, HttpClientOptions};
> +
> +// Re-export for backward compatibility; the canonical definition is now in
> +// backup_repo.rs alongside BackupRepositoryArgs.
> +pub use crate::REPO_URL_SCHEMA;
>  
>  pub mod key_source;
>  
> @@ -30,6 +32,10 @@ const ENV_VAR_PBS_FINGERPRINT: &str = "PBS_FINGERPRINT";
>  const ENV_VAR_PBS_PASSWORD: &str = "PBS_PASSWORD";
>  const ENV_VAR_PBS_ENCRYPTION_PASSWORD: &str = "PBS_ENCRYPTION_PASSWORD";
>  const ENV_VAR_PBS_REPOSITORY: &str = "PBS_REPOSITORY";
> +const ENV_VAR_PBS_SERVER: &str = "PBS_SERVER";
> +const ENV_VAR_PBS_PORT: &str = "PBS_PORT";
> +const ENV_VAR_PBS_DATASTORE: &str = "PBS_DATASTORE";
> +const ENV_VAR_PBS_AUTH_ID: &str = "PBS_AUTH_ID";
>  
>  /// Directory with system [credential]s. See systemd-creds(1).
>  ///
> @@ -44,11 +50,6 @@ const CRED_PBS_REPOSITORY: &str = "proxmox-backup-client.repository";
>  /// Credential name of the the fingerprint.
>  const CRED_PBS_FINGERPRINT: &str = "proxmox-backup-client.fingerprint";
>  
> -pub const REPO_URL_SCHEMA: Schema = StringSchema::new("Repository URL.")
> -    .format(&BACKUP_REPO_URL)
> -    .max_length(256)
> -    .schema();
> -
>  pub const CHUNK_SIZE_SCHEMA: Schema = IntegerSchema::new("Chunk size in KB. Must be a power of 2.")
>      .minimum(64)
>      .maximum(4096)
> @@ -233,41 +234,131 @@ pub fn get_fingerprint() -> Option<String> {
>          .unwrap_or_default()
>  }
>  
> +/// Build [`BackupRepositoryArgs`] from the fields in a JSON Value.
> +fn args_from_value(param: &Value) -> BackupRepositoryArgs {
> +    BackupRepositoryArgs {
> +        repository: param["repository"].as_str().map(String::from),
> +        server: param["server"].as_str().map(String::from),
> +        port: param["port"].as_u64().map(|p| p as u16),
> +        datastore: param["datastore"].as_str().map(String::from),
> +        auth_id: param["auth-id"]
> +            .as_str()
> +            .and_then(|s| s.parse::<Authid>().ok()),
> +    }
> +}
> +
> +/// Build [`BackupRepositoryArgs`] from `PBS_*` environment variables.
> +fn args_from_env() -> BackupRepositoryArgs {
> +    BackupRepositoryArgs {
> +        repository: None,
> +        server: std::env::var(ENV_VAR_PBS_SERVER).ok(),
> +        port: std::env::var(ENV_VAR_PBS_PORT)
> +            .ok()
> +            .and_then(|p| p.parse::<u16>().ok()),
> +        datastore: std::env::var(ENV_VAR_PBS_DATASTORE).ok(),
> +        auth_id: std::env::var(ENV_VAR_PBS_AUTH_ID)
> +            .ok()
> +            .and_then(|s| s.parse::<Authid>().ok()),
> +    }
> +}
> +
> +/// Remove repository-related keys from a JSON Value and return the parsed [`BackupRepository`].
> +///
> +/// This is used by commands that forward the remaining parameters to the server API after stripping
> +/// the repository fields.
>  pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {
> -    if let Some(url) = param
> +    let map = param
>          .as_object_mut()
> -        .ok_or_else(|| format_err!("unable to get repository (parameter is not an object)"))?
> -        .remove("repository")
> -    {
> -        return url
> -            .as_str()
> -            .ok_or_else(|| format_err!("invalid repository value (must be a string)"))?
> -            .parse();
> +        .ok_or_else(|| format_err!("unable to get repository (parameter is not an object)"))?;
> +
> +    let to_string = |v: Value| v.as_str().map(String::from);
> +
> +    let args = BackupRepositoryArgs {
> +        repository: map.remove("repository").and_then(to_string),
> +        server: map.remove("server").and_then(to_string),
> +        port: map
> +            .remove("port")
> +            .and_then(|v| v.as_u64())
> +            .map(|p| p as u16),
> +        datastore: map.remove("datastore").and_then(to_string),
> +        auth_id: map
> +            .remove("auth-id")
> +            .and_then(to_string)
> +            .map(|s| s.parse::<Authid>())
> +            .transpose()?,
> +    };
> +
> +    match BackupRepository::try_from(args) {
> +        Ok(repo) => Ok(repo),
> +        Err(_) => {
> +            // Fall back to environment.
> +            if let Some(url) = get_default_repository() {
> +                return url.parse();
> +            }
> +            let env_args = args_from_env();
> +            if env_args.has_atoms() {
> +                return BackupRepository::try_from(env_args);
> +            }
> +            bail!("repository not passed via CLI options and unable to get (default) repository from environment");

a similar issue like below also exists here I think, the code path below
was just how I triggered it..

> +        }
>      }
> -
> -    get_default_repository()
> -        .ok_or_else(|| format_err!("unable to get default repository"))?
> -        .parse()
>  }
>  
> +/// Extract a [`BackupRepository`] from CLI parameters.
> +///
> +/// Resolution precedence:
> +/// 1. CLI `--repository` + CLI atoms → error (mutually exclusive)

this doesn't work nicely in practice atm, e.g. when I run

`proxmox-backup-client backup .. --repository "something" --auth-id "something"` :

Error: unable to get (default) repository

instead of a nice error point at the invalid combination of arguments..

> +/// 2. CLI atoms (at least `--datastore`)
> +/// 3. CLI `--repository`
> +/// 4. `PBS_REPOSITORY` environment variable / systemd credential
> +/// 5. `PBS_SERVER`/`PBS_PORT`/`PBS_DATASTORE`/`PBS_AUTH_ID` environment variables
>  pub fn extract_repository_from_value(param: &Value) -> Result<BackupRepository, Error> {
> -    let repo_url = param["repository"]
> -        .as_str()
> -        .map(String::from)
> -        .or_else(get_default_repository)
> -        .ok_or_else(|| format_err!("unable to get (default) repository"))?;
> +    let args = args_from_value(param);
>  
> -    let repo: BackupRepository = repo_url.parse()?;
> -
> -    Ok(repo)
> +    match BackupRepository::try_from(args) {
> +        Ok(repo) => Ok(repo),
> +        Err(_) => {

probably this error here

> +            // Fall back to environment.
> +            if let Some(url) = get_default_repository() {
> +                return url.parse();
> +            }
> +            let env_args = args_from_env();
> +            if env_args.has_atoms() {
> +                return BackupRepository::try_from(env_args);
> +            }
> +            bail!("unable to get (default) repository");

should be printed here? or we should not fall back to the environment if
at least one atom or the URL is given as argument? what about mix and
match, e.g. setting the server via the env, but the datastore and
namespace via arguments?

> +        }
> +    }
>  }
>  
> +/// Extract a [`BackupRepository`] from a parameter map (used for shell
> +/// completion callbacks).
>  pub fn extract_repository_from_map(param: &HashMap<String, String>) -> Option<BackupRepository> {
> -    param
> -        .get("repository")
> -        .map(String::from)
> -        .or_else(get_default_repository)
> -        .and_then(|repo_url| repo_url.parse::<BackupRepository>().ok())
> +    let args = BackupRepositoryArgs {
> +        repository: param.get("repository").cloned(),
> +        server: param.get("server").cloned(),
> +        port: param.get("port").and_then(|p| p.parse().ok()),
> +        datastore: param.get("datastore").cloned(),
> +        auth_id: param.get("auth-id").and_then(|s| s.parse().ok()),
> +    };
> +
> +    if let Ok(repo) = BackupRepository::try_from(args) {
> +        return Some(repo);
> +    }
> +
> +    // Fall back to environment: compound URL, then atoms.
> +    if let Some(url) = get_default_repository() {
> +        if let Ok(repo) = url.parse() {
> +            return Some(repo);
> +        }
> +    }
> +
> +    let env_args = args_from_env();
> +    if env_args.has_atoms() {
> +        return BackupRepository::try_from(env_args).ok();
> +    }
> +
> +    None
>  }

this helper probably requires similar attention for consistency's sake

>  
>  pub fn connect(repo: &BackupRepository) -> Result<HttpClient, Error> {

> [..]