Re: [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Would name it "proxmox-rest-server-config" or "proxmox-api-config", and here
I'm asking myself (without really trying to answer it myself): why isn't
this part of proxmox-rest-server, it's only relevant when used in combination
of there, simply to allow use-sites that do not depend on the rest of that
crates code?

Also, mostly important for  the other two crates: did you check the history
of the files you mode? If there's anything relevant, I'd favor isolating that
and then merging it in, like I did when splitting out rest-server, but it can
sometimes be a bit of a PITA, so  can be decided case-by-case for those new
crates..

Am 18/10/2023 um 12:39 schrieb Dominik Csapak:

> diff --git a/proxmox-server-config/src/lib.rs b/proxmox-server-config/src/lib.rs
> new file mode 100644
> index 0000000..8377978
> --- /dev/null
> +++ b/proxmox-server-config/src/lib.rs
> @@ -0,0 +1,302 @@
> +//! A generic server config abstraction
> +//!
> +//! Used for proxmox daemons to have a central point for configuring things

"Used for API daemons of Proxmox projects as a central point for configuring their
base environment, like ..."

(Proxmox is our TM and is not a project itself)

> +//! like base/log/task/certificate directories, user for creating files, etc.
> +
> +use std::os::linux::fs::MetadataExt;
> +use std::path::{Path, PathBuf};
> +use std::sync::OnceLock;
> +
> +use anyhow::{format_err, Context, Error};
> +use nix::unistd::User;
> +
> +use proxmox_sys::fs::{create_path, CreateOptions};

this alone here is probably causing 90% compile time ^^
we really need to split proxmox-sys more, but that's orthogonal to this
series

> +
> +static SERVER_CONFIG: OnceLock<ServerConfig> = OnceLock::new();
> +
> +/// Get the global [`ServerConfig`] instance.
> +///
> +/// Before using this, you must create a new [`ServerConfig`] and call
> +/// [`setup`](ServerConfig::setup) on it.
> +///
> +/// Returns an [`Error`](anyhow::Error) when no server config was yet initialized.
> +pub fn get_server_config() -> Result<&'static ServerConfig, Error> {
> +    SERVER_CONFIG
> +        .get()
> +        .ok_or_else(|| format_err!("not server config initialized"))
> +}
> +
> +/// A Server configuration object.
> +///
> +/// contains the name, user and used directories of a server, like the log directory or
> +/// the state directory.
> +///
> +/// Is created by calling [`new`](Self::new) and can be set as a global object
> +/// with [`setup`](Self::setup)
> +///
> +/// # Example
> +///
> +/// On server initialize run something like this:
> +/// ```
> +/// use proxmox_server_config::ServerConfig;
> +///
> +/// # fn function() -> Result<(), anyhow::Error> {
> +/// # let some_user = nix::unistd::User::from_uid(nix::unistd::ROOT).unwrap().unwrap();
> +/// # let privileged_user = nix::unistd::User::from_uid(nix::unistd::ROOT).unwrap().unwrap();
> +/// ServerConfig::new("name-of-daemon", "/some/base/dir", some_user)?
> +///     .with_privileged_user(privileged_user)?
> +///     .with_task_dir("/var/log/tasks")?
> +///     .with_config_dir("/etc/some-dir")?
> +///     // ...
> +///     .setup()?;
> +/// # Ok(())
> +/// # }
> +/// ```
> +///
> +/// Then you can use it in other parts of your daemon:
> +///
> +/// ```
> +/// use proxmox_server_config:: get_server_config;
> +///
> +/// # fn function() -> Result<(), anyhow::Error> {
> +/// let task_dir = get_server_config()?.task_dir();
> +/// let user = get_server_config()?.user();
> +/// // ...and so on
> +/// # Ok(())
> +/// # }
> +/// ```
> +pub struct ServerConfig {

Lacks doc comments for members, please add some even if they're
private.

> +    name: String,
> +    base_dir: PathBuf,
> +    user: User,
> +    privileged_user: OnceLock<User>,
> +
> +    task_dir: OnceLock<PathBuf>,

not sure if this should be an option, e.g., for simple CRUD
API daemons there might not be any worker tasks at all.


> +    log_dir: OnceLock<PathBuf>,

maybe: access_log_dir (as this is the api access log that other

> +    cert_dir: OnceLock<PathBuf>,
> +    state_dir: OnceLock<PathBuf>,
> +    run_dir: OnceLock<PathBuf>,
> +    config_dir: OnceLock<PathBuf>,
> +}
> +