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 C8A66AF59 for ; Tue, 8 Aug 2023 14:14:23 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A96999753 for ; Tue, 8 Aug 2023 14:13:53 +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 ; Tue, 8 Aug 2023 14:13:52 +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 2DD8943784 for ; Tue, 8 Aug 2023 14:13:52 +0200 (CEST) From: Hannes Laimer To: pbs-devel@lists.proxmox.com Date: Tue, 8 Aug 2023 14:13:39 +0200 Message-Id: <20230808121344.199500-2-h.laimer@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230808121344.199500-1-h.laimer@proxmox.com> References: <20230808121344.199500-1-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.017 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 Subject: [pbs-devel] [PATCH proxmox-backup v3 1/6] api2: 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, 08 Aug 2023 12:14:23 -0000 Signed-off-by: Hannes Laimer --- pbs-api-types/src/jobs.rs | 5 ++- src/api2/config/remote.rs | 2 +- src/api2/config/sync.rs | 41 +++++++++++++------- src/api2/node/tasks.rs | 4 +- src/api2/pull.rs | 62 ++++++++++++++++++++++--------- src/server/email_notifications.rs | 18 +++++---- 6 files changed, 90 insertions(+), 42 deletions(-) diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs index 23e19b7b..85fdbe41 100644 --- a/pbs-api-types/src/jobs.rs +++ b/pbs-api-types/src/jobs.rs @@ -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..12ce70f6 100644 --- a/src/api2/node/tasks.rs +++ b/src/api2/node/tasks.rs @@ -75,14 +75,14 @@ fn check_job_privs(auth_id: &Authid, user_info: &CachedUserInfo, upid: &UPID) -> let local_store = captures.get(3); let local_ns = captures.get(4).map(|m| m.as_str()); - if let (Some(remote), Some(remote_store), Some(local_store)) = + if let (remote, Some(remote_store), Some(local_store)) = (remote, remote_store, local_store) { return check_pull_privs( auth_id, local_store.as_str(), local_ns, - remote.as_str(), + remote.map(|remote| remote.as_str()), remote_store.as_str(), false, ); diff --git a/src/api2/pull.rs b/src/api2/pull.rs index daeba7cf..664ecce5 100644 --- a/src/api2/pull.rs +++ b/src/api2/pull.rs @@ -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.clone().unwrap_or("localhost".to_string()), sync_job.remote_store, sync_job.store, sync_job.ns.clone().unwrap_or_default(), @@ -124,11 +134,28 @@ pub fn do_sync_job( worker, "sync datastore '{}' from '{}/{}'", sync_job.store, - sync_job.remote, + sync_job.remote.clone().unwrap_or("local".to_string()), 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 +207,7 @@ pub fn do_sync_job( }, remote: { schema: REMOTE_ID_SCHEMA, + optional: true, }, "remote-store": { schema: DATASTORE_SCHEMA, @@ -224,7 +252,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 +276,7 @@ async fn pull( &auth_id, &store, ns_str.as_deref(), - &remote, + remote.as_deref(), &remote_store, delete, )?; @@ -256,7 +284,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 +308,7 @@ async fn pull( worker, "pull datastore '{}' from '{}/{}'", store, - remote, + remote.as_deref().unwrap_or("localhost"), remote_store, ); @@ -299,4 +327,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/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