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

[Dominik Csapak <d.csapak@proxmox.com>]

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?

> +#[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