From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id E8FF096839 for ; Wed, 25 Jan 2023 13:19:58 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 72E71F95E for ; Wed, 25 Jan 2023 13:19:31 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 25 Jan 2023 13:19:29 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 5B6804615B for ; Wed, 25 Jan 2023 13:19:29 +0100 (CET) From: Christoph Heiss To: pbs-devel@lists.proxmox.com Date: Wed, 25 Jan 2023 13:18:58 +0100 Message-Id: <20230125121902.404950-4-c.heiss@proxmox.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230125121902.404950-1-c.heiss@proxmox.com> References: <20230125121902.404950-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 2.448 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_HI -5 Sender listed at https://www.dnswl.org/, high trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs, upload-speed.rs, main.rs, proxmox.com, benchmark.rs] Subject: [pbs-devel] [RFC PATCH v2 proxmox-backup 3/3] client: Add `--protected` CLI flag to backup command X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Jan 2023 12:19:59 -0000 This implements setting the backup as protected after finishing it, either using the newly introduced API parameter on the `/finish` endpoint, falling back to the "old" way using the separate endpoint, thus supporting this in a backwards-compatible way. The same caveat as in bug #4289 [0] applies here as well: If the datastore has "Verify New Snapshots" set, the latter way might fail to set the backup as protected. [0] https://bugzilla.proxmox.com/show_bug.cgi?id=4289 Signed-off-by: Christoph Heiss --- Note: BackupWriter::start() actually does fine with just a &HttpClient, since HttpClient::start_h2_connection() then .clone()'s it anyway and not consume it. Changes v1 -> v2: * Moved BackupWriter::finish adaption to this patch * Only set API parameter if available using new server feature mechanism * Implement best-effort backwards-compatible way for older server examples/upload-speed.rs | 2 +- pbs-client/src/backup_writer.rs | 6 +-- pbs-client/src/tools/mod.rs | 25 ++++++++- proxmox-backup-client/src/benchmark.rs | 2 +- proxmox-backup-client/src/main.rs | 72 ++++++++++++++++++++------ 5 files changed, 86 insertions(+), 21 deletions(-) diff --git a/examples/upload-speed.rs b/examples/upload-speed.rs index f9fc52a8..e4b570ec 100644 --- a/examples/upload-speed.rs +++ b/examples/upload-speed.rs @@ -18,7 +18,7 @@ async fn upload_speed() -> Result { let backup_time = proxmox_time::epoch_i64(); let client = BackupWriter::start( - client, + &client, None, datastore, &BackupNamespace::root(), diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs index be6da2a6..4ecc6622 100644 --- a/pbs-client/src/backup_writer.rs +++ b/pbs-client/src/backup_writer.rs @@ -76,7 +76,7 @@ impl BackupWriter { // FIXME: extract into (flattened) parameter struct? #[allow(clippy::too_many_arguments)] pub async fn start( - client: HttpClient, + client: &HttpClient, crypt_config: Option>, datastore: &str, ns: &BackupNamespace, @@ -165,10 +165,10 @@ impl BackupWriter { self.h2.upload("PUT", path, param, content_type, data).await } - pub async fn finish(self: Arc) -> Result<(), Error> { + pub async fn finish(self: Arc, param: Option) -> Result<(), Error> { let h2 = self.h2.clone(); - h2.post("finish", None) + h2.post("finish", param) .map_ok(move |_| { self.abort.abort(); }) diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs index fa1d5092..140ef9c7 100644 --- a/pbs-client/src/tools/mod.rs +++ b/pbs-client/src/tools/mod.rs @@ -15,7 +15,9 @@ use proxmox_router::cli::{complete_file_name, shellword_split}; use proxmox_schema::*; use proxmox_sys::fs::file_get_json; -use pbs_api_types::{Authid, BackupNamespace, RateLimitConfig, UserWithTokens, BACKUP_REPO_URL}; +use pbs_api_types::{ + Authid, BackupNamespace, RateLimitConfig, ServerFeature, UserWithTokens, BACKUP_REPO_URL, +}; use crate::{BackupRepository, HttpClient, HttpClientOptions}; @@ -502,6 +504,27 @@ pub async fn complete_namespace_do( result } +/// Returns a list of (backwards-incompatible) features supported by the server. +pub async fn get_supported_server_features(client: &HttpClient) -> Vec { + match client.get("api2/json/version", None).await { + Ok(mut result) if result["data"]["features"].is_array() => { + match serde_json::from_value::>(result["data"]["features"].take()) { + Ok(features) => features, + Err(e) => { + log::warn!("failed to deserialize feature list: {}", e); + vec![] + } + } + } + // Old server with no feature advertising yet. + Ok(_) => vec![], + Err(e) => { + log::error!("could not connect to server - {}", e); + vec![] + } + } +} + pub fn base_directories() -> Result { xdg::BaseDirectories::with_prefix("proxmox-backup").map_err(Error::from) } diff --git a/proxmox-backup-client/src/benchmark.rs b/proxmox-backup-client/src/benchmark.rs index b3047308..1262fb46 100644 --- a/proxmox-backup-client/src/benchmark.rs +++ b/proxmox-backup-client/src/benchmark.rs @@ -229,7 +229,7 @@ async fn test_upload_speed( log::debug!("Connecting to backup server"); let client = BackupWriter::start( - client, + &client, crypt_config.clone(), repo.store(), &BackupNamespace::root(), diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs index 55198108..bab67207 100644 --- a/proxmox-backup-client/src/main.rs +++ b/proxmox-backup-client/src/main.rs @@ -25,15 +25,16 @@ use pxar::accessor::{MaybeReady, ReadAt, ReadAtOperation}; use pbs_api_types::{ Authid, BackupDir, BackupGroup, BackupNamespace, BackupPart, BackupType, CryptMode, Fingerprint, GroupListItem, HumanByte, PruneJobOptions, PruneListItem, RateLimitConfig, - SnapshotListItem, StorageStatus, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, - BACKUP_TYPE_SCHEMA, TRAFFIC_CONTROL_BURST_SCHEMA, TRAFFIC_CONTROL_RATE_SCHEMA, + ServerFeature, SnapshotListItem, StorageStatus, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, + BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, TRAFFIC_CONTROL_BURST_SCHEMA, + TRAFFIC_CONTROL_RATE_SCHEMA, }; use pbs_client::catalog_shell::Shell; use pbs_client::tools::{ complete_archive_name, complete_auth_id, complete_backup_group, complete_backup_snapshot, complete_backup_source, complete_chunk_size, complete_group_or_snapshot, complete_img_archive_name, complete_namespace, complete_pxar_archive_name, complete_repository, - connect, connect_rate_limited, extract_repository_from_value, + connect, connect_rate_limited, extract_repository_from_value, get_supported_server_features, key_source::{ crypto_parameters, format_key_source, get_encryption_key_password, KEYFD_SCHEMA, KEYFILE_SCHEMA, MASTER_PUBKEY_FD_SCHEMA, MASTER_PUBKEY_FILE_SCHEMA, @@ -663,6 +664,12 @@ fn spawn_catalog_upload( optional: true, default: false, }, + "protected": { + type: Boolean, + description: "Set backup as protected after upload.", + optional: true, + default: false, + }, } } )] @@ -716,6 +723,7 @@ async fn create_backup( let empty = Vec::new(); let exclude_args = param["exclude"].as_array().unwrap_or(&empty); + let protected = param["protected"].as_bool().unwrap_or(false); let mut pattern_list = Vec::with_capacity(exclude_args.len()); for entry in exclude_args { @@ -837,6 +845,9 @@ async fn create_backup( log::info!("Client name: {}", proxmox_sys::nodename()); + let server_features = get_supported_server_features(&client).await; + log::debug!("Supported server features: {:?}", server_features); + let start_time = std::time::Instant::now(); log::info!( @@ -876,8 +887,8 @@ async fn create_backup( } }; - let client = BackupWriter::start( - client, + let writer = BackupWriter::start( + &client, crypt_config.clone(), repo.store(), &backup_ns, @@ -887,7 +898,7 @@ async fn create_backup( ) .await?; - let download_previous_manifest = match client.previous_backup_time().await { + let download_previous_manifest = match writer.previous_backup_time().await { Ok(Some(backup_time)) => { log::info!( "Downloading previous manifest ({})", @@ -906,7 +917,7 @@ async fn create_backup( }; let previous_manifest = if download_previous_manifest { - match client.download_previous_manifest().await { + match writer.download_previous_manifest().await { Ok(previous_manifest) => { match previous_manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref)) { Ok(()) => Some(Arc::new(previous_manifest)), @@ -951,7 +962,7 @@ async fn create_backup( }; log_file("config file", &filename, &target); - let stats = client + let stats = writer .upload_blob_from_file(&filename, &target, upload_options) .await?; manifest.add_file(target, stats.size, stats.csum, crypto.mode)?; @@ -965,7 +976,7 @@ async fn create_backup( }; log_file("log file", &filename, &target); - let stats = client + let stats = writer .upload_blob_from_file(&filename, &target, upload_options) .await?; manifest.add_file(target, stats.size, stats.csum, crypto.mode)?; @@ -974,7 +985,7 @@ async fn create_backup( // start catalog upload on first use if catalog.is_none() { let catalog_upload_res = - spawn_catalog_upload(client.clone(), crypto.mode == CryptMode::Encrypt)?; + spawn_catalog_upload(writer.clone(), crypto.mode == CryptMode::Encrypt)?; catalog = Some(catalog_upload_res.catalog_writer); catalog_result_rx = Some(catalog_upload_res.result); } @@ -1001,7 +1012,7 @@ async fn create_backup( }; let stats = backup_directory( - &client, + &writer, &filename, &target, chunk_size_opt, @@ -1024,7 +1035,7 @@ async fn create_backup( }; let stats = - backup_image(&client, &filename, &target, chunk_size_opt, upload_options) + backup_image(&writer, &filename, &target, chunk_size_opt, upload_options) .await?; manifest.add_file(target, stats.size, stats.csum, crypto.mode)?; } @@ -1060,7 +1071,7 @@ async fn create_backup( encrypt: false, ..UploadOptions::default() }; - let stats = client + let stats = writer .upload_blob_from_data(rsa_encrypted_key, target, options) .await?; manifest.add_file(target.to_string(), stats.size, stats.csum, crypto.mode)?; @@ -1078,11 +1089,42 @@ async fn create_backup( encrypt: false, ..UploadOptions::default() }; - client + writer .upload_blob_from_data(manifest.into_bytes(), MANIFEST_BLOB_NAME, options) .await?; - client.finish().await?; + + let mut params = json!({}); + + let has_protected_param = server_features.contains(&ServerFeature::FinishHasProtectedParam); + if has_protected_param { + params["protected"] = json!(protected); + } + + writer.finish(Some(params)).await?; + + // In case the server does not support the new `protected` parameter and the user has requested + // to set the backup as protected, fall back to the separate API endpoint as a "best-effort" + // solution. This might fail for larger backups if the datastore has "Verify New Snapshots" + // enabled. + if !has_protected_param && protected { + let path = format!("api2/json/admin/datastore/{}/protected", repo.store()); + + let mut params = json!({ + "backup-id": backup_id, + "backup-time": backup_time, + "backup-type": backup_type, + "protected": protected, + }); + + if backup_ns.is_root() { + params["ns"] = json!(backup_ns); + } + + if let Err(e) = client.put(&path, Some(params)).await { + log::warn!("Unable to set backup as protected: {}", e); + } + } let end_time = std::time::Instant::now(); let elapsed = end_time.duration_since(start_time); -- 2.34.1