Re: [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type

["Michael Köppl" <m.koeppl@proxmox.com>]

2 comments inline

On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
> This type will be used to define views.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>
> Notes:
>     Changes since v2:
>       - use ApiSectionDataEntry, as suggested by Dominik
>       - Renamed ViewFilter* -> View*
>       - Use SAFE_ID_FORMAT to check filter values where possible
>     
>     Changes since RFC:
>       - Change config structure, instead of 'include-{x} value' we now do
>         'include {x}:value'
>
>  lib/pdm-api-types/src/lib.rs   |   8 ++
>  lib/pdm-api-types/src/views.rs | 202 +++++++++++++++++++++++++++++++++
>  2 files changed, 210 insertions(+)
>  create mode 100644 lib/pdm-api-types/src/views.rs
>
> diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs
> index ee4dfb2b..20327aba 100644
> --- a/lib/pdm-api-types/src/lib.rs
> +++ b/lib/pdm-api-types/src/lib.rs
> @@ -106,6 +106,8 @@ pub mod subscription;
>  
>  pub mod sdn;
>  
> +pub mod views;
> +
>  const_regex! {
>      // just a rough check - dummy acceptor is used before persisting
>      pub OPENSSL_CIPHERS_REGEX = r"^[0-9A-Za-z_:, +!\-@=.]+$";
> @@ -153,6 +155,12 @@ pub const REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
>      .max_length(32)
>      .schema();
>  
> +pub const VIEW_ID_SCHEMA: Schema = StringSchema::new("View name.")
> +    .format(&PROXMOX_SAFE_ID_FORMAT)
> +    .min_length(2)
> +    .max_length(32)
> +    .schema();
> +
>  pub const VMID_SCHEMA: Schema = IntegerSchema::new("A guest ID").minimum(1).schema();
>  pub const SNAPSHOT_NAME_SCHEMA: Schema = StringSchema::new("The name of the snapshot")
>      .format(&PROXMOX_SAFE_ID_FORMAT)
> diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
> new file mode 100644
> index 00000000..165b29d6
> --- /dev/null
> +++ b/lib/pdm-api-types/src/views.rs
> @@ -0,0 +1,202 @@
> +use std::{fmt::Display, str::FromStr, sync::OnceLock};
> +
> +use anyhow::bail;
> +use proxmox_section_config::{typed::ApiSectionDataEntry, SectionConfig, SectionConfigPlugin};
> +use serde::{Deserialize, Serialize};
> +
> +use proxmox_schema::{
> +    api, api_types::SAFE_ID_FORMAT, ApiStringFormat, ApiType, ArraySchema, Schema, StringSchema,
> +    Updater,
> +};
> +
> +use crate::{remotes::REMOTE_ID_SCHEMA, resource::ResourceType, VIEW_ID_SCHEMA};
> +
> +/// Schema for filter rules.
> +pub const FILTER_RULE_SCHEMA: Schema = StringSchema::new("Filter rule for resources.")
> +    .format(&ApiStringFormat::VerifyFn(verify_filter_rule))
> +    .type_text(
> +        "resource-type:storage|qemu|lxc|sdn-zone|datastore|node>\

This line seems to be missing a <

> +            |resource-pool:<pool-name>\
> +            |tag:<tag-name>\
> +            |remote:<remote-name>\
> +            |resource-id:<resource-id>",
> +    )
> +    .schema();
> +
> +/// Schema for list of filter rules.
> +pub const FILTER_RULE_LIST_SCHEMA: Schema =
> +    ArraySchema::new("List of filter rules.", &FILTER_RULE_SCHEMA).schema();
> +
> +#[api(
> +    properties: {
> +        "id": {
> +            schema: VIEW_ID_SCHEMA,
> +        },
> +        "include": {
> +            schema: FILTER_RULE_LIST_SCHEMA,
> +            optional: true,
> +        },
> +        "exclude": {
> +            schema: FILTER_RULE_LIST_SCHEMA,
> +            optional: true,
> +        }
> +    }
> +)]
> +#[derive(Clone, Debug, Default, Deserialize, Serialize, Updater, PartialEq)]
> +#[serde(rename_all = "kebab-case")]
> +/// View definition
> +pub struct ViewConfig {
> +    /// View name.
> +    #[updater(skip)]
> +    pub id: String,
> +
> +    /// List of includes.
> +    #[serde(default, skip_serializing_if = "Vec::is_empty")]
> +    #[updater(serde(skip_serializing_if = "Option::is_none"))]
> +    pub include: Vec<FilterRule>,
> +
> +    /// List of excludes.
> +    #[serde(default, skip_serializing_if = "Vec::is_empty")]
> +    #[updater(serde(skip_serializing_if = "Option::is_none"))]
> +    pub exclude: Vec<FilterRule>,
> +}
> +
> +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
> +#[serde(rename_all = "kebab-case")]
> +/// Enum for the different sections in the 'views.cfg' file.
> +pub enum ViewConfigEntry {
> +    /// 'view' section
> +    View(ViewConfig),
> +}
> +
> +const VIEW_SECTION_NAME: &str = "view";
> +
> +impl ApiSectionDataEntry for ViewConfigEntry {
> +    fn section_config() -> &'static SectionConfig {
> +        static CONFIG: OnceLock<SectionConfig> = OnceLock::new();
> +
> +        CONFIG.get_or_init(|| {
> +            let mut this = SectionConfig::new(&VIEW_ID_SCHEMA);
> +
> +            this.register_plugin(SectionConfigPlugin::new(
> +                VIEW_SECTION_NAME.into(),
> +                Some("id".to_string()),
> +                ViewConfig::API_SCHEMA.unwrap_object_schema(),
> +            ));
> +            this
> +        })
> +    }
> +
> +    fn section_type(&self) -> &'static str {
> +        match self {
> +            ViewConfigEntry::View(_) => VIEW_SECTION_NAME,
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Debug, PartialEq)]
> +/// Filter rule for includes/excludes.
> +pub enum FilterRule {
> +    /// Match a resource type.
> +    ResourceType(ResourceType),
> +    /// Match a resource pools (for PVE guests).
> +    ResourcePool(String),
> +    /// Match a (global) resource ID, e.g. 'remote/<remote>/guest/<vmid>'.
> +    ResourceId(String),
> +    /// Match a tag (for PVE guests).
> +    Tag(String),
> +    /// Match a remote.
> +    Remote(String),
> +}
> +
> +impl FromStr for FilterRule {
> +    type Err = anyhow::Error;
> +
> +    fn from_str(s: &str) -> Result<Self, Self::Err> {
> +        Ok(match s.split_once(':') {
> +            Some(("resource-type", value)) => FilterRule::ResourceType(value.parse()?),
> +            Some(("resource-pool", value)) => {
> +                if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
> +                    bail!("invalid tag value: {value}");

This should probably have different error message such as "invalid
resource pool ID".

> +                }
> +                FilterRule::ResourcePool(value.to_string())
> +            }
> +            Some(("resource-id", value)) => {
> +                // TODO: Define schema and use it to validate. Can't use SAFE_ID_FORMAT since it does
> +                // not allow '/'.
> +                if value.is_empty() {
> +                    bail!("empty resource-id rule not allowed");
> +                }
> +                FilterRule::ResourceId(value.to_string())
> +            }
> +            Some(("tag", value)) => {
> +                if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
> +                    bail!("invalid tag value: {value}");
> +                }
> +                FilterRule::Tag(value.to_string())
> +            }
> +            Some(("remote", value)) => {
> +                let _ = REMOTE_ID_SCHEMA.parse_simple_value(value)?;
> +                FilterRule::Remote(value.to_string())
> +            }
> +            Some((ty, _)) => bail!("invalid type: {ty}"),
> +            None => bail!("invalid filter rule: {s}"),
> +        })
> +    }
> +}
> +
> +// used for serializing below, caution!
> +impl Display for FilterRule {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        match self {
> +            FilterRule::ResourceType(resource_type) => write!(f, "resource-type:{resource_type}"),
> +            FilterRule::ResourcePool(pool) => write!(f, "resource-pool:{pool}"),
> +            FilterRule::ResourceId(id) => write!(f, "resource-id:{id}"),
> +            FilterRule::Tag(tag) => write!(f, "tag:{tag}"),
> +            FilterRule::Remote(remote) => write!(f, "remote:{remote}"),
> +        }
> +    }
> +}
> +
> +proxmox_serde::forward_deserialize_to_from_str!(FilterRule);
> +proxmox_serde::forward_serialize_to_display!(FilterRule);
> +
> +fn verify_filter_rule(input: &str) -> Result<(), anyhow::Error> {
> +    FilterRule::from_str(input).map(|_| ())
> +}
> +
> +#[cfg(test)]
> +mod test {
> +    use anyhow::Error;
> +
> +    use crate::views::FilterRule;
> +
> +    fn parse_and_check_display(input: &str) -> Result<bool, Error> {
> +        let rule: FilterRule = input.parse()?;
> +
> +        Ok(input == format!("{rule}"))
> +    }
> +
> +    #[test]
> +    fn test_filter_rule() {
> +        assert!(parse_and_check_display("abc").is_err());
> +        assert!(parse_and_check_display("abc:").is_err());
> +
> +        assert!(parse_and_check_display("resource-type:").is_err());
> +        assert!(parse_and_check_display("resource-type:lxc").unwrap());
> +        assert!(parse_and_check_display("resource-type:qemu").unwrap());
> +        assert!(parse_and_check_display("resource-type:abc").is_err());
> +
> +        assert!(parse_and_check_display("resource-pool:").is_err());
> +        assert!(parse_and_check_display("resource-pool:somepool").unwrap());
> +
> +        assert!(parse_and_check_display("resource-id:").is_err());
> +        assert!(parse_and_check_display("resource-id:remote/someremote/guest/100").unwrap());
> +
> +        assert!(parse_and_check_display("tag:").is_err());
> +        assert!(parse_and_check_display("tag:sometag").unwrap());
> +
> +        assert!(parse_and_check_display("remote:someremote").unwrap());
> +        assert!(parse_and_check_display("remote:a").is_err());
> +    }
> +}



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel