Re: [pdm-devel] [PATCH proxmox v2 1/4] access-control: add acl feature to only expose types and the AclTree

["Shannon Sterz" <s.sterz@proxmox.com>]

On Thu Oct 23, 2025 at 11:24 AM CEST, Dominik Csapak wrote:
> one tiny nit inline
>
> otherwise looks good to me
>
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
>
> On 10/22/25 3:11 PM, Shannon Sterz wrote:
>> this is useful, when an application wants to only handle an acl tree
>> without depending on more complex features provided by the rest of the
>> `impl` feature or its bigger dependencies (e.g. openssl).
>>
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>>   proxmox-access-control/Cargo.toml  |  6 +++++-
>>   proxmox-access-control/src/acl.rs  | 19 ++++++++++++++++++-
>>   proxmox-access-control/src/init.rs | 13 ++++++++++++-
>>   proxmox-access-control/src/lib.rs  |  4 ++--
>>   4 files changed, 37 insertions(+), 5 deletions(-)
>>
>> diff --git a/proxmox-access-control/Cargo.toml b/proxmox-access-control/Cargo.toml
>> index d23d272d..0eaf9fdf 100644
>> --- a/proxmox-access-control/Cargo.toml
>> +++ b/proxmox-access-control/Cargo.toml
>> @@ -35,18 +35,22 @@ proxmox-uuid = { workspace = true }
>>
>>   [features]
>>   default = []
>> +acl = [
>> +    "dep:proxmox-section-config",
>> +]
>>   api = [
>>       "impl",
>>       "dep:hex",
>>   ]
>>   impl = [
>> +    "acl",
>>       "dep:nix",
>>       "dep:openssl",
>>       "dep:proxmox-config-digest",
>>       "dep:proxmox-product-config",
>>       "dep:proxmox-router",
>> -    "dep:proxmox-section-config",
>>       "dep:proxmox-shared-memory",
>>       "dep:proxmox-sys",
>>       "dep:serde_json",
>>   ]
>> +
>> diff --git a/proxmox-access-control/src/acl.rs b/proxmox-access-control/src/acl.rs
>> index 270292ac..877b003c 100644
>> --- a/proxmox-access-control/src/acl.rs
>> +++ b/proxmox-access-control/src/acl.rs
>> @@ -1,15 +1,25 @@
>> -use std::collections::{BTreeMap, BTreeSet, HashMap};
>> +#[cfg(feature = "impl")]
>> +use std::collections::BTreeSet;
>> +use std::collections::{BTreeMap, HashMap};
>> +#[cfg(feature = "impl")]
>>   use std::io::Write;
>> +#[cfg(feature = "impl")]
>>   use std::path::Path;
>> +#[cfg(feature = "impl")]
>>   use std::sync::{Arc, OnceLock, RwLock};
>>
>>   use anyhow::{bail, Error};
>>
>>   use proxmox_auth_api::types::{Authid, Userid};
>> +#[cfg(feature = "impl")]
>>   use proxmox_config_digest::ConfigDigest;
>> +#[cfg(feature = "impl")]
>>   use proxmox_product_config::{open_api_lockfile, replace_privileged_config, ApiLockGuard};
>>
>>   use crate::init::{access_conf, acl_config, acl_config_lock};
>> +use crate::init::access_conf;
>
> this now imports 'access_conf' twice, probably that line would belong to
> the next patch?

ah sorry this was my mistake when detangling dependencies for the
features. the `use crate::init::{access_conf, acl_config,
acl_config_lock};` here should be removed. `access_config` and
`access_config_lock` are only needed for `impl`.

>
>> +#[cfg(feature = "impl")]
>> +use crate::init::{acl_config, acl_config_lock};
>>
>>   pub fn split_acl_path(path: &str) -> Vec<&str> {
>>       let items = path.split('/');
>> @@ -302,6 +312,7 @@ impl AclTree {
>>           node.insert_user_role(auth_id.to_owned(), role.to_string(), propagate);
>>       }
>>
>> +    #[cfg(feature = "impl")]
>>       fn write_node_config(node: &AclTreeNode, path: &str, w: &mut dyn Write) -> Result<(), Error> {
>>           let mut role_ug_map0: HashMap<_, BTreeSet<_>> = HashMap::new();
>>           let mut role_ug_map1: HashMap<_, BTreeSet<_>> = HashMap::new();
>> @@ -402,6 +413,7 @@ impl AclTree {
>>           Ok(())
>>       }
>>
>> +    #[cfg(feature = "impl")]
>>       fn write_config(&self, w: &mut dyn Write) -> Result<(), Error> {
>>           Self::write_node_config(&self.root, "", w)
>>       }
>> @@ -449,6 +461,7 @@ impl AclTree {
>>           Ok(())
>>       }
>>
>> +    #[cfg(feature = "impl")]
>>       fn load(filename: &Path) -> Result<(Self, ConfigDigest), Error> {
>>           let mut tree = Self::new();
>>
>> @@ -553,11 +566,13 @@ impl AclTree {
>>   }
>>
>>   /// Get exclusive lock
>> +#[cfg(feature = "impl")]
>>   pub fn lock_config() -> Result<ApiLockGuard, Error> {
>>       open_api_lockfile(acl_config_lock(), None, true)
>>   }
>>
>>   /// Reads the [`AclTree`] from `acl.cfg` in the configuration directory.
>> +#[cfg(feature = "impl")]
>>   pub fn config() -> Result<(AclTree, ConfigDigest), Error> {
>>       let path = acl_config();
>>       AclTree::load(&path)
>> @@ -568,6 +583,7 @@ pub fn config() -> Result<(AclTree, ConfigDigest), Error> {
>>   ///
>>   /// Since the AclTree is used for every API request's permission check, this caching mechanism
>>   /// allows to skip reading and parsing the file again if it is unchanged.
>> +#[cfg(feature = "impl")]
>>   pub fn cached_config() -> Result<Arc<AclTree>, Error> {
>>       struct ConfigCache {
>>           data: Option<Arc<AclTree>>,
>> @@ -621,6 +637,7 @@ pub fn cached_config() -> Result<Arc<AclTree>, Error> {
>>
>>   /// Saves an [`AclTree`] to `acl.cfg` in the configuration directory, ensuring proper ownership and
>>   /// file permissions.
>> +#[cfg(feature = "impl")]
>>   pub fn save_config(acl: &AclTree) -> Result<(), Error> {
>>       let mut raw: Vec<u8> = Vec::new();
>>       acl.write_config(&mut raw)?;
>> diff --git a/proxmox-access-control/src/init.rs b/proxmox-access-control/src/init.rs
>> index 39a12352..cf5f795d 100644
>> --- a/proxmox-access-control/src/init.rs
>> +++ b/proxmox-access-control/src/init.rs
>> @@ -1,4 +1,5 @@
>>   use std::collections::HashMap;
>> +#[cfg(feature = "impl")]
>>   use std::path::{Path, PathBuf};
>>   use std::sync::OnceLock;
>>
>> @@ -8,6 +9,7 @@ use proxmox_auth_api::types::{Authid, Userid};
>>   use proxmox_section_config::SectionConfigData;
>>
>>   static ACCESS_CONF: OnceLock<&'static dyn AccessControlConfig> = OnceLock::new();
>> +#[cfg(feature = "impl")]
>>   static ACCESS_CONF_DIR: OnceLock<PathBuf> = OnceLock::new();
>>
>>   /// This trait specifies the functions a product needs to implement to get ACL tree based access
>> @@ -105,6 +107,7 @@ pub trait AccessControlConfig: Send + Sync {
>>       }
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   pub fn init<P: AsRef<Path>>(
>>       acm_config: &'static dyn AccessControlConfig,
>>       config_dir: P,
>> @@ -113,13 +116,14 @@ pub fn init<P: AsRef<Path>>(
>>       init_access_config_dir(config_dir)
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   pub(crate) fn init_access_config_dir<P: AsRef<Path>>(config_dir: P) -> Result<(), Error> {
>>       ACCESS_CONF_DIR
>>           .set(config_dir.as_ref().to_owned())
>>           .map_err(|_e| format_err!("cannot initialize acl tree config twice!"))
>>   }
>>
>> -pub(crate) fn init_access_config(config: &'static dyn AccessControlConfig) -> Result<(), Error> {
>> +pub fn init_access_config(config: &'static dyn AccessControlConfig) -> Result<(), Error> {
>>       ACCESS_CONF
>>           .set(config)
>>           .map_err(|_e| format_err!("cannot initialize acl tree config twice!"))
>> @@ -131,32 +135,39 @@ pub(crate) fn access_conf() -> &'static dyn AccessControlConfig {
>>           .expect("please initialize the acm config before using it!")
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   fn conf_dir() -> &'static PathBuf {
>>       ACCESS_CONF_DIR
>>           .get()
>>           .expect("please initialize acm config dir before using it!")
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   pub(crate) fn acl_config() -> PathBuf {
>>       conf_dir().join("acl.cfg")
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   pub(crate) fn acl_config_lock() -> PathBuf {
>>       conf_dir().join(".acl.lck")
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   pub(crate) fn user_config() -> PathBuf {
>>       conf_dir().join("user.cfg")
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   pub(crate) fn user_config_lock() -> PathBuf {
>>       conf_dir().join(".user.lck")
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   pub(crate) fn token_shadow() -> PathBuf {
>>       conf_dir().join("token.shadow")
>>   }
>>
>> +#[cfg(feature = "impl")]
>>   pub(crate) fn token_shadow_lock() -> PathBuf {
>>       conf_dir().join("token.shadow.lock")
>>   }
>> diff --git a/proxmox-access-control/src/lib.rs b/proxmox-access-control/src/lib.rs
>> index 62683924..9195c999 100644
>> --- a/proxmox-access-control/src/lib.rs
>> +++ b/proxmox-access-control/src/lib.rs
>> @@ -2,13 +2,13 @@
>>
>>   pub mod types;
>>
>> -#[cfg(feature = "impl")]
>> +#[cfg(feature = "acl")]
>>   pub mod acl;
>>
>>   #[cfg(feature = "api")]
>>   pub mod api;
>>
>> -#[cfg(feature = "impl")]
>> +#[cfg(feature = "acl")]
>>   pub mod init;
>>
>>   #[cfg(feature = "impl")]



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel