From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 299E21FF185 for ; Mon, 17 Nov 2025 15:59:00 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9974E1D15D; Mon, 17 Nov 2025 15:59:01 +0100 (CET) Mime-Version: 1.0 Date: Mon, 17 Nov 2025 15:58:57 +0100 Message-Id: To: "Dominik Csapak" X-Mailer: aerc 0.20.0 References: <20251117125041.1931382-1-d.csapak@proxmox.com> <20251117125041.1931382-4-d.csapak@proxmox.com> In-Reply-To: <20251117125041.1931382-4-d.csapak@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1763391508464 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.937 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_MAILER 2 Automated Mailer Tag Left in Email SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH datacenter-manager v3 03/18] server: api: implement CRUD api for views X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" comments in-line. On Mon Nov 17, 2025 at 1:44 PM CET, Dominik Csapak wrote: > namely list/read/update/delete api calls in `/config/views` api > > Signed-off-by: Dominik Csapak > --- > server/src/api/config/mod.rs | 2 + > server/src/api/config/views.rs | 254 +++++++++++++++++++++++++++++++++ > 2 files changed, 256 insertions(+) > create mode 100644 server/src/api/config/views.rs > > diff --git a/server/src/api/config/mod.rs b/server/src/api/config/mod.rs > index 7b58c756..8f646c15 100644 > --- a/server/src/api/config/mod.rs > +++ b/server/src/api/config/mod.rs > @@ -6,6 +6,7 @@ pub mod access; > pub mod acme; > pub mod certificate; > pub mod notes; > +pub mod views; > > #[sortable] > const SUBDIRS: SubdirMap = &sorted!([ > @@ -13,6 +14,7 @@ const SUBDIRS: SubdirMap = &sorted!([ > ("acme", &acme::ROUTER), > ("certificate", &certificate::ROUTER), > ("notes", ¬es::ROUTER), > + ("views", &views::ROUTER) > ]); > > pub const ROUTER: Router = Router::new() > diff --git a/server/src/api/config/views.rs b/server/src/api/config/views.rs > new file mode 100644 > index 00000000..8c5a1d29 > --- /dev/null > +++ b/server/src/api/config/views.rs > @@ -0,0 +1,254 @@ > +use anyhow::Error; > + > +use proxmox_access_control::CachedUserInfo; > +use proxmox_config_digest::ConfigDigest; > +use proxmox_router::{http_bail, http_err, Permission, Router, RpcEnvironment}; > +use proxmox_schema::{api, param_bail}; > + > +use pdm_api_types::{ > + views::{ViewConfig, ViewConfigEntry, ViewConfigUpdater}, > + PRIV_RESOURCE_AUDIT, PRIV_RESOURCE_MODIFY, > +}; > +use serde::{Deserialize, Serialize}; tiny nit: this should be sorted with anyhow in a block and the imports from pdm_api_types::views should get their own `use` statement. > + > +const VIEW_ROUTER: Router = Router::new() > + .put(&API_METHOD_UPDATE_VIEW) > + .delete(&API_METHOD_REMOVE_VIEW) > + .get(&API_METHOD_READ_VIEW); > + > +pub const ROUTER: Router = Router::new() > + .get(&API_METHOD_GET_VIEWS) > + .post(&API_METHOD_ADD_VIEW) > + .match_all("id", &VIEW_ROUTER); > + > +#[api( > + protected: true, > + access: { > + permission: &Permission::Anybody, > + description: "Returns the views the user has access to.", > + }, > + returns: { > + description: "List of views.", > + type: Array, > + items: { > + type: String, > + description: "The name of a view." > + }, > + }, > +)] > +/// List views. > +pub fn get_views(rpcenv: &mut dyn RpcEnvironment) -> Result, Error> { > + let (config, _) = pdm_config::views::config()?; > + > + let user_info = CachedUserInfo::new()?; > + let auth_id = rpcenv.get_auth_id().unwrap().parse()?; > + let top_level_allowed = 0 != user_info.lookup_privs(&auth_id, &["view"]); hm this is a little weird. you check if the user has *any* privileges on the top level. so this would be true even if the privilege in question is `SYS_AUDIT` or similar. currently the roles that we have, bundle all auditing privileges in all roles, but that might change. so i think this should be: let top_level_allowed = user_info .check_privs(&auth_id, &["view"], PRIV_RESOURCE_AUDIT, false) .is_ok() correct me if im wrong, though. > + > + let views: Vec = config > + .into_iter() > + .filter_map(|(view, value)| { > + if !top_level_allowed > + && user_info > + .check_privs(&auth_id, &["view", &view], PRIV_RESOURCE_AUDIT, false) > + .is_err() > + { > + return None; > + }; > + match value { > + ViewConfigEntry::View(conf) => Some(conf), > + } > + }) > + .collect(); > + > + Ok(views) > +} > + > +#[api( > + protected: true, > + input: { > + properties: { > + view: { > + flatten: true, > + type: ViewConfig, > + }, > + digest: { > + type: ConfigDigest, > + optional: true, > + }, > + }, > + }, > + access: { > + permission: &Permission::Privilege(&["view"], PRIV_RESOURCE_MODIFY, false), > + }, > +)] > +/// Add new view nit: should probably end in a period. > +pub fn add_view(view: ViewConfig, digest: Option) -> Result<(), Error> { > + let _lock = pdm_config::views::lock_config()?; > + > + let (mut config, config_digest) = pdm_config::views::config()?; > + > + config_digest.detect_modification(digest.as_ref())?; > + > + let id = view.id.clone(); > + > + if let Some(ViewConfigEntry::View(_)) = config.insert(id.clone(), ViewConfigEntry::View(view)) { > + param_bail!("id", "view '{}' already exists.", id) > + } > + > + pdm_config::views::save_config(&config)?; > + > + Ok(()) > +} > + > +#[api()] > +#[derive(Serialize, Deserialize)] > +#[serde(rename_all = "kebab-case")] > +/// Deletable property name > +pub enum DeletableProperty { > + /// Delete the include filters. > + Include, > + /// Delete the exclude filters. > + Exclude, > + /// Delete the layout. > + Layout, > +} > + > +#[api( > + protected: true, > + input: { > + properties: { > + id: { > + type: String, > + description: "", > + }, > + view: { > + flatten: true, > + type: ViewConfigUpdater, > + }, > + delete: { > + description: "List of properties to delete.", > + type: Array, > + optional: true, > + items: { > + type: DeletableProperty, > + } > + }, > + digest: { > + type: ConfigDigest, > + optional: true, > + }, > + }, > + }, > + access: { > + permission: &Permission::Privilege(&["view", "{id}"], PRIV_RESOURCE_MODIFY, false), > + }, > +)] > +/// Update View nit: same here > +pub fn update_view( > + id: String, > + view: ViewConfigUpdater, > + delete: Option>, > + digest: Option, > +) -> Result<(), Error> { > + let _lock = pdm_config::views::lock_config()?; > + > + let (mut config, config_digest) = pdm_config::views::config()?; > + > + config_digest.detect_modification(digest.as_ref())?; > + > + let entry = config > + .get_mut(&id) > + .ok_or_else(|| http_err!(NOT_FOUND, "no such remote {id}"))?; > + > + let ViewConfigEntry::View(conf) = entry; > + > + if let Some(delete) = delete { > + for delete_prop in delete { > + match delete_prop { > + DeletableProperty::Include => conf.include = Vec::new(), > + DeletableProperty::Exclude => conf.exclude = Vec::new(), > + DeletableProperty::Layout => conf.layout = String::new(), > + } > + } > + } > + > + if let Some(include) = view.include { > + conf.include = include; > + } > + > + if let Some(exclude) = view.exclude { > + conf.exclude = exclude; > + } > + > + if let Some(layout) = view.layout { > + conf.layout = layout; > + } > + > + pdm_config::views::save_config(&config)?; > + > + Ok(()) > +} > + > +#[api( > + protected: true, > + input: { > + properties: { > + id: { > + type: String, > + description: "", > + }, > + digest: { > + type: ConfigDigest, > + optional: true, > + }, > + }, > + }, > + access: { > + permission: &Permission::Privilege(&["view"], PRIV_RESOURCE_MODIFY, false), > + }, > +)] > +/// Delete the view with the given id. > +pub fn remove_view(id: String, digest: Option) -> Result<(), Error> { > + let _lock = pdm_config::views::lock_config()?; > + > + let (mut config, config_digest) = pdm_config::views::config()?; > + > + config_digest.detect_modification(digest.as_ref())?; > + > + match config.remove(&id) { > + Some(ViewConfigEntry::View(_)) => {} > + None => http_bail!(NOT_FOUND, "view '{id}' does not exist."), > + } > + > + pdm_config::views::save_config(&config)?; > + > + Ok(()) > +} > + > +#[api( > + input: { > + properties: { > + id: { > + type: String, > + description: "", > + }, > + }, > + }, > + access: { > + permission: &Permission::Privilege(&["view", "{id}"], PRIV_RESOURCE_AUDIT, false), > + }, > +)] > +/// Get the config of a single view. > +pub fn read_view(id: String) -> Result { > + let (config, _) = pdm_config::views::config()?; > + > + let view = config > + .get(&id) > + .ok_or_else(|| http_err!(NOT_FOUND, "no such view '{id}'"))?; > + > + let view = match view { > + ViewConfigEntry::View(view) => view.clone(), > + }; > + > + Ok(view) > +} _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel