From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 947E51FF13E for ; Fri, 03 Apr 2026 18:57:22 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 535D19169; Fri, 3 Apr 2026 18:57:53 +0200 (CEST) From: Christoph Heiss To: pdm-devel@lists.proxmox.com Subject: [PATCH installer v3 29/38] tree-wide: used moved `Fqdn` type to proxmox-network-types Date: Fri, 3 Apr 2026 18:54:01 +0200 Message-ID: <20260403165437.2166551-30-c.heiss@proxmox.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260403165437.2166551-1-c.heiss@proxmox.com> References: <20260403165437.2166551-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775235375708 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.067 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: KUJSIPHBWYRNT6SR6PYRCE7Y4T25BHWV X-Message-ID-Hash: KUJSIPHBWYRNT6SR6PYRCE7Y4T25BHWV X-MailFrom: c.heiss@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Now that the `Fqdn` has been moved to the proxmox-network-types crate, use it from there. No functional changes. Signed-off-by: Christoph Heiss --- Changes v2 -> v3: * new patch Cargo.toml | 6 + proxmox-auto-installer/Cargo.toml | 1 + proxmox-auto-installer/src/answer.rs | 4 +- proxmox-auto-installer/src/utils.rs | 6 +- proxmox-installer-common/Cargo.toml | 1 + proxmox-installer-common/src/options.rs | 3 +- proxmox-installer-common/src/utils.rs | 241 --------------------- proxmox-tui-installer/Cargo.toml | 1 + proxmox-tui-installer/src/setup.rs | 5 +- proxmox-tui-installer/src/views/network.rs | 6 +- 10 files changed, 20 insertions(+), 254 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3075bcc..379ee6b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,3 +27,9 @@ serde_plain = "1.0" toml = "0.8" proxmox-auto-installer.path = "./proxmox-auto-installer" proxmox-installer-common.path = "./proxmox-installer-common" +proxmox-network-types = "1.0" + +# Local path overrides +# NOTE: You must run `cargo update` after changing this for it to take effect! +[patch.crates-io] +# proxmox-network-types.path = "../proxmox/proxmox-network-types" diff --git a/proxmox-auto-installer/Cargo.toml b/proxmox-auto-installer/Cargo.toml index 8a5283e..0086e5d 100644 --- a/proxmox-auto-installer/Cargo.toml +++ b/proxmox-auto-installer/Cargo.toml @@ -14,6 +14,7 @@ homepage = "https://www.proxmox.com" anyhow.workspace = true log.workspace = true proxmox-installer-common = { workspace = true, features = ["http"] } +proxmox-network-types.workspace = true serde = { workspace = true, features = ["derive"] } serde_json.workspace = true serde_plain.workspace = true diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs index d12e088..40e6557 100644 --- a/proxmox-auto-installer/src/answer.rs +++ b/proxmox-auto-installer/src/answer.rs @@ -4,8 +4,10 @@ use proxmox_installer_common::{ BtrfsCompressOption, BtrfsRaidLevel, FsType, NetworkInterfacePinningOptions, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel, }, - utils::{CidrAddress, Fqdn}, + utils::CidrAddress, }; +use proxmox_network_types::fqdn::Fqdn; + use serde::{Deserialize, Serialize}; use std::{ collections::{BTreeMap, HashMap}, diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 09b3408..9998491 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -535,11 +535,7 @@ pub fn parse_answer( .map(|o| o.mapping) .unwrap_or_default(), - hostname: network_settings - .fqdn - .host() - .unwrap_or(setup_info.config.product.default_hostname()) - .to_string(), + hostname: network_settings.fqdn.host().to_owned(), domain: network_settings.fqdn.domain(), cidr: network_settings.address, gateway: network_settings.gateway, diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml index b3ce3d7..7469627 100644 --- a/proxmox-installer-common/Cargo.toml +++ b/proxmox-installer-common/Cargo.toml @@ -13,6 +13,7 @@ regex.workspace = true serde = { workspace = true, features = [ "derive" ] } serde_json.workspace = true serde_plain.workspace = true +proxmox-network-types.workspace = true # `http` feature hex = { version = "0.4", optional = true } diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index dcf4fe7..feb0dc4 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -10,7 +10,8 @@ use std::{cmp, fmt}; use crate::disk_checks::check_raid_min_disks; use crate::net::{MAX_IFNAME_LEN, MIN_IFNAME_LEN}; use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo}; -use crate::utils::{CidrAddress, Fqdn}; +use crate::utils::CidrAddress; +use proxmox_network_types::fqdn::Fqdn; #[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, PartialEq)] #[serde(rename_all(deserialize = "lowercase", serialize = "UPPERCASE"))] diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs index ffc862e..e86abdf 100644 --- a/proxmox-installer-common/src/utils.rs +++ b/proxmox-installer-common/src/utils.rs @@ -139,244 +139,3 @@ fn check_mask_limit(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseEr Ok(()) } } - -/// Possible errors that might occur when parsing FQDNs. -#[derive(Debug, Eq, PartialEq)] -pub enum FqdnParseError { - MissingHostname, - NumericHostname, - InvalidPart(String), - TooLong(usize), -} - -impl fmt::Display for FqdnParseError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use FqdnParseError::*; - match self { - MissingHostname => write!(f, "missing hostname part"), - NumericHostname => write!(f, "hostname cannot be purely numeric"), - InvalidPart(part) => write!( - f, - "FQDN must only consist of alphanumeric characters and dashes. Invalid part: '{part}'", - ), - TooLong(len) => write!(f, "FQDN too long: {len} > {}", Fqdn::MAX_LENGTH), - } - } -} - -/// A type for safely representing fully-qualified domain names (FQDNs). -/// -/// It considers following RFCs: -/// - [RFC952] (sec. "ASSUMPTIONS", 1.) -/// - [RFC1035] (sec. 2.3. "Conventions") -/// - [RFC1123] (sec. 2.1. "Host Names and Numbers") -/// - [RFC3492] -/// - [RFC4343] -/// -/// .. and applies some restriction given by Debian, e.g. 253 instead of 255 -/// maximum total length and maximum 63 characters per label, per the -/// [hostname(7)]. -/// -/// Additionally: -/// - It enforces the restriction as per Bugzilla #1054, in that -/// purely numeric hostnames are not allowed - against RFC1123 sec. 2.1. -/// -/// Some terminology: -/// - "label" - a single part of a FQDN, e.g. {label}.{label}.{tld} -/// -/// [RFC952]: -/// [RFC1035]: -/// [RFC1123]: -/// [RFC3492]: -/// [RFC4343]: -/// [hostname(7)]: -#[derive(Clone, Debug, Eq)] -pub struct Fqdn { - parts: Vec, -} - -impl Fqdn { - /// Maximum length of a single label of the FQDN - const MAX_LABEL_LENGTH: usize = 63; - /// Maximum total length of the FQDN - const MAX_LENGTH: usize = 253; - - pub fn from(fqdn: &str) -> Result { - if fqdn.len() > Self::MAX_LENGTH { - return Err(FqdnParseError::TooLong(fqdn.len())); - } - - let parts = fqdn - .split('.') - .map(ToOwned::to_owned) - .collect::>(); - - for part in &parts { - if !Self::validate_single(part) { - return Err(FqdnParseError::InvalidPart(part.clone())); - } - } - - if parts.len() < 2 { - Err(FqdnParseError::MissingHostname) - } else if parts[0].chars().all(|c| c.is_ascii_digit()) { - // Do not allow a purely numeric hostname, see: - // https://bugzilla.proxmox.com/show_bug.cgi?id=1054 - Err(FqdnParseError::NumericHostname) - } else { - Ok(Self { parts }) - } - } - - pub fn host(&self) -> Option<&str> { - self.has_host().then_some(&self.parts[0]) - } - - pub fn domain(&self) -> String { - let parts = if self.has_host() { - &self.parts[1..] - } else { - &self.parts - }; - - parts.join(".") - } - - /// Checks whether the FQDN has a hostname associated with it, i.e. is has more than 1 part. - fn has_host(&self) -> bool { - self.parts.len() > 1 - } - - fn validate_single(s: &str) -> bool { - !s.is_empty() - && s.len() <= Self::MAX_LABEL_LENGTH - // First character must be alphanumeric - && s.chars() - .next() - .map(|c| c.is_ascii_alphanumeric()) - .unwrap_or_default() - // .. last character as well, - && s.chars() - .last() - .map(|c| c.is_ascii_alphanumeric()) - .unwrap_or_default() - // and anything between must be alphanumeric or - - && s.chars() - .skip(1) - .take(s.len().saturating_sub(2)) - .all(|c| c.is_ascii_alphanumeric() || c == '-') - } -} - -impl FromStr for Fqdn { - type Err = FqdnParseError; - - fn from_str(value: &str) -> Result { - Self::from(value) - } -} - -impl fmt::Display for Fqdn { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.parts.join(".")) - } -} - -impl<'de> Deserialize<'de> for Fqdn { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - let s: String = Deserialize::deserialize(deserializer)?; - s.parse() - .map_err(|_| serde::de::Error::custom("invalid FQDN")) - } -} - -impl PartialEq for Fqdn { - // Case-insensitive comparison, as per RFC 952 "ASSUMPTIONS", RFC 1035 sec. 2.3.3. "Character - // Case" and RFC 4343 as a whole - fn eq(&self, other: &Self) -> bool { - if self.parts.len() != other.parts.len() { - return false; - } - - self.parts - .iter() - .zip(other.parts.iter()) - .all(|(a, b)| a.to_lowercase() == b.to_lowercase()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn fqdn_construct() { - use FqdnParseError::*; - assert!(Fqdn::from("foo.example.com").is_ok()); - assert!(Fqdn::from("foo-bar.com").is_ok()); - assert!(Fqdn::from("a-b.com").is_ok()); - - assert_eq!(Fqdn::from("foo"), Err(MissingHostname)); - - assert_eq!(Fqdn::from("-foo.com"), Err(InvalidPart("-foo".to_owned()))); - assert_eq!(Fqdn::from("foo-.com"), Err(InvalidPart("foo-".to_owned()))); - assert_eq!(Fqdn::from("foo.com-"), Err(InvalidPart("com-".to_owned()))); - assert_eq!(Fqdn::from("-o-.com"), Err(InvalidPart("-o-".to_owned()))); - - // https://bugzilla.proxmox.com/show_bug.cgi?id=1054 - assert_eq!(Fqdn::from("123.com"), Err(NumericHostname)); - assert!(Fqdn::from("foo123.com").is_ok()); - assert!(Fqdn::from("123foo.com").is_ok()); - - assert!(Fqdn::from(&format!("{}.com", "a".repeat(63))).is_ok()); - assert_eq!( - Fqdn::from(&format!("{}.com", "a".repeat(250))), - Err(TooLong(254)), - ); - assert_eq!( - Fqdn::from(&format!("{}.com", "a".repeat(64))), - Err(InvalidPart("a".repeat(64))), - ); - - // https://bugzilla.proxmox.com/show_bug.cgi?id=5230 - assert_eq!( - Fqdn::from("123@foo.com"), - Err(InvalidPart("123@foo".to_owned())) - ); - } - - #[test] - fn fqdn_parts() { - let fqdn = Fqdn::from("pve.example.com").unwrap(); - assert_eq!(fqdn.host().unwrap(), "pve"); - assert_eq!(fqdn.domain(), "example.com"); - assert_eq!( - fqdn.parts, - &["pve".to_owned(), "example".to_owned(), "com".to_owned()] - ); - } - - #[test] - fn fqdn_display() { - assert_eq!( - Fqdn::from("foo.example.com").unwrap().to_string(), - "foo.example.com" - ); - } - - #[test] - fn fqdn_compare() { - assert_eq!(Fqdn::from("example.com"), Fqdn::from("example.com")); - assert_eq!(Fqdn::from("example.com"), Fqdn::from("ExAmPle.Com")); - assert_eq!(Fqdn::from("ExAmPle.Com"), Fqdn::from("example.com")); - assert_ne!( - Fqdn::from("subdomain.ExAmPle.Com"), - Fqdn::from("example.com") - ); - assert_ne!(Fqdn::from("foo.com"), Fqdn::from("bar.com")); - assert_ne!(Fqdn::from("example.com"), Fqdn::from("example.net")); - } -} diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml index cc2baeb..1ca91cb 100644 --- a/proxmox-tui-installer/Cargo.toml +++ b/proxmox-tui-installer/Cargo.toml @@ -9,6 +9,7 @@ homepage = "https://www.proxmox.com" [dependencies] proxmox-installer-common.workspace = true +proxmox-network-types.workspace = true anyhow.workspace = true serde_json.workspace = true diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs index 3ab1869..98dbcac 100644 --- a/proxmox-tui-installer/src/setup.rs +++ b/proxmox-tui-installer/src/setup.rs @@ -36,10 +36,7 @@ impl From for InstallConfig { mngmt_nic: options.network.ifname, network_interface_pin_map: pinning_opts.map(|o| o.mapping.clone()).unwrap_or_default(), - // Safety: At this point, it is know that we have a valid FQDN, as - // this is set by the TUI network panel, which only lets the user - // continue if a valid FQDN is provided. - hostname: options.network.fqdn.host().expect("valid FQDN").to_owned(), + hostname: options.network.fqdn.host().to_owned(), domain: options.network.fqdn.domain(), cidr: options.network.address, gateway: options.network.gateway, diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs index 970c353..53e0d65 100644 --- a/proxmox-tui-installer/src/views/network.rs +++ b/proxmox-tui-installer/src/views/network.rs @@ -12,13 +12,15 @@ use std::{ sync::{Arc, Mutex}, }; -use super::{CidrAddressEditView, FormView}; use proxmox_installer_common::{ net::MAX_IFNAME_LEN, options::{NetworkInterfacePinningOptions, NetworkOptions}, setup::{Interface, NetworkInfo}, - utils::{CidrAddress, Fqdn}, + utils::CidrAddress, }; +use proxmox_network_types::fqdn::Fqdn; + +use super::{CidrAddressEditView, FormView}; struct NetworkViewOptions { selected_mac: String, -- 2.53.0