From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 6A1AF1FF17A for ; Fri, 18 Jul 2025 10:36:12 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5A1F316C0B; Fri, 18 Jul 2025 10:37:20 +0200 (CEST) Message-ID: Date: Fri, 18 Jul 2025 10:37:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Lukas Wagner , Proxmox Backup Server development discussion References: <20250715125332.954494-1-c.ebner@proxmox.com> <20250715125332.954494-12-c.ebner@proxmox.com> <5f86f319-23e6-4573-b63e-9432f2856b00@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <5f86f319-23e6-4573-b63e-9432f2856b00@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1752827833474 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 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: Re: [pbs-devel] [PATCH proxmox-backup v8 02/45] config: introduce s3 object store client configuration 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On 7/18/25 9:22 AM, Lukas Wagner wrote: > With my minor complaints fixed: > > Reviewed-by: Lukas Wagner > > On 2025-07-15 14:52, Christian Ebner wrote: >> Adds the client configuration for s3 object store as dedicated >> configuration files, with secrets being stored separately from the >> regular configuration and excluded from api responses for security >> reasons. >> >> Signed-off-by: Christian Ebner >> --- >> changes since version 7: >> - no changes >> >> pbs-config/Cargo.toml | 1 + >> pbs-config/src/lib.rs | 1 + >> pbs-config/src/s3.rs | 83 +++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 85 insertions(+) >> create mode 100644 pbs-config/src/s3.rs >> >> diff --git a/pbs-config/Cargo.toml b/pbs-config/Cargo.toml >> index 284149658..74afb3c64 100644 >> --- a/pbs-config/Cargo.toml >> +++ b/pbs-config/Cargo.toml >> @@ -19,6 +19,7 @@ serde_json.workspace = true >> >> proxmox-notify.workspace = true >> proxmox-router = { workspace = true, default-features = false } >> +proxmox-s3-client.workspace = true >> proxmox-schema.workspace = true >> proxmox-section-config.workspace = true >> proxmox-shared-memory.workspace = true >> diff --git a/pbs-config/src/lib.rs b/pbs-config/src/lib.rs >> index 9c4d77c24..d03c079ab 100644 >> --- a/pbs-config/src/lib.rs >> +++ b/pbs-config/src/lib.rs >> @@ -10,6 +10,7 @@ pub mod network; >> pub mod notifications; >> pub mod prune; >> pub mod remote; >> +pub mod s3; >> pub mod sync; >> pub mod tape_job; >> pub mod token_shadow; >> diff --git a/pbs-config/src/s3.rs b/pbs-config/src/s3.rs >> new file mode 100644 >> index 000000000..ec3998834 >> --- /dev/null >> +++ b/pbs-config/src/s3.rs >> @@ -0,0 +1,83 @@ >> +use std::collections::HashMap; >> +use std::sync::LazyLock; >> + >> +use anyhow::Error; >> + >> +use proxmox_s3_client::{S3ClientConfig, S3ClientSecretsConfig}; >> +use proxmox_schema::*; >> +use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin}; >> + >> +use pbs_api_types::JOB_ID_SCHEMA; >> + >> +use crate::{open_backup_lockfile, replace_backup_config, BackupLockGuard}; >> + >> +pub static CONFIG: LazyLock = LazyLock::new(init); >> + >> +fn init() -> SectionConfig { >> + let obj_schema = match S3ClientConfig::API_SCHEMA { >> + Schema::Object(ref obj_schema) => obj_schema, >> + _ => unreachable!(), >> + }; >> + let secrets_obj_schema = match S3ClientSecretsConfig::API_SCHEMA { >> + Schema::Object(ref obj_schema) => obj_schema, >> + _ => unreachable!(), >> + }; > > You can use API_SCHEMA::unwrap_object_schema here, that's a bit nicer to read :) agreed, incorporated this for the next iteration of the patches >> + >> + let plugin = >> + SectionConfigPlugin::new("s3client".to_string(), Some(String::from("id")), obj_schema); >> + let secrets_plugin = SectionConfigPlugin::new( >> + "s3secrets".to_string(), >> + Some(String::from("secrets-id")), >> + secrets_obj_schema, >> + ); >> + let mut config = SectionConfig::new(&JOB_ID_SCHEMA); >> + config.register_plugin(plugin); >> + config.register_plugin(secrets_plugin); >> + >> + config >> +} >> + >> +pub const S3_CFG_FILENAME: &str = "/etc/proxmox-backup/s3.cfg"; >> +pub const S3_SECRETS_CFG_FILENAME: &str = "/etc/proxmox-backup/s3-secrets.cfg"; >> +pub const S3_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.s3.lck"; > > You can use the pbs_buildcfg::configdir macro to build these paths. Also please > add some docstrings to public consts like these. same, added the helper so we always use the configured path from buildcfg as base path for these constans. > >> + >> +/// Get exclusive lock >> +pub fn lock_config() -> Result { >> + open_backup_lockfile(S3_CFG_LOCKFILE, None, true) >> +} >> + >> +pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> { >> + parse_config(S3_CFG_FILENAME) >> +} >> + >> +pub fn secrets_config() -> Result<(SectionConfigData, [u8; 32]), Error> { >> + parse_config(S3_SECRETS_CFG_FILENAME) >> +} >> + >> +pub fn save_config(config: &SectionConfigData, secrets: &SectionConfigData) -> Result<(), Error> { >> + let raw = CONFIG.write(S3_CFG_FILENAME, config)?; >> + replace_backup_config(S3_CFG_FILENAME, raw.as_bytes())?; >> + >> + let secrets_raw = CONFIG.write(S3_SECRETS_CFG_FILENAME, secrets)?; >> + // Secrets are stored with `backup` permissions to allow reading from >> + // not protected api endpoints as well. >> + replace_backup_config(S3_SECRETS_CFG_FILENAME, secrets_raw.as_bytes())?; >> + >> + Ok(()) >> +} > > ^ These public functions lack docstrings added these as well ... > >> + >> +// shell completion helper ... and expanded a bit on this one since already at it. >> +pub fn complete_s3_client_id(_arg: &str, _param: &HashMap) -> Vec { >> + match config() { >> + Ok((data, _digest)) => data.sections.keys().map(|id| id.to_string()).collect(), >> + Err(_) => Vec::new(), >> + } >> +} >> + >> +fn parse_config(path: &str) -> Result<(SectionConfigData, [u8; 32]), Error> { >> + let content = proxmox_sys::fs::file_read_optional_string(path)?; >> + let content = content.unwrap_or_default(); >> + let digest = openssl::sha::sha256(content.as_bytes()); >> + let data = CONFIG.parse(path, &content)?; >> + Ok((data, digest)) >> +} > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel