From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 8D6E21FF17C for ; Wed, 23 Jul 2025 16:30:37 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DE4A315B14; Wed, 23 Jul 2025 16:31:56 +0200 (CEST) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Wed, 23 Jul 2025 16:31:52 +0200 Message-Id: <20250723143152.3829064-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.39.5 MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.020 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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. [datastore.rs, restore.rs, mod.rs, backup.rs, setup.store] Subject: [pbs-devel] [PATCH] tape: forbid operations on a s3 datastore 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: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" namely: * backup to tape from s3 (including a configuring such a job) * restore to s3 from tape It does not work currently, but it probably does not make sense to allow that at all for several reasons: * both are designed to be 'off-site', so copying data from one off-site location to another directly does not make sense most of the time * (modern) tape operations can reach relatively high speeds (> 300MB/s) and up/downloading to an (most likely remote) s3 storage will slow down the tape Note that we could make the check in the restore case more efficient (since we already have the parsed DataStore struct), but this to be done only once for each tape restore operation and most of the time there aren't that many datastores involved, so the extra runtime cost is probably not that bad vs having multiple code paths for the error. Signed-off-by: Dominik Csapak --- pbs-config/src/datastore.rs | 13 +++++++++++++ src/api2/config/tape_backup_job.rs | 7 +++++++ src/api2/tape/backup.rs | 6 +++++- src/api2/tape/restore.rs | 6 +++++- src/tape/mod.rs | 12 +++++++++++- 5 files changed, 41 insertions(+), 3 deletions(-) diff --git a/pbs-config/src/datastore.rs b/pbs-config/src/datastore.rs index 396dcb37..5a5553db 100644 --- a/pbs-config/src/datastore.rs +++ b/pbs-config/src/datastore.rs @@ -113,3 +113,16 @@ pub fn complete_calendar_event(_arg: &str, _param: &HashMap) -> .map(|s| String::from(*s)) .collect() } + +/// Returns the datastore backend type from it's name +pub fn datastore_backend_type(store: &str) -> Result { + let (config, _) = config()?; + let store_config: DataStoreConfig = config.lookup("datastore", &store)?; + + let backend_config: pbs_api_types::DatastoreBackendConfig = serde_json::from_value( + pbs_api_types::DatastoreBackendConfig::API_SCHEMA + .parse_property_string(store_config.backend.as_deref().unwrap_or(""))?, + )?; + + Ok(backend_config.ty.unwrap_or_default()) +} diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_backup_job.rs index f2abf652..786acde0 100644 --- a/src/api2/config/tape_backup_job.rs +++ b/src/api2/config/tape_backup_job.rs @@ -13,6 +13,8 @@ use pbs_api_types::{ use pbs_config::CachedUserInfo; +use crate::tape::assert_datastore_type; + #[api( input: { properties: {}, @@ -71,6 +73,8 @@ pub fn create_tape_backup_job( job: TapeBackupJobConfig, _rpcenv: &mut dyn RpcEnvironment, ) -> Result<(), Error> { + assert_datastore_type(&job.setup.store)?; + let _lock = pbs_config::tape_job::lock()?; let (mut config, _digest) = pbs_config::tape_job::config()?; @@ -180,6 +184,9 @@ pub fn update_tape_backup_job( delete: Option>, digest: Option, ) -> Result<(), Error> { + if let Some(store) = &update.setup.store { + assert_datastore_type(&store)?; + } let _lock = pbs_config::tape_job::lock()?; let (mut config, expected_digest) = pbs_config::tape_job::config()?; diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs index 31293a9a..16a26b83 100644 --- a/src/api2/tape/backup.rs +++ b/src/api2/tape/backup.rs @@ -20,7 +20,7 @@ use pbs_config::CachedUserInfo; use pbs_datastore::backup_info::{BackupDir, BackupInfo}; use pbs_datastore::{DataStore, StoreProgress}; -use crate::tape::TapeNotificationMode; +use crate::tape::{assert_datastore_type, TapeNotificationMode}; use crate::{ server::{ jobstate::{compute_schedule_status, Job, JobState}, @@ -140,6 +140,8 @@ pub fn do_tape_backup_job( schedule: Option, to_stdout: bool, ) -> Result { + assert_datastore_type(&setup.store)?; + let job_id = format!( "{}:{}:{}:{}", setup.store, @@ -302,6 +304,8 @@ pub fn backup( force_media_set: bool, rpcenv: &mut dyn RpcEnvironment, ) -> Result { + assert_datastore_type(&setup.store)?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; check_backup_permission(&auth_id, &setup.store, &setup.pool, &setup.drive)?; diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs index 2cc1baab..9e55cae2 100644 --- a/src/api2/tape/restore.rs +++ b/src/api2/tape/restore.rs @@ -37,7 +37,7 @@ use pbs_tape::{ }; use crate::backup::check_ns_modification_privs; -use crate::tape::TapeNotificationMode; +use crate::tape::{assert_datastore_type, TapeNotificationMode}; use crate::{ tape::{ drive::{lock_tape_device, request_and_load_media, set_tape_device_state, TapeDriver}, @@ -354,6 +354,10 @@ pub fn restore( bail!("no datastores given"); } + for (_, (target, _)) in &used_datastores { + assert_datastore_type(target.name())?; + } + for (target, namespaces) in used_datastores.values() { check_datastore_privs( &user_info, diff --git a/src/tape/mod.rs b/src/tape/mod.rs index f276f948..69fc373b 100644 --- a/src/tape/mod.rs +++ b/src/tape/mod.rs @@ -1,6 +1,6 @@ //! Magnetic tape backup -use anyhow::{format_err, Error}; +use anyhow::{bail, format_err, Error}; use proxmox_auth_api::types::Userid; use proxmox_sys::fs::{create_path, CreateOptions}; @@ -155,3 +155,13 @@ impl From<(Option, Option)> for TapeNotificationMode { } } } + +/// Asserts that the datastore is on a supported backend type +pub fn assert_datastore_type(store: &str) -> Result<(), Error> { + match pbs_config::datastore::datastore_backend_type(store)? { + pbs_api_types::DatastoreBackendType::Filesystem => Ok(()), + pbs_api_types::DatastoreBackendType::S3 => { + bail!("direct s3/tape operations are not supported") + } + } +} -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel