From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id A2E811FF2CF for ; Thu, 11 Jul 2024 17:54:56 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6F25A36F3B; Thu, 11 Jul 2024 17:55:19 +0200 (CEST) Message-ID: <6be4ffe9-f3f9-4379-9393-d4f67361be47@proxmox.com> Date: Thu, 11 Jul 2024 17:54:42 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Christoph Heiss References: <20240710132756.1149508-1-c.heiss@proxmox.com> <20240710132756.1149508-13-c.heiss@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20240710132756.1149508-13-c.heiss@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.638 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: Re: [pve-devel] [PATCH installer 12/14] fix #5536: add post-hook utility for sending notifications after auto-install 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: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On 7/10/24 15:27, Christoph Heiss wrote: > +impl Answer { > + pub fn from_reader(reader: impl BufRead) -> Result { > + let mut buffer = String::new(); > + let lines = reader.lines(); > + for line in lines { > + buffer.push_str(&line.unwrap()); > + buffer.push('\n'); > + } > + > + toml::from_str(&buffer).map_err(|err| format_err!("Failed parsing answer file: {err}")) > + } > +} maybe better impl TryFrom for Answer ? Or at least call the method try_from_reader if it returns a result? > #[derive(Clone, Deserialize, Debug)] > #[serde(deny_unknown_fields)] > pub struct Global { > diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs > index bf6f8fb..aab0f1f 100644 > --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs > +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs > @@ -42,16 +42,7 @@ fn auto_installer_setup(in_test_mode: bool) -> Result<(Answer, UdevInfo)> { > .map_err(|err| format_err!("Failed to retrieve udev info details: {err}"))? > }; > > - let mut buffer = String::new(); > - let lines = std::io::stdin().lock().lines(); > - for line in lines { > - buffer.push_str(&line.unwrap()); > - buffer.push('\n'); > - } > - > - let answer: Answer = > - toml::from_str(&buffer).map_err(|err| format_err!("Failed parsing answer file: {err}"))?; > - > + let answer = Answer::from_reader(std::io::stdin().lock())?; > Ok((answer, udev_info)) > } > > @@ -91,8 +82,6 @@ fn main() -> ExitCode { > } > } > > - // TODO: (optionally) do a HTTP post with basic system info, like host SSH public key(s) here > - > ExitCode::SUCCESS > } > > diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs > index a6a8de0..4317430 100644 > --- a/proxmox-fetch-answer/src/fetch_plugins/http.rs > +++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs > @@ -68,7 +68,7 @@ impl FetchFromHTTP { > let payload = SysInfo::as_json()?; > info!("Sending POST request to '{answer_url}'."); > let answer = > - proxmox_installer_common::http::post(answer_url, fingerprint.as_deref(), payload)?; > + proxmox_installer_common::http::post(&answer_url, fingerprint.as_deref(), payload)?; > Ok(answer) > } > > diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs > index 4472922..f07389b 100644 > --- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs > +++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs > @@ -31,7 +31,7 @@ impl FetchFromPartition { > } > > fn path_exists_logged(file_name: &str, search_path: &str) -> Option { > - let path = Path::new(search_path).join(&file_name); > + let path = Path::new(search_path).join(file_name); > info!("Testing partition search path {path:?}"); > match path.try_exists() { > Ok(true) => Some(path), > diff --git a/proxmox-installer-common/src/http.rs b/proxmox-installer-common/src/http.rs > index 4a5d444..b754ed8 100644 > --- a/proxmox-installer-common/src/http.rs > +++ b/proxmox-installer-common/src/http.rs > @@ -15,7 +15,7 @@ use ureq::{Agent, AgentBuilder}; > /// * `url` - URL to call > /// * `fingerprint` - SHA256 cert fingerprint if certificate pinning should be used. Optional. > /// * `payload` - The payload to send to the server. Expected to be a JSON formatted string. > -pub fn post(url: String, fingerprint: Option<&str>, payload: String) -> Result { > +pub fn post(url: &str, fingerprint: Option<&str>, payload: String) -> Result { > let answer; > > if let Some(fingerprint) = fingerprint { > @@ -27,7 +27,7 @@ pub fn post(url: String, fingerprint: Option<&str>, payload: String) -> Result let agent: Agent = AgentBuilder::new().tls_config(Arc::new(tls_config)).build(); > > answer = agent > - .post(&url) > + .post(url) > .set("Content-Type", "application/json; charset=utf-8") > .send_string(&payload)? > .into_string()?; > @@ -47,7 +47,7 @@ pub fn post(url: String, fingerprint: Option<&str>, payload: String) -> Result .tls_config(Arc::new(tls_config)) > .build(); > answer = agent > - .post(&url) > + .post(url) > .set("Content-Type", "application/json; charset=utf-8") > .timeout(std::time::Duration::from_secs(60)) > .send_string(&payload)? > diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs > index 9f6131b..b209587 100644 > --- a/proxmox-installer-common/src/options.rs > +++ b/proxmox-installer-common/src/options.rs > @@ -45,6 +45,10 @@ impl FsType { > pub fn is_btrfs(&self) -> bool { > matches!(self, FsType::Btrfs(_)) > } > + > + pub fn is_lvm(&self) -> bool { > + matches!(self, FsType::Ext4 | FsType::Xfs) > + } > } > > impl fmt::Display for FsType { > diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs > index 29137bf..479a3b5 100644 > --- a/proxmox-installer-common/src/setup.rs > +++ b/proxmox-installer-common/src/setup.rs > @@ -335,7 +335,7 @@ pub struct RuntimeInfo { > pub hvm_supported: bool, > } > > -#[derive(Copy, Clone, Eq, Deserialize, PartialEq)] > +#[derive(Copy, Clone, Eq, Deserialize, PartialEq, Serialize)] > #[serde(rename_all = "lowercase")] > pub enum BootType { > Bios, > @@ -383,7 +383,7 @@ pub struct Gateway { > pub gateway: IpAddr, > } > > -#[derive(Clone, Deserialize)] > +#[derive(Clone, Deserialize, Serialize)] > #[serde(rename_all = "UPPERCASE")] > pub enum InterfaceState { > Up, > @@ -403,7 +403,7 @@ impl InterfaceState { > } > } > > -#[derive(Clone, Deserialize)] > +#[derive(Clone, Deserialize, Serialize)] > pub struct Interface { > pub name: String, > > diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs > index 57b1753..2579c80 100644 > --- a/proxmox-installer-common/src/utils.rs > +++ b/proxmox-installer-common/src/utils.rs > @@ -114,6 +114,8 @@ impl<'de> Deserialize<'de> for CidrAddress { > } > } > > +serde_plain::derive_serialize_from_display!(CidrAddress); > + > fn mask_limit(addr: &IpAddr) -> usize { > if addr.is_ipv4() { > 32 > diff --git a/proxmox-post-hook/Cargo.toml b/proxmox-post-hook/Cargo.toml > new file mode 100644 > index 0000000..ee4f679 > --- /dev/null > +++ b/proxmox-post-hook/Cargo.toml > @@ -0,0 +1,19 @@ > +[package] > +name = "proxmox-post-hook" > +version = "0.1.0" > +edition = "2021" > +authors = [ > + "Christoph Heiss ", > + "Proxmox Support Team ", > +] > +license = "AGPL-3" > +exclude = [ "build", "debian" ] > +homepage = "https://www.proxmox.com" > + > +[dependencies] > +anyhow.workspace = true > +proxmox-auto-installer.workspace = true > +proxmox-installer-common = { workspace = true, features = ["http"] } > +serde.workspace = true > +serde_json.workspace = true > +toml.workspace = true > diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs > new file mode 100644 > index 0000000..9e5680b > --- /dev/null > +++ b/proxmox-post-hook/src/main.rs > @@ -0,0 +1,372 @@ > +//! Post installation hook for the Proxmox installer, mainly for combination > +//! with the auto-installer. > +//! > +//! If a `[posthook]` section is specified in the given answer file, it will > +//! send a HTTP POST request to that URL, with an optional certificate fingerprint > +//! for usage with (self-signed) TLS certificates. > +//! In the body of the request, information about the newly installed system is sent. > +//! > +//! Relies on `proxmox-chroot` as an external dependency to (bind-)mount the > +//! previously installed system. > + > +use std::{ > + collections::BTreeMap, > + fs::{self, File}, > + io::BufReader, > + path::PathBuf, > + process::{Command, ExitCode}, > +}; > + > +use anyhow::{anyhow, bail, Context, Result}; > +use proxmox_auto_installer::{ > + answer::{Answer, PostNotificationHookInfo}, > + udevinfo::UdevInfo, > +}; > +use proxmox_installer_common::{ > + options::{Disk, FsType}, > + setup::{ > + load_installer_setup_files, BootType, InstallConfig, ProxmoxProduct, RuntimeInfo, SetupInfo, > + }, > + utils::CidrAddress, > +}; > +use serde::{Deserialize, Serialize}; > + > +/// Holds all the public keys for the different algorithms available. > +#[derive(Serialize)] > +struct SshPublicHostKeys { > + // ECDSA-based public host key > + ecdsa: String, > + // ED25519-based public host key > + ed25519: String, > + // RSA-based public host key > + rsa: String, > +} > + > +/// A single disk configured as boot disk. > +#[derive(Clone, Serialize)] > +#[serde(rename_all = "kebab-case")] > +struct BootDiskInfo { > + /// Size in bytes > + size: usize, > + /// Properties as given by udev > + udev_properties: BTreeMap, > +} > + > +/// Holds information about the management network interface. > +#[derive(Serialize)] > +#[serde(rename_all = "kebab-case")] > +struct NetworkInterfaceInfo { > + /// MAC address of the interface > + mac: String, > + /// (Designated) IP address of the interface > + address: CidrAddress, > + /// Properties as given by udev > + udev_properties: BTreeMap, > +} > + > +/// All data sent as request payload with the post-hook POST request. > +#[derive(Serialize)] > +#[serde(rename_all = "kebab-case")] > +struct PostHookInfo { > + /// major.minor version of Debian as installed, retrieved from /etc/debian_version > + debian_version: String, > + /// PVE/PMG/PBS version as reported by `pveversion`, `pmgversion` or > + /// `proxmox-backup-manager version`, respectively. > + product_version: String, > + /// Installed kernel version > + kernel_version: String, > + /// Either `bios` or `efi` > + boot_type: BootType, > + /// Filesystem used for boot disk(s) > + filesystem: FsType, > + /// Fully qualified domain name of the installed system > + fqdn: String, > + /// Unique systemd-id128 identifier of the installed system (128-bit, 16 bytes) > + machine_id: String, > + /// Boot disks selected during installation as used for installation of the system > + bootdisk: Vec, > + /// Primary management network interface chosen during installation > + management_nic: NetworkInterfaceInfo, > + /// Public parts of SSH host keys of the installed system > + ssh_public_host_keys: SshPublicHostKeys, > +} > + > +/// Used for deserializing the output of `proxmox-backup-manager version --output-format json` > +/// Only contains the properties we are interested in for simplicity sake. > +#[derive(Deserialize)] > +#[serde(rename_all(deserialize = "PascalCase"))] > +struct PbsVersionInfo { > + /// Actual package name as reported by `proxmox-backup-manager` > + package: String, > + /// Installed package version as reported by `proxmox-backup-manager` > + version: String, > +} > + > +/// Defines the size of a gibibyte in bytes. > +const SIZE_GIB: usize = 1024 * 1024 * 1024; > + > +impl PostHookInfo { > + /// Gathers all needed information about the newly installed system for (optionally) sending > + /// it to a specified server. > + /// > + /// # Arguments > + /// > + /// * `answer` - Answer file as provided by the user > + fn gather(answer: &Answer) -> Result { > + println!("Gathering installed system data .."); > + > + let config: InstallConfig = > + serde_json::from_reader(BufReader::new(File::open("/tmp/low-level-config.json")?))?; > + > + let (setup_info, _, run_env) = > + load_installer_setup_files(&PathBuf::from(proxmox_installer_common::RUNTIME_DIR)) When converting the param to AsRef you should be able to just use RUNTIME_DIR here then. > + .map_err(|err| anyhow!(err))?; Maybe use .context() here? > + > + let udev: UdevInfo = { > + let path = > + PathBuf::from(proxmox_installer_common::RUNTIME_DIR).join("run-env-udev.json"); > + serde_json::from_reader(BufReader::new(File::open(path)?))? > + }; > + > + with_chroot(|target_path| { > + // Reads a file, specified by an absolute path _inside_ the chroot > + // from the target. > + let read_file = |path: &str| { > + fs::read_to_string(format!("{}/{}", target_path, path)) > + .map(|s| s.trim().to_owned()) > + .with_context(|| path.to_owned()) > + }; > + > + // Runs a command inside the target chroot. > + let run_cmd = |cmd: &[&str]| { > + Command::new("chroot") > + .arg(target_path) > + .args(cmd) > + .output() > + .with_context(|| format!("failed to run '{cmd:?}'")) > + .and_then(|r| Ok(String::from_utf8(r.stdout)?)) > + }; > + > + let arch = run_cmd(&["dpkg", "--print-architecture"])?; > + > + let kernel_version = run_cmd(&[ > + "dpkg-query", > + "--showformat", > + "${db:Status-Abbrev}|${Architecture}|${Package}\\n", > + "--show", > + "proxmox-kernel-[0-9]*", > + ]) > + .and_then(|s| Self::parse_dpkg_query_kernel_output(&s, arch.trim()))?; > + > + Ok(Self { > + debian_version: read_file("/etc/debian_version")?, > + product_version: Self::gather_product_version(&setup_info, &run_cmd)?, > + kernel_version, > + boot_type: run_env.boot_type, > + filesystem: answer.disks.fs_type, > + fqdn: answer.global.fqdn.to_string(), > + machine_id: read_file("/etc/machine-id")?, > + bootdisk: Self::gather_disks(&config, &run_env, &udev)?, > + management_nic: Self::gather_nic(&config, &run_env, &udev)?, > + ssh_public_host_keys: SshPublicHostKeys { > + ecdsa: read_file("/etc/ssh/ssh_host_ecdsa_key.pub")?, > + ed25519: read_file("/etc/ssh/ssh_host_ed25519_key.pub")?, > + rsa: read_file("/etc/ssh/ssh_host_rsa_key.pub")?, > + }, > + }) > + }) > + } > + > + /// Retrieves all needed information about the boot disks that were selected during > + /// installation, most notable the udev properties. > + /// > + /// # Arguments > + /// > + /// * `config` - Low-level installation configuration > + /// * `run_env` - Runtime envirornment information gathered by the installer at the start > + /// * `udev` - udev information for all system devices > + fn gather_disks( > + config: &InstallConfig, > + run_env: &RuntimeInfo, > + udev: &UdevInfo, > + ) -> Result> { > + if config.filesys.is_lvm() { > + Ok(udev > + .disks > + .values() > + .find(|props| props.get("DEVNAME") == config.target_hd.as_ref()) > + .map(|props| BootDiskInfo { > + size: (config.hdsize * (SIZE_GIB as f64)) as usize, > + udev_properties: props.clone(), > + }) > + .as_slice() > + .to_vec()) Can use .into_iter().collect() for no clones > + } else { > + Ok(config > + .disk_selection > + .values() > + .filter_map(|index| Some(run_env.disks[index.parse::().ok()?].to_owned())) > + .filter_map(|disk: Disk| { > + let props = udev > + .disks > + .values() > + .find(|props| props.get("DEVNAME") == Some(&disk.path))?; > + > + Some(BootDiskInfo { > + size: (config.hdsize * (SIZE_GIB as f64)) as usize, > + udev_properties: props.clone(), > + }) > + }) > + .collect()) > + } > + } Maybe we should not silently fail here? (because of filter_map). I got the feeling using the fact that an Iterator> can be collected into a Result,..> could be used to streamline this code here. > + /// Retrieves all needed information about the management network interface that was selected > + /// during installation, most notable the udev properties. > + /// > + /// # Arguments > + /// > + /// * `config` - Low-level installation configuration > + /// * `run_env` - Runtime envirornment information gathered by the installer at the start > + /// * `udev` - udev information for all system devices > + fn gather_nic( > + config: &InstallConfig, > + run_env: &RuntimeInfo, > + udev: &UdevInfo, > + ) -> Result { > + let nic = run_env > + .network > + .interfaces > + .get(&config.mngmt_nic) > + .ok_or_else(|| anyhow!("could not find network interface '{}'", config.mngmt_nic))?; > + > + // Use the actual IP address from the low-level install config, as the runtime info > + // contains the original IP address from DHCP. > + let address = config.cidr.clone(); > + > + let props = udev > + .nics > + .values() > + .find(|props| props.get("INTERFACE") == Some(&nic.name)) > + .ok_or_else(|| anyhow!("could not find udev information for NIC '{}'", nic.name))?; I see this a lot for network interfaces / disks in this code - maybe something that could be provided by the struct? That way magical constants such as "INTERFACE" or "DEVNAME" don't have to be used throughout the code. > + Ok(NetworkInterfaceInfo { > + mac: nic.mac.clone(), > + address, > + udev_properties: props.clone(), > + }) > + } > + > + /// Retrieves the version of the installed product from the chroot. > + /// > + /// # Arguments > + /// > + /// * `setup_info` - Filled-out struct with information about the product > + /// * `run_cmd` - Callback to run a command inside the target chroot. > + fn gather_product_version( > + setup_info: &SetupInfo, > + run_cmd: &dyn Fn(&[&str]) -> Result, > + ) -> Result { > + match setup_info.config.product { > + ProxmoxProduct::PVE => run_cmd(&["pveversion"])? > + .split_once(' ') > + .map(|(ver, _)| ver.to_owned()) > + .ok_or(anyhow!("failed to parse `pveversion` output")), > + ProxmoxProduct::PMG => run_cmd(&["pmgversion"])? > + .split_once(' ') > + .map(|(ver, _)| ver.to_owned()) > + .ok_or(anyhow!("failed to parse `pveversion` output")), > + ProxmoxProduct::PBS => { > + let info: Vec = serde_json::from_str(&run_cmd(&[ > + "proxmox-backup-manager", > + "version", > + "--output-format", > + "json", > + ])?) > + .context("failed to parse json output from 'proxmox-backup-manager'")?; > + > + if info.is_empty() { > + bail!("got empty version information"); > + } > + > + Ok(format!("{}/{}", info[0].package, info[0].version)) > + } > + } > + } > + > + /// Tries to parses `dpkg-query` output generated from the following command: > + /// dpkg-query --showformat '${db:Status-Abbrev}|${Architecture}|${Package}\n' --show 'proxmox-kernel-[0-9]*' > + /// and report the version of the actual kernel package. > + /// > + /// The output to parse looks like this: > + /// ii |all|proxmox-kernel-6.8 > + /// un ||proxmox-kernel-6.8.8-2-pve > + /// ii |amd64|proxmox-kernel-6.8.8-2-pve-signed > + fn parse_dpkg_query_kernel_output(output: &str, dpkg_arch: &str) -> Result { > + for pkg in output.lines() { > + let parts = pkg.split('|').collect::>(); > + > + if let [status, arch, name] = parts[..] { > + if status.trim() == "ii" && arch.trim() == dpkg_arch { > + return Ok(name.trim().to_owned()); > + } > + } > + } > + > + bail!("failed to find kernel package") > + } > +} > + > +/// Runs the specified callback with the mounted chroot, passing along the > +/// absolute path to where / is mounted. > +/// The callback is *not* run inside the chroot itself, that is left to the caller. > +fn with_chroot Result>(callback: F) -> Result { > + let ec = Command::new("proxmox-chroot") > + .arg("prepare") > + .status() > + .context("failed to run proxmox-chroot")?; > + > + if !ec.success() { > + bail!("failed to create chroot for installed system"); > + } > + > + // See also proxmox-chroot/src/main.rs > + let result = callback("/target"); > + > + let ec = Command::new("proxmox-chroot").arg("cleanup").status(); > + if ec.is_err() || !ec.map(|ec| ec.success()).unwrap_or(false) { > + eprintln!("failed to clean up chroot for installed system"); > + } > + > + result > +} > + > +fn do_main() -> Result<()> { > + let answer = Answer::from_reader(std::io::stdin().lock())?; > + > + if let Some(PostNotificationHookInfo { > + url, > + cert_fingerprint, > + }) = &answer.posthook > + { > + println!("Found posthook; sending POST request to '{url}'."); > + > + let info = serde_json::to_string(&PostHookInfo::gather(&answer)?)?; > + proxmox_installer_common::http::post(url, cert_fingerprint.as_deref(), info)?; > + } else { > + println!("No posthook found; skipping"); > + } > + > + Ok(()) > +} > + > +fn main() -> ExitCode { > + match do_main() { > + Ok(()) => ExitCode::SUCCESS, > + Err(err) => { > + eprintln!("\nError occurred during posthook:"); > + eprintln!("{err:#}"); > + ExitCode::FAILURE > + } > + } > +} _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel