From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id D3F4D9D062 for ; Wed, 25 Oct 2023 10:57:13 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B6BA311F97 for ; Wed, 25 Oct 2023 10:56:43 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 25 Oct 2023 10:56:42 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id A17AB450E0 for ; Wed, 25 Oct 2023 10:56:42 +0200 (CEST) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Wed, 25 Oct 2023 10:56:22 +0200 Message-ID: <20231025085634.171618-2-c.heiss@proxmox.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231025085634.171618-1-c.heiss@proxmox.com> References: <20231025085634.171618-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.021 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 Subject: [pve-devel] [PATCH installer v2 1/3] tui: refactor `NetworkOptions` to have a `defaults_from()` function X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Oct 2023 08:57:13 -0000 This aligns it with the other constructors for options struct by introducing a `::defaults_from()` function. Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/main.rs | 15 +++--- proxmox-tui-installer/src/options.rs | 75 +++++++++++++--------------- proxmox-tui-installer/src/setup.rs | 10 ++-- 3 files changed, 45 insertions(+), 55 deletions(-) diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index 0a61e39..a342a08 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -166,7 +166,6 @@ enum InstallerStep { #[derive(Clone)] struct InstallerState { options: InstallerOptions, - /// FIXME: Remove: setup_info: SetupInfo, runtime_info: RuntimeInfo, locales: LocaleInfo, @@ -184,7 +183,7 @@ fn main() { _ => cfg!(debug_assertions), }; - let (locales, runtime_info) = match installer_setup(in_test_mode) { + let (setup_info, locales, runtime_info) = match installer_setup(in_test_mode) { Ok(result) => result, Err(err) => initial_setup_error(&mut siv, &err), }; @@ -197,10 +196,10 @@ fn main() { bootdisk: BootdiskOptions::defaults_from(&runtime_info.disks[0]), timezone: TimezoneOptions::defaults_from(&runtime_info, &locales), password: Default::default(), - network: NetworkOptions::from(&runtime_info.network), + network: NetworkOptions::defaults_from(&setup_info, &runtime_info.network), autoreboot: false, }, - setup_info: setup_info().clone(), // FIXME: REMOVE + setup_info, runtime_info, locales, steps: HashMap::new(), @@ -211,20 +210,20 @@ fn main() { siv.run(); } -fn installer_setup(in_test_mode: bool) -> Result<(LocaleInfo, RuntimeInfo), String> { +fn installer_setup(in_test_mode: bool) -> Result<(SetupInfo, LocaleInfo, RuntimeInfo), String> { let base_path = if in_test_mode { "./testdir" } else { "/" }; let mut path = PathBuf::from(base_path); path.push("run"); path.push("proxmox-installer"); - let installer_info = { + let installer_info: SetupInfo = { let mut path = path.clone(); path.push("iso-info.json"); setup::read_json(&path).map_err(|err| format!("Failed to retrieve setup info: {err}"))? }; - init_setup_info(installer_info); + init_setup_info(installer_info.clone()); let locale_info = { let mut path = path.clone(); @@ -245,7 +244,7 @@ fn installer_setup(in_test_mode: bool) -> Result<(LocaleInfo, RuntimeInfo), Stri if runtime_info.disks.is_empty() { Err("The installer could not find any supported hard disks.".to_owned()) } else { - Ok((locale_info, runtime_info)) + Ok((installer_info, locale_info, runtime_info)) } } diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs index abc8c7b..85b39b8 100644 --- a/proxmox-tui-installer/src/options.rs +++ b/proxmox-tui-installer/src/options.rs @@ -1,7 +1,7 @@ use std::net::{IpAddr, Ipv4Addr}; use std::{cmp, fmt}; -use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo}; +use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo}; use crate::utils::{CidrAddress, Fqdn}; use crate::SummaryOption; @@ -339,49 +339,25 @@ pub struct NetworkOptions { impl NetworkOptions { const DEFAULT_DOMAIN: &str = "example.invalid"; -} -impl Default for NetworkOptions { - fn default() -> Self { - let fqdn = format!( - "{}.{}", - crate::current_product().default_hostname(), - Self::DEFAULT_DOMAIN - ); - // TODO: Retrieve automatically - Self { + pub fn defaults_from(setup: &SetupInfo, network: &NetworkInfo) -> Self { + let mut this = Self { ifname: String::new(), - fqdn: fqdn.parse().unwrap(), + fqdn: Self::construct_fqdn(network, setup.config.product.default_hostname()), // Safety: The provided mask will always be valid. address: CidrAddress::new(Ipv4Addr::UNSPECIFIED, 0).unwrap(), gateway: Ipv4Addr::UNSPECIFIED.into(), dns_server: Ipv4Addr::UNSPECIFIED.into(), - } - } -} - -impl From<&NetworkInfo> for NetworkOptions { - fn from(info: &NetworkInfo) -> Self { - let mut this = Self::default(); + }; - if let Some(ip) = info.dns.dns.first() { + if let Some(ip) = network.dns.dns.first() { this.dns_server = *ip; } - let hostname = info - .hostname - .as_deref() - .unwrap_or_else(|| crate::current_product().default_hostname()); - let domain = info.dns.domain.as_deref().unwrap_or(Self::DEFAULT_DOMAIN); - - if let Ok(fqdn) = Fqdn::from(&format!("{hostname}.{domain}")) { - this.fqdn = fqdn; - } - - if let Some(routes) = &info.routes { + if let Some(routes) = &network.routes { let mut filled = false; if let Some(gw) = &routes.gateway4 { - if let Some(iface) = info.interfaces.get(&gw.dev) { + if let Some(iface) = network.interfaces.get(&gw.dev) { this.ifname = iface.name.clone(); if let Some(addresses) = &iface.addresses { if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) { @@ -394,7 +370,7 @@ impl From<&NetworkInfo> for NetworkOptions { } if !filled { if let Some(gw) = &routes.gateway6 { - if let Some(iface) = info.interfaces.get(&gw.dev) { + if let Some(iface) = network.interfaces.get(&gw.dev) { if let Some(addresses) = &iface.addresses { if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) { this.ifname = iface.name.clone(); @@ -409,6 +385,23 @@ impl From<&NetworkInfo> for NetworkOptions { this } + + fn construct_fqdn(network: &NetworkInfo, default_hostname: &str) -> Fqdn { + let hostname = network.hostname.as_deref().unwrap_or(default_hostname); + + let domain = network + .dns + .domain + .as_deref() + .unwrap_or(Self::DEFAULT_DOMAIN); + + Fqdn::from(&format!("{hostname}.{domain}")).unwrap_or_else(|_| { + // Safety: This will always result in a valid FQDN, as we control & know + // the values of default_hostname (one of "pve", "pmg" or "pbs") and + // constant-defined DEFAULT_DOMAIN. + Fqdn::from(&format!("{}.{}", default_hostname, Self::DEFAULT_DOMAIN)).unwrap() + }) + } } #[derive(Clone, Debug)] @@ -460,8 +453,8 @@ mod tests { }; use std::{collections::HashMap, path::PathBuf}; - fn fill_setup_info() { - crate::init_setup_info(SetupInfo { + fn dummy_setup_info() -> SetupInfo { + SetupInfo { config: ProductConfig { fullname: "Proxmox VE".to_owned(), product: ProxmoxProduct::PVE, @@ -474,12 +467,12 @@ mod tests { locations: IsoLocations { iso: PathBuf::new(), }, - }); + } } #[test] fn network_options_from_setup_network_info() { - fill_setup_info(); + let setup = dummy_setup_info(); let mut interfaces = HashMap::new(); interfaces.insert( @@ -512,7 +505,7 @@ mod tests { }; assert_eq!( - NetworkOptions::from(&info), + NetworkOptions::defaults_from(&setup, &info), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("foo.bar.com").unwrap(), @@ -524,7 +517,7 @@ mod tests { info.hostname = None; assert_eq!( - NetworkOptions::from(&info), + NetworkOptions::defaults_from(&setup, &info), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("pve.bar.com").unwrap(), @@ -536,7 +529,7 @@ mod tests { info.dns.domain = None; assert_eq!( - NetworkOptions::from(&info), + NetworkOptions::defaults_from(&setup, &info), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("pve.example.invalid").unwrap(), @@ -548,7 +541,7 @@ mod tests { info.hostname = Some("foo".to_owned()); assert_eq!( - NetworkOptions::from(&info), + NetworkOptions::defaults_from(&setup, &info), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("foo.example.invalid").unwrap(), diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs index 7238131..5575759 100644 --- a/proxmox-tui-installer/src/setup.rs +++ b/proxmox-tui-installer/src/setup.rs @@ -196,12 +196,10 @@ impl From for InstallConfig { mngmt_nic: options.network.ifname, - hostname: options - .network - .fqdn - .host() - .unwrap_or_else(|| crate::current_product().default_hostname()) - .to_owned(), + // 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(), domain: options.network.fqdn.domain(), cidr: options.network.address, gateway: options.network.gateway, -- 2.42.0