[pve-devel] [PATCH proxmox-ve-rs 02/21] firewall: add ip range types
Max Carrara
m.carrara at proxmox.com
Tue Aug 13 18:08:20 CEST 2024
On Wed Jun 26, 2024 at 2:15 PM CEST, Stefan Hanreich wrote:
> Currently we are using tuples to represent IP ranges which is
> suboptimal. Validation logic and invariant checking needs to happen at
> every site using the IP range rather than having a unified struct for
> enforcing those invariants.
That's something I completely support; as you know I'm a fan of
representing state / invariants / etc. via types ;)
>
> Signed-off-by: Stefan Hanreich <s.hanreich at proxmox.com>
> ---
> .../src/firewall/types/address.rs | 230 +++++++++++++++++-
> 1 file changed, 228 insertions(+), 2 deletions(-)
>
> diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs
> index e48ac1b..ddf4652 100644
> --- a/proxmox-ve-config/src/firewall/types/address.rs
> +++ b/proxmox-ve-config/src/firewall/types/address.rs
> @@ -1,9 +1,9 @@
> -use std::fmt;
> +use std::fmt::{self, Display};
> use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
> use std::ops::Deref;
>
> use anyhow::{bail, format_err, Error};
> -use serde_with::DeserializeFromStr;
> +use serde_with::{DeserializeFromStr, SerializeDisplay};
>
> #[derive(Clone, Copy, Debug, Eq, PartialEq)]
> pub enum Family {
> @@ -239,6 +239,202 @@ impl<T: Into<Ipv6Addr>> From<T> for Ipv6Cidr {
> }
> }
>
> +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)]
> +pub enum IpRangeError {
> + MismatchedFamilies,
> + StartGreaterThanEnd,
> + InvalidFormat,
> +}
> +
> +impl std::error::Error for IpRangeError {}
> +
> +impl Display for IpRangeError {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + f.write_str(match self {
> + IpRangeError::MismatchedFamilies => "mismatched ip address families",
> + IpRangeError::StartGreaterThanEnd => "start is greater than end",
> + IpRangeError::InvalidFormat => "invalid ip range format",
> + })
> + }
> +}
> +
> +/// represents a range of IPv4 or IPv6 addresses
Small thing: I'd prefer
Represents a range of IPv4 or IPv6 addresses.
instead.
Mainly because most docstrings are written that way, and you do it in
later patches in a couple places as well anyways. Just gonna mention
this here once as you do this a couple times in this series in order to
avoid unnecessary noise.
IMO it's a really minor thing, but since you told me off-list that
you're in the process of neatly documenting everything, I thought I'd
mention it here.
> +///
> +/// For more information see [`AddressRange`]
> +#[derive(Clone, Copy, Debug, PartialEq, Eq, SerializeDisplay, DeserializeFromStr)]
> +pub enum IpRange {
> + V4(AddressRange<Ipv4Addr>),
> + V6(AddressRange<Ipv6Addr>),
> +}
> +
> +impl IpRange {
> + /// returns the family of the IpRange
> + pub fn family(&self) -> Family {
> + match self {
> + IpRange::V4(_) => Family::V4,
> + IpRange::V6(_) => Family::V6,
> + }
> + }
> +
> + /// creates a new [`IpRange`] from two [`IpAddr`]
> + ///
> + /// # Errors
> + ///
> + /// This function will return an error if start and end IP address are not from the same family.
> + pub fn new(start: impl Into<IpAddr>, end: impl Into<IpAddr>) -> Result<Self, IpRangeError> {
> + match (start.into(), end.into()) {
> + (IpAddr::V4(start), IpAddr::V4(end)) => Self::new_v4(start, end),
> + (IpAddr::V6(start), IpAddr::V6(end)) => Self::new_v6(start, end),
> + _ => Err(IpRangeError::MismatchedFamilies),
> + }
> + }
> +
> + /// construct a new Ipv4 Range
> + pub fn new_v4(
> + start: impl Into<Ipv4Addr>,
> + end: impl Into<Ipv4Addr>,
> + ) -> Result<Self, IpRangeError> {
> + Ok(IpRange::V4(AddressRange::new_v4(start, end)?))
> + }
> +
> + pub fn new_v6(
> + start: impl Into<Ipv6Addr>,
> + end: impl Into<Ipv6Addr>,
> + ) -> Result<Self, IpRangeError> {
> + Ok(IpRange::V6(AddressRange::new_v6(start, end)?))
> + }
> +}
> +
> +impl std::str::FromStr for IpRange {
> + type Err = IpRangeError;
> +
> + fn from_str(s: &str) -> Result<Self, Self::Err> {
> + if let Ok(range) = s.parse() {
> + return Ok(IpRange::V4(range));
> + }
> +
> + if let Ok(range) = s.parse() {
> + return Ok(IpRange::V6(range));
> + }
> +
> + Err(IpRangeError::InvalidFormat)
> + }
> +}
> +
> +impl fmt::Display for IpRange {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + match self {
> + IpRange::V4(range) => range.fmt(f),
> + IpRange::V6(range) => range.fmt(f),
> + }
> + }
> +}
> +
> +/// represents a range of IP addresses from start to end
> +///
> +/// This type is for encapsulation purposes for the [`IpRange`] enum and should be instantiated via
> +/// that enum.
> +///
> +/// # Invariants
> +///
> +/// * start and end have the same IP address family
> +/// * start is lesser than or equal to end
> +///
> +/// # Textual representation
> +///
> +/// Two IP addresses separated by a hyphen, e.g.: `127.0.0.1-127.0.0.255`
> +#[derive(Clone, Copy, Debug, PartialEq, Eq)]
> +pub struct AddressRange<T> {
> + start: T,
> + end: T,
> +}
> +
> +impl AddressRange<Ipv4Addr> {
> + pub(crate) fn new_v4(
> + start: impl Into<Ipv4Addr>,
> + end: impl Into<Ipv4Addr>,
> + ) -> Result<AddressRange<Ipv4Addr>, IpRangeError> {
> + let (start, end) = (start.into(), end.into());
> +
> + if start > end {
> + return Err(IpRangeError::StartGreaterThanEnd);
> + }
> +
> + Ok(Self { start, end })
> + }
> +}
> +
> +impl AddressRange<Ipv6Addr> {
> + pub(crate) fn new_v6(
> + start: impl Into<Ipv6Addr>,
> + end: impl Into<Ipv6Addr>,
> + ) -> Result<AddressRange<Ipv6Addr>, IpRangeError> {
> + let (start, end) = (start.into(), end.into());
> +
> + if start > end {
> + return Err(IpRangeError::StartGreaterThanEnd);
> + }
> +
> + Ok(Self { start, end })
> + }
> +}
> +
> +impl<T> AddressRange<T> {
> + pub fn start(&self) -> &T {
> + &self.start
> + }
> +
> + pub fn end(&self) -> &T {
> + &self.end
> + }
> +}
> +
> +impl std::str::FromStr for AddressRange<Ipv4Addr> {
> + type Err = IpRangeError;
> +
> + fn from_str(s: &str) -> Result<Self, Self::Err> {
> + if let Some((start, end)) = s.split_once('-') {
> + let start_address = start
> + .parse::<Ipv4Addr>()
> + .map_err(|_| IpRangeError::InvalidFormat)?;
> +
> + let end_address = end
> + .parse::<Ipv4Addr>()
> + .map_err(|_| IpRangeError::InvalidFormat)?;
> +
> + return Self::new_v4(start_address, end_address);
> + }
> +
> + Err(IpRangeError::InvalidFormat)
> + }
> +}
> +
> +impl std::str::FromStr for AddressRange<Ipv6Addr> {
> + type Err = IpRangeError;
> +
> + fn from_str(s: &str) -> Result<Self, Self::Err> {
> + if let Some((start, end)) = s.split_once('-') {
> + let start_address = start
> + .parse::<Ipv6Addr>()
> + .map_err(|_| IpRangeError::InvalidFormat)?;
> +
> + let end_address = end
> + .parse::<Ipv6Addr>()
> + .map_err(|_| IpRangeError::InvalidFormat)?;
> +
> + return Self::new_v6(start_address, end_address);
> + }
> +
> + Err(IpRangeError::InvalidFormat)
> + }
> +}
> +
> +impl<T: fmt::Display> fmt::Display for AddressRange<T> {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + write!(f, "{}-{}", self.start, self.end)
> + }
> +}
> +
> #[derive(Clone, Debug)]
> #[cfg_attr(test, derive(Eq, PartialEq))]
> pub enum IpEntry {
> @@ -612,4 +808,34 @@ mod tests {
> ])
> .expect_err("cannot mix ip families in ip list");
> }
> +
> + #[test]
> + fn test_ip_range() {
> + IpRange::new([10, 0, 0, 2], [10, 0, 0, 1]).unwrap_err();
> +
> + IpRange::new(
> + [0x2001, 0x0db8, 0, 0, 0, 0, 0, 0x1000],
> + [0x2001, 0x0db8, 0, 0, 0, 0, 0, 0],
> + )
> + .unwrap_err();
> +
> + let v4_range = IpRange::new([10, 0, 0, 0], [10, 0, 0, 100]).unwrap();
> + assert_eq!(v4_range.family(), Family::V4);
> +
> + let v6_range = IpRange::new(
> + [0x2001, 0x0db8, 0, 0, 0, 0, 0, 0],
> + [0x2001, 0x0db8, 0, 0, 0, 0, 0, 0x1000],
> + )
> + .unwrap();
> + assert_eq!(v6_range.family(), Family::V6);
> +
> + "10.0.0.1-10.0.0.100".parse::<IpRange>().unwrap();
> + "2001:db8::1-2001:db8::f".parse::<IpRange>().unwrap();
> +
> + "10.0.0.1-2001:db8::1000".parse::<IpRange>().unwrap_err();
> + "2001:db8::1-192.168.0.2".parse::<IpRange>().unwrap_err();
> +
> + "10.0.0.1-10.0.0.0".parse::<IpRange>().unwrap_err();
> + "2001:db8::1-2001:db8::0".parse::<IpRange>().unwrap_err();
> + }
> }
More information about the pve-devel
mailing list