tsarevvv/proxmox-backup
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

api: make Remote for SyncJob optional

Browse Source 
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Tested-by: Gabriel Goller <g.goller@proxmox.com>
This commit is contained in:
Hannes Laimer 2023-11-21 15:31:54 +01:00 committed by Thomas Lamprecht
parent d1100551fd
commit 4ec733279b
7 changed files with 87 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
pbs-api-types/src/jobs.rs
View File

@ -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<BackupNamespace>,
#[serde(skip_serializing_if = "Option::is_none")]
pub owner: Option<Authid>,
pub remote: String,
#[serde(skip_serializing_if = "Option::is_none")]
/// None implies local sync.
pub remote: Option<String>,
pub remote_store: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub remote_ns: Option<BackupNamespace>,

2
src/api2/config/remote.rs
View File

@ -268,7 +268,7 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
let job_list: Vec<SyncJobConfig> = 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 '{}')",

41
src/api2/config/sync.rs
View File

@ -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<Sync
#[serde(rename_all = "kebab-case")]
/// Deletable property name
pub enum DeletableProperty {
/// Delete the remote property(-> 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));

3
src/api2/node/tasks.rs
View File

@ -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,
);

54
src/api2/pull.rs
View File

@ -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<String, Error> {
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<BackupNamespace>,
remote: String,
remote: Option<String>,
remote_store: String,
remote_ns: Option<BackupNamespace>,
remove_vanished: Option<bool>,
@ -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,
);

4
src/bin/proxmox-backup-manager.rs
View File

@ -535,7 +535,7 @@ fn get_remote(param: &HashMap<String, String>) -> Option<String> {
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<String, String>) -> Option<(Option<String>,
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

18
src/server/email_notifications.rs
View File

@ -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)?;