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 2AFFE9829E for ; Fri, 6 Oct 2023 16:06:07 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0E3881B9F for ; Fri, 6 Oct 2023 16:05:37 +0200 (CEST) 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 ; Fri, 6 Oct 2023 16:05:35 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6F062448E8 for ; Fri, 6 Oct 2023 16:05:35 +0200 (CEST) From: Hannes Laimer To: pbs-devel@lists.proxmox.com Date: Fri, 6 Oct 2023 16:05:24 +0200 Message-Id: <20231006140529.723988-2-h.laimer@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231006140529.723988-1-h.laimer@proxmox.com> References: <20231006140529.723988-1-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.012 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [sync.rs, remote.rs, pull.rs, jobs.rs, proxmox-backup-manager.rs, job.store, tasks.rs] Subject: [pbs-devel] [PATCH proxmox-backup v5 1/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: Fri, 06 Oct 2023 14:06:07 -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 | 76 +++++++++++++++++++++++-------- src/bin/proxmox-backup-manager.rs | 6 +-- src/server/email_notifications.rs | 18 ++++---- 7 files changed, 107 insertions(+), 48 deletions(-) diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs index 23e19b7b..aedee090 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.") @@ -467,6 +467,7 @@ pub const TRANSFER_LAST_SCHEMA: Schema = }, remote: { schema: REMOTE_ID_SCHEMA, + optional: true, }, "remote-store": { schema: DATASTORE_SCHEMA, @@ -515,7 +516,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 76dd3b89..307cf3cd 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 daeba7cf..9ed83046 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}; @@ -8,7 +8,7 @@ use proxmox_sys::task_log; use pbs_api_types::{ Authid, BackupNamespace, GroupFilter, RateLimitConfig, SyncJobConfig, DATASTORE_SCHEMA, - GROUP_FILTER_LIST_SCHEMA, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_BACKUP, + GROUP_FILTER_LIST_SCHEMA, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_PRUNE, PRIV_REMOTE_READ, REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA, TRANSFER_LAST_SCHEMA, }; @@ -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(), - &sync_job.remote, + sync_job.remote.as_deref().unwrap_or("local"), &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,13 @@ 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 + && sync_job.ns == sync_job.remote_ns + { + bail!("can't sync, source equals the target"); + } + let (email, notify) = crate::server::lookup_datastore_notify_settings(&sync_job.store); let upid_str = WorkerTask::spawn( @@ -122,13 +139,33 @@ 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, ); - pull_store(&worker, &client, pull_params).await?; + if sync_job.remote.is_some() { + pull_store(&worker, &client, pull_params).await?; + } else { + if let (Some(target_ns), Some(source_ns)) = (sync_job.ns, sync_job.remote_ns) { + if target_ns.path().starts_with(source_ns.path()) + && sync_job.store == sync_job.remote_store + && sync_job.max_depth.map_or(true, |sync_depth| { + target_ns.depth() + sync_depth > MAX_NAMESPACE_DEPTH + }) { + task_log!( + worker, + "Can't sync namespace into one of its sub-namespaces, would exceed maximum namespace depth, skipping" + ); + } + } else { + pull_store(&worker, &client, pull_params).await?; + } + } task_log!(worker, "sync job '{}' end", &job_id); @@ -180,6 +217,7 @@ pub fn do_sync_job( }, remote: { schema: REMOTE_ID_SCHEMA, + optional: true, }, "remote-store": { schema: DATASTORE_SCHEMA, @@ -224,7 +262,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, @@ -248,7 +286,7 @@ async fn pull( &auth_id, &store, ns_str.as_deref(), - &remote, + remote.as_deref(), &remote_store, delete, )?; @@ -256,7 +294,7 @@ async fn pull( let pull_params = PullParameters::new( &store, ns, - &remote, + remote.as_deref().unwrap_or("local"), &remote_store, remote_ns.unwrap_or_default(), auth_id.clone(), @@ -280,7 +318,7 @@ async fn pull( worker, "pull datastore '{}' from '{}/{}'", store, - remote, + remote.as_deref().unwrap_or("-"), remote_store, ); @@ -299,4 +337,4 @@ async fn pull( Ok(upid_str) } -pub const ROUTER: Router = Router::new().post(&API_METHOD_PULL); +pub const ROUTER: Router = Router::new().post(&API_METHOD_PULL); \ No newline at end of file diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs index b4cb6cb3..1dd51951 100644 --- a/src/bin/proxmox-backup-manager.rs +++ b/src/bin/proxmox-backup-manager.rs @@ -535,21 +535,21 @@ 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 }) } -fn get_remote_store(param: &HashMap) -> Option<(String, String)> { +fn get_remote_store(param: &HashMap) -> Option<(Option, String)> { let mut job: Option = None; let remote = param.get("remote").map(|r| r.to_owned()).or_else(|| { 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