On Mon, Feb 16, 2026 at 11:43:42AM +0100, 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),
IMO using `Self::` here, as well as putting a single `Ok()` around the
whole match makes this more readable. It would also condense the `other`
line into a single line.
> + "" => 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"),
Could avoid the macro machinery by using `f.write_str("second")` etc.
here.
> + 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);
> +
> +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
Missing punctuation (full stop) here ...
> + #[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.
... while here we have the lower-case versions as comments but *with*
punctuation afterwards. 🥴
> + 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");
> + }
> +}
> --
> 2.47.3
