Re: [RFC proxmox 12/22] firewall-api-types: add FirewallIpsetReference type

[Stefan Hanreich <s.hanreich@proxmox.com>]

comments inline

On 2/16/26 11:45 AM, Dietmar Maurer wrote:
> This adds a new type to reference ipsets with proper scope handling
> (Datacenter, Guest, SDN, or None for legacy ipsets).
> 
> The implementation includes:
> - FirewallIpsetScope enum for scope variants
> - FirewallIpsetReference struct with validation
> - Proper encapsulation with constructor and accessor methods
> - FromStr implementation for parsing ipset references
> 
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
>  proxmox-firewall-api-types/src/ipset.rs | 191 ++++++++++++++++++++++++
>  proxmox-firewall-api-types/src/lib.rs   |   3 +
>  2 files changed, 194 insertions(+)
>  create mode 100644 proxmox-firewall-api-types/src/ipset.rs
> 
> diff --git a/proxmox-firewall-api-types/src/ipset.rs b/proxmox-firewall-api-types/src/ipset.rs
> new file mode 100644
> index 00000000..02659394
> --- /dev/null
> +++ b/proxmox-firewall-api-types/src/ipset.rs
> @@ -0,0 +1,191 @@
> +use std::fmt;
> +use std::str::FromStr;
> +
> +use anyhow::{bail, Error};
> +
> +#[cfg(feature = "enum-fallback")]
> +use proxmox_fixed_string::FixedString;
> +
> +/// The scope of an ipset.
> +#[derive(Debug, Clone, Copy, Eq, PartialEq)]
> +pub enum FirewallIpsetScope {
> +    /// Datacenter scope.
> +    Datacenter,
> +    /// Guest scope.
> +    Guest,
> +    /// SDN scope.
> +    Sdn,
> +    /// No scope (e.g. for legacy ipsets).
> +    None,

would it potentially be better to remove the None here...

> +    #[cfg(feature = "enum-fallback")]
> +    /// Unknown variants for forward compatibility.
> +    UnknownEnumValue(FixedString),
> +}
> +
> +/// A reference to an ipset, including its scope.
> +#[derive(Debug, Clone, Eq, PartialEq)]
> +pub struct FirewallIpsetReference {
> +    scope: FirewallIpsetScope,

...and then use Option<FirewallIpsetScope> here? Would make it clearer
imo that the scope is optional. Might make handling the scope less
ergonomic though. Same goes for the Alias scope in the next patch.

[snip]