namespaces: move max-depth check to api type

and use it when creating a sync job, and simplify the check on updating
(only check the final, resulting config instead of each intermediate
version).

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
This commit is contained in:
Fabian Grünbichler 2022-05-13 11:25:01 +02:00
parent 11567dfbad
commit 66abc4cb7d
3 changed files with 32 additions and 28 deletions

View File

@ -719,6 +719,17 @@ impl BackupNamespace {
} }
Ok(new) Ok(new)
} }
/// Check whether adding `depth` levels of sub-namespaces exceeds the max depth limit
pub fn check_max_depth(&self, depth: usize) -> Result<(), Error> {
let ns_depth = self.depth();
if ns_depth + depth > MAX_NAMESPACE_DEPTH {
bail!(
"namespace '{self}'s depth and recursion depth exceed limit: {ns_depth} + {depth} > {MAX_NAMESPACE_DEPTH}",
);
}
Ok(())
}
} }
impl fmt::Display for BackupNamespace { impl fmt::Display for BackupNamespace {

View File

@ -1,8 +1,6 @@
use ::serde::{Deserialize, Serialize}; use ::serde::{Deserialize, Serialize};
use anyhow::{bail, Error}; use anyhow::{bail, Error};
use hex::FromHex; use hex::FromHex;
use pbs_api_types::BackupNamespace;
use pbs_api_types::MAX_NAMESPACE_DEPTH;
use serde_json::Value; use serde_json::Value;
use proxmox_router::{http_bail, Permission, Router, RpcEnvironment}; use proxmox_router::{http_bail, Permission, Router, RpcEnvironment};
@ -147,6 +145,15 @@ pub fn create_sync_job(
bail!("permission check failed"); bail!("permission check failed");
} }
if let Some(max_depth) = config.max_depth {
if let Some(ref ns) = config.ns {
ns.check_max_depth(max_depth)?;
}
if let Some(ref ns) = config.remote_ns {
ns.check_max_depth(max_depth)?;
}
}
let (mut section_config, _digest) = sync::config()?; let (mut section_config, _digest) = sync::config()?;
if section_config.sections.get(&config.id).is_some() { if section_config.sections.get(&config.id).is_some() {
@ -321,18 +328,6 @@ pub fn update_sync_job(
} }
} }
let check_max_depth = |ns: &BackupNamespace, depth| -> Result<(), Error> {
if ns.depth() + depth >= MAX_NAMESPACE_DEPTH {
bail!(
"namespace and recursion depth exceed limit: {} + {} >= {}",
ns.depth(),
depth,
MAX_NAMESPACE_DEPTH
);
}
Ok(())
};
if let Some(comment) = update.comment { if let Some(comment) = update.comment {
let comment = comment.trim().to_string(); let comment = comment.trim().to_string();
if comment.is_empty() { if comment.is_empty() {
@ -346,9 +341,6 @@ pub fn update_sync_job(
data.store = store; data.store = store;
} }
if let Some(ns) = update.ns { if let Some(ns) = update.ns {
if let Some(explicit_depth) = update.max_depth.or(data.max_depth) {
check_max_depth(&ns, explicit_depth)?;
}
data.ns = Some(ns); data.ns = Some(ns);
} }
if let Some(remote) = update.remote { if let Some(remote) = update.remote {
@ -358,9 +350,6 @@ pub fn update_sync_job(
data.remote_store = remote_store; data.remote_store = remote_store;
} }
if let Some(remote_ns) = update.remote_ns { if let Some(remote_ns) = update.remote_ns {
if let Some(explicit_depth) = update.max_depth.or(data.max_depth) {
check_max_depth(&remote_ns, explicit_depth)?;
}
data.remote_ns = Some(remote_ns); data.remote_ns = Some(remote_ns);
} }
if let Some(owner) = update.owner { if let Some(owner) = update.owner {
@ -394,13 +383,16 @@ pub fn update_sync_job(
data.remove_vanished = update.remove_vanished; data.remove_vanished = update.remove_vanished;
} }
if let Some(max_depth) = update.max_depth { if let Some(max_depth) = update.max_depth {
data.max_depth = Some(max_depth);
}
if let Some(max_depth) = data.max_depth {
if let Some(ref ns) = data.ns { if let Some(ref ns) = data.ns {
check_max_depth(ns, max_depth)?; ns.check_max_depth(max_depth)?;
} }
if let Some(ref ns) = data.remote_ns { if let Some(ref ns) = data.remote_ns {
check_max_depth(ns, max_depth)?; ns.check_max_depth(max_depth)?;
} }
data.max_depth = Some(max_depth);
} }
if !check_sync_job_modify_access(&user_info, &auth_id, &data) { if !check_sync_job_modify_access(&user_info, &auth_id, &data) {

View File

@ -1,6 +1,5 @@
//! Sync datastore from remote server //! Sync datastore from remote server
use std::cmp::min;
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use std::convert::TryFrom; use std::convert::TryFrom;
use std::io::{Seek, SeekFrom}; use std::io::{Seek, SeekFrom};
@ -18,8 +17,8 @@ use proxmox_sys::task_log;
use pbs_api_types::{ use pbs_api_types::{
Authid, BackupNamespace, DatastoreWithNamespace, GroupFilter, GroupListItem, NamespaceListItem, Authid, BackupNamespace, DatastoreWithNamespace, GroupFilter, GroupListItem, NamespaceListItem,
Operation, RateLimitConfig, Remote, SnapshotListItem, MAX_NAMESPACE_DEPTH, Operation, RateLimitConfig, Remote, SnapshotListItem, PRIV_DATASTORE_AUDIT,
PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY,
}; };
use pbs_client::{ use pbs_client::{
@ -81,8 +80,10 @@ impl PullParameters {
) -> Result<Self, Error> { ) -> Result<Self, Error> {
let store = DataStore::lookup_datastore(store, Some(Operation::Write))?; let store = DataStore::lookup_datastore(store, Some(Operation::Write))?;
let max_depth = if let Some(max_depth) = max_depth {
max_depth.map(|max_depth| min(max_depth, MAX_NAMESPACE_DEPTH - remote_ns.depth())); ns.check_max_depth(max_depth)?;
remote_ns.check_max_depth(max_depth)?;
}
let (remote_config, _digest) = pbs_config::remote::config()?; let (remote_config, _digest) = pbs_config::remote::config()?;
let remote: Remote = remote_config.lookup("remote", remote)?; let remote: Remote = remote_config.lookup("remote", remote)?;