* [PATCH] client: support individual repository component parameters
@ 2026-03-23 21:11 Thomas Lamprecht
2026-03-24 13:42 ` Fabian Grünbichler
2026-03-24 15:58 ` Maximiliano Sandoval
0 siblings, 2 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2026-03-23 21:11 UTC (permalink / raw)
To: pbs-devel
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:
+
+.. 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.
+
``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");
+ }
}
-
- 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)
+/// 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(_) => {
+ // 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");
+ }
+ }
}
+/// 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
}
pub fn connect(repo: &BackupRepository) -> Result<HttpClient, Error> {
diff --git a/proxmox-backup-client/src/benchmark.rs b/proxmox-backup-client/src/benchmark.rs
index ad8c60ed9..c113e2ca2 100644
--- a/proxmox-backup-client/src/benchmark.rs
+++ b/proxmox-backup-client/src/benchmark.rs
@@ -22,7 +22,7 @@ use pbs_key_config::{load_and_decrypt_key, KeyDerivationConfig};
use pbs_tools::crypt_config::CryptConfig;
use crate::{
- connect, extract_repository_from_value, record_repository, KEYFILE_SCHEMA, REPO_URL_SCHEMA,
+ connect, extract_repository_from_value, record_repository, BackupRepositoryArgs, KEYFILE_SCHEMA,
};
#[api()]
@@ -105,9 +105,9 @@ static BENCHMARK_RESULT_2020_TOP: BenchmarkResult = BenchmarkResult {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
keyfile: {
schema: KEYFILE_SCHEMA,
diff --git a/proxmox-backup-client/src/catalog.rs b/proxmox-backup-client/src/catalog.rs
index b1b22ff24..50d8e997a 100644
--- a/proxmox-backup-client/src/catalog.rs
+++ b/proxmox-backup-client/src/catalog.rs
@@ -20,16 +20,16 @@ use crate::{
complete_backup_snapshot, complete_group_or_snapshot, complete_namespace,
complete_pxar_archive_name, complete_repository, connect, crypto_parameters, decrypt_key,
dir_or_last_from_group, extract_repository_from_value, format_key_source, optional_ns_param,
- record_repository, BackupDir, BufferedDynamicReader, CatalogReader, DynamicIndexReader,
- IndexFile, Shell, KEYFD_SCHEMA, REPO_URL_SCHEMA,
+ record_repository, BackupDir, BackupRepositoryArgs, BufferedDynamicReader, CatalogReader,
+ DynamicIndexReader, IndexFile, Shell, KEYFD_SCHEMA,
};
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -170,9 +170,9 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> {
"archive-name": {
type: BackupArchiveName,
},
- "repository": {
- optional: true,
- schema: REPO_URL_SCHEMA,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
"keyfile": {
optional: true,
diff --git a/proxmox-backup-client/src/group.rs b/proxmox-backup-client/src/group.rs
index 2c12db2ee..5a3786f07 100644
--- a/proxmox-backup-client/src/group.rs
+++ b/proxmox-backup-client/src/group.rs
@@ -6,7 +6,7 @@ use proxmox_schema::api;
use crate::{
complete_backup_group, complete_namespace, complete_repository, merge_group_into,
- REPO_URL_SCHEMA,
+ BackupRepositoryArgs,
};
use pbs_api_types::{BackupGroup, BackupNamespace};
use pbs_client::tools::{connect, remove_repository_from_value};
@@ -29,9 +29,9 @@ pub fn group_mgmt_cli() -> CliCommandMap {
type: String,
description: "Backup group",
},
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 2ada87bdd..55fef0cca 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -41,13 +41,13 @@ use pbs_client::tools::{
crypto_parameters, format_key_source, get_encryption_key_password, KEYFD_SCHEMA,
KEYFILE_SCHEMA, MASTER_PUBKEY_FD_SCHEMA, MASTER_PUBKEY_FILE_SCHEMA,
},
- raise_nofile_limit, CHUNK_SIZE_SCHEMA, REPO_URL_SCHEMA,
+ raise_nofile_limit, CHUNK_SIZE_SCHEMA,
};
use pbs_client::{
delete_ticket_info, parse_backup_specification, view_task_result, BackupDetectionMode,
- BackupReader, BackupRepository, BackupSpecificationType, BackupStats, BackupWriter,
- BackupWriterOptions, ChunkStream, FixedChunkStream, HttpClient, IndexType, InjectionData,
- PxarBackupStream, RemoteChunkReader, UploadOptions, BACKUP_SOURCE_SCHEMA,
+ BackupReader, BackupRepository, BackupRepositoryArgs, BackupSpecificationType, BackupStats,
+ BackupWriter, BackupWriterOptions, ChunkStream, FixedChunkStream, HttpClient, IndexType,
+ InjectionData, PxarBackupStream, RemoteChunkReader, UploadOptions, BACKUP_SOURCE_SCHEMA,
};
use pbs_datastore::catalog::{BackupCatalogWriter, CatalogReader, CatalogWriter};
use pbs_datastore::chunk_store::verify_chunk_size;
@@ -340,9 +340,9 @@ pub fn optional_ns_param(param: &Value) -> Result<BackupNamespace, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
"ns": {
type: BackupNamespace,
@@ -433,9 +433,9 @@ fn merge_group_into(to: &mut serde_json::Map<String, Value>, group: BackupGroup)
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
group: {
type: String,
@@ -478,9 +478,9 @@ async fn change_backup_owner(group: String, mut param: Value) -> Result<(), Erro
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
}
}
@@ -500,9 +500,9 @@ async fn api_login(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
}
}
@@ -519,9 +519,9 @@ fn api_logout(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
"output-format": {
schema: OUTPUT_FORMAT,
@@ -572,9 +572,9 @@ async fn api_version(param: Value) -> Result<(), Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
"output-format": {
schema: OUTPUT_FORMAT,
@@ -662,9 +662,9 @@ fn spawn_catalog_upload(
schema: BACKUP_SOURCE_SCHEMA,
}
},
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
"include-dev": {
description:
@@ -1439,9 +1439,9 @@ async fn dump_image<W: Write>(
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -1837,9 +1837,9 @@ async fn restore(
default: false,
description: "Minimal output - only show removals.",
},
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
},
},
@@ -1929,9 +1929,9 @@ async fn prune(
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
"output-format": {
schema: OUTPUT_FORMAT,
diff --git a/proxmox-backup-client/src/mount.rs b/proxmox-backup-client/src/mount.rs
index e815c8a9c..bc666a18f 100644
--- a/proxmox-backup-client/src/mount.rs
+++ b/proxmox-backup-client/src/mount.rs
@@ -17,11 +17,12 @@ use proxmox_router::{cli::*, ApiHandler, ApiMethod, RpcEnvironment};
use proxmox_schema::*;
use proxmox_sortable_macro::sortable;
-use pbs_api_types::{ArchiveType, BackupArchiveName, BackupNamespace};
+use pbs_api_types::{ArchiveType, Authid, BackupArchiveName, BackupNamespace, DATASTORE_SCHEMA};
use pbs_client::tools::key_source::{
crypto_parameters, format_key_source, get_encryption_key_password,
};
use pbs_client::{BackupReader, RemoteChunkReader};
+use pbs_client::{BACKUP_REPO_PORT_SCHEMA, BACKUP_REPO_SERVER_SCHEMA, REPO_URL_SCHEMA};
use pbs_datastore::cached_chunk_reader::CachedChunkReader;
use pbs_datastore::index::IndexFile;
use pbs_key_config::decrypt_key;
@@ -32,7 +33,7 @@ use crate::helper;
use crate::{
complete_group_or_snapshot, complete_img_archive_name, complete_namespace,
complete_pxar_archive_name, complete_repository, connect, dir_or_last_from_group,
- extract_repository_from_value, optional_ns_param, record_repository, REPO_URL_SCHEMA,
+ extract_repository_from_value, optional_ns_param, record_repository,
};
#[sortable]
@@ -54,6 +55,10 @@ const API_METHOD_MOUNT: ApiMethod = ApiMethod::new(
&StringSchema::new("Target directory path.").schema()
),
("repository", true, &REPO_URL_SCHEMA),
+ ("server", true, &BACKUP_REPO_SERVER_SCHEMA),
+ ("port", true, &BACKUP_REPO_PORT_SCHEMA),
+ ("datastore", true, &DATASTORE_SCHEMA),
+ ("auth-id", true, &Authid::API_SCHEMA),
(
"keyfile",
true,
@@ -85,6 +90,10 @@ WARNING: Only do this with *trusted* backups!",
),
("archive-name", false, &BackupArchiveName::API_SCHEMA),
("repository", true, &REPO_URL_SCHEMA),
+ ("server", true, &BACKUP_REPO_SERVER_SCHEMA),
+ ("port", true, &BACKUP_REPO_PORT_SCHEMA),
+ ("datastore", true, &DATASTORE_SCHEMA),
+ ("auth-id", true, &Authid::API_SCHEMA),
(
"keyfile",
true,
diff --git a/proxmox-backup-client/src/namespace.rs b/proxmox-backup-client/src/namespace.rs
index 2929e394b..53d4c31af 100644
--- a/proxmox-backup-client/src/namespace.rs
+++ b/proxmox-backup-client/src/namespace.rs
@@ -2,7 +2,7 @@ use anyhow::{bail, Error};
use serde_json::{json, Value};
use pbs_api_types::BackupNamespace;
-use pbs_client::tools::REPO_URL_SCHEMA;
+use pbs_client::BackupRepositoryArgs;
use proxmox_router::cli::{
format_and_print_result, get_output_format, CliCommand, CliCommandMap, OUTPUT_FORMAT,
@@ -17,9 +17,9 @@ use crate::{
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -84,9 +84,9 @@ async fn list_namespaces(param: Value, max_depth: Option<usize>) -> Result<(), E
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -124,9 +124,9 @@ async fn create_namespace(param: Value) -> Result<(), Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
diff --git a/proxmox-backup-client/src/snapshot.rs b/proxmox-backup-client/src/snapshot.rs
index 500fd2bb8..7c7217891 100644
--- a/proxmox-backup-client/src/snapshot.rs
+++ b/proxmox-backup-client/src/snapshot.rs
@@ -17,8 +17,8 @@ use pbs_tools::json::required_string_param;
use crate::{
api_datastore_list_snapshots, complete_backup_group, complete_backup_snapshot,
complete_namespace, complete_repository, connect, crypto_parameters,
- extract_repository_from_value, optional_ns_param, record_repository, BackupDir, KEYFD_SCHEMA,
- KEYFILE_SCHEMA, REPO_URL_SCHEMA,
+ extract_repository_from_value, optional_ns_param, record_repository, BackupDir,
+ BackupRepositoryArgs, KEYFD_SCHEMA, KEYFILE_SCHEMA,
};
fn snapshot_args(ns: &BackupNamespace, snapshot: &BackupDir) -> Result<Value, Error> {
@@ -32,9 +32,9 @@ fn snapshot_args(ns: &BackupNamespace, snapshot: &BackupDir) -> Result<Value, Er
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -108,9 +108,9 @@ async fn list_snapshots(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -161,9 +161,9 @@ async fn list_snapshot_files(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -200,9 +200,9 @@ async fn forget_snapshots(param: Value) -> Result<(), Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -281,9 +281,9 @@ async fn upload_log(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -338,9 +338,9 @@ async fn show_notes(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -380,9 +380,9 @@ async fn update_notes(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -437,9 +437,9 @@ async fn show_protection(param: Value) -> Result<(), Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
diff --git a/proxmox-backup-client/src/task.rs b/proxmox-backup-client/src/task.rs
index 472db0860..dba50cc85 100644
--- a/proxmox-backup-client/src/task.rs
+++ b/proxmox-backup-client/src/task.rs
@@ -10,14 +10,14 @@ use pbs_tools::json::required_string_param;
use pbs_api_types::UPID;
-use crate::{complete_repository, connect, extract_repository_from_value, REPO_URL_SCHEMA};
+use crate::{complete_repository, connect, extract_repository_from_value, BackupRepositoryArgs};
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
limit: {
description: "The maximal number of tasks to list.",
@@ -87,9 +87,9 @@ async fn task_list(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
upid: {
type: UPID,
@@ -112,9 +112,9 @@ async fn task_log(param: Value) -> Result<Value, Error> {
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
upid: {
type: UPID,
diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
index bf6cf9aed..99685bafc 100644
--- a/proxmox-file-restore/src/main.rs
+++ b/proxmox-file-restore/src/main.rs
@@ -31,9 +31,8 @@ use pbs_client::tools::{
crypto_parameters_keep_fd, format_key_source, get_encryption_key_password, KEYFD_SCHEMA,
KEYFILE_SCHEMA,
},
- REPO_URL_SCHEMA,
};
-use pbs_client::{BackupReader, BackupRepository, RemoteChunkReader};
+use pbs_client::{BackupReader, BackupRepository, BackupRepositoryArgs, RemoteChunkReader};
use pbs_datastore::catalog::{ArchiveEntry, CatalogReader, DirEntryAttribute};
use pbs_datastore::dynamic_index::BufferedDynamicReader;
use pbs_datastore::index::IndexFile;
@@ -212,9 +211,9 @@ async fn list_files(
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
@@ -361,9 +360,9 @@ async fn list(
#[api(
input: {
properties: {
- repository: {
- schema: REPO_URL_SCHEMA,
- optional: true,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
ns: {
type: BackupNamespace,
diff --git a/src/bin/proxmox_backup_debug/diff.rs b/src/bin/proxmox_backup_debug/diff.rs
index 116216e51..f13cc6ab5 100644
--- a/src/bin/proxmox_backup_debug/diff.rs
+++ b/src/bin/proxmox_backup_debug/diff.rs
@@ -19,9 +19,8 @@ use pbs_client::tools::key_source::{
};
use pbs_client::tools::{
complete_archive_name, complete_group_or_snapshot, connect, extract_repository_from_value,
- REPO_URL_SCHEMA,
};
-use pbs_client::{BackupReader, BackupRepository, RemoteChunkReader};
+use pbs_client::{BackupReader, BackupRepository, BackupRepositoryArgs, RemoteChunkReader};
use pbs_datastore::dynamic_index::{BufferedDynamicReader, DynamicIndexReader, LocalDynamicReadAt};
use pbs_datastore::index::IndexFile;
use pbs_key_config::decrypt_key;
@@ -72,9 +71,9 @@ pub fn diff_commands() -> CommandLineInterface {
"archive-name": {
type: BackupArchiveName,
},
- "repository": {
- optional: true,
- schema: REPO_URL_SCHEMA,
+ repo: {
+ type: BackupRepositoryArgs,
+ flatten: true,
},
"keyfile": {
optional: true,
--
2.47.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] client: support individual repository component parameters
2026-03-23 21:11 [PATCH] client: support individual repository component parameters Thomas Lamprecht
@ 2026-03-24 13:42 ` Fabian Grünbichler
2026-03-25 11:28 ` Thomas Lamprecht
2026-03-24 15:58 ` Maximiliano Sandoval
1 sibling, 1 reply; 4+ messages in thread
From: Fabian Grünbichler @ 2026-03-24 13:42 UTC (permalink / raw)
To: pbs-devel, Thomas Lamprecht
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> {
> [..]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] client: support individual repository component parameters
2026-03-23 21:11 [PATCH] client: support individual repository component parameters Thomas Lamprecht
2026-03-24 13:42 ` Fabian Grünbichler
@ 2026-03-24 15:58 ` Maximiliano Sandoval
1 sibling, 0 replies; 4+ messages in thread
From: Maximiliano Sandoval @ 2026-03-24 15:58 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: pbs-devel
Thomas Lamprecht <t.lamprecht@proxmox.com> writes:
I will test this during the week. Having something akin to these
parameters has been in my TODO-list/wishlist since forever.
Some comments below.
> 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``.
I would mention here that this requires the datastore option, like the
docs a couple of chunks below do.
> +
> +``--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:
> +
> +.. 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
This makes me wonder if there should too be a dedicated --api-token
parameter too (that basically appends `!tokenname` to the user). I ran
into this issue a couple of times in the past, while having it in the
documentation is good, perhaps it is not the first place one would look
for this.
> +
> 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.
I am not sure about the use of "default", since one has to specify one
or the other. I would personally say something like:
Backup server address. Requires to be used in conjunction to --datastore
or PBS_DATASTORE. This option is not compatible with --repository or
PBS_REPOSITORY.
> +
> +``PBS_PORT``
> + Default backup server port. Defaults to ``8007`` if unset.
> +
> [...]
> "keyfile": {
> optional: true,
--
Maximiliano
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] client: support individual repository component parameters
2026-03-24 13:42 ` Fabian Grünbichler
@ 2026-03-25 11:28 ` Thomas Lamprecht
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2026-03-25 11:28 UTC (permalink / raw)
To: Fabian Grünbichler, pbs-devel
Am 24.03.26 um 14:42 schrieb Fabian Grünbichler:
> On March 23, 2026 10:11 pm, Thomas Lamprecht wrote:
>> +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`..
yeah, that was more for inside perl use cases, but it's bogus; will drop.
[...]
>> @@ -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
Yeah, that makes sense to incorporate in this series, thanks for the pointer.
>> 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};
[...]
>> +/// 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..
ack
>> +/// 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?
I checked a few CLI tools and most allow mixing environment and CLI for
different such atoms, e.g. restic or borg, but also others like mysql or
psql, and rethinking this also what I would expect to happen. I'll recheck
this and try to add some unit tests to check and encode the inteded behavior.
>> +/// 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
>
Will recheck more closely, thanks for taking a look!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-25 11:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-23 21:11 [PATCH] client: support individual repository component parameters Thomas Lamprecht
2026-03-24 13:42 ` Fabian Grünbichler
2026-03-25 11:28 ` Thomas Lamprecht
2026-03-24 15:58 ` Maximiliano Sandoval
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.