[pbs-devel] [PATCH proxmox-backup RFC 07/10] api: replace datastore_lookup with new, state-typed datastore returning functions
Hannes Laimer
h.laimer at proxmox.com
Tue Sep 3 14:33:58 CEST 2024
Signed-off-by: Hannes Laimer <h.laimer at proxmox.com>
---
src/api2/admin/datastore.rs | 143 ++++++++++++++++++------------------
src/api2/admin/namespace.rs | 8 +-
src/api2/backup/mod.rs | 4 +-
src/api2/reader/mod.rs | 4 +-
src/api2/status.rs | 8 +-
src/api2/tape/restore.rs | 6 +-
6 files changed, 86 insertions(+), 87 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 976617d9..f1eed9fc 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -36,13 +36,12 @@ use pxar::EntryKind;
use pbs_api_types::{
print_ns_and_snapshot, print_store_and_ns, Authid, BackupContent, BackupNamespace, BackupType,
Counts, CryptMode, DataStoreConfig, DataStoreListItem, DataStoreStatus,
- GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, Operation,
- PruneJobOptions, SnapshotListItem, SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA,
- BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA,
- DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
- PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
- PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA,
- VERIFICATION_OUTDATED_AFTER_SCHEMA,
+ GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, PruneJobOptions,
+ SnapshotListItem, SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA,
+ BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA,
+ IGNORE_VERIFIED_BACKUPS_SCHEMA, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT,
+ PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ,
+ PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
};
use pbs_client::pxar::{create_tar, create_zip};
use pbs_config::CachedUserInfo;
@@ -88,25 +87,28 @@ fn get_group_note_path(
// 1. check privs on NS (full or limited access)
// 2. load datastore
// 3. if needed (only limited access), check owner of group
-fn check_privs_and_load_store(
- store: &str,
+fn check_privs<T: CanRead>(
+ store: Arc<DataStore<T>>,
ns: &BackupNamespace,
auth_id: &Authid,
full_access_privs: u64,
partial_access_privs: u64,
- operation: Option<Operation>,
backup_group: &pbs_api_types::BackupGroup,
-) -> Result<Arc<DataStore>, Error> {
- let limited = check_ns_privs_full(store, ns, auth_id, full_access_privs, partial_access_privs)?;
-
- let datastore = DataStore::lookup_datastore(store, operation)?;
+) -> Result<(), Error> {
+ let limited = check_ns_privs_full(
+ store.name(),
+ ns,
+ auth_id,
+ full_access_privs,
+ partial_access_privs,
+ )?;
if limited {
- let owner = datastore.get_owner(ns, backup_group)?;
+ let owner = store.get_owner(ns, backup_group)?;
check_backup_owner(&owner, auth_id)?;
}
- Ok(datastore)
+ Ok(())
}
fn read_backup_index(
@@ -195,7 +197,7 @@ pub fn list_groups(
PRIV_DATASTORE_BACKUP,
)?;
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore_read(&store)?;
datastore
.iter_backup_groups(ns.clone())? // FIXME: Namespaces and recursion parameters!
@@ -286,14 +288,13 @@ pub async fn delete_group(
tokio::task::spawn_blocking(move || {
let ns = ns.unwrap_or_default();
-
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_write(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_MODIFY,
PRIV_DATASTORE_PRUNE,
- Some(Operation::Write),
&group,
)?;
@@ -341,13 +342,13 @@ pub async fn list_snapshot_files(
tokio::task::spawn_blocking(move || {
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_read(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_READ,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Read),
&backup_dir.group,
)?;
@@ -395,13 +396,13 @@ pub async fn delete_snapshot(
tokio::task::spawn_blocking(move || {
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_write(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_MODIFY,
PRIV_DATASTORE_PRUNE,
- Some(Operation::Write),
&backup_dir.group,
)?;
@@ -477,7 +478,7 @@ unsafe fn list_snapshots_blocking(
PRIV_DATASTORE_BACKUP,
)?;
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore_read(&store)?;
// FIXME: filter also owner before collecting, for doing that nicely the owner should move into
// backup group and provide an error free (Err -> None) accessor
@@ -691,7 +692,7 @@ pub async fn status(
}
};
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore_read(&store)?;
let (counts, gc_status) = if verbose {
let filter_owner = if store_privs & PRIV_DATASTORE_AUDIT != 0 {
@@ -804,7 +805,7 @@ pub fn verify(
PRIV_DATASTORE_BACKUP,
)?;
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore_write(&store)?;
let ignore_verified = ignore_verified.unwrap_or(true);
let worker_id;
@@ -974,13 +975,13 @@ pub fn prune(
) -> Result<Value, Error> {
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_write(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_MODIFY,
PRIV_DATASTORE_PRUNE,
- Some(Operation::Write),
&group,
)?;
@@ -1149,7 +1150,7 @@ pub fn prune_datastore(
true,
)?;
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
+ let datastore = DataStore::lookup_datastore_write(&store)?;
let ns = prune_options.ns.clone().unwrap_or_default();
let worker_id = format!("{}:{}", store, ns);
@@ -1187,7 +1188,7 @@ pub fn start_garbage_collection(
_info: &ApiMethod,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Value, Error> {
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
+ let datastore = DataStore::lookup_datastore_write(&store)?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let job = Job::new("garbage_collection", &store)
@@ -1238,7 +1239,7 @@ pub fn garbage_collection_status(
..Default::default()
};
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore_read(&store)?;
let status_in_memory = datastore.last_gc_status();
let state_file = JobState::load("garbage_collection", &store)
.map_err(|err| log::error!("could not open GC statefile for {store}: {err}"))
@@ -1373,13 +1374,13 @@ pub fn download_file(
let backup_ns = optional_ns_param(¶m)?;
let backup_dir: pbs_api_types::BackupDir = Deserialize::deserialize(¶m)?;
- let datastore = check_privs_and_load_store(
- store,
+ let datastore = DataStore::lookup_datastore_read(store)?;
+ check_privs(
+ datastore.clone(),
&backup_ns,
&auth_id,
PRIV_DATASTORE_READ,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Read),
&backup_dir.group,
)?;
@@ -1458,13 +1459,13 @@ pub fn download_file_decoded(
let backup_ns = optional_ns_param(¶m)?;
let backup_dir_api: pbs_api_types::BackupDir = Deserialize::deserialize(¶m)?;
- let datastore = check_privs_and_load_store(
- store,
+ let datastore = DataStore::lookup_datastore_read(store)?;
+ check_privs(
+ datastore.clone(),
&backup_ns,
&auth_id,
PRIV_DATASTORE_READ,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Read),
&backup_dir_api.group,
)?;
@@ -1589,13 +1590,13 @@ pub fn upload_backup_log(
let backup_dir_api: pbs_api_types::BackupDir = Deserialize::deserialize(¶m)?;
- let datastore = check_privs_and_load_store(
- store,
+ let datastore = DataStore::lookup_datastore_write(store)?;
+ check_privs(
+ datastore.clone(),
&backup_ns,
&auth_id,
0,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Write),
&backup_dir_api.group,
)?;
let backup_dir = datastore.backup_dir(backup_ns.clone(), backup_dir_api.clone())?;
@@ -1686,13 +1687,13 @@ pub async fn catalog(
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_read(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_READ,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Read),
&backup_dir.group,
)?;
@@ -1806,13 +1807,13 @@ pub fn pxar_file_download(
let ns = optional_ns_param(¶m)?;
let backup_dir: pbs_api_types::BackupDir = Deserialize::deserialize(¶m)?;
- let datastore = check_privs_and_load_store(
- store,
+ let datastore = DataStore::lookup_datastore_read(store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_READ,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Read),
&backup_dir.group,
)?;
@@ -1944,7 +1945,7 @@ pub fn get_rrd_stats(
cf: RRDMode,
_param: Value,
) -> Result<Value, Error> {
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore(&store)?;
let disk_manager = crate::tools::disks::DiskManage::new();
let mut rrd_fields = vec![
@@ -2017,13 +2018,13 @@ pub fn get_group_notes(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_read(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_AUDIT,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Read),
&backup_group,
)?;
@@ -2065,13 +2066,13 @@ pub fn set_group_notes(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_write(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_MODIFY,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Write),
&backup_group,
)?;
@@ -2111,13 +2112,13 @@ pub fn get_notes(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_read(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_AUDIT,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Read),
&backup_dir.group,
)?;
@@ -2164,13 +2165,13 @@ pub fn set_notes(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_write(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_MODIFY,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Write),
&backup_dir.group,
)?;
@@ -2214,13 +2215,13 @@ pub fn get_protection(
) -> Result<bool, Error> {
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_read(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_AUDIT,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Read),
&backup_dir.group,
)?;
@@ -2264,13 +2265,13 @@ pub async fn set_protection(
tokio::task::spawn_blocking(move || {
let ns = ns.unwrap_or_default();
- let datastore = check_privs_and_load_store(
- &store,
+ let datastore = DataStore::lookup_datastore_write(&store)?;
+ check_privs(
+ datastore.clone(),
&ns,
&auth_id,
PRIV_DATASTORE_MODIFY,
PRIV_DATASTORE_BACKUP,
- Some(Operation::Write),
&backup_dir.group,
)?;
@@ -2324,7 +2325,7 @@ pub async fn set_backup_owner(
PRIV_DATASTORE_BACKUP,
)?;
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
+ let datastore = DataStore::lookup_datastore_write(&store)?;
let backup_group = datastore.backup_group(ns, backup_group);
diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs
index 889dc1a3..10af8693 100644
--- a/src/api2/admin/namespace.rs
+++ b/src/api2/admin/namespace.rs
@@ -6,7 +6,7 @@ use proxmox_router::{http_bail, ApiMethod, Permission, Router, RpcEnvironment};
use proxmox_schema::*;
use pbs_api_types::{
- Authid, BackupNamespace, NamespaceListItem, Operation, DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA,
+ Authid, BackupNamespace, NamespaceListItem, DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA,
PROXMOX_SAFE_ID_FORMAT,
};
@@ -55,7 +55,7 @@ pub fn create_namespace(
check_ns_modification_privs(&store, &ns, &auth_id)?;
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
+ let datastore = DataStore::lookup_datastore_write(&store)?;
datastore.create_namespace(&parent, name)
}
@@ -98,7 +98,7 @@ pub fn list_namespaces(
// get result up-front to avoid cloning NS, it's relatively cheap anyway (no IO normally)
let parent_access = check_ns_privs(&store, &parent, &auth_id, NS_PRIVS_OK);
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore_read(&store)?;
let iter = match datastore.recursive_iter_backup_ns_ok(parent, max_depth) {
Ok(iter) => iter,
@@ -156,7 +156,7 @@ pub fn delete_namespace(
check_ns_modification_privs(&store, &ns, &auth_id)?;
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
+ let datastore = DataStore::lookup_datastore_write(&store)?;
if !datastore.remove_namespace_recursive(&ns, delete_groups)? {
if delete_groups {
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index ea0d0292..e6a92117 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -19,7 +19,7 @@ use proxmox_sortable_macro::sortable;
use proxmox_sys::fs::lock_dir_noblock_shared;
use pbs_api_types::{
- Authid, BackupNamespace, BackupType, Operation, SnapshotVerifyState, VerifyState,
+ Authid, BackupNamespace, BackupType, SnapshotVerifyState, VerifyState,
BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
BACKUP_TYPE_SCHEMA, CHUNK_DIGEST_SCHEMA, DATASTORE_SCHEMA, PRIV_DATASTORE_BACKUP,
};
@@ -96,7 +96,7 @@ fn upgrade_to_backup_protocol(
)
.map_err(|err| http_err!(FORBIDDEN, "{err}"))?;
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
+ let datastore = DataStore::lookup_datastore_write(&store)?;
let protocols = parts
.headers
diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
index 23051653..48a8c5fc 100644
--- a/src/api2/reader/mod.rs
+++ b/src/api2/reader/mod.rs
@@ -19,7 +19,7 @@ use proxmox_sortable_macro::sortable;
use proxmox_sys::fs::lock_dir_noblock_shared;
use pbs_api_types::{
- Authid, Operation, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
+ Authid, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CHUNK_DIGEST_SCHEMA, DATASTORE_SCHEMA,
PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_READ,
};
@@ -93,7 +93,7 @@ fn upgrade_to_backup_reader_protocol(
bail!("no permissions on /{}", acl_path.join("/"));
}
- let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore_read(&store)?;
let backup_dir = pbs_api_types::BackupDir::deserialize(¶m)?;
diff --git a/src/api2/status.rs b/src/api2/status.rs
index f1ae0ef5..30a0cf76 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -8,9 +8,7 @@ use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment, SubdirMap};
use proxmox_rrd::api_types::{RRDMode, RRDTimeFrame};
use proxmox_schema::api;
-use pbs_api_types::{
- Authid, DataStoreStatusListItem, Operation, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
-};
+use pbs_api_types::{Authid, DataStoreStatusListItem, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP};
use pbs_config::CachedUserInfo;
use pbs_datastore::DataStore;
@@ -49,7 +47,7 @@ pub async fn datastore_status(
let user_privs = user_info.lookup_privs(&auth_id, &["datastore", store]);
let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP)) != 0;
if !allowed {
- if let Ok(datastore) = DataStore::lookup_datastore(store, Some(Operation::Lookup)) {
+ if let Ok(datastore) = DataStore::lookup_datastore_read(store) {
if can_access_any_namespace(datastore, &auth_id, &user_info) {
list.push(DataStoreStatusListItem::empty(store, None));
}
@@ -57,7 +55,7 @@ pub async fn datastore_status(
continue;
}
- let datastore = match DataStore::lookup_datastore(store, Some(Operation::Read)) {
+ let datastore = match DataStore::lookup_datastore_read(store) {
Ok(datastore) => datastore,
Err(err) => {
list.push(DataStoreStatusListItem::empty(store, Some(err.to_string())));
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index b28db6e3..95ce13c7 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -20,7 +20,7 @@ use proxmox_worker_task::WorkerTaskContext;
use pbs_api_types::{
parse_ns_and_snapshot, print_ns_and_snapshot, Authid, BackupDir, BackupNamespace, CryptMode,
- NotificationMode, Operation, TapeRestoreNamespace, Userid, DATASTORE_MAP_ARRAY_SCHEMA,
+ NotificationMode, TapeRestoreNamespace, Userid, DATASTORE_MAP_ARRAY_SCHEMA,
DATASTORE_MAP_LIST_SCHEMA, DRIVE_NAME_SCHEMA, MAX_NAMESPACE_DEPTH, PRIV_DATASTORE_BACKUP,
PRIV_DATASTORE_MODIFY, PRIV_TAPE_READ, TAPE_RESTORE_NAMESPACE_SCHEMA,
TAPE_RESTORE_SNAPSHOT_SCHEMA, UPID_SCHEMA,
@@ -144,10 +144,10 @@ impl TryFrom<String> for DataStoreMap {
if let Some(index) = store.find('=') {
let mut target = store.split_off(index);
target.remove(0); // remove '='
- let datastore = DataStore::lookup_datastore(&target, Some(Operation::Write))?;
+ let datastore = DataStore::lookup_datastore_write(&target)?;
map.insert(store, datastore);
} else if default.is_none() {
- default = Some(DataStore::lookup_datastore(&store, Some(Operation::Write))?);
+ default = Some(DataStore::lookup_datastore_write(&store)?);
} else {
bail!("multiple default stores given");
}
--
2.39.2
More information about the pbs-devel
mailing list