* [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs
@ 2023-10-18 10:39 Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate Dominik Csapak
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Dominik Csapak @ 2023-10-18 10:39 UTC (permalink / raw)
To: pbs-devel
# General
Sending this as RFC because of the multiple ways (see 'Alternatives'
section) we could implement this (or something similar). I'm not sure
what the general opinion about this pattern is.
# Goal
When creating a new daemon/server, we currently have much code in pbs
that would have to be duplicated, mostly relating to setting up the
server directories/files. Some crates features already have an interface
to be slightly configurable, but not all of them.
This change adds a new crate 'proxmox-server-config' that provides an
interface which should be easy to use and reusable across multiple
daemons/server applications for our use-cases.
In this series i opted to factor out the two main remaining crates that
are not seperated yet: the jobstates and the certificate management.
(Though i guess there are more, depending on what another daemon might
want to do)
# Implementation
I use a static OnceLock to save a global `ServerConfig` struct, that
contains some configurations available, which are either setup, or
derived from the minimum required configuration (base_dir, name, user).
This can then be used to access them from any point in the program or in
crates where needed.
With this, we don't have to carry around the different directories as
parameters, nor do we have to sprinkle things like `config!("foo.bar")`,
around the code to get the path we want.
# Alternatives
The main alternatives to this approaches are:
* copy the code across different server/daemons:
obviously the easiest way, but doesn't scale well and prone to
diverging implementations.
* parameters:
this has two ways of working:
- all functions must take the configuration (dir(s)/user(s))
blows up the parameters, and we have to use our current dir macros
even more
- each crate/module has an initializer that takes the config, which
would then e.g. save that into a static OnceLock once again,
disadvantage here is that we may duplicate values across multiple
crates (should not be much though)
There are probably more ways to implement this (e.g. maybe a global
trait object that must be implemented by each server?)
# Future work
We could extend this pattern for other crates (e.g. rest-server) so
that we don't have to carry around the parameters anymore.
Also we could make the ServerConfig extendable, where each server could
configure it's own necessary variables (e.g with a hashmap)
proxmox:
Dominik Csapak (3):
new proxmox-server-config crate
new proxmox-jobstate crate
new proxmox-cert-management crate
Cargo.toml | 5 +
proxmox-cert-management/Cargo.toml | 23 ++
proxmox-cert-management/debian/changelog | 5 +
proxmox-cert-management/debian/control | 53 ++++
proxmox-cert-management/debian/copyright | 18 ++
proxmox-cert-management/debian/debcargo.toml | 7 +
proxmox-cert-management/src/lib.rs | 182 +++++++++++
proxmox-jobstate/Cargo.toml | 26 ++
proxmox-jobstate/debian/changelog | 5 +
proxmox-jobstate/debian/control | 55 ++++
proxmox-jobstate/debian/copyright | 18 ++
proxmox-jobstate/debian/debcargo.toml | 7 +
proxmox-jobstate/src/api_types.rs | 40 +++
proxmox-jobstate/src/jobstate.rs | 315 +++++++++++++++++++
proxmox-jobstate/src/lib.rs | 46 +++
proxmox-server-config/Cargo.toml | 16 +
proxmox-server-config/debian/changelog | 5 +
proxmox-server-config/debian/control | 37 +++
proxmox-server-config/debian/copyright | 18 ++
proxmox-server-config/debian/debcargo.toml | 7 +
proxmox-server-config/src/lib.rs | 302 ++++++++++++++++++
21 files changed, 1190 insertions(+)
create mode 100644 proxmox-cert-management/Cargo.toml
create mode 100644 proxmox-cert-management/debian/changelog
create mode 100644 proxmox-cert-management/debian/control
create mode 100644 proxmox-cert-management/debian/copyright
create mode 100644 proxmox-cert-management/debian/debcargo.toml
create mode 100644 proxmox-cert-management/src/lib.rs
create mode 100644 proxmox-jobstate/Cargo.toml
create mode 100644 proxmox-jobstate/debian/changelog
create mode 100644 proxmox-jobstate/debian/control
create mode 100644 proxmox-jobstate/debian/copyright
create mode 100644 proxmox-jobstate/debian/debcargo.toml
create mode 100644 proxmox-jobstate/src/api_types.rs
create mode 100644 proxmox-jobstate/src/jobstate.rs
create mode 100644 proxmox-jobstate/src/lib.rs
create mode 100644 proxmox-server-config/Cargo.toml
create mode 100644 proxmox-server-config/debian/changelog
create mode 100644 proxmox-server-config/debian/control
create mode 100644 proxmox-server-config/debian/copyright
create mode 100644 proxmox-server-config/debian/debcargo.toml
create mode 100644 proxmox-server-config/src/lib.rs
proxmox-backup:
Dominik Csapak (1):
use proxmox_jobstate and proxmox_cert_management crates
Cargo.toml | 8 +
pbs-api-types/Cargo.toml | 1 +
pbs-api-types/src/jobs.rs | 41 +---
src/auth_helpers.rs | 184 +---------------
src/bin/proxmox-backup-api.rs | 8 +-
src/bin/proxmox-backup-manager.rs | 2 +
src/bin/proxmox-backup-proxy.rs | 2 +
src/server/jobstate.rs | 347 +-----------------------------
src/server/mod.rs | 21 ++
9 files changed, 52 insertions(+), 562 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate
2023-10-18 10:39 [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Dominik Csapak
@ 2023-10-18 10:39 ` Dominik Csapak
2023-10-25 16:38 ` Thomas Lamprecht
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 2/3] new proxmox-jobstate crate Dominik Csapak
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Dominik Csapak @ 2023-10-18 10:39 UTC (permalink / raw)
To: pbs-devel
aims to provide global server config that can be initialized from the
main daemon entry point, and used in other crates without passing around
the individual directories and the user
uses `OnceLock` with `static` to provide a global reference to the
finished instance.
general use is intended like this:
ServerConfig::new("daemon-name", "/some/base/dir", some_user)?
.with_task_dir("/some/other/dir")
...
.setup()?;
and then use it in other places like this:
let task_dir = get_server_config()?.task_dir();
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
Cargo.toml | 1 +
proxmox-server-config/Cargo.toml | 16 ++
proxmox-server-config/debian/changelog | 5 +
proxmox-server-config/debian/control | 37 +++
proxmox-server-config/debian/copyright | 18 ++
proxmox-server-config/debian/debcargo.toml | 7 +
proxmox-server-config/src/lib.rs | 302 +++++++++++++++++++++
7 files changed, 386 insertions(+)
create mode 100644 proxmox-server-config/Cargo.toml
create mode 100644 proxmox-server-config/debian/changelog
create mode 100644 proxmox-server-config/debian/control
create mode 100644 proxmox-server-config/debian/copyright
create mode 100644 proxmox-server-config/debian/debcargo.toml
create mode 100644 proxmox-server-config/src/lib.rs
diff --git a/Cargo.toml b/Cargo.toml
index f8bc181..6b22c58 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -103,6 +103,7 @@ proxmox-router = { version = "2.1.1", path = "proxmox-router" }
proxmox-schema = { version = "2.0.0", path = "proxmox-schema" }
proxmox-section-config = { version = "2.0.0", path = "proxmox-section-config" }
proxmox-serde = { version = "0.1.1", path = "proxmox-serde", features = [ "serde_json" ] }
+proxmox-server-config = { version = "0.1", path = "proxmox-server-config" }
proxmox-sortable-macro = { version = "0.1.3", path = "proxmox-sortable-macro" }
proxmox-sys = { version = "0.5.0", path = "proxmox-sys" }
proxmox-tfa = { version = "4.0.4", path = "proxmox-tfa" }
diff --git a/proxmox-server-config/Cargo.toml b/proxmox-server-config/Cargo.toml
new file mode 100644
index 0000000..f0f4de2
--- /dev/null
+++ b/proxmox-server-config/Cargo.toml
@@ -0,0 +1,16 @@
+[package]
+name = "proxmox-server-config"
+version = "0.1.0"
+authors.workspace = true
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
+description = "Generic Server Config crate"
+
+exclude.workspace = true
+
+[dependencies]
+anyhow.workspace = true
+nix.workspace = true
+
+proxmox-sys.workspace = true
diff --git a/proxmox-server-config/debian/changelog b/proxmox-server-config/debian/changelog
new file mode 100644
index 0000000..4c21324
--- /dev/null
+++ b/proxmox-server-config/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-server-config (0.1.0-1) stable; urgency=medium
+
+ * initial version
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 03 Oct 2023 10:56:15 +0200
diff --git a/proxmox-server-config/debian/control b/proxmox-server-config/debian/control
new file mode 100644
index 0000000..48a3ff3
--- /dev/null
+++ b/proxmox-server-config/debian/control
@@ -0,0 +1,37 @@
+Source: rust-proxmox-server-config
+Section: rust
+Priority: optional
+Build-Depends: debhelper (>= 12),
+ dh-cargo (>= 25),
+ cargo:native <!nocheck>,
+ rustc:native <!nocheck>,
+ libstd-rust-dev <!nocheck>,
+ librust-anyhow-1+default-dev <!nocheck>,
+ librust-nix-0.26+default-dev (>= 0.26.1-~~) <!nocheck>,
+ librust-proxmox-sys-0.5+default-dev <!nocheck>
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Standards-Version: 4.6.1
+Vcs-Git: git://git.proxmox.com/git/proxmox.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+X-Cargo-Crate: proxmox-server-config
+Rules-Requires-Root: no
+
+Package: librust-proxmox-server-config-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-anyhow-1+default-dev,
+ librust-nix-0.26+default-dev (>= 0.26.1-~~),
+ librust-proxmox-sys-0.5+default-dev
+Provides:
+ librust-proxmox-server-config+default-dev (= ${binary:Version}),
+ librust-proxmox-server-config-0-dev (= ${binary:Version}),
+ librust-proxmox-server-config-0+default-dev (= ${binary:Version}),
+ librust-proxmox-server-config-0.1-dev (= ${binary:Version}),
+ librust-proxmox-server-config-0.1+default-dev (= ${binary:Version}),
+ librust-proxmox-server-config-0.1.0-dev (= ${binary:Version}),
+ librust-proxmox-server-config-0.1.0+default-dev (= ${binary:Version})
+Description: Generic Server Config crate - Rust source code
+ This package contains the source for the Rust proxmox-server-config crate,
+ packaged by debcargo for use with cargo and dh-cargo.
diff --git a/proxmox-server-config/debian/copyright b/proxmox-server-config/debian/copyright
new file mode 100644
index 0000000..0d9eab3
--- /dev/null
+++ b/proxmox-server-config/debian/copyright
@@ -0,0 +1,18 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+
+Files:
+ *
+Copyright: 2019 - 2023 Proxmox Server Solutions GmbH <support@proxmox.com>
+License: AGPL-3.0-or-later
+ This program is free software: you can redistribute it and/or modify it under
+ the terms of the GNU Affero General Public License as published by the Free
+ Software Foundation, either version 3 of the License, or (at your option) any
+ later version.
+ .
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ details.
+ .
+ You should have received a copy of the GNU Affero General Public License along
+ with this program. If not, see <https://www.gnu.org/licenses/>.
diff --git a/proxmox-server-config/debian/debcargo.toml b/proxmox-server-config/debian/debcargo.toml
new file mode 100644
index 0000000..b7864cd
--- /dev/null
+++ b/proxmox-server-config/debian/debcargo.toml
@@ -0,0 +1,7 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+[source]
+vcs_git = "git://git.proxmox.com/git/proxmox.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
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
+//! 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};
+
+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 {
+ name: String,
+ base_dir: PathBuf,
+ user: User,
+ privileged_user: OnceLock<User>,
+
+ task_dir: OnceLock<PathBuf>,
+ log_dir: OnceLock<PathBuf>,
+ cert_dir: OnceLock<PathBuf>,
+ state_dir: OnceLock<PathBuf>,
+ run_dir: OnceLock<PathBuf>,
+ config_dir: OnceLock<PathBuf>,
+}
+
+fn check_dir_permissions<M: MetadataExt>(metadata: M, user: &User) -> bool {
+ let mode = metadata.st_mode();
+ let user_write = metadata.st_uid() == user.uid.as_raw() && mode & 0o200 > 0;
+ let group_write = metadata.st_gid() == user.gid.as_raw() && mode & 0o020 > 0;
+ let other_write = mode & 0o002 > 0;
+
+ user_write || group_write || other_write
+}
+
+impl ServerConfig {
+ /// Creates a new instance of [`ServerConfig`], with sensible defaults derived from the given
+ /// `name` and `base_dir`. Permissions are checked against the given `user`.
+ pub fn new<P: AsRef<Path>>(name: &str, base_dir: P, user: User) -> std::io::Result<Self> {
+ let base_dir = base_dir.as_ref();
+
+ let metadata = std::fs::metadata(base_dir)?;
+ if !metadata.is_dir() {
+ return Err(std::io::Error::new(
+ std::io::ErrorKind::NotFound,
+ "base directory does not exists or is not a directory",
+ ));
+ }
+ if !check_dir_permissions(metadata, &user) {
+ return Err(std::io::Error::new(
+ std::io::ErrorKind::PermissionDenied,
+ format!(
+ "user '{}' does not have enough permissions on base directory",
+ user.name
+ ),
+ ));
+ }
+
+ Ok(Self {
+ name: name.to_string(),
+ base_dir: base_dir.to_owned(),
+ user,
+ privileged_user: OnceLock::new(),
+ task_dir: OnceLock::new(),
+ log_dir: OnceLock::new(),
+ cert_dir: OnceLock::new(),
+ state_dir: OnceLock::new(),
+ run_dir: OnceLock::new(),
+ config_dir: OnceLock::new(),
+ })
+ }
+
+ /// Finishes the server config setup by creating the dirs and creating a global
+ /// reference to it with [`OnceLock`]
+ ///
+ /// Creates the configured directories. If no error is returned, all directories are created
+ /// and writable with the configured user. After this, you can get a reference to the
+ /// config with [`get_server_config`].
+ pub fn setup(self) -> Result<(), Error> {
+ self.create_dirs()?;
+
+ SERVER_CONFIG
+ .set(self)
+ .map_err(|_| format_err!("Server config already set"))?;
+
+ Ok(())
+ }
+
+ fn create_dirs(&self) -> Result<(), Error> {
+ let opts = CreateOptions::new()
+ .owner(self.user.uid)
+ .group(self.user.gid);
+
+ create_path(&self.base_dir, Some(opts.clone()), Some(opts.clone()))
+ .context("could not create base directory")?;
+
+ create_path(self.task_dir(), Some(opts.clone()), Some(opts.clone()))
+ .context("could not create task directory")?;
+
+ create_path(self.log_dir(), Some(opts.clone()), Some(opts.clone()))
+ .context("could not create log directory")?;
+
+ create_path(self.cert_dir(), Some(opts.clone()), Some(opts.clone()))
+ .context("could not create certificate directory")?;
+
+ create_path(self.state_dir(), Some(opts.clone()), Some(opts.clone()))
+ .context("could not create state directory")?;
+
+ create_path(self.run_dir(), Some(opts.clone()), Some(opts))
+ .context("could not create run directory")?;
+
+ Ok(())
+ }
+
+ /// Returns the configured user
+ pub fn user(&self) -> &User {
+ &self.user
+ }
+
+ /// Returns the configured privileged user. Defaults to the regular configured user.
+ pub fn privileged_user(&self) -> &User {
+ self.privileged_user.get_or_init(|| self.user.clone())
+ }
+
+ /// Set the privileged user
+ pub fn set_privileged_user(&mut self, user: User) -> Result<(), Error> {
+ self.privileged_user
+ .set(user)
+ .map_err(|_| format_err!("already set privileged user"))
+ }
+
+ /// Builder style method to set the privileged user
+ pub fn with_privileged_user(mut self, user: User) -> Result<Self, Error> {
+ self.set_privileged_user(user)?;
+ Ok(self)
+ }
+
+ /// Set the task directory.
+ pub fn set_task_dir<P: AsRef<Path>>(&mut self, path: P) -> Result<(), Error> {
+ self.task_dir
+ .set(path.as_ref().to_owned())
+ .map_err(|_| format_err!("already set task dir"))
+ }
+
+ /// Builder style method to set the task directory.
+ pub fn with_task_dir<P: AsRef<Path>>(mut self, path: P) -> Result<Self, Error> {
+ self.set_task_dir(path)?;
+ Ok(self)
+ }
+
+ /// Get the task directory
+ pub fn task_dir(&self) -> &Path {
+ self.task_dir.get_or_init(|| self.base_dir.join("tasks"))
+ }
+
+ /// Set the log directory.
+ pub fn set_log_dir<P: AsRef<Path>>(&mut self, path: P) -> Result<(), Error> {
+ self.log_dir
+ .set(path.as_ref().to_owned())
+ .map_err(|_| format_err!("already set log dir"))
+ }
+
+ /// Builder style method to set the log directory.
+ pub fn with_log_dir<P: AsRef<Path>>(mut self, path: P) -> Result<Self, Error> {
+ self.set_log_dir(path)?;
+ Ok(self)
+ }
+
+ /// Get the log directory
+ pub fn log_dir(&self) -> &Path {
+ self.log_dir.get_or_init(|| self.base_dir.join("logs"))
+ }
+
+ /// Set the cert directory.
+ pub fn set_cert_dir<P: AsRef<Path>>(&mut self, path: P) -> Result<(), Error> {
+ self.cert_dir
+ .set(path.as_ref().to_owned())
+ .map_err(|_| format_err!("already set cert dir"))
+ }
+
+ /// Builder style method to set the cert directory.
+ pub fn with_cert_dir<P: AsRef<Path>>(mut self, path: P) -> Result<Self, Error> {
+ self.set_cert_dir(path)?;
+ Ok(self)
+ }
+
+ /// Get the cert directory
+ pub fn cert_dir(&self) -> &Path {
+ self.cert_dir
+ .get_or_init(|| self.base_dir.join("certificates"))
+ }
+
+ /// Set the state directory.
+ pub fn set_state_dir<P: AsRef<Path>>(&mut self, path: P) -> Result<(), Error> {
+ self.state_dir
+ .set(path.as_ref().to_owned())
+ .map_err(|_| format_err!("already set state dir"))
+ }
+
+ /// Builder style method to set the state directory.
+ pub fn with_state_dir<P: AsRef<Path>>(mut self, path: P) -> Result<Self, Error> {
+ self.set_state_dir(path)?;
+ Ok(self)
+ }
+
+ /// Get the state directory
+ pub fn state_dir(&self) -> &Path {
+ self.state_dir.get_or_init(|| self.base_dir.join("states"))
+ }
+
+ /// Set the run directory.
+ pub fn set_run_dir<P: AsRef<Path>>(&mut self, path: P) -> Result<(), Error> {
+ self.run_dir
+ .set(path.as_ref().to_owned())
+ .map_err(|_| format_err!("already set run dir"))
+ }
+
+ /// Builder style method to set the run directory.
+ pub fn with_run_dir<P: AsRef<Path>>(mut self, path: P) -> Result<Self, Error> {
+ self.set_run_dir(path)?;
+ Ok(self)
+ }
+
+ /// Get the run directory
+ pub fn run_dir(&self) -> &Path {
+ self.run_dir
+ .get_or_init(|| PathBuf::from(format!("/run/{}", self.name)))
+ }
+
+ /// Set the config directory.
+ pub fn set_config_dir<P: AsRef<Path>>(&mut self, path: P) -> Result<(), Error> {
+ self.config_dir
+ .set(path.as_ref().to_owned())
+ .map_err(|_| format_err!("already set config dir"))
+ }
+
+ /// Builder style method to set the config directory.
+ pub fn with_config_dir<P: AsRef<Path>>(mut self, path: P) -> Result<Self, Error> {
+ self.set_config_dir(path)?;
+ Ok(self)
+ }
+
+ /// Get the config directory
+ pub fn config_dir(&self) -> &Path {
+ self.config_dir.get_or_init(|| self.base_dir.join("config"))
+ }
+}
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pbs-devel] [RFC proxmox 2/3] new proxmox-jobstate crate
2023-10-18 10:39 [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate Dominik Csapak
@ 2023-10-18 10:39 ` Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 3/3] new proxmox-cert-management crate Dominik Csapak
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Dominik Csapak @ 2023-10-18 10:39 UTC (permalink / raw)
To: pbs-devel
split out from proxmox-backup/pbs-api-types
includes the JobScheduleStatus api type
depends on the new ServerConfig for the global user/directory config
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
Cargo.toml | 2 +
proxmox-jobstate/Cargo.toml | 26 +++
proxmox-jobstate/debian/changelog | 5 +
proxmox-jobstate/debian/control | 55 +++++
proxmox-jobstate/debian/copyright | 18 ++
proxmox-jobstate/debian/debcargo.toml | 7 +
proxmox-jobstate/src/api_types.rs | 40 ++++
proxmox-jobstate/src/jobstate.rs | 315 ++++++++++++++++++++++++++
proxmox-jobstate/src/lib.rs | 46 ++++
9 files changed, 514 insertions(+)
create mode 100644 proxmox-jobstate/Cargo.toml
create mode 100644 proxmox-jobstate/debian/changelog
create mode 100644 proxmox-jobstate/debian/control
create mode 100644 proxmox-jobstate/debian/copyright
create mode 100644 proxmox-jobstate/debian/debcargo.toml
create mode 100644 proxmox-jobstate/src/api_types.rs
create mode 100644 proxmox-jobstate/src/jobstate.rs
create mode 100644 proxmox-jobstate/src/lib.rs
diff --git a/Cargo.toml b/Cargo.toml
index 6b22c58..4b4b787 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -11,6 +11,7 @@ members = [
"proxmox-http-error",
"proxmox-human-byte",
"proxmox-io",
+ "proxmox-jobstate",
"proxmox-lang",
"proxmox-ldap",
"proxmox-login",
@@ -22,6 +23,7 @@ members = [
"proxmox-schema",
"proxmox-section-config",
"proxmox-serde",
+ "proxmox-server-config",
"proxmox-shared-memory",
"proxmox-sortable-macro",
"proxmox-subscription",
diff --git a/proxmox-jobstate/Cargo.toml b/proxmox-jobstate/Cargo.toml
new file mode 100644
index 0000000..d2ba93b
--- /dev/null
+++ b/proxmox-jobstate/Cargo.toml
@@ -0,0 +1,26 @@
+[package]
+name = "proxmox-jobstate"
+version = "0.1.0"
+authors.workspace = true
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
+description = "Jobstate handling"
+
+exclude.workspace = true
+
+[dependencies]
+serde = { workspace = true, features = ["derive"] }
+
+proxmox-schema = { workspace = true, features = [ "api-macro" ] }
+
+[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
+anyhow.workspace = true
+nix.workspace = true
+once_cell.workspace = true
+serde_json.workspace = true
+
+proxmox-rest-server = { workspace = true, features = [] }
+proxmox-server-config.workspace = true
+proxmox-sys.workspace = true
+proxmox-time.workspace = true
diff --git a/proxmox-jobstate/debian/changelog b/proxmox-jobstate/debian/changelog
new file mode 100644
index 0000000..484a98a
--- /dev/null
+++ b/proxmox-jobstate/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-jobstate (0.1.0-1) stable; urgency=medium
+
+ * initial split out of proxmox-backup
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 26 Sep 2023 13:51:49 +0200
diff --git a/proxmox-jobstate/debian/control b/proxmox-jobstate/debian/control
new file mode 100644
index 0000000..fe3c0bc
--- /dev/null
+++ b/proxmox-jobstate/debian/control
@@ -0,0 +1,55 @@
+Source: rust-proxmox-jobstate
+Section: rust
+Priority: optional
+Build-Depends: debhelper (>= 12),
+ dh-cargo (>= 25),
+ cargo:native <!nocheck>,
+ rustc:native <!nocheck>,
+ libstd-rust-dev <!nocheck>,
+ librust-anyhow-1+default-dev <!nocheck>,
+ librust-nix-0.26+default-dev (>= 0.26.1-~~) <!nocheck>,
+ librust-once-cell-1+default-dev (>= 1.3.1-~~) <!nocheck>,
+ librust-proxmox-rest-server-0.4+default-dev <!nocheck>,
+ librust-proxmox-schema-2+api-macro-dev <!nocheck>,
+ librust-proxmox-schema-2+default-dev <!nocheck>,
+ librust-proxmox-server-config-0.1+default-dev <!nocheck>,
+ librust-proxmox-sys-0.5+default-dev <!nocheck>,
+ librust-proxmox-time-1+default-dev (>= 1.1.4-~~) <!nocheck>,
+ librust-serde-1+default-dev <!nocheck>,
+ librust-serde-1+derive-dev <!nocheck>,
+ librust-serde-json-1+default-dev <!nocheck>
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Standards-Version: 4.6.1
+Vcs-Git: git://git.proxmox.com/git/proxmox.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+X-Cargo-Crate: proxmox-jobstate
+Rules-Requires-Root: no
+
+Package: librust-proxmox-jobstate-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-anyhow-1+default-dev,
+ librust-nix-0.26+default-dev (>= 0.26.1-~~),
+ librust-once-cell-1+default-dev (>= 1.3.1-~~),
+ librust-proxmox-rest-server-0.4+default-dev,
+ librust-proxmox-schema-2+api-macro-dev,
+ librust-proxmox-schema-2+default-dev,
+ librust-proxmox-server-config-0.1+default-dev,
+ librust-proxmox-sys-0.5+default-dev,
+ librust-proxmox-time-1+default-dev (>= 1.1.4-~~),
+ librust-serde-1+default-dev,
+ librust-serde-1+derive-dev,
+ librust-serde-json-1+default-dev
+Provides:
+ librust-proxmox-jobstate+default-dev (= ${binary:Version}),
+ librust-proxmox-jobstate-0-dev (= ${binary:Version}),
+ librust-proxmox-jobstate-0+default-dev (= ${binary:Version}),
+ librust-proxmox-jobstate-0.1-dev (= ${binary:Version}),
+ librust-proxmox-jobstate-0.1+default-dev (= ${binary:Version}),
+ librust-proxmox-jobstate-0.1.0-dev (= ${binary:Version}),
+ librust-proxmox-jobstate-0.1.0+default-dev (= ${binary:Version})
+Description: Jobstate handling - Rust source code
+ This package contains the source for the Rust proxmox-jobstate crate, packaged
+ by debcargo for use with cargo and dh-cargo.
diff --git a/proxmox-jobstate/debian/copyright b/proxmox-jobstate/debian/copyright
new file mode 100644
index 0000000..0d9eab3
--- /dev/null
+++ b/proxmox-jobstate/debian/copyright
@@ -0,0 +1,18 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+
+Files:
+ *
+Copyright: 2019 - 2023 Proxmox Server Solutions GmbH <support@proxmox.com>
+License: AGPL-3.0-or-later
+ This program is free software: you can redistribute it and/or modify it under
+ the terms of the GNU Affero General Public License as published by the Free
+ Software Foundation, either version 3 of the License, or (at your option) any
+ later version.
+ .
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ details.
+ .
+ You should have received a copy of the GNU Affero General Public License along
+ with this program. If not, see <https://www.gnu.org/licenses/>.
diff --git a/proxmox-jobstate/debian/debcargo.toml b/proxmox-jobstate/debian/debcargo.toml
new file mode 100644
index 0000000..b7864cd
--- /dev/null
+++ b/proxmox-jobstate/debian/debcargo.toml
@@ -0,0 +1,7 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+[source]
+vcs_git = "git://git.proxmox.com/git/proxmox.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
diff --git a/proxmox-jobstate/src/api_types.rs b/proxmox-jobstate/src/api_types.rs
new file mode 100644
index 0000000..c04d72a
--- /dev/null
+++ b/proxmox-jobstate/src/api_types.rs
@@ -0,0 +1,40 @@
+use proxmox_schema::api;
+use serde::{Deserialize, Serialize};
+
+#[api(
+ properties: {
+ "next-run": {
+ description: "Estimated time of the next run (UNIX epoch).",
+ optional: true,
+ type: Integer,
+ },
+ "last-run-state": {
+ description: "Result of the last run.",
+ optional: true,
+ type: String,
+ },
+ "last-run-upid": {
+ description: "Task UPID of the last run.",
+ optional: true,
+ type: String,
+ },
+ "last-run-endtime": {
+ description: "Endtime of the last run.",
+ optional: true,
+ type: Integer,
+ },
+ }
+)]
+#[derive(Serialize, Deserialize, Default, Clone, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// Job Scheduling Status
+pub struct JobScheduleStatus {
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub next_run: Option<i64>,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub last_run_state: Option<String>,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub last_run_upid: Option<String>,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub last_run_endtime: Option<i64>,
+}
diff --git a/proxmox-jobstate/src/jobstate.rs b/proxmox-jobstate/src/jobstate.rs
new file mode 100644
index 0000000..9522c5b
--- /dev/null
+++ b/proxmox-jobstate/src/jobstate.rs
@@ -0,0 +1,315 @@
+use std::path::{Path, PathBuf};
+
+use anyhow::{bail, format_err, Error};
+use nix::unistd::User;
+use serde::{Deserialize, Serialize};
+
+use proxmox_rest_server::{upid_read_status, worker_is_active_local, TaskState};
+use proxmox_schema::upid::UPID;
+use proxmox_sys::fs::{create_path, file_read_optional_string, replace_file, CreateOptions};
+use proxmox_time::CalendarEvent;
+
+use crate::JobScheduleStatus;
+
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Represents the State of a specific Job
+pub enum JobState {
+ /// A job was created at 'time', but never started/finished
+ Created { time: i64 },
+ /// The Job was last started in 'upid',
+ Started { upid: String },
+ /// The Job was last started in 'upid', which finished with 'state', and was last updated at 'updated'
+ Finished {
+ upid: String,
+ state: TaskState,
+ updated: Option<i64>,
+ },
+}
+
+/// Represents a Job and holds the correct lock
+pub struct Job {
+ jobtype: String,
+ jobname: String,
+ /// The State of the job
+ pub state: JobState,
+ _lock: std::fs::File,
+}
+
+/// Create jobstate stat dir with correct permission
+///
+/// It is necessary to initialize a global [`ServerConfig`](proxmox_server_config::ServerConfig)
+/// first.
+pub fn create_jobstate_dir() -> Result<(), Error> {
+ let server_config = proxmox_server_config::get_server_config()?;
+ let path = server_config.state_dir().join("jobstates");
+
+ let user = server_config.user();
+ let opts = CreateOptions::new().owner(user.uid).group(user.gid);
+
+ create_path(path, Some(opts.clone()), Some(opts))
+ .map_err(|err: Error| format_err!("unable to create job state dir - {err}"))?;
+
+ Ok(())
+}
+
+fn get_path(jobtype: &str, jobname: &str) -> Result<PathBuf, Error> {
+ let server_config = proxmox_server_config::get_server_config()?;
+ let mut path = server_config.state_dir().join("jobstates");
+ path.push(format!("{jobtype}-{jobname}.json"));
+ Ok(path)
+}
+
+fn get_user() -> Result<&'static User, Error> {
+ Ok(proxmox_server_config::get_server_config()?.user())
+}
+
+fn get_lock<P>(path: P) -> Result<std::fs::File, Error>
+where
+ P: AsRef<Path>,
+{
+ let mut path = path.as_ref().to_path_buf();
+ path.set_extension("lck");
+ let user = get_user()?;
+ let options = proxmox_sys::fs::CreateOptions::new()
+ .perm(nix::sys::stat::Mode::from_bits_truncate(0o660))
+ .owner(user.uid)
+ .group(user.gid);
+ let timeout = std::time::Duration::new(10, 0);
+
+ let file = proxmox_sys::fs::open_file_locked(&path, timeout, true, options)?;
+ Ok(file)
+}
+
+/// Removes the statefile of a job, this is useful if we delete a job
+pub fn remove_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> {
+ let mut path = get_path(jobtype, jobname)?;
+ let _lock = get_lock(&path)?;
+ if let Err(err) = std::fs::remove_file(&path) {
+ if err.kind() != std::io::ErrorKind::NotFound {
+ bail!("cannot remove statefile for {jobtype} - {jobname}: {err}");
+ }
+ }
+ path.set_extension("lck");
+ if let Err(err) = std::fs::remove_file(&path) {
+ if err.kind() != std::io::ErrorKind::NotFound {
+ bail!("cannot remove lockfile for {jobtype} - {jobname}: {err}");
+ }
+ }
+ Ok(())
+}
+
+/// Creates the statefile with the state 'Created'
+/// overwrites if it exists already
+pub fn create_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> {
+ let mut job = Job::new(jobtype, jobname)?;
+ job.write_state()
+}
+
+/// Tries to update the state file with the current time
+/// if the job is currently running, does nothing.
+/// Intended for use when the schedule changes.
+pub fn update_job_last_run_time(jobtype: &str, jobname: &str) -> Result<(), Error> {
+ let mut job = match Job::new(jobtype, jobname) {
+ Ok(job) => job,
+ Err(_) => return Ok(()), // was locked (running), so do not update
+ };
+ let time = proxmox_time::epoch_i64();
+
+ job.state = match JobState::load(jobtype, jobname)? {
+ JobState::Created { .. } => JobState::Created { time },
+ JobState::Started { .. } => return Ok(()), // currently running (without lock?)
+ JobState::Finished {
+ upid,
+ state,
+ updated: _,
+ } => JobState::Finished {
+ upid,
+ state,
+ updated: Some(time),
+ },
+ };
+ job.write_state()
+}
+
+/// Returns the last run time of a job by reading the statefile
+/// Note that this is not locked
+pub fn last_run_time(jobtype: &str, jobname: &str) -> Result<i64, Error> {
+ match JobState::load(jobtype, jobname)? {
+ JobState::Created { time } => Ok(time),
+ JobState::Finished {
+ updated: Some(time),
+ ..
+ } => Ok(time),
+ JobState::Started { upid }
+ | JobState::Finished {
+ upid,
+ state: _,
+ updated: None,
+ } => {
+ let upid: UPID = upid
+ .parse()
+ .map_err(|err| format_err!("could not parse upid from state: {err}"))?;
+ Ok(upid.starttime)
+ }
+ }
+}
+
+impl JobState {
+ /// Loads and deserializes the jobstate from type and name.
+ /// When the loaded state indicates a started UPID,
+ /// we go and check if it has already stopped, and
+ /// returning the correct state.
+ ///
+ /// This does not update the state in the file.
+ pub fn load(jobtype: &str, jobname: &str) -> Result<Self, Error> {
+ if let Some(state) = file_read_optional_string(get_path(jobtype, jobname)?)? {
+ match serde_json::from_str(&state)? {
+ JobState::Started { upid } => {
+ let parsed: UPID = upid
+ .parse()
+ .map_err(|err| format_err!("error parsing upid: {err}"))?;
+
+ if !worker_is_active_local(&parsed) {
+ let state = upid_read_status(&parsed)
+ .map_err(|err| format_err!("error reading upid log status: {err}"))?;
+
+ Ok(JobState::Finished {
+ upid,
+ state,
+ updated: None,
+ })
+ } else {
+ Ok(JobState::Started { upid })
+ }
+ }
+ other => Ok(other),
+ }
+ } else {
+ Ok(JobState::Created {
+ time: proxmox_time::epoch_i64() - 30,
+ })
+ }
+ }
+}
+
+impl Job {
+ /// Creates a new instance of a job with the correct lock held
+ /// (will be hold until the job is dropped again).
+ ///
+ /// This does not load the state from the file, to do that,
+ /// 'load' must be called
+ pub fn new(jobtype: &str, jobname: &str) -> Result<Self, Error> {
+ let path = get_path(jobtype, jobname)?;
+
+ let _lock = get_lock(path)?;
+
+ Ok(Self {
+ jobtype: jobtype.to_string(),
+ jobname: jobname.to_string(),
+ state: JobState::Created {
+ time: proxmox_time::epoch_i64(),
+ },
+ _lock,
+ })
+ }
+
+ /// Start the job and update the statefile accordingly
+ /// Fails if the job was already started
+ pub fn start(&mut self, upid: &str) -> Result<(), Error> {
+ if let JobState::Started { .. } = self.state {
+ bail!("cannot start job that is started!");
+ }
+
+ self.state = JobState::Started {
+ upid: upid.to_string(),
+ };
+
+ self.write_state()
+ }
+
+ /// Finish the job and update the statefile accordingly with the given taskstate
+ /// Fails if the job was not yet started
+ pub fn finish(&mut self, state: TaskState) -> Result<(), Error> {
+ let upid = match &self.state {
+ JobState::Created { .. } => bail!("cannot finish when not started"),
+ JobState::Started { upid } => upid,
+ JobState::Finished { upid, .. } => upid,
+ }
+ .to_string();
+
+ self.state = JobState::Finished {
+ upid,
+ state,
+ updated: None,
+ };
+
+ self.write_state()
+ }
+
+ pub fn jobtype(&self) -> &str {
+ &self.jobtype
+ }
+
+ pub fn jobname(&self) -> &str {
+ &self.jobname
+ }
+
+ fn write_state(&mut self) -> Result<(), Error> {
+ let serialized = serde_json::to_string(&self.state)?;
+ let path = get_path(&self.jobtype, &self.jobname)?;
+
+ let backup_user = get_user()?;
+ let mode = nix::sys::stat::Mode::from_bits_truncate(0o0644);
+ // set the correct owner/group/permissions while saving file
+ // owner(rw) = backup, group(r)= backup
+ let options = CreateOptions::new()
+ .perm(mode)
+ .owner(backup_user.uid)
+ .group(backup_user.gid);
+
+ replace_file(path, serialized.as_bytes(), options, false)
+ }
+}
+
+pub fn compute_schedule_status(
+ job_state: &JobState,
+ schedule: Option<&str>,
+) -> Result<JobScheduleStatus, Error> {
+ let (upid, endtime, state, last) = match job_state {
+ JobState::Created { time } => (None, None, None, *time),
+ JobState::Started { upid } => {
+ let parsed_upid: UPID = upid.parse()?;
+ (Some(upid), None, None, parsed_upid.starttime)
+ }
+ JobState::Finished {
+ upid,
+ state,
+ updated,
+ } => {
+ let last = updated.unwrap_or_else(|| state.endtime());
+ (
+ Some(upid),
+ Some(state.endtime()),
+ Some(state.to_string()),
+ last,
+ )
+ }
+ };
+
+ let mut status = JobScheduleStatus {
+ last_run_upid: upid.map(String::from),
+ last_run_state: state,
+ last_run_endtime: endtime,
+ ..Default::default()
+ };
+
+ if let Some(schedule) = schedule {
+ if let Ok(event) = schedule.parse::<CalendarEvent>() {
+ // ignore errors
+ status.next_run = event.compute_next_event(last).unwrap_or(None);
+ }
+ }
+
+ Ok(status)
+}
diff --git a/proxmox-jobstate/src/lib.rs b/proxmox-jobstate/src/lib.rs
new file mode 100644
index 0000000..0d1ea70
--- /dev/null
+++ b/proxmox-jobstate/src/lib.rs
@@ -0,0 +1,46 @@
+/// Generic JobState handling
+///
+/// A 'Job' can have 3 states
+/// - Created, when a schedule was created but never executed
+/// - Started, when a job is running right now
+/// - Finished, when a job was running in the past
+///
+/// and is identified by 2 values: jobtype and jobname (e.g. 'syncjob' and 'myfirstsyncjob')
+///
+/// This module Provides 2 helper structs to handle those coniditons
+/// 'Job' which handles locking and writing to a file
+/// 'JobState' which is the actual state
+///
+/// an example usage would be
+/// ```no_run
+/// # use anyhow::{bail, Error};
+/// # use proxmox_rest_server::TaskState;
+/// # use proxmox_jobstate::*;
+/// # fn some_code() -> TaskState { TaskState::OK { endtime: 0 } }
+/// # fn code() -> Result<(), Error> {
+/// // locks the correct file under /var/lib
+/// // or fails if someone else holds the lock
+/// let mut job = match Job::new("jobtype", "jobname") {
+/// Ok(job) => job,
+/// Err(err) => bail!("could not lock jobstate"),
+/// };
+///
+/// // job holds the lock, we can start it
+/// job.start("someupid")?;
+/// // do something
+/// let task_state = some_code();
+/// job.finish(task_state)?;
+///
+/// // release the lock
+/// drop(job);
+/// # Ok(())
+/// # }
+///
+/// ```
+mod api_types;
+pub use api_types::*;
+
+#[cfg(not(target_arch = "wasm32"))]
+mod jobstate;
+#[cfg(not(target_arch = "wasm32"))]
+pub use jobstate::*;
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pbs-devel] [RFC proxmox 3/3] new proxmox-cert-management crate
2023-10-18 10:39 [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 2/3] new proxmox-jobstate crate Dominik Csapak
@ 2023-10-18 10:39 ` Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox-backup 1/1] use proxmox_jobstate and proxmox_cert_management crates Dominik Csapak
2023-10-24 8:17 ` [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Lukas Wagner
4 siblings, 0 replies; 9+ messages in thread
From: Dominik Csapak @ 2023-10-18 10:39 UTC (permalink / raw)
To: pbs-devel
refactored from proxmox-backup, but uses the new ServerConfig global
settings for the users and directories.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
Cargo.toml | 2 +
proxmox-cert-management/Cargo.toml | 23 +++
proxmox-cert-management/debian/changelog | 5 +
proxmox-cert-management/debian/control | 53 ++++++
proxmox-cert-management/debian/copyright | 18 ++
proxmox-cert-management/debian/debcargo.toml | 7 +
proxmox-cert-management/src/lib.rs | 182 +++++++++++++++++++
7 files changed, 290 insertions(+)
create mode 100644 proxmox-cert-management/Cargo.toml
create mode 100644 proxmox-cert-management/debian/changelog
create mode 100644 proxmox-cert-management/debian/control
create mode 100644 proxmox-cert-management/debian/copyright
create mode 100644 proxmox-cert-management/debian/debcargo.toml
create mode 100644 proxmox-cert-management/src/lib.rs
diff --git a/Cargo.toml b/Cargo.toml
index 4b4b787..0a4ad06 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -5,6 +5,7 @@ members = [
"proxmox-async",
"proxmox-auth-api",
"proxmox-borrow",
+ "proxmox-cert-management",
"proxmox-client",
"proxmox-compression",
"proxmox-http",
@@ -91,6 +92,7 @@ webauthn-rs = "0.3"
zstd = { version = "0.12", features = [ "bindgen" ] }
# workspace dependencies
+proxmox-auth-api = { version = "0.3", path = "proxmox-auth-api" }
proxmox-api-macro = { version = "1.0.6", path = "proxmox-api-macro" }
proxmox-async = { version = "0.4.1", path = "proxmox-async" }
proxmox-compression = { version = "0.2.0", path = "proxmox-compression" }
diff --git a/proxmox-cert-management/Cargo.toml b/proxmox-cert-management/Cargo.toml
new file mode 100644
index 0000000..816f4b6
--- /dev/null
+++ b/proxmox-cert-management/Cargo.toml
@@ -0,0 +1,23 @@
+[package]
+name = "proxmox-cert-management"
+version = "0.1.0"
+authors.workspace = true
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
+description = "Certificate management and utilities"
+
+exclude.workspace = true
+
+[dependencies]
+anyhow.workspace = true
+base64.workspace = true
+lazy_static.workspace = true
+nix.workspace = true
+openssl.workspace = true
+
+proxmox-auth-api = { workspace = true, features = ["api-types"] }
+proxmox-lang.workspace = true
+proxmox-sys.workspace = true
+proxmox-server-config.workspace = true
+proxmox-time.workspace = true
diff --git a/proxmox-cert-management/debian/changelog b/proxmox-cert-management/debian/changelog
new file mode 100644
index 0000000..ed5d4d6
--- /dev/null
+++ b/proxmox-cert-management/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-cert-management (0.1.0-1) stable; urgency=medium
+
+ * initial version
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 17 Oct 2023 13:56:35 +0200
diff --git a/proxmox-cert-management/debian/control b/proxmox-cert-management/debian/control
new file mode 100644
index 0000000..593ea6c
--- /dev/null
+++ b/proxmox-cert-management/debian/control
@@ -0,0 +1,53 @@
+Source: rust-proxmox-cert-management
+Section: rust
+Priority: optional
+Build-Depends: debhelper (>= 12),
+ dh-cargo (>= 25),
+ cargo:native <!nocheck>,
+ rustc:native <!nocheck>,
+ libstd-rust-dev <!nocheck>,
+ librust-anyhow-1+default-dev <!nocheck>,
+ librust-base64-0.13+default-dev <!nocheck>,
+ librust-lazy-static-1+default-dev (>= 1.4-~~) <!nocheck>,
+ librust-nix-0.26+default-dev (>= 0.26.1-~~) <!nocheck>,
+ librust-openssl-0.10+default-dev <!nocheck>,
+ librust-proxmox-auth-api-0.3+api-types-dev <!nocheck>,
+ librust-proxmox-auth-api-0.3+default-dev <!nocheck>,
+ librust-proxmox-lang-1+default-dev (>= 1.1-~~) <!nocheck>,
+ librust-proxmox-server-config-0.1+default-dev <!nocheck>,
+ librust-proxmox-sys-0.5+default-dev <!nocheck>,
+ librust-proxmox-time-1+default-dev (>= 1.1.4-~~) <!nocheck>
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Standards-Version: 4.6.1
+Vcs-Git: git://git.proxmox.com/git/proxmox.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+X-Cargo-Crate: proxmox-cert-management
+Rules-Requires-Root: no
+
+Package: librust-proxmox-cert-management-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-anyhow-1+default-dev,
+ librust-base64-0.13+default-dev,
+ librust-lazy-static-1+default-dev (>= 1.4-~~),
+ librust-nix-0.26+default-dev (>= 0.26.1-~~),
+ librust-openssl-0.10+default-dev,
+ librust-proxmox-auth-api-0.3+api-types-dev,
+ librust-proxmox-auth-api-0.3+default-dev,
+ librust-proxmox-lang-1+default-dev (>= 1.1-~~),
+ librust-proxmox-server-config-0.1+default-dev,
+ librust-proxmox-sys-0.5+default-dev,
+ librust-proxmox-time-1+default-dev (>= 1.1.4-~~)
+Provides:
+ librust-proxmox-cert-management+default-dev (= ${binary:Version}),
+ librust-proxmox-cert-management-0-dev (= ${binary:Version}),
+ librust-proxmox-cert-management-0+default-dev (= ${binary:Version}),
+ librust-proxmox-cert-management-0.1-dev (= ${binary:Version}),
+ librust-proxmox-cert-management-0.1+default-dev (= ${binary:Version}),
+ librust-proxmox-cert-management-0.1.0-dev (= ${binary:Version}),
+ librust-proxmox-cert-management-0.1.0+default-dev (= ${binary:Version})
+Description: Certificate management and utilities - Rust source code
+ This package contains the source for the Rust proxmox-cert-management crate,
+ packaged by debcargo for use with cargo and dh-cargo.
diff --git a/proxmox-cert-management/debian/copyright b/proxmox-cert-management/debian/copyright
new file mode 100644
index 0000000..0d9eab3
--- /dev/null
+++ b/proxmox-cert-management/debian/copyright
@@ -0,0 +1,18 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+
+Files:
+ *
+Copyright: 2019 - 2023 Proxmox Server Solutions GmbH <support@proxmox.com>
+License: AGPL-3.0-or-later
+ This program is free software: you can redistribute it and/or modify it under
+ the terms of the GNU Affero General Public License as published by the Free
+ Software Foundation, either version 3 of the License, or (at your option) any
+ later version.
+ .
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ details.
+ .
+ You should have received a copy of the GNU Affero General Public License along
+ with this program. If not, see <https://www.gnu.org/licenses/>.
diff --git a/proxmox-cert-management/debian/debcargo.toml b/proxmox-cert-management/debian/debcargo.toml
new file mode 100644
index 0000000..b7864cd
--- /dev/null
+++ b/proxmox-cert-management/debian/debcargo.toml
@@ -0,0 +1,7 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+[source]
+vcs_git = "git://git.proxmox.com/git/proxmox.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
diff --git a/proxmox-cert-management/src/lib.rs b/proxmox-cert-management/src/lib.rs
new file mode 100644
index 0000000..dd327f7
--- /dev/null
+++ b/proxmox-cert-management/src/lib.rs
@@ -0,0 +1,182 @@
+use anyhow::{bail, format_err, Error};
+use lazy_static::lazy_static;
+use openssl::pkey::{PKey, Private, Public};
+use openssl::rsa::Rsa;
+use openssl::sha;
+
+use proxmox_auth_api::types::Userid;
+use proxmox_lang::try_block;
+use proxmox_server_config::get_server_config;
+use proxmox_sys::fs::{file_get_contents, replace_file, CreateOptions};
+
+fn compute_csrf_secret_digest(timestamp: i64, secret: &[u8], userid: &Userid) -> String {
+ let mut hasher = sha::Sha256::new();
+ let data = format!("{:08X}:{}:", timestamp, userid);
+ hasher.update(data.as_bytes());
+ hasher.update(secret);
+
+ base64::encode_config(hasher.finish(), base64::STANDARD_NO_PAD)
+}
+
+pub fn assemble_csrf_prevention_token(secret: &[u8], userid: &Userid) -> String {
+ let epoch = proxmox_time::epoch_i64();
+
+ let digest = compute_csrf_secret_digest(epoch, secret, userid);
+
+ format!("{:08X}:{}", epoch, digest)
+}
+
+pub fn verify_csrf_prevention_token(
+ secret: &[u8],
+ userid: &Userid,
+ token: &str,
+ min_age: i64,
+ max_age: i64,
+) -> Result<i64, Error> {
+ use std::collections::VecDeque;
+
+ let mut parts: VecDeque<&str> = token.split(':').collect();
+
+ try_block!({
+ if parts.len() != 2 {
+ bail!("format error - wrong number of parts.");
+ }
+
+ let timestamp = parts.pop_front().unwrap();
+ let sig = parts.pop_front().unwrap();
+
+ let ttime = i64::from_str_radix(timestamp, 16)
+ .map_err(|err| format_err!("timestamp format error - {}", err))?;
+
+ let digest = compute_csrf_secret_digest(ttime, secret, userid);
+
+ if digest != sig {
+ bail!("invalid signature.");
+ }
+
+ let now = proxmox_time::epoch_i64();
+
+ let age = now - ttime;
+ if age < min_age {
+ bail!("timestamp newer than expected.");
+ }
+
+ if age > max_age {
+ bail!("timestamp too old.");
+ }
+
+ Ok(age)
+ })
+ .map_err(|err| format_err!("invalid csrf token - {}", err))
+}
+
+pub fn generate_csrf_key() -> Result<(), Error> {
+ let server_config = get_server_config()?;
+ let path = server_config.config_dir().join("csrf.key");
+
+ if path.exists() {
+ return Ok(());
+ }
+
+ let rsa = Rsa::generate(2048).unwrap();
+
+ let pem = rsa.private_key_to_pem()?;
+
+ use nix::sys::stat::Mode;
+
+ replace_file(
+ &path,
+ &pem,
+ CreateOptions::new()
+ .perm(Mode::from_bits_truncate(0o0640))
+ .owner(server_config.privileged_user().uid)
+ .group(server_config.user().gid),
+ true,
+ )?;
+
+ Ok(())
+}
+
+pub fn generate_auth_key() -> Result<(), Error> {
+ let server_config = get_server_config()?;
+ let priv_path = server_config.config_dir().join("authkey.key");
+
+ let mut public_path = priv_path.clone();
+ public_path.set_extension("pub");
+
+ if priv_path.exists() && public_path.exists() {
+ return Ok(());
+ }
+
+ let rsa = Rsa::generate(4096).unwrap();
+
+ let priv_pem = rsa.private_key_to_pem()?;
+
+ use nix::sys::stat::Mode;
+
+ replace_file(
+ &priv_path,
+ &priv_pem,
+ CreateOptions::new().perm(Mode::from_bits_truncate(0o0600)),
+ true,
+ )?;
+
+ let public_pem = rsa.public_key_to_pem()?;
+
+ replace_file(
+ &public_path,
+ &public_pem,
+ CreateOptions::new()
+ .perm(Mode::from_bits_truncate(0o0640))
+ .owner(server_config.privileged_user().uid)
+ .group(server_config.user().gid),
+ true,
+ )?;
+
+ Ok(())
+}
+
+pub fn csrf_secret() -> &'static [u8] {
+ lazy_static! {
+ static ref SECRET: Vec<u8> = {
+ let dir = get_server_config().unwrap().config_dir().join("csrf.key");
+ file_get_contents(dir).unwrap()
+ };
+ }
+
+ &SECRET
+}
+
+fn load_public_auth_key() -> Result<PKey<Public>, Error> {
+ let pem_path = get_server_config()?.config_dir().join("authkey.pub");
+ let pem = file_get_contents(pem_path)?;
+ let rsa = Rsa::public_key_from_pem(&pem)?;
+ let key = PKey::from_rsa(rsa)?;
+
+ Ok(key)
+}
+
+pub fn public_auth_key() -> &'static PKey<Public> {
+ lazy_static! {
+ static ref KEY: PKey<Public> = load_public_auth_key().unwrap();
+ }
+
+ &KEY
+}
+
+fn load_private_auth_key() -> Result<PKey<Private>, Error> {
+ let pem_path = get_server_config()?.config_dir().join("authkey.key");
+ let pem = file_get_contents(pem_path)?;
+ let rsa = Rsa::private_key_from_pem(&pem)?;
+ let key = PKey::from_rsa(rsa)?;
+
+ Ok(key)
+}
+
+pub fn private_auth_key() -> &'static PKey<Private> {
+ lazy_static! {
+ static ref KEY: PKey<Private> = load_private_auth_key().unwrap();
+ }
+
+ &KEY
+}
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pbs-devel] [RFC proxmox-backup 1/1] use proxmox_jobstate and proxmox_cert_management crates
2023-10-18 10:39 [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Dominik Csapak
` (2 preceding siblings ...)
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 3/3] new proxmox-cert-management crate Dominik Csapak
@ 2023-10-18 10:39 ` Dominik Csapak
2023-10-24 8:17 ` [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Lukas Wagner
4 siblings, 0 replies; 9+ messages in thread
From: Dominik Csapak @ 2023-10-18 10:39 UTC (permalink / raw)
To: pbs-devel
and remove the old code here.
For this to work now, we have to use the ServerConfig setup, so that the
directories/users are accessible globally.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
Cargo.toml | 8 +
pbs-api-types/Cargo.toml | 1 +
pbs-api-types/src/jobs.rs | 41 +---
src/auth_helpers.rs | 184 +---------------
src/bin/proxmox-backup-api.rs | 8 +-
src/bin/proxmox-backup-manager.rs | 2 +
src/bin/proxmox-backup-proxy.rs | 2 +
src/server/jobstate.rs | 347 +-----------------------------
src/server/mod.rs | 21 ++
9 files changed, 52 insertions(+), 562 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index cfbf2ba1..de0c8138 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -58,11 +58,13 @@ proxmox-apt = "0.10.5"
proxmox-async = "0.4"
proxmox-auth-api = "0.3"
proxmox-borrow = "1"
+proxmox-cert-management = "0.1"
proxmox-compression = "0.2"
proxmox-fuse = "0.1.3"
proxmox-http = { version = "0.9.0", features = [ "client", "http-helpers", "websocket" ] } # see below
proxmox-human-byte = "0.1"
proxmox-io = "1.0.1" # tools and client use "tokio" feature
+proxmox-jobstate = "0.1"
proxmox-lang = "1.1"
proxmox-ldap = "0.2.1"
proxmox-metrics = "0.3"
@@ -74,6 +76,7 @@ proxmox-router = { version = "2.0.0", default_features = false }
proxmox-schema = "2.0.0"
proxmox-section-config = "2"
proxmox-serde = "0.1.1"
+proxmox-server-config = "0.1"
proxmox-shared-memory = "0.3.0"
proxmox-sortable-macro = "0.1.2"
proxmox-subscription = { version = "0.4", features = [ "api-types" ] }
@@ -202,10 +205,12 @@ zstd.workspace = true
proxmox-apt.workspace = true
proxmox-async.workspace = true
proxmox-auth-api = { workspace = true, features = [ "api", "pam-authenticator" ] }
+proxmox-cert-management.workspace = true
proxmox-compression.workspace = true
proxmox-http = { workspace = true, features = [ "client-trait", "proxmox-async", "rate-limited-stream" ] } # pbs-client doesn't use these
proxmox-human-byte.workspace = true
proxmox-io.workspace = true
+proxmox-jobstate.workspace = true
proxmox-lang.workspace = true
proxmox-ldap.workspace = true
proxmox-metrics.workspace = true
@@ -215,6 +220,7 @@ proxmox-router = { workspace = true, features = [ "cli", "server"] }
proxmox-schema = { workspace = true, features = [ "api-macro" ] }
proxmox-section-config.workspace = true
proxmox-serde = { workspace = true, features = [ "serde_json" ] }
+proxmox-server-config.workspace = true
proxmox-shared-memory.workspace = true
proxmox-sortable-macro.workspace = true
proxmox-subscription.workspace = true
@@ -252,6 +258,7 @@ proxmox-rrd.workspace = true
#proxmox-http = { path = "../proxmox/proxmox-http" }
#proxmox-human-byte = { path = "../proxmox/proxmox-human-byte" }
#proxmox-io = { path = "../proxmox/proxmox-io" }
+#proxmox-jobstate = { path = "../proxmox/proxmox-jobstate" }
#proxmox-lang = { path = "../proxmox/proxmox-lang" }
#proxmox-ldap = { path = "../proxmox/proxmox-ldap" }
#proxmox-metrics = { path = "../proxmox/proxmox-metrics" }
@@ -261,6 +268,7 @@ proxmox-rrd.workspace = true
#proxmox-schema = { path = "../proxmox/proxmox-schema" }
#proxmox-section-config = { path = "../proxmox/proxmox-section-config" }
#proxmox-serde = { path = "../proxmox/proxmox-serde" }
+#proxmox-server-config = { path = "../proxmox/proxmox-server-config" }
#proxmox-shared-memory = { path = "../proxmox/proxmox-shared-memory" }
#proxmox-sortable-macro = { path = "../proxmox/proxmox-sortable-macro" }
#proxmox-subscription = { path = "../proxmox/proxmox-subscription" }
diff --git a/pbs-api-types/Cargo.toml b/pbs-api-types/Cargo.toml
index 31b69f62..13f7d76b 100644
--- a/pbs-api-types/Cargo.toml
+++ b/pbs-api-types/Cargo.toml
@@ -16,6 +16,7 @@ serde_plain.workspace = true
proxmox-auth-api = { workspace = true, features = [ "api-types" ] }
proxmox-human-byte.workspace = true
+proxmox-jobstate.workspace=true
proxmox-lang.workspace=true
proxmox-schema = { workspace = true, features = [ "api-macro" ] }
proxmox-serde.workspace = true
diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index 23e19b7b..a932c29d 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -13,6 +13,9 @@ use crate::{
SINGLE_LINE_COMMENT_SCHEMA,
};
+// re-exported for compatibility
+pub use proxmox_jobstate::JobScheduleStatus;
+
const_regex! {
/// Regex for verification jobs 'DATASTORE:ACTUAL_JOB_ID'
@@ -63,44 +66,6 @@ pub const REMOVE_VANISHED_BACKUPS_SCHEMA: Schema = BooleanSchema::new(
.default(false)
.schema();
-#[api(
- properties: {
- "next-run": {
- description: "Estimated time of the next run (UNIX epoch).",
- optional: true,
- type: Integer,
- },
- "last-run-state": {
- description: "Result of the last run.",
- optional: true,
- type: String,
- },
- "last-run-upid": {
- description: "Task UPID of the last run.",
- optional: true,
- type: String,
- },
- "last-run-endtime": {
- description: "Endtime of the last run.",
- optional: true,
- type: Integer,
- },
- }
-)]
-#[derive(Serialize, Deserialize, Default, Clone, PartialEq)]
-#[serde(rename_all = "kebab-case")]
-/// Job Scheduling Status
-pub struct JobScheduleStatus {
- #[serde(skip_serializing_if = "Option::is_none")]
- pub next_run: Option<i64>,
- #[serde(skip_serializing_if = "Option::is_none")]
- pub last_run_state: Option<String>,
- #[serde(skip_serializing_if = "Option::is_none")]
- pub last_run_upid: Option<String>,
- #[serde(skip_serializing_if = "Option::is_none")]
- pub last_run_endtime: Option<i64>,
-}
-
#[api()]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
diff --git a/src/auth_helpers.rs b/src/auth_helpers.rs
index c2eaaef1..41c3c569 100644
--- a/src/auth_helpers.rs
+++ b/src/auth_helpers.rs
@@ -1,189 +1,17 @@
-use std::path::PathBuf;
-
-use anyhow::{bail, format_err, Error};
-use lazy_static::lazy_static;
-use openssl::pkey::{PKey, Private, Public};
-use openssl::rsa::Rsa;
-use openssl::sha;
+use anyhow::Error;
use pbs_config::BackupLockGuard;
-use proxmox_lang::try_block;
-use proxmox_sys::fs::{file_get_contents, replace_file, CreateOptions};
-use pbs_api_types::Userid;
use pbs_buildcfg::configdir;
use serde_json::json;
pub use crate::auth::setup_auth_context;
-fn compute_csrf_secret_digest(timestamp: i64, secret: &[u8], userid: &Userid) -> String {
- let mut hasher = sha::Sha256::new();
- let data = format!("{:08X}:{}:", timestamp, userid);
- hasher.update(data.as_bytes());
- hasher.update(secret);
-
- base64::encode_config(hasher.finish(), base64::STANDARD_NO_PAD)
-}
-
-pub fn assemble_csrf_prevention_token(secret: &[u8], userid: &Userid) -> String {
- let epoch = proxmox_time::epoch_i64();
-
- let digest = compute_csrf_secret_digest(epoch, secret, userid);
-
- format!("{:08X}:{}", epoch, digest)
-}
-
-pub fn verify_csrf_prevention_token(
- secret: &[u8],
- userid: &Userid,
- token: &str,
- min_age: i64,
- max_age: i64,
-) -> Result<i64, Error> {
- use std::collections::VecDeque;
-
- let mut parts: VecDeque<&str> = token.split(':').collect();
-
- try_block!({
- if parts.len() != 2 {
- bail!("format error - wrong number of parts.");
- }
-
- let timestamp = parts.pop_front().unwrap();
- let sig = parts.pop_front().unwrap();
-
- let ttime = i64::from_str_radix(timestamp, 16)
- .map_err(|err| format_err!("timestamp format error - {}", err))?;
-
- let digest = compute_csrf_secret_digest(ttime, secret, userid);
-
- if digest != sig {
- bail!("invalid signature.");
- }
-
- let now = proxmox_time::epoch_i64();
-
- let age = now - ttime;
- if age < min_age {
- bail!("timestamp newer than expected.");
- }
-
- if age > max_age {
- bail!("timestamp too old.");
- }
-
- Ok(age)
- })
- .map_err(|err| format_err!("invalid csrf token - {}", err))
-}
-
-pub fn generate_csrf_key() -> Result<(), Error> {
- let path = PathBuf::from(configdir!("/csrf.key"));
-
- if path.exists() {
- return Ok(());
- }
-
- let rsa = Rsa::generate(2048).unwrap();
-
- let pem = rsa.private_key_to_pem()?;
-
- use nix::sys::stat::Mode;
-
- let backup_user = pbs_config::backup_user()?;
-
- replace_file(
- &path,
- &pem,
- CreateOptions::new()
- .perm(Mode::from_bits_truncate(0o0640))
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid),
- true,
- )?;
-
- Ok(())
-}
-
-pub fn generate_auth_key() -> Result<(), Error> {
- let priv_path = PathBuf::from(configdir!("/authkey.key"));
-
- let mut public_path = priv_path.clone();
- public_path.set_extension("pub");
-
- if priv_path.exists() && public_path.exists() {
- return Ok(());
- }
-
- let rsa = Rsa::generate(4096).unwrap();
-
- let priv_pem = rsa.private_key_to_pem()?;
-
- use nix::sys::stat::Mode;
-
- replace_file(
- &priv_path,
- &priv_pem,
- CreateOptions::new().perm(Mode::from_bits_truncate(0o0600)),
- true,
- )?;
-
- let public_pem = rsa.public_key_to_pem()?;
-
- let backup_user = pbs_config::backup_user()?;
-
- replace_file(
- &public_path,
- &public_pem,
- CreateOptions::new()
- .perm(Mode::from_bits_truncate(0o0640))
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid),
- true,
- )?;
-
- Ok(())
-}
-
-pub fn csrf_secret() -> &'static [u8] {
- lazy_static! {
- static ref SECRET: Vec<u8> = file_get_contents(configdir!("/csrf.key")).unwrap();
- }
-
- &SECRET
-}
-
-fn load_public_auth_key() -> Result<PKey<Public>, Error> {
- let pem = file_get_contents(configdir!("/authkey.pub"))?;
- let rsa = Rsa::public_key_from_pem(&pem)?;
- let key = PKey::from_rsa(rsa)?;
-
- Ok(key)
-}
-
-pub fn public_auth_key() -> &'static PKey<Public> {
- lazy_static! {
- static ref KEY: PKey<Public> = load_public_auth_key().unwrap();
- }
-
- &KEY
-}
-
-fn load_private_auth_key() -> Result<PKey<Private>, Error> {
- let pem = file_get_contents(configdir!("/authkey.key"))?;
- let rsa = Rsa::private_key_from_pem(&pem)?;
- let key = PKey::from_rsa(rsa)?;
-
- Ok(key)
-}
-
-pub fn private_auth_key() -> &'static PKey<Private> {
- lazy_static! {
- static ref KEY: PKey<Private> = load_private_auth_key().unwrap();
- }
-
- &KEY
-}
+// re-exported
+pub use proxmox_cert_management::{
+ assemble_csrf_prevention_token, csrf_secret, generate_auth_key, generate_csrf_key,
+ private_auth_key, public_auth_key,
+};
const LDAP_PASSWORDS_FILENAME: &str = configdir!("/ldap_passwords.json");
diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index c6c24449..4d0837eb 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -6,6 +6,7 @@ use futures::*;
use http::Response;
use hyper::{Body, StatusCode};
+use proxmox_backup::server::setup_server_config;
use proxmox_lang::try_block;
use proxmox_router::RpcEnvironmentType;
use proxmox_sys::fs::CreateOptions;
@@ -48,13 +49,12 @@ async fn run() -> Result<(), Error> {
bail!("unable to inititialize syslog - {}", err);
}
- config::create_configdir()?;
+ setup_server_config()?;
+
+ config::check_configdir_permissions()?;
config::update_self_signed_cert(false)?;
- proxmox_backup::server::create_run_dir()?;
- proxmox_backup::server::create_state_dir()?;
- proxmox_backup::server::create_active_operations_dir()?;
proxmox_backup::server::jobstate::create_jobstate_dir()?;
proxmox_backup::tape::create_tape_status_dir()?;
proxmox_backup::tape::create_drive_state_dir()?;
diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index b4cb6cb3..a5d777bb 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -507,6 +507,8 @@ async fn run() -> Result<(), Error> {
fn main() -> Result<(), Error> {
proxmox_backup::tools::setup_safe_path_env();
+ proxmox_backup::server::setup_server_config()?;
+
proxmox_async::runtime::main(run())
}
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index f38a02bd..7eafdbc4 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -197,6 +197,8 @@ async fn run() -> Result<(), Error> {
bail!("unable to inititialize syslog - {err}");
}
+ server::setup_server_config()?;
+
proxmox_backup::auth_helpers::setup_auth_context(false);
let rrd_cache = initialize_rrd_cache()?;
diff --git a/src/server/jobstate.rs b/src/server/jobstate.rs
index be9dac42..e4a74c65 100644
--- a/src/server/jobstate.rs
+++ b/src/server/jobstate.rs
@@ -1,343 +1,6 @@
-//! Generic JobState handling
-//!
-//! A 'Job' can have 3 states
-//! - Created, when a schedule was created but never executed
-//! - Started, when a job is running right now
-//! - Finished, when a job was running in the past
-//!
-//! and is identified by 2 values: jobtype and jobname (e.g. 'syncjob' and 'myfirstsyncjob')
-//!
-//! This module Provides 2 helper structs to handle those coniditons
-//! 'Job' which handles locking and writing to a file
-//! 'JobState' which is the actual state
-//!
-//! an example usage would be
-//! ```no_run
-//! # use anyhow::{bail, Error};
-//! # use proxmox_rest_server::TaskState;
-//! # use proxmox_backup::server::jobstate::*;
-//! # fn some_code() -> TaskState { TaskState::OK { endtime: 0 } }
-//! # fn code() -> Result<(), Error> {
-//! // locks the correct file under /var/lib
-//! // or fails if someone else holds the lock
-//! let mut job = match Job::new("jobtype", "jobname") {
-//! Ok(job) => job,
-//! Err(err) => bail!("could not lock jobstate"),
-//! };
-//!
-//! // job holds the lock, we can start it
-//! job.start("someupid")?;
-//! // do something
-//! let task_state = some_code();
-//! job.finish(task_state)?;
-//!
-//! // release the lock
-//! drop(job);
-//! # Ok(())
-//! # }
-//!
-//! ```
-use std::path::{Path, PathBuf};
+//! Generic JobState handling. Deprecated and reexported from [proxmox_jobstate](proxmox_jobstate)
-use anyhow::{bail, format_err, Error};
-use serde::{Deserialize, Serialize};
-
-use proxmox_sys::fs::{create_path, file_read_optional_string, replace_file, CreateOptions};
-
-use proxmox_time::CalendarEvent;
-
-use pbs_api_types::{JobScheduleStatus, UPID};
-use pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR_M;
-use pbs_config::{open_backup_lockfile, BackupLockGuard};
-
-use proxmox_rest_server::{upid_read_status, worker_is_active_local, TaskState};
-
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-/// Represents the State of a specific Job
-pub enum JobState {
- /// A job was created at 'time', but never started/finished
- Created { time: i64 },
- /// The Job was last started in 'upid',
- Started { upid: String },
- /// The Job was last started in 'upid', which finished with 'state', and was last updated at 'updated'
- Finished {
- upid: String,
- state: TaskState,
- updated: Option<i64>,
- },
-}
-
-/// Represents a Job and holds the correct lock
-pub struct Job {
- jobtype: String,
- jobname: String,
- /// The State of the job
- pub state: JobState,
- _lock: BackupLockGuard,
-}
-
-const JOB_STATE_BASEDIR: &str = concat!(PROXMOX_BACKUP_STATE_DIR_M!(), "/jobstates");
-
-/// Create jobstate stat dir with correct permission
-pub fn create_jobstate_dir() -> Result<(), Error> {
- let backup_user = pbs_config::backup_user()?;
-
- let opts = CreateOptions::new()
- .owner(backup_user.uid)
- .group(backup_user.gid);
-
- create_path(JOB_STATE_BASEDIR, Some(opts.clone()), Some(opts))
- .map_err(|err: Error| format_err!("unable to create job state dir - {err}"))?;
-
- Ok(())
-}
-
-fn get_path(jobtype: &str, jobname: &str) -> PathBuf {
- let mut path = PathBuf::from(JOB_STATE_BASEDIR);
- path.push(format!("{jobtype}-{jobname}.json"));
- path
-}
-
-fn get_lock<P>(path: P) -> Result<BackupLockGuard, Error>
-where
- P: AsRef<Path>,
-{
- let mut path = path.as_ref().to_path_buf();
- path.set_extension("lck");
- open_backup_lockfile(&path, None, true)
-}
-
-/// Removes the statefile of a job, this is useful if we delete a job
-pub fn remove_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> {
- let mut path = get_path(jobtype, jobname);
- let _lock = get_lock(&path)?;
- if let Err(err) = std::fs::remove_file(&path) {
- if err.kind() != std::io::ErrorKind::NotFound {
- bail!("cannot remove statefile for {jobtype} - {jobname}: {err}");
- }
- }
- path.set_extension("lck");
- if let Err(err) = std::fs::remove_file(&path) {
- if err.kind() != std::io::ErrorKind::NotFound {
- bail!("cannot remove lockfile for {jobtype} - {jobname}: {err}");
- }
- }
- Ok(())
-}
-
-/// Creates the statefile with the state 'Created'
-/// overwrites if it exists already
-pub fn create_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> {
- let mut job = Job::new(jobtype, jobname)?;
- job.write_state()
-}
-
-/// Tries to update the state file with the current time
-/// if the job is currently running, does nothing.
-/// Intended for use when the schedule changes.
-pub fn update_job_last_run_time(jobtype: &str, jobname: &str) -> Result<(), Error> {
- let mut job = match Job::new(jobtype, jobname) {
- Ok(job) => job,
- Err(_) => return Ok(()), // was locked (running), so do not update
- };
- let time = proxmox_time::epoch_i64();
-
- job.state = match JobState::load(jobtype, jobname)? {
- JobState::Created { .. } => JobState::Created { time },
- JobState::Started { .. } => return Ok(()), // currently running (without lock?)
- JobState::Finished {
- upid,
- state,
- updated: _,
- } => JobState::Finished {
- upid,
- state,
- updated: Some(time),
- },
- };
- job.write_state()
-}
-
-/// Returns the last run time of a job by reading the statefile
-/// Note that this is not locked
-pub fn last_run_time(jobtype: &str, jobname: &str) -> Result<i64, Error> {
- match JobState::load(jobtype, jobname)? {
- JobState::Created { time } => Ok(time),
- JobState::Finished {
- updated: Some(time),
- ..
- } => Ok(time),
- JobState::Started { upid }
- | JobState::Finished {
- upid,
- state: _,
- updated: None,
- } => {
- let upid: UPID = upid
- .parse()
- .map_err(|err| format_err!("could not parse upid from state: {err}"))?;
- Ok(upid.starttime)
- }
- }
-}
-
-impl JobState {
- /// Loads and deserializes the jobstate from type and name.
- /// When the loaded state indicates a started UPID,
- /// we go and check if it has already stopped, and
- /// returning the correct state.
- ///
- /// This does not update the state in the file.
- pub fn load(jobtype: &str, jobname: &str) -> Result<Self, Error> {
- if let Some(state) = file_read_optional_string(get_path(jobtype, jobname))? {
- match serde_json::from_str(&state)? {
- JobState::Started { upid } => {
- let parsed: UPID = upid
- .parse()
- .map_err(|err| format_err!("error parsing upid: {err}"))?;
-
- if !worker_is_active_local(&parsed) {
- let state = upid_read_status(&parsed).unwrap_or(TaskState::Unknown {
- endtime: parsed.starttime,
- });
-
- Ok(JobState::Finished {
- upid,
- state,
- updated: None,
- })
- } else {
- Ok(JobState::Started { upid })
- }
- }
- other => Ok(other),
- }
- } else {
- Ok(JobState::Created {
- time: proxmox_time::epoch_i64() - 30,
- })
- }
- }
-}
-
-impl Job {
- /// Creates a new instance of a job with the correct lock held
- /// (will be hold until the job is dropped again).
- ///
- /// This does not load the state from the file, to do that,
- /// 'load' must be called
- pub fn new(jobtype: &str, jobname: &str) -> Result<Self, Error> {
- let path = get_path(jobtype, jobname);
-
- let _lock = get_lock(path)?;
-
- Ok(Self {
- jobtype: jobtype.to_string(),
- jobname: jobname.to_string(),
- state: JobState::Created {
- time: proxmox_time::epoch_i64(),
- },
- _lock,
- })
- }
-
- /// Start the job and update the statefile accordingly
- /// Fails if the job was already started
- pub fn start(&mut self, upid: &str) -> Result<(), Error> {
- if let JobState::Started { .. } = self.state {
- bail!("cannot start job that is started!");
- }
-
- self.state = JobState::Started {
- upid: upid.to_string(),
- };
-
- self.write_state()
- }
-
- /// Finish the job and update the statefile accordingly with the given taskstate
- /// Fails if the job was not yet started
- pub fn finish(&mut self, state: TaskState) -> Result<(), Error> {
- let upid = match &self.state {
- JobState::Created { .. } => bail!("cannot finish when not started"),
- JobState::Started { upid } => upid,
- JobState::Finished { upid, .. } => upid,
- }
- .to_string();
-
- self.state = JobState::Finished {
- upid,
- state,
- updated: None,
- };
-
- self.write_state()
- }
-
- pub fn jobtype(&self) -> &str {
- &self.jobtype
- }
-
- pub fn jobname(&self) -> &str {
- &self.jobname
- }
-
- fn write_state(&mut self) -> Result<(), Error> {
- let serialized = serde_json::to_string(&self.state)?;
- let path = get_path(&self.jobtype, &self.jobname);
-
- let backup_user = pbs_config::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0644);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = backup, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(backup_user.uid)
- .group(backup_user.gid);
-
- replace_file(path, serialized.as_bytes(), options, false)
- }
-}
-
-pub fn compute_schedule_status(
- job_state: &JobState,
- schedule: Option<&str>,
-) -> Result<JobScheduleStatus, Error> {
- let (upid, endtime, state, last) = match job_state {
- JobState::Created { time } => (None, None, None, *time),
- JobState::Started { upid } => {
- let parsed_upid: UPID = upid.parse()?;
- (Some(upid), None, None, parsed_upid.starttime)
- }
- JobState::Finished {
- upid,
- state,
- updated,
- } => {
- let last = updated.unwrap_or_else(|| state.endtime());
- (
- Some(upid),
- Some(state.endtime()),
- Some(state.to_string()),
- last,
- )
- }
- };
-
- let mut status = JobScheduleStatus {
- last_run_upid: upid.map(String::from),
- last_run_state: state,
- last_run_endtime: endtime,
- ..Default::default()
- };
-
- if let Some(schedule) = schedule {
- if let Ok(event) = schedule.parse::<CalendarEvent>() {
- // ignore errors
- status.next_run = event.compute_next_event(last).unwrap_or(None);
- }
- }
-
- Ok(status)
-}
+pub use proxmox_jobstate::{
+ compute_schedule_status, create_jobstate_dir, create_state_file, last_run_time,
+ remove_state_file, update_job_last_run_time, Job, JobState,
+};
diff --git a/src/server/mod.rs b/src/server/mod.rs
index 4e3b68ac..06b1d92e 100644
--- a/src/server/mod.rs
+++ b/src/server/mod.rs
@@ -5,6 +5,7 @@
//! tokio/hyper.
use anyhow::{format_err, Error};
+use proxmox_server_config::ServerConfig;
use serde_json::Value;
use proxmox_sys::fs::{create_path, CreateOptions};
@@ -92,3 +93,23 @@ pub fn create_active_operations_dir() -> Result<(), Error> {
.map_err(|err: Error| format_err!("unable to create active operations dir - {err}"))?;
Ok(())
}
+
+/// Setup basic directories and users configs
+pub fn setup_server_config() -> Result<(), Error> {
+ let root = nix::unistd::User::from_uid(nix::unistd::ROOT)
+ .unwrap()
+ .unwrap();
+
+ ServerConfig::new(
+ "proxmox-backup-api",
+ "/var/lib/proxmox-backup",
+ pbs_config::backup_user()?,
+ )?
+ .with_privileged_user(root)?
+ .with_log_dir(pbs_buildcfg::PROXMOX_BACKUP_LOG_DIR_M!())?
+ .with_state_dir(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR_M!())?
+ .with_run_dir(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR_M!())?
+ .with_config_dir(pbs_buildcfg::CONFIGDIR)?
+ .with_cert_dir(pbs_buildcfg::CONFIGDIR)?
+ .setup()
+}
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs
2023-10-18 10:39 [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Dominik Csapak
` (3 preceding siblings ...)
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox-backup 1/1] use proxmox_jobstate and proxmox_cert_management crates Dominik Csapak
@ 2023-10-24 8:17 ` Lukas Wagner
4 siblings, 0 replies; 9+ messages in thread
From: Lukas Wagner @ 2023-10-24 8:17 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
In general these changes look good to me.
I think we should take the opportunity to write some unit tests for the
jobstate/cert-management crates though.
I guess with the new server-config crate it should relatively
straightforward to use some temporary directory as a base directory for
the test runs.
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
On 10/18/23 12:39, Dominik Csapak wrote:
> # General
>
> Sending this as RFC because of the multiple ways (see 'Alternatives'
> section) we could implement this (or something similar). I'm not sure
> what the general opinion about this pattern is.
>
> # Goal
>
> When creating a new daemon/server, we currently have much code in pbs
> that would have to be duplicated, mostly relating to setting up the
> server directories/files. Some crates features already have an interface
> to be slightly configurable, but not all of them.
>
> This change adds a new crate 'proxmox-server-config' that provides an
> interface which should be easy to use and reusable across multiple
> daemons/server applications for our use-cases.
>
> In this series i opted to factor out the two main remaining crates that
> are not seperated yet: the jobstates and the certificate management.
> (Though i guess there are more, depending on what another daemon might
> want to do)
>
> # Implementation
>
> I use a static OnceLock to save a global `ServerConfig` struct, that
> contains some configurations available, which are either setup, or
> derived from the minimum required configuration (base_dir, name, user).
>
> This can then be used to access them from any point in the program or in
> crates where needed.
>
> With this, we don't have to carry around the different directories as
> parameters, nor do we have to sprinkle things like `config!("foo.bar")`,
> around the code to get the path we want.
>
> # Alternatives
>
> The main alternatives to this approaches are:
>
> * copy the code across different server/daemons:
> obviously the easiest way, but doesn't scale well and prone to
> diverging implementations.
>
> * parameters:
> this has two ways of working:
> - all functions must take the configuration (dir(s)/user(s))
> blows up the parameters, and we have to use our current dir macros
> even more
> - each crate/module has an initializer that takes the config, which
> would then e.g. save that into a static OnceLock once again,
> disadvantage here is that we may duplicate values across multiple
> crates (should not be much though)
>
> There are probably more ways to implement this (e.g. maybe a global
> trait object that must be implemented by each server?)
>
> # Future work
>
> We could extend this pattern for other crates (e.g. rest-server) so
> that we don't have to carry around the parameters anymore.
>
> Also we could make the ServerConfig extendable, where each server could
> configure it's own necessary variables (e.g with a hashmap)
>
> proxmox:
>
> Dominik Csapak (3):
> new proxmox-server-config crate
> new proxmox-jobstate crate
> new proxmox-cert-management crate
>
> Cargo.toml | 5 +
> proxmox-cert-management/Cargo.toml | 23 ++
> proxmox-cert-management/debian/changelog | 5 +
> proxmox-cert-management/debian/control | 53 ++++
> proxmox-cert-management/debian/copyright | 18 ++
> proxmox-cert-management/debian/debcargo.toml | 7 +
> proxmox-cert-management/src/lib.rs | 182 +++++++++++
> proxmox-jobstate/Cargo.toml | 26 ++
> proxmox-jobstate/debian/changelog | 5 +
> proxmox-jobstate/debian/control | 55 ++++
> proxmox-jobstate/debian/copyright | 18 ++
> proxmox-jobstate/debian/debcargo.toml | 7 +
> proxmox-jobstate/src/api_types.rs | 40 +++
> proxmox-jobstate/src/jobstate.rs | 315 +++++++++++++++++++
> proxmox-jobstate/src/lib.rs | 46 +++
> proxmox-server-config/Cargo.toml | 16 +
> proxmox-server-config/debian/changelog | 5 +
> proxmox-server-config/debian/control | 37 +++
> proxmox-server-config/debian/copyright | 18 ++
> proxmox-server-config/debian/debcargo.toml | 7 +
> proxmox-server-config/src/lib.rs | 302 ++++++++++++++++++
> 21 files changed, 1190 insertions(+)
> create mode 100644 proxmox-cert-management/Cargo.toml
> create mode 100644 proxmox-cert-management/debian/changelog
> create mode 100644 proxmox-cert-management/debian/control
> create mode 100644 proxmox-cert-management/debian/copyright
> create mode 100644 proxmox-cert-management/debian/debcargo.toml
> create mode 100644 proxmox-cert-management/src/lib.rs
> create mode 100644 proxmox-jobstate/Cargo.toml
> create mode 100644 proxmox-jobstate/debian/changelog
> create mode 100644 proxmox-jobstate/debian/control
> create mode 100644 proxmox-jobstate/debian/copyright
> create mode 100644 proxmox-jobstate/debian/debcargo.toml
> create mode 100644 proxmox-jobstate/src/api_types.rs
> create mode 100644 proxmox-jobstate/src/jobstate.rs
> create mode 100644 proxmox-jobstate/src/lib.rs
> create mode 100644 proxmox-server-config/Cargo.toml
> create mode 100644 proxmox-server-config/debian/changelog
> create mode 100644 proxmox-server-config/debian/control
> create mode 100644 proxmox-server-config/debian/copyright
> create mode 100644 proxmox-server-config/debian/debcargo.toml
> create mode 100644 proxmox-server-config/src/lib.rs
>
> proxmox-backup:
>
> Dominik Csapak (1):
> use proxmox_jobstate and proxmox_cert_management crates
>
> Cargo.toml | 8 +
> pbs-api-types/Cargo.toml | 1 +
> pbs-api-types/src/jobs.rs | 41 +---
> src/auth_helpers.rs | 184 +---------------
> src/bin/proxmox-backup-api.rs | 8 +-
> src/bin/proxmox-backup-manager.rs | 2 +
> src/bin/proxmox-backup-proxy.rs | 2 +
> src/server/jobstate.rs | 347 +-----------------------------
> src/server/mod.rs | 21 ++
> 9 files changed, 52 insertions(+), 562 deletions(-)
>
--
- Lukas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate Dominik Csapak
@ 2023-10-25 16:38 ` Thomas Lamprecht
2023-10-30 11:05 ` Dominik Csapak
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Lamprecht @ 2023-10-25 16:38 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
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>,
> +}
> +
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate
2023-10-25 16:38 ` Thomas Lamprecht
@ 2023-10-30 11:05 ` Dominik Csapak
2023-10-30 11:41 ` Thomas Lamprecht
0 siblings, 1 reply; 9+ messages in thread
From: Dominik Csapak @ 2023-10-30 11:05 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox Backup Server development discussion
On 10/25/23 18:38, Thomas Lamprecht wrote:
> 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?
>
you're right that it's currently only used together with the
rest-server, but i though it might bite us in the future if we put that
together. We might want to have some daemons that does not have a rest
part in the future?
e.g. like we now have a few of them in PVE (pvestatd, qmeventd, spiceproxy)
also having more smaller crates does improve compile time for boxes
with many cores (not really an argument for or against, just a side effect)
> 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..
>
i'll look into it. I did not spend much time on such things for the RFC.
just wanted to see how others feel about the general approach
> 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)
yes, of course
>
>> +//! 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
>
true, IMHO at least the 'fs' part would warrant it's own crate
>> +
[snip]
>> +pub struct ServerConfig {
>
> Lacks doc comments for members, please add some even if they're
> private.
sure
>
>> + 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.
>
that's one reason i used OnceLock, since it's only created when either
one sets it manually, or access it the first time
but Option would work too ofc if that's preferred
>
>> + log_dir: OnceLock<PathBuf>,
>
> maybe: access_log_dir (as this is the api access log that other
if we move it to rest server i agree, but otherwise i'd leave it more
generic (other daemons might have logging needs besides the access log)
>
>> + cert_dir: OnceLock<PathBuf>,
>> + state_dir: OnceLock<PathBuf>,
>> + run_dir: OnceLock<PathBuf>,
>> + config_dir: OnceLock<PathBuf>,
>> +}
>> +
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate
2023-10-30 11:05 ` Dominik Csapak
@ 2023-10-30 11:41 ` Thomas Lamprecht
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2023-10-30 11:41 UTC (permalink / raw)
To: Dominik Csapak, Proxmox Backup Server development discussion
Am 30/10/2023 um 12:05 schrieb Dominik Csapak:
> On 10/25/23 18:38, Thomas Lamprecht wrote:
>> 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?
>>
>
> you're right that it's currently only used together with the
> rest-server, but i though it might bite us in the future if we put that
> together. We might want to have some daemons that does not have a rest
> part in the future?
then adapt to the actual reality that happens in the future? It's not like
this is then set in stone, and especially with rust one can adapt this
relatively easily, or at least without fear of subtle breakage.
> e.g. like we now have a few of them in PVE (pvestatd, qmeventd, spiceproxy)
Yes, which either mostly exists due to the lack of (sane) threading in perl,
or are simple and have no need for worker stuff (like qmeventd, or the syscall
proxy, already written in rust).
> also having more smaller crates does improve compile time for boxes
> with many cores (not really an argument for or against, just a side effect)
This crate is so small, that the static overhead of having it probably even
increases total build time ;-)
But I digress, I do not argue against having small, or even micro crates,
on the contrary they can be great for certain stuff.
And "certain stuff" is doing the heavy lifting there, as there are only some
cases where one can be sure up-front that a micro crate is best. E.g., one
ideal one would have a small scope, and allow one to be almost certain that
the ABI never has to be broken. A config trait crate for our (still a bit messy)
daemons isn't one of those IMO. Most others can only be decided (for sure) in
retrospect, until then the code should live as close to the actual use case
as possible (i.e., a micro crate can be fine, having it in the crate that uses
it can be fine, but not putting it in-between in some monster helper crate
like proxmox-sys please!).
Here it could be OK'ish, but I'd favor having this in rest-server until there's
an actual use case to reason about.
>> 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..
>>
>
> i'll look into it. I did not spend much time on such things for the RFC.
> just wanted to see how others feel about the general approach
If it's messy, or hardly worth it, then specifying the previous location
(including some verison, or even commit rev) would already make it lots
easier to find, if really needed.
>>> + 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.
>>
>
> that's one reason i used OnceLock, since it's only created when either
> one sets it manually, or access it the first time
>
> but Option would work too ofc if that's preferred
no hard preference, but Option feels more explicit for a, well optional,
use-case – but if we keep this code in proxmox-rest-server for now it
wouldn't matter anyway.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-10-30 11:41 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-18 10:39 [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate Dominik Csapak
2023-10-25 16:38 ` Thomas Lamprecht
2023-10-30 11:05 ` Dominik Csapak
2023-10-30 11:41 ` Thomas Lamprecht
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 2/3] new proxmox-jobstate crate Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 3/3] new proxmox-cert-management crate Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox-backup 1/1] use proxmox_jobstate and proxmox_cert_management crates Dominik Csapak
2023-10-24 8:17 ` [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Lukas Wagner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox