[snip] # workspace dependencies -proxmox-access-control = { version = "0.2.5", path = "proxmox-access-control" } -proxmox-acme = { version = "1.0.0", path = "proxmox-acme", default-features = false } -proxmox-api-macro = { version = "1.4.0", path = "proxmox-api-macro" } -proxmox-apt-api-types = { version = "2.0.0", path = "proxmox-apt-api-types" } -proxmox-auth-api = { version = "1.0.0", path = "proxmox-auth-api" } -proxmox-async = { version = "0.5.0", path = "proxmox-async" } -proxmox-base64 = { version = "1.0.0", path = "proxmox-base64" } -proxmox-compression = { version = "1.0.0", path = "proxmox-compression" } -proxmox-daemon = { version = "1.0.0", path = "proxmox-daemon" } -proxmox-http = { version = "1.0.0", path = "proxmox-http" } -proxmox-http-error = { version = "1.0.0", path = "proxmox-http-error" } -proxmox-human-byte = { version = "1.0.0", path = "proxmox-human-byte" } -proxmox-io = { version = "1.2.0", path = "proxmox-io" } -proxmox-lang = { version = "1.5", path = "proxmox-lang" } -proxmox-log = { version = "1.0.0", path = "proxmox-log" } -proxmox-login = { version = "1.0.0", path = "proxmox-login" } -proxmox-product-config = { version = "1.0.0", path = "proxmox-product-config" } -proxmox-config-digest = { version = "1.0.0", path = "proxmox-config-digest" } -proxmox-rest-server = { version = "1.0.0", path = "proxmox-rest-server" } -proxmox-router = { version = "3.2.2", path = "proxmox-router" } -proxmox-schema = { version = "4.1.0", path = "proxmox-schema" } -proxmox-section-config = { version = "3.1.0", path = "proxmox-section-config" } -proxmox-sendmail = { version = "1.0.0", path = "proxmox-sendmail" } -proxmox-serde = { version = "1.0.0", path = "proxmox-serde", features = [ "serde_json" ] } -proxmox-shared-memory = { version = "1.0.0", path = "proxmox-shared-memory" } +proxmox-acme = { version = "0.5.3", path = "proxmox-acme", default-features = false } +proxmox-api-macro = { version = "1.3.2", path = "proxmox-api-macro" } +proxmox-apt-api-types = { version = "1.0.2", path = "proxmox-apt-api-types" } +proxmox-auth-api = { version = "0.4.0", path = "proxmox-auth-api" } +proxmox-async = { version = "0.4.1", path = "proxmox-async" } +proxmox-compression = { version = "0.2.4", path = "proxmox-compression" } +proxmox-daemon = { version = "0.1.0", path = "proxmox-daemon" } +proxmox-http = { version = "0.9.5", path = "proxmox-http" } +proxmox-http-error = { version = "0.1.0", path = "proxmox-http-error" } +proxmox-human-byte = { version = "0.1.0", path = "proxmox-human-byte" } +proxmox-io = { version = "1.1.0", path = "proxmox-io" } +proxmox-lang = { version = "1.3", path = "proxmox-lang" } +proxmox-log= { version = "0.2.9", path = "proxmox-log" } +proxmox-login = { version = "0.2.0", path = "proxmox-login" } +proxmox-network-types = { version = "0.1.0", path = "proxmox-network-types" } +proxmox-product-config = { version = "0.2.0", path = "proxmox-product-config" } +proxmox-config-digest = { version = "0.1.0", path = "proxmox-config-digest" } +proxmox-rest-server = { version = "0.8.8", path = "proxmox-rest-server" } +proxmox-router = { version = "3.1.1", path = "proxmox-router" } +proxmox-schema = { version = "4.0.0", path = "proxmox-schema" } +proxmox-section-config = { version = "3.0.0", path = "proxmox-section-config" } +proxmox-sendmail = { version = "0.1.0", path = "proxmox-sendmail" } +proxmox-serde = { version = "0.1.1", path = "proxmox-serde", features = [ "serde_json" ] } +proxmox-shared-memory = { version = "0.3.0", path = "proxmox-shared-memory" } proxmox-sortable-macro = { version = "0.1.3", path = "proxmox-sortable-macro" }^ Reverts to bookworm deps - you sure the patches are otherwise based on trixie?
Missed this hunk, my bad. The rest should be rebased on trixie correctly.
diff --git a/proxmox-network-types/src/ip_address.rs b/proxmox-network-types/src/ip_address.rs new file mode 100644 index 000000000000..355547b17ae0 --- /dev/null +++ b/proxmox-network-types/src/ip_address.rs @@ -0,0 +1,1410 @@ +//! Provides helpers to deal with IP addresses / CIDRs + +use std::net::{AddrParseError, IpAddr, Ipv4Addr, Ipv6Addr}; + +use serde_with::{DeserializeFromStr, SerializeDisplay}; +use thiserror::Error; + +/// The family (v4 or v6) of an IP address or CIDR prefix +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Family { + V4, + V6, +} + +impl Family { + pub fn is_ipv4(&self) -> bool { + *self == Self::V4 + }nit: ↑↓ those could take self copied since it's Family is Copy.
Done.
+ + pub fn is_ipv6(&self) -> bool { + *self == Self::V6 + } +} + +impl std::fmt::Display for Family { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + Family::V4 => f.write_str("Ipv4"), + Family::V6 => f.write_str("Ipv6"),For a Display implementation it IMO makes no sense to mix capital/lowercase in "IP". If we need to keep this for ser/de purposes, add a comment please. Otherwise change it to IPv4/IPv6.
Agree.
+ } + } +} + +#[derive(Error, Debug)] +pub enum CidrError { + #[error("invalid netmask")] + InvalidNetmask, + #[error("invalid IP address")] + InvalidAddress(#[from] AddrParseError), +} + +/// Represents either an [`Ipv4Cidr`] or [`Ipv6Cidr`] CIDR prefix +#[derive( + Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash, SerializeDisplay, DeserializeFromStr, +)] +pub enum Cidr { + Ipv4(Ipv4Cidr), + Ipv6(Ipv6Cidr), +} + +impl Cidr { + pub fn new_v4(addr: impl Into<Ipv4Addr>, mask: u8) -> Result<Self, CidrError> { + Ok(Cidr::Ipv4(Ipv4Cidr::new(addr, mask)?)) + } + + pub fn new_v6(addr: impl Into<Ipv6Addr>, mask: u8) -> Result<Self, CidrError> { + Ok(Cidr::Ipv6(Ipv6Cidr::new(addr, mask)?)) + } + + /// which [`Family`] this CIDR belongs to + pub const fn family(&self) -> Family { + match self { + Cidr::Ipv4(_) => Family::V4, + Cidr::Ipv6(_) => Family::V6, + } + } + + pub fn is_ipv4(&self) -> bool { + matches!(self, Cidr::Ipv4(_)) + } + + pub fn is_ipv6(&self) -> bool { + matches!(self, Cidr::Ipv6(_)) + } + + /// Whether a given IP address is contained in this [`Cidr`] + /// + /// This only works if both [`IpAddr`] are in the same family, otherwise the function returns + /// false. + pub fn contains_address(&self, ip: &IpAddr) -> bool { + match (self, ip) { + (Cidr::Ipv4(cidr), IpAddr::V4(ip)) => cidr.contains_address(ip), + (Cidr::Ipv6(cidr), IpAddr::V6(ip)) => cidr.contains_address(ip), + _ => false, + } + } +} + +impl std::fmt::Display for Cidr { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + Self::Ipv4(ip) => f.write_str(ip.to_string().as_str()), + Self::Ipv6(ip) => f.write_str(ip.to_string().as_str()),No need to allocate a new string if you just forward to the inner value, either use `write!(f, ip)` (more convenient), or explicitly forward via `Display::fmt(ip, f)`: Self::Ipv4(ip) => fmt::Display::fmt(ip, f), Self::Ipv6(ip) => fmt::Display::fmt(ip, f),
Would have been `write!(f, "{}", ip)`, so I went with Display::fmt :)
+ } + } +} + +impl std::str::FromStr for Cidr { + type Err = CidrError; + + fn from_str(s: &str) -> Result<Self, Self::Err> { + if let Ok(ip) = s.parse::<Ipv4Cidr>() { + return Ok(Cidr::Ipv4(ip)); + } + + Ok(Cidr::Ipv6(s.parse()?)) + } +} + +impl From<Ipv4Cidr> for Cidr { + fn from(cidr: Ipv4Cidr) -> Self { + Cidr::Ipv4(cidr) + } +} + +impl From<Ipv6Cidr> for Cidr { + fn from(cidr: Ipv6Cidr) -> Self { + Cidr::Ipv6(cidr) + } +} + +impl From<IpAddr> for Cidr { + fn from(value: IpAddr) -> Self { + match value { + IpAddr::V4(addr) => Ipv4Cidr::from(addr).into(), + IpAddr::V6(addr) => Ipv6Cidr::from(addr).into(), + } + } +} + +const IPV4_LENGTH: u8 = 32; + +/// An IPv4 CIDR (e.g. 192.0.2.0/24) +#[derive( + SerializeDisplay, DeserializeFromStr, Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash,nit: would prefer the standard stuff first
Agree
[snip] +impl std::str::FromStr for Ipv4Cidr { + type Err = CidrError; + + fn from_str(s: &str) -> Result<Self, Self::Err> { + Ok(match s.find('/') { + None => Self { + addr: s.parse()?, + mask: 32, + }, + Some(pos) => { + let mask: u8 = s[(pos + 1)..] + .parse() + .map_err(|_| CidrError::InvalidNetmask)?; + + Self::new(s[..pos].parse::<Ipv4Addr>()?, mask)? + } + })^ since 1.52 this could be more readable with `.split_once('/')`, then we don't need to slice manually with `..pos`/`(pos + 1)..`.
Fixed this.
+ } +} + +impl std::fmt::Display for Ipv4Cidr { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}/{}", &self.addr, self.mask)^ unnecessary &
Agree.
[snip] +#[cfg(test)] +mod tests { + use super::*; + use std::net::{Ipv4Addr, Ipv6Addr};^ std import should be grouped befure the `super::*` one.
cargo fmt puts super over std :( Thanks for the review! _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
