Re: [pdm-devel] [PATCH datacenter-manager v3 03/18] server: api: implement CRUD api for views

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

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 <d.csapak@proxmox.com>
> ---
>  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", &notes::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<Vec<ViewConfig>, 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<ViewConfig> = 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<ConfigDigest>) -> 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<Vec<DeletableProperty>>,
> +    digest: Option<ConfigDigest>,
> +) -> 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<ConfigDigest>) -> 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<ViewConfig, Error> {
> +    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