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 1B2899BEFC for ; Tue, 21 Nov 2023 15:32:36 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EB565BB42 for ; Tue, 21 Nov 2023 15:32:05 +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 ; Tue, 21 Nov 2023 15:32:03 +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 97A9041863 for ; Tue, 21 Nov 2023 15:32:03 +0100 (CET) From: Hannes Laimer To: pbs-devel@lists.proxmox.com Date: Tue, 21 Nov 2023 15:31:54 +0100 Message-Id: <20231121143155.370916-6-h.laimer@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231121143155.370916-1-h.laimer@proxmox.com> References: <20231121143155.370916-1-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.007 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH proxmox-backup v6 5/6] api: make Remote for SyncJob optional 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: Tue, 21 Nov 2023 14:32:36 -0000 Signed-off-by: Hannes Laimer --- pbs-api-types/src/jobs.rs | 9 ++++-- src/api2/config/remote.rs | 2 +- src/api2/config/sync.rs | 41 +++++++++++++++-------- src/api2/node/tasks.rs | 3 +- src/api2/pull.rs | 54 ++++++++++++++++++++++--------- src/bin/proxmox-backup-manager.rs | 4 +-- src/server/email_notifications.rs | 18 ++++++----- 7 files changed, 87 insertions(+), 44 deletions(-) diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs index 7b5a1f3d..b8640216 100644 --- a/pbs-api-types/src/jobs.rs +++ b/pbs-api-types/src/jobs.rs @@ -17,8 +17,8 @@ const_regex! { /// Regex for verification jobs 'DATASTORE:ACTUAL_JOB_ID' pub VERIFICATION_JOB_WORKER_ID_REGEX = concat!(r"^(", PROXMOX_SAFE_ID_REGEX_STR!(), r"):"); - /// Regex for sync jobs 'REMOTE:REMOTE_DATASTORE:LOCAL_DATASTORE:(?:LOCAL_NS_ANCHOR:)ACTUAL_JOB_ID' - pub SYNC_JOB_WORKER_ID_REGEX = concat!(r"^(", PROXMOX_SAFE_ID_REGEX_STR!(), r"):(", PROXMOX_SAFE_ID_REGEX_STR!(), r"):(", PROXMOX_SAFE_ID_REGEX_STR!(), r")(?::(", BACKUP_NS_RE!(), r"))?:"); + /// Regex for sync jobs '(REMOTE|\-):REMOTE_DATASTORE:LOCAL_DATASTORE:(?:LOCAL_NS_ANCHOR:)ACTUAL_JOB_ID' + pub SYNC_JOB_WORKER_ID_REGEX = concat!(r"^(", PROXMOX_SAFE_ID_REGEX_STR!(), r"|\-):(", PROXMOX_SAFE_ID_REGEX_STR!(), r"):(", PROXMOX_SAFE_ID_REGEX_STR!(), r")(?::(", BACKUP_NS_RE!(), r"))?:"); } pub const JOB_ID_SCHEMA: Schema = StringSchema::new("Job ID.") @@ -471,6 +471,7 @@ pub const TRANSFER_LAST_SCHEMA: Schema = }, remote: { schema: REMOTE_ID_SCHEMA, + optional: true, }, "remote-store": { schema: DATASTORE_SCHEMA, @@ -519,7 +520,9 @@ pub struct SyncJobConfig { pub ns: Option, #[serde(skip_serializing_if = "Option::is_none")] pub owner: Option, - pub remote: String, + #[serde(skip_serializing_if = "Option::is_none")] + /// None implies local sync. + pub remote: Option, pub remote_store: String, #[serde(skip_serializing_if = "Option::is_none")] pub remote_ns: Option, diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs index 31922b94..2511c5d5 100644 --- a/src/api2/config/remote.rs +++ b/src/api2/config/remote.rs @@ -268,7 +268,7 @@ pub fn delete_remote(name: String, digest: Option) -> Result<(), Error> let job_list: Vec = sync_jobs.convert_to_typed_array("sync")?; for job in job_list { - if job.remote == name { + if job.remote.map_or(false, |id| id == name) { param_bail!( "name", "remote '{}' is used by sync job '{}' (datastore '{}')", diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs index 01e5f2ce..21634bd5 100644 --- a/src/api2/config/sync.rs +++ b/src/api2/config/sync.rs @@ -8,8 +8,8 @@ use proxmox_schema::{api, param_bail}; use pbs_api_types::{ Authid, SyncJobConfig, SyncJobConfigUpdater, JOB_ID_SCHEMA, PRIV_DATASTORE_AUDIT, - PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_REMOTE_AUDIT, - PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA, + PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, + PRIV_REMOTE_AUDIT, PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA, }; use pbs_config::sync; @@ -25,8 +25,13 @@ pub fn check_sync_job_read_access( return false; } - let remote_privs = user_info.lookup_privs(auth_id, &["remote", &job.remote]); - remote_privs & PRIV_REMOTE_AUDIT != 0 + if let Some(remote) = &job.remote { + let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote]); + remote_privs & PRIV_REMOTE_AUDIT != 0 + } else { + let source_ds_privs = user_info.lookup_privs(auth_id, &["datastore", &job.remote_store]); + source_ds_privs & PRIV_DATASTORE_AUDIT != 0 + } } /// checks whether user can run the corresponding pull job @@ -63,8 +68,13 @@ pub fn check_sync_job_modify_access( return false; } - let remote_privs = user_info.lookup_privs(auth_id, &["remote", &job.remote, &job.remote_store]); - remote_privs & PRIV_REMOTE_READ != 0 + if let Some(remote) = &job.remote { + let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote, &job.remote_store]); + remote_privs & PRIV_REMOTE_READ != 0 + } else { + let source_ds_privs = user_info.lookup_privs(auth_id, &["datastore", &job.remote_store]); + source_ds_privs & PRIV_DATASTORE_READ != 0 + } } #[api( @@ -191,6 +201,8 @@ pub fn read_sync_job(id: String, rpcenv: &mut dyn RpcEnvironment) -> Result meaning local). + Remote, /// Delete the owner property. Owner, /// Delete the comment property. @@ -275,6 +287,9 @@ pub fn update_sync_job( if let Some(delete) = delete { for delete_prop in delete { match delete_prop { + DeletableProperty::Remote => { + data.remote = None; + } DeletableProperty::Owner => { data.owner = None; } @@ -334,7 +349,7 @@ pub fn update_sync_job( data.ns = Some(ns); } if let Some(remote) = update.remote { - data.remote = remote; + data.remote = Some(remote); } if let Some(remote_store) = update.remote_store { data.remote_store = remote_store; @@ -503,7 +518,7 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator let mut job = SyncJobConfig { id: "regular".to_string(), - remote: "remote0".to_string(), + remote: Some("remote0".to_string()), remote_store: "remotestore1".to_string(), remote_ns: None, store: "localstore0".to_string(), @@ -538,11 +553,11 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job)); // reading without proper read permissions on local end must fail - job.remote = "remote1".to_string(); + job.remote = Some("remote1".to_string()); assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job)); // reading without proper read permissions on remote end must fail - job.remote = "remote0".to_string(); + job.remote = Some("remote0".to_string()); job.store = "localstore1".to_string(); assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job)); @@ -555,10 +570,10 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator )); // writing without proper write permissions on local end must fail - job.remote = "remote1".to_string(); + job.remote = Some("remote1".to_string()); // writing without proper write permissions on remote end must fail - job.remote = "remote0".to_string(); + job.remote = Some("remote0".to_string()); job.store = "localstore1".to_string(); assert!(!check_sync_job_modify_access( &user_info, @@ -567,7 +582,7 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator )); // reset remote to one where users have access - job.remote = "remote1".to_string(); + job.remote = Some("remote1".to_string()); // user with read permission can only read, but not modify/run assert!(check_sync_job_read_access(&user_info, &read_auth_id, &job)); diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs index 866361c6..8f08d3af 100644 --- a/src/api2/node/tasks.rs +++ b/src/api2/node/tasks.rs @@ -78,11 +78,12 @@ fn check_job_privs(auth_id: &Authid, user_info: &CachedUserInfo, upid: &UPID) -> if let (Some(remote), Some(remote_store), Some(local_store)) = (remote, remote_store, local_store) { + let remote_str = remote.as_str(); return check_pull_privs( auth_id, local_store.as_str(), local_ns, - remote.as_str(), + (remote_str != "-").then_some(remote_str), remote_store.as_str(), false, ); diff --git a/src/api2/pull.rs b/src/api2/pull.rs index 8cd28fa0..eb9a2199 100644 --- a/src/api2/pull.rs +++ b/src/api2/pull.rs @@ -1,5 +1,5 @@ //! Sync datastore from remote server -use anyhow::{format_err, Error}; +use anyhow::{bail, format_err, Error}; use futures::{future::FutureExt, select}; use proxmox_router::{Permission, Router, RpcEnvironment}; @@ -22,7 +22,7 @@ pub fn check_pull_privs( auth_id: &Authid, store: &str, ns: Option<&str>, - remote: &str, + remote: Option<&str>, remote_store: &str, delete: bool, ) -> Result<(), Error> { @@ -39,12 +39,22 @@ pub fn check_pull_privs( PRIV_DATASTORE_BACKUP, false, )?; - user_info.check_privs( - auth_id, - &["remote", remote, remote_store], - PRIV_REMOTE_READ, - false, - )?; + + if let Some(remote) = remote { + user_info.check_privs( + auth_id, + &["remote", remote, remote_store], + PRIV_REMOTE_READ, + false, + )?; + } else { + user_info.check_privs( + auth_id, + &["datastore", remote_store], + PRIV_DATASTORE_BACKUP, + false, + )?; + } if delete { user_info.check_privs( @@ -65,7 +75,7 @@ impl TryFrom<&SyncJobConfig> for PullParameters { PullParameters::new( &sync_job.store, sync_job.ns.clone().unwrap_or_default(), - Some(&sync_job.remote), + sync_job.remote.as_deref(), &sync_job.remote_store, sync_job.remote_ns.clone().unwrap_or_default(), sync_job @@ -91,7 +101,7 @@ pub fn do_sync_job( ) -> Result { let job_id = format!( "{}:{}:{}:{}:{}", - sync_job.remote, + sync_job.remote.as_deref().unwrap_or("-"), sync_job.remote_store, sync_job.store, sync_job.ns.clone().unwrap_or_default(), @@ -99,6 +109,10 @@ pub fn do_sync_job( ); let worker_type = job.jobtype().to_string(); + if sync_job.remote.is_none() && sync_job.store == sync_job.remote_store { + bail!("can't sync to same datastore"); + } + let (email, notify) = crate::server::lookup_datastore_notify_settings(&sync_job.store); let upid_str = WorkerTask::spawn( @@ -121,9 +135,12 @@ pub fn do_sync_job( } task_log!( worker, - "sync datastore '{}' from '{}/{}'", + "sync datastore '{}' from '{}{}'", sync_job.store, - sync_job.remote, + sync_job + .remote + .as_deref() + .map_or(String::new(), |remote| format!("{remote}/")), sync_job.remote_store, ); @@ -179,6 +196,7 @@ pub fn do_sync_job( }, remote: { schema: REMOTE_ID_SCHEMA, + optional: true, }, "remote-store": { schema: DATASTORE_SCHEMA, @@ -223,7 +241,7 @@ The delete flag additionally requires the Datastore.Prune privilege on '/datasto async fn pull( store: String, ns: Option, - remote: String, + remote: Option, remote_store: String, remote_ns: Option, remove_vanished: Option, @@ -236,6 +254,10 @@ async fn pull( let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let delete = remove_vanished.unwrap_or(false); + if remote.is_none() && store == remote_store { + bail!("can't sync to same datastore"); + } + let ns = ns.unwrap_or_default(); let ns_str = if ns.is_root() { None @@ -247,7 +269,7 @@ async fn pull( &auth_id, &store, ns_str.as_deref(), - &remote, + remote.as_deref(), &remote_store, delete, )?; @@ -255,7 +277,7 @@ async fn pull( let pull_params = PullParameters::new( &store, ns, - Some(&remote), + remote.as_deref(), &remote_store, remote_ns.unwrap_or_default(), auth_id.clone(), @@ -278,7 +300,7 @@ async fn pull( worker, "pull datastore '{}' from '{}/{}'", store, - remote, + remote.as_deref().unwrap_or("-"), remote_store, ); diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs index b4948e43..eadfe547 100644 --- a/src/bin/proxmox-backup-manager.rs +++ b/src/bin/proxmox-backup-manager.rs @@ -535,7 +535,7 @@ fn get_remote(param: &HashMap) -> Option { param.get("remote").map(|r| r.to_owned()).or_else(|| { if let Some(id) = param.get("id") { if let Ok(job) = get_sync_job(id) { - return Some(job.remote); + return job.remote; } } None @@ -549,7 +549,7 @@ fn get_remote_store(param: &HashMap) -> Option<(Option, if let Some(id) = param.get("id") { job = get_sync_job(id).ok(); if let Some(ref job) = job { - return Some(job.remote.clone()); + return job.remote.clone(); } } None diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs index ea1476d7..18881782 100644 --- a/src/server/email_notifications.rs +++ b/src/server/email_notifications.rs @@ -484,15 +484,17 @@ pub fn send_sync_status( } }; + let tmp_src_string; + let source_str = if let Some(remote) = &job.remote { + tmp_src_string = format!("Sync remote '{}'", remote); + &tmp_src_string + } else { + "Sync local" + }; + let subject = match result { - Ok(()) => format!( - "Sync remote '{}' datastore '{}' successful", - job.remote, job.remote_store, - ), - Err(_) => format!( - "Sync remote '{}' datastore '{}' failed", - job.remote, job.remote_store, - ), + Ok(()) => format!("{} datastore '{}' successful", source_str, job.remote_store,), + Err(_) => format!("{} datastore '{}' failed", source_str, job.remote_store,), }; send_job_status_mail(email, &subject, &text)?; -- 2.39.2