comments inline On 2/16/26 11:45 AM, Dietmar Maurer wrote: > This adds several firewall logging-related types: > - FirewallLogLevel: enum for syslog-style log levels (emerg to nolog) > - FirewallLogRateLimit: configuration for rate-limiting log messages > - FirewallPacketRate: packet rate representation (e.g., '100/second') > - FirewallPacketRateTimescale: time units for rate limiting > > Includes comprehensive tests for parsing, display, and roundtrip > serialization. > > Signed-off-by: Dietmar Maurer <[email protected]> > --- > proxmox-firewall-api-types/src/lib.rs | 5 + > proxmox-firewall-api-types/src/log.rs | 312 ++++++++++++++++++++++++++ > 2 files changed, 317 insertions(+) > create mode 100644 proxmox-firewall-api-types/src/log.rs > > diff --git a/proxmox-firewall-api-types/src/lib.rs > b/proxmox-firewall-api-types/src/lib.rs > index b8004c76..d9ff4548 100644 > --- a/proxmox-firewall-api-types/src/lib.rs > +++ b/proxmox-firewall-api-types/src/lib.rs > @@ -1,2 +1,7 @@ > +mod log; > +pub use log::{ > + FirewallLogLevel, FirewallLogRateLimit, FirewallPacketRate, > FirewallPacketRateTimescale, > +}; > + > mod policy; > pub use policy::{FirewallFWPolicy, FirewallIOPolicy}; > diff --git a/proxmox-firewall-api-types/src/log.rs > b/proxmox-firewall-api-types/src/log.rs > new file mode 100644 > index 00000000..fb2df49e > --- /dev/null > +++ b/proxmox-firewall-api-types/src/log.rs > @@ -0,0 +1,312 @@ > +use std::fmt; > +use std::str::FromStr; > + > +use anyhow::{bail, Error}; > +use serde::{Deserialize, Serialize}; > + > +use proxmox_schema::api; > + > +#[cfg(feature = "enum-fallback")] > +use proxmox_fixed_string::FixedString; > + > +/// Firewall log rate limit time scales. > +#[derive(Copy, Clone, Default, PartialEq)] > +#[cfg_attr(test, derive(Debug))] > +pub enum FirewallPacketRateTimescale { > + /// second > + #[default] > + Second, > + /// minute > + Minute, > + /// hour > + Hour, > + /// day > + Day, > + #[cfg(feature = "enum-fallback")] > + /// Unknown variants for forward compatibility. > + UnknownEnumValue(FixedString), > +} > + > +impl FromStr for FirewallPacketRateTimescale { > + type Err = Error; > + > + fn from_str(str: &str) -> Result<Self, Error> { > + match str { > + "second" => Ok(FirewallPacketRateTimescale::Second), > + "minute" => Ok(FirewallPacketRateTimescale::Minute), > + "hour" => Ok(FirewallPacketRateTimescale::Hour), > + "day" => Ok(FirewallPacketRateTimescale::Day), > + "" => bail!("empty time scale specification"), > + #[cfg(not(feature = "enum-fallback"))] > + _ => bail!("Invalid time scale provided"), > + #[cfg(feature = "enum-fallback")] > + other => Ok(FirewallPacketRateTimescale::UnknownEnumValue( > + FixedString::from_str(other)?, > + )), > + } > + } > +} > + > +impl fmt::Display for FirewallPacketRateTimescale { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + match self { > + FirewallPacketRateTimescale::Second => write!(f, "second"), > + FirewallPacketRateTimescale::Minute => write!(f, "minute"), > + FirewallPacketRateTimescale::Hour => write!(f, "hour"), > + FirewallPacketRateTimescale::Day => write!(f, "day"), > + #[cfg(feature = "enum-fallback")] > + &FirewallPacketRateTimescale::UnknownEnumValue(scale) => > scale.fmt(f), > + } > + } > +} > + > +/// Packet rate for log rate limiting. > +#[derive(Copy, Clone, PartialEq)] > +#[cfg_attr(test, derive(Debug))] > +pub struct FirewallPacketRate { > + /// Number of packets > + pub packets: u64, > + /// Time scale for the rate > + pub timescale: FirewallPacketRateTimescale, > +} > + > +serde_plain::derive_deserialize_from_fromstr!(FirewallPacketRate, "valid > packet rate"); > +serde_plain::derive_serialize_from_display!(FirewallPacketRate);
we have similar macros in proxmox-serde as well [1]. Would potentially save an external dependency? applies to all instances in this patch series ofc. [1] https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox-serde/src/serde_macros.rs;h=9d88218361c805cd5b2fa6056e0c49dda9636fb2;hb=HEAD > +impl fmt::Display for FirewallPacketRate { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + write!(f, "{}/{}", self.packets, self.timescale) > + } > +} > + > +impl FromStr for FirewallPacketRate { > + type Err = Error; > + > + fn from_str(str: &str) -> Result<Self, Error> { > + match str.split_once('/') { > + None => Ok(FirewallPacketRate { > + packets: u64::from_str(str)?, > + timescale: FirewallPacketRateTimescale::default(), > + }), > + Some((rate, unit)) => Ok(FirewallPacketRate { > + packets: u64::from_str(rate)?, > + timescale: FirewallPacketRateTimescale::from_str(unit)?, > + }), > + } > + } > +} > + > +#[api( > + default_key: "enable", > + properties: { > + burst: { > + default: 5, > + minimum: 0, > + optional: true, > + type: Integer, > + }, > + enable: { > + default: true, > + }, > + rate: { > + default: "1/second", > + optional: true, > + type: String, > + }, > + }, > +)] > +/// Firewall log rate limit configuration. > +#[derive(Deserialize, Serialize, Clone, PartialEq)] > +#[cfg_attr(test, derive(Debug))] > +pub struct FirewallLogRateLimit { > + /// Initial burst of packages which will always get logged before the > rate > + /// is applied > + #[serde(deserialize_with = "proxmox_serde::perl::deserialize_u64")] > + #[serde(default, skip_serializing_if = "Option::is_none")] > + pub burst: Option<u64>, > + > + /// Enable or disable log rate limiting > + #[serde(deserialize_with = "proxmox_serde::perl::deserialize_bool")] > + pub enable: bool, > + > + /// Frequency with which the burst bucket gets refilled > + #[serde(default, skip_serializing_if = "Option::is_none")] > + pub rate: Option<FirewallPacketRate>, > +} > + > +#[api] > +/// Firewall log levels. > +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Deserialize, Serialize)] > +pub enum FirewallLogLevel { > + #[serde(rename = "emerg")] > + /// emerg. > + Emergency, > + #[serde(rename = "alert")] > + /// alert. > + Alert, > + #[serde(rename = "crit")] > + /// crit. > + Critical, > + #[serde(rename = "err")] > + /// err. > + Error, > + #[serde(rename = "warning")] > + /// warning. > + Warning, > + #[serde(rename = "notice")] > + /// notice. > + Notice, > + #[serde(rename = "info")] > + /// info. > + Info, > + #[serde(rename = "debug")] > + /// debug. > + Debug, > + #[serde(rename = "nolog")] > + #[default] > + /// nolog. > + Nolog, > +} > + > +serde_plain::derive_display_from_serialize!(FirewallLogLevel); > +serde_plain::derive_fromstr_from_deserialize!(FirewallLogLevel); > + > +#[cfg(test)] > +mod tests { > + use proxmox_schema::property_string::PropertyString; > + > + use super::*; > + > + #[test] > + fn test_parse_rate_limit() { > + let mut parsed_rate_limit: PropertyString<FirewallLogRateLimit> = > + serde_plain::from_str("1,burst=123,rate=44").expect("valid rate > limit"); > + > + assert_eq!( > + parsed_rate_limit.into_inner(), > + FirewallLogRateLimit { > + enable: true, > + burst: Some(123), > + rate: Some(FirewallPacketRate { > + packets: 44, > + timescale: FirewallPacketRateTimescale::Second, > + }), > + } > + ); > + > + parsed_rate_limit = serde_plain::from_str("1").expect("valid rate > limit"); > + > + assert_eq!( > + parsed_rate_limit.into_inner(), > + FirewallLogRateLimit { > + enable: true, > + burst: None, > + rate: None > + } > + ); > + > + parsed_rate_limit = > + serde_plain::from_str("enable=0,rate=123/hour").expect("valid > rate limit"); > + > + assert_eq!( > + parsed_rate_limit.into_inner(), > + FirewallLogRateLimit { > + enable: false, > + burst: None, > + rate: Some(FirewallPacketRate { > + packets: 123, > + timescale: FirewallPacketRateTimescale::Hour, > + }), > + } > + ); > + > + serde_plain::from_str::<PropertyString<FirewallLogRateLimit>>("2") > + .expect_err("invalid value for enable"); > + > + > serde_plain::from_str::<PropertyString<FirewallLogRateLimit>>("enabled=0,rate=123") > + .expect_err("invalid key in log ratelimit"); > + > + #[cfg(not(feature = "enum-fallback"))] > + > serde_plain::from_str::<PropertyString<FirewallLogRateLimit>>("enable=0,rate=123/proxmox,") > + .expect_err("invalid unit for rate"); > + } > + > + #[test] > + fn test_packet_rate_parse() { > + // Test parsing with all timescales > + let rate: FirewallPacketRate = "100/second".parse().expect("valid > rate"); > + assert_eq!(rate.packets, 100); > + assert_eq!(rate.timescale, FirewallPacketRateTimescale::Second); > + > + let rate: FirewallPacketRate = "50/minute".parse().expect("valid > rate"); > + assert_eq!(rate.packets, 50); > + assert_eq!(rate.timescale, FirewallPacketRateTimescale::Minute); > + > + let rate: FirewallPacketRate = "10/hour".parse().expect("valid > rate"); > + assert_eq!(rate.packets, 10); > + assert_eq!(rate.timescale, FirewallPacketRateTimescale::Hour); > + > + let rate: FirewallPacketRate = "1/day".parse().expect("valid rate"); > + assert_eq!(rate.packets, 1); > + assert_eq!(rate.timescale, FirewallPacketRateTimescale::Day); > + > + // Test default timescale when no unit specified > + let rate: FirewallPacketRate = "42".parse().expect("valid rate > without unit"); > + assert_eq!(rate.packets, 42); > + assert_eq!(rate.timescale, FirewallPacketRateTimescale::Second); > + } > + > + #[test] > + fn test_packet_rate_display() { > + let rate = FirewallPacketRate { > + packets: 100, > + timescale: FirewallPacketRateTimescale::Second, > + }; > + assert_eq!(rate.to_string(), "100/second"); > + > + let rate = FirewallPacketRate { > + packets: 5, > + timescale: FirewallPacketRateTimescale::Hour, > + }; > + assert_eq!(rate.to_string(), "5/hour"); > + } > + > + #[test] > + fn test_packet_rate_roundtrip() { > + let original = FirewallPacketRate { > + packets: 123, > + timescale: FirewallPacketRateTimescale::Minute, > + }; > + let serialized = original.to_string(); > + let parsed: FirewallPacketRate = > serialized.parse().expect("roundtrip parse"); > + assert_eq!(original, parsed); > + } > + > + #[test] > + fn test_packet_rate_parse_errors() { > + // Empty timescale > + "100/" > + .parse::<FirewallPacketRate>() > + .expect_err("empty timescale"); > + > + // Invalid timescale > + #[cfg(not(feature = "enum-fallback"))] > + "100/invalid" > + .parse::<FirewallPacketRate>() > + .expect_err("invalid timescale"); > + #[cfg(feature = "enum-fallback")] > + "100/invalid" > + .parse::<FirewallPacketRate>() > + .expect("valid timescale (enum fallback feature)"); > + > + // Invalid packet count > + "abc/second" > + .parse::<FirewallPacketRate>() > + .expect_err("invalid packet count"); > + > + // Negative number > + "-5/second" > + .parse::<FirewallPacketRate>() > + .expect_err("negative packet count"); > + } > +}
