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 <[email protected]>
> ---
> 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]