* [pve-devel] [PATCH installer 01/14] chroot: print full anyhow message
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 02/14] tree-wide: fix some typos Christoph Heiss
` (14 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
This forces anyhow to give more context to the stringified error, which
helps tremendously when trying to make sense of these messages.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-chroot/src/main.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/proxmox-chroot/src/main.rs b/proxmox-chroot/src/main.rs
index ca6f3a9..519e3f3 100644
--- a/proxmox-chroot/src/main.rs
+++ b/proxmox-chroot/src/main.rs
@@ -79,7 +79,7 @@ fn main() {
Commands::Cleanup(args) => cleanup(args),
};
if let Err(err) = res {
- eprintln!("{err}");
+ eprintln!("{err:#}");
std::process::exit(1);
}
}
@@ -108,7 +108,7 @@ fn cleanup(args: &CommandCleanup) -> Result<()> {
let fs = get_fs(args.filesystem)?;
if let Err(e) = bind_umount() {
- eprintln!("{e}")
+ eprintln!("{e:#}")
}
match fs {
@@ -188,7 +188,7 @@ fn mount_fs() -> Result<()> {
.arg(product.to_string())
.output();
match res {
- Err(e) => bail!("{e}"),
+ Err(e) => bail!("{e:#}"),
Ok(output) => {
if output.status.success() {
println!(
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 02/14] tree-wide: fix some typos
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 01/14] chroot: print full anyhow message Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 03/14] tree-wide: collect hardcoded installer runtime directory strings into constant Christoph Heiss
` (13 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
.../src/bin/proxmox-auto-installer.rs | 2 +-
proxmox-chroot/src/main.rs | 2 +-
proxmox-fetch-answer/src/fetch_plugins/partition.rs | 12 ++++++------
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 9fcec1e..bf6f8fb 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -57,7 +57,7 @@ fn auto_installer_setup(in_test_mode: bool) -> Result<(Answer, UdevInfo)> {
fn main() -> ExitCode {
if let Err(err) = init_log() {
- panic!("could not initilize logging: {}", err);
+ panic!("could not initialize logging: {}", err);
}
let in_test_mode = match env::args().nth(1).as_deref() {
diff --git a/proxmox-chroot/src/main.rs b/proxmox-chroot/src/main.rs
index 519e3f3..7885ce7 100644
--- a/proxmox-chroot/src/main.rs
+++ b/proxmox-chroot/src/main.rs
@@ -14,7 +14,7 @@ static BINDMOUNTS: [&str; 4] = ["dev", "proc", "run", "sys"];
const TARGET_DIR: &str = "/target";
const ZPOOL_NAME: &str = "rpool";
-/// Helper tool to prepare eveything to `chroot` into an installation
+/// Helper tool to prepare everything to `chroot` into an installation
#[derive(Parser, Debug)]
#[command(author, version, about, long_about = None)]
struct Cli {
diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
index 0479c8f..4472922 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -45,20 +45,20 @@ fn path_exists_logged(file_name: &str, search_path: &str) -> Option<PathBuf> {
/// Searches for upper and lower case existence of the partlabel in the search_path
///
-/// # Arguemnts
+/// # Arguments
/// * `partlabel_source` - Partition Label, used as upper and lower case
-/// * `search_path` - Path where to search for the partiiton label
+/// * `search_path` - Path where to search for the partition label
fn scan_partlabels(partlabel: &str, search_path: &str) -> Result<PathBuf> {
let partlabel_upper_case = partlabel.to_uppercase();
if let Some(path) = path_exists_logged(&partlabel_upper_case, search_path) {
- info!("Found partition with label '{partlabel_upper_case}'");
- return Ok(path);
+ info!("Found partition with label '{partlabel_upper_case}'");
+ return Ok(path);
}
let partlabel_lower_case = partlabel.to_lowercase();
if let Some(path) = path_exists_logged(&partlabel_lower_case, search_path) {
- info!("Found partition with label '{partlabel_lower_case}'");
- return Ok(path);
+ info!("Found partition with label '{partlabel_lower_case}'");
+ return Ok(path);
}
bail!("Could not detect upper or lower case labels for '{partlabel}'");
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 03/14] tree-wide: collect hardcoded installer runtime directory strings into constant
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 01/14] chroot: print full anyhow message Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 02/14] tree-wide: fix some typos Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 04/14] common: simplify filesystem type serializing & Display trait impl Christoph Heiss
` (12 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-auto-installer/src/sysinfo.rs | 10 +++++++---
proxmox-chroot/src/main.rs | 11 ++++++++---
proxmox-installer-common/src/lib.rs | 2 ++
proxmox-installer-common/src/setup.rs | 11 ++++++-----
4 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/proxmox-auto-installer/src/sysinfo.rs b/proxmox-auto-installer/src/sysinfo.rs
index 05f7f50..112e898 100644
--- a/proxmox-auto-installer/src/sysinfo.rs
+++ b/proxmox-auto-installer/src/sysinfo.rs
@@ -1,7 +1,10 @@
use anyhow::{bail, Result};
-use proxmox_installer_common::setup::{IsoInfo, ProductConfig, SetupInfo};
+use proxmox_installer_common::{
+ setup::{IsoInfo, ProductConfig, SetupInfo},
+ RUNTIME_DIR,
+};
use serde::Serialize;
-use std::{collections::HashMap, fs, io};
+use std::{collections::HashMap, fs, io, path::PathBuf};
use crate::utils::get_nic_list;
@@ -17,7 +20,8 @@ pub struct SysInfo {
impl SysInfo {
pub fn get() -> Result<Self> {
- let setup_info: SetupInfo = match fs::File::open("/run/proxmox-installer/iso-info.json") {
+ let path = PathBuf::from(RUNTIME_DIR).join("iso-info.json").to_owned();
+ let setup_info: SetupInfo = match fs::File::open(path) {
Ok(iso_info_file) => {
let reader = io::BufReader::new(iso_info_file);
serde_json::from_reader(reader)?
diff --git a/proxmox-chroot/src/main.rs b/proxmox-chroot/src/main.rs
index 7885ce7..594f9ab 100644
--- a/proxmox-chroot/src/main.rs
+++ b/proxmox-chroot/src/main.rs
@@ -1,4 +1,8 @@
-use std::{fs, io, path, process::Command};
+use std::{
+ fs, io,
+ path::{self, PathBuf},
+ process::Command,
+};
use anyhow::{bail, Result};
use clap::{Args, Parser, Subcommand, ValueEnum};
@@ -6,6 +10,7 @@ use nix::mount::{mount, umount, MsFlags};
use proxmox_installer_common::{
options::FsType,
setup::{InstallConfig, SetupInfo},
+ RUNTIME_DIR,
};
use regex::Regex;
@@ -145,8 +150,8 @@ fn get_low_level_config() -> Result<InstallConfig> {
}
fn get_iso_info() -> Result<SetupInfo> {
- let file = fs::File::open("/run/proxmox-installer/iso-info.json")?;
- let reader = io::BufReader::new(file);
+ let path = PathBuf::from(RUNTIME_DIR).join("iso-info.json");
+ let reader = io::BufReader::new(fs::File::open(path)?);
let setup_info: SetupInfo = serde_json::from_reader(reader)?;
Ok(setup_info)
}
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index f0093f5..850e825 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -2,3 +2,5 @@ pub mod disk_checks;
pub mod options;
pub mod setup;
pub mod utils;
+
+pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 64d05af..9aa4063 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -161,11 +161,12 @@ pub struct LocaleInfo {
/// Fetches basic information needed for the installer which is required to work
pub 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 base_path = if in_test_mode {
+ format!("./testdir/{}", crate::RUNTIME_DIR)
+ } else {
+ crate::RUNTIME_DIR.to_owned()
+ };
+ let path = PathBuf::from(base_path);
let installer_info: SetupInfo = {
let mut path = path.clone();
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 04/14] common: simplify filesystem type serializing & Display trait impl
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (2 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 03/14] tree-wide: collect hardcoded installer runtime directory strings into constant Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-11 14:32 ` Stefan Hanreich
2024-07-10 13:27 ` [pve-devel] [PATCH installer 05/14] common: setup: serialize `target_hd` as string explicitly Christoph Heiss
` (11 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
Implements the proper de-/serializer directly on the type and then
use serde_plain::derive_display_from_serialize where applicable, instead
of separate serializer functions somewhere else.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-installer-common/Cargo.toml | 1 +
proxmox-installer-common/src/options.rs | 105 +++++++++++++-----------
proxmox-installer-common/src/setup.rs | 59 +------------
3 files changed, 61 insertions(+), 104 deletions(-)
diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
index 70f828a..4b72041 100644
--- a/proxmox-installer-common/Cargo.toml
+++ b/proxmox-installer-common/Cargo.toml
@@ -11,3 +11,4 @@ homepage = "https://www.proxmox.com"
regex = "1.7"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
+serde_plain = "1.0"
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index e77914b..9f6131b 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -1,4 +1,4 @@
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
use std::net::{IpAddr, Ipv4Addr};
use std::{cmp, fmt};
@@ -7,55 +7,33 @@ use crate::setup::{
};
use crate::utils::{CidrAddress, Fqdn};
-#[derive(Copy, Clone, Debug, Deserialize, Eq, PartialEq)]
-#[serde(rename_all = "lowercase")]
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
+#[serde(rename_all(deserialize = "lowercase", serialize = "UPPERCASE"))]
pub enum BtrfsRaidLevel {
Raid0,
Raid1,
Raid10,
}
-impl fmt::Display for BtrfsRaidLevel {
- fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- use BtrfsRaidLevel::*;
- match self {
- Raid0 => write!(f, "RAID0"),
- Raid1 => write!(f, "RAID1"),
- Raid10 => write!(f, "RAID10"),
- }
- }
-}
+serde_plain::derive_display_from_serialize!(BtrfsRaidLevel);
-#[derive(Copy, Clone, Debug, Deserialize, Eq, PartialEq)]
-#[serde(rename_all = "lowercase")]
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
+#[serde(rename_all(deserialize = "lowercase", serialize = "UPPERCASE"))]
pub enum ZfsRaidLevel {
Raid0,
Raid1,
Raid10,
- #[serde(rename = "raidz-1")]
+ #[serde(rename = "RAIDZ-1")]
RaidZ,
- #[serde(rename = "raidz-2")]
+ #[serde(rename = "RAIDZ-2")]
RaidZ2,
- #[serde(rename = "raidz-3")]
+ #[serde(rename = "RAIDZ-3")]
RaidZ3,
}
-impl fmt::Display for ZfsRaidLevel {
- fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- use ZfsRaidLevel::*;
- match self {
- Raid0 => write!(f, "RAID0"),
- Raid1 => write!(f, "RAID1"),
- Raid10 => write!(f, "RAID10"),
- RaidZ => write!(f, "RAIDZ-1"),
- RaidZ2 => write!(f, "RAIDZ-2"),
- RaidZ3 => write!(f, "RAIDZ-3"),
- }
- }
-}
+serde_plain::derive_display_from_serialize!(ZfsRaidLevel);
-#[derive(Copy, Clone, Debug, Deserialize, Eq, PartialEq)]
-#[serde(rename_all = "lowercase")]
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum FsType {
Ext4,
Xfs,
@@ -81,6 +59,49 @@ impl fmt::Display for FsType {
}
}
+impl<'de> Deserialize<'de> for FsType {
+ fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+ where
+ D: serde::Deserializer<'de>,
+ {
+ let fs: String = Deserialize::deserialize(deserializer)?;
+
+ match fs.as_str() {
+ "ext4" => Ok(FsType::Ext4),
+ "xfs" => Ok(FsType::Xfs),
+ "zfs (RAID0)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid0)),
+ "zfs (RAID1)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid1)),
+ "zfs (RAID10)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid10)),
+ "zfs (RAIDZ-1)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ)),
+ "zfs (RAIDZ-2)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ2)),
+ "zfs (RAIDZ-3)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ3)),
+ "btrfs (RAID0)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid0)),
+ "btrfs (RAID1)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid1)),
+ "btrfs (RAID10)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid10)),
+ _ => Err(serde::de::Error::custom("could not find file system: {fs}")),
+ }
+ }
+}
+
+impl Serialize for FsType {
+ fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+ where
+ S: serde::Serializer,
+ {
+ let value = match self {
+ // proxinstall::$fssetup
+ FsType::Ext4 => "ext4",
+ FsType::Xfs => "xfs",
+ // proxinstall::get_zfs_raid_setup()
+ FsType::Zfs(level) => &format!("zfs ({level})"),
+ // proxinstall::get_btrfs_raid_setup()
+ FsType::Btrfs(level) => &format!("btrfs ({level})"),
+ };
+
+ serializer.collect_str(value)
+ }
+}
+
#[derive(Clone, Debug)]
pub struct LvmBootdiskOptions {
pub total_size: f64,
@@ -119,8 +140,8 @@ impl BtrfsBootdiskOptions {
}
}
-#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq)]
-#[serde(rename_all(deserialize = "lowercase"))]
+#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
+#[serde(rename_all = "lowercase")]
pub enum ZfsCompressOption {
#[default]
On,
@@ -132,11 +153,7 @@ pub enum ZfsCompressOption {
Zstd,
}
-impl fmt::Display for ZfsCompressOption {
- fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- write!(f, "{}", format!("{self:?}").to_lowercase())
- }
-}
+serde_plain::derive_display_from_serialize!(ZfsCompressOption);
impl From<&ZfsCompressOption> for String {
fn from(value: &ZfsCompressOption) -> Self {
@@ -149,7 +166,7 @@ pub const ZFS_COMPRESS_OPTIONS: &[ZfsCompressOption] = {
&[On, Off, Lzjb, Lz4, Zle, Gzip, Zstd]
};
-#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq)]
+#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
#[serde(rename_all = "kebab-case")]
pub enum ZfsChecksumOption {
#[default]
@@ -158,11 +175,7 @@ pub enum ZfsChecksumOption {
Sha256,
}
-impl fmt::Display for ZfsChecksumOption {
- fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- write!(f, "{}", format!("{self:?}").to_lowercase())
- }
-}
+serde_plain::derive_display_from_serialize!(ZfsChecksumOption);
impl From<&ZfsChecksumOption> for String {
fn from(value: &ZfsChecksumOption) -> Self {
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 9aa4063..c581100 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -12,10 +12,7 @@ use std::{
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
use crate::{
- options::{
- BtrfsRaidLevel, Disk, FsType, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption,
- ZfsRaidLevel,
- },
+ options::{Disk, FsType, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption},
utils::CidrAddress,
};
@@ -201,9 +198,7 @@ pub fn installer_setup(in_test_mode: bool) -> Result<(SetupInfo, LocaleInfo, Run
#[derive(Debug, Deserialize, Serialize)]
pub struct InstallZfsOption {
pub ashift: usize,
- #[serde(serialize_with = "serialize_as_display")]
pub compress: ZfsCompressOption,
- #[serde(serialize_with = "serialize_as_display")]
pub checksum: ZfsChecksumOption,
pub copies: usize,
pub arc_max: usize,
@@ -449,10 +444,6 @@ pub fn spawn_low_level_installer(test_mode: bool) -> io::Result<process::Child>
pub struct InstallConfig {
pub autoreboot: usize,
- #[serde(
- serialize_with = "serialize_fstype",
- deserialize_with = "deserialize_fs_type"
- )]
pub filesys: FsType,
pub hdsize: f64,
#[serde(skip_serializing_if = "Option::is_none")]
@@ -511,51 +502,3 @@ where
serializer.serialize_none()
}
}
-
-fn serialize_fstype<S>(value: &FsType, serializer: S) -> Result<S::Ok, S::Error>
-where
- S: Serializer,
-{
- use FsType::*;
- let value = match value {
- // proxinstall::$fssetup
- Ext4 => "ext4",
- Xfs => "xfs",
- // proxinstall::get_zfs_raid_setup()
- Zfs(ZfsRaidLevel::Raid0) => "zfs (RAID0)",
- Zfs(ZfsRaidLevel::Raid1) => "zfs (RAID1)",
- Zfs(ZfsRaidLevel::Raid10) => "zfs (RAID10)",
- Zfs(ZfsRaidLevel::RaidZ) => "zfs (RAIDZ-1)",
- Zfs(ZfsRaidLevel::RaidZ2) => "zfs (RAIDZ-2)",
- Zfs(ZfsRaidLevel::RaidZ3) => "zfs (RAIDZ-3)",
- // proxinstall::get_btrfs_raid_setup()
- Btrfs(BtrfsRaidLevel::Raid0) => "btrfs (RAID0)",
- Btrfs(BtrfsRaidLevel::Raid1) => "btrfs (RAID1)",
- Btrfs(BtrfsRaidLevel::Raid10) => "btrfs (RAID10)",
- };
-
- serializer.collect_str(value)
-}
-
-pub fn deserialize_fs_type<'de, D>(deserializer: D) -> Result<FsType, D::Error>
-where
- D: Deserializer<'de>,
-{
- use FsType::*;
- let de_fs: String = Deserialize::deserialize(deserializer)?;
-
- match de_fs.as_str() {
- "ext4" => Ok(Ext4),
- "xfs" => Ok(Xfs),
- "zfs (RAID0)" => Ok(Zfs(ZfsRaidLevel::Raid0)),
- "zfs (RAID1)" => Ok(Zfs(ZfsRaidLevel::Raid1)),
- "zfs (RAID10)" => Ok(Zfs(ZfsRaidLevel::Raid10)),
- "zfs (RAIDZ-1)" => Ok(Zfs(ZfsRaidLevel::RaidZ)),
- "zfs (RAIDZ-2)" => Ok(Zfs(ZfsRaidLevel::RaidZ2)),
- "zfs (RAIDZ-3)" => Ok(Zfs(ZfsRaidLevel::RaidZ3)),
- "btrfs (RAID0)" => Ok(Btrfs(BtrfsRaidLevel::Raid0)),
- "btrfs (RAID1)" => Ok(Btrfs(BtrfsRaidLevel::Raid1)),
- "btrfs (RAID10)" => Ok(Btrfs(BtrfsRaidLevel::Raid10)),
- _ => Err(de::Error::custom("could not find file system: {de_fs}")),
- }
-}
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [pve-devel] [PATCH installer 04/14] common: simplify filesystem type serializing & Display trait impl
2024-07-10 13:27 ` [pve-devel] [PATCH installer 04/14] common: simplify filesystem type serializing & Display trait impl Christoph Heiss
@ 2024-07-11 14:32 ` Stefan Hanreich
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Hanreich @ 2024-07-11 14:32 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
On 7/10/24 15:27, Christoph Heiss wrote:
> +impl<'de> Deserialize<'de> for FsType {
> + fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
> + where
> + D: serde::Deserializer<'de>,
> + {
> + let fs: String = Deserialize::deserialize(deserializer)?;
> +
> + match fs.as_str() {
> + "ext4" => Ok(FsType::Ext4),
> + "xfs" => Ok(FsType::Xfs),
> + "zfs (RAID0)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid0)),
> + "zfs (RAID1)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid1)),
> + "zfs (RAID10)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid10)),
> + "zfs (RAIDZ-1)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ)),
> + "zfs (RAIDZ-2)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ2)),
> + "zfs (RAIDZ-3)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ3)),
> + "btrfs (RAID0)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid0)),
> + "btrfs (RAID1)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid1)),
> + "btrfs (RAID10)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid10)),
> + _ => Err(serde::de::Error::custom("could not find file system: {fs}")),
> + }
> + }
> +}
Maybe we could implement FromStr here and use
serde_plain::derive_deserialize_from_fromstr ?
We could even implement FromStr for BtrfsRaidLevel and ZfsRaidLevel and
then use that here, but it might be a bit overkill for just this....
> +impl Serialize for FsType {
> + fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
> + where
> + S: serde::Serializer,
> + {
> + let value = match self {
> + // proxinstall::$fssetup
> + FsType::Ext4 => "ext4",
> + FsType::Xfs => "xfs",
> + // proxinstall::get_zfs_raid_setup()
> + FsType::Zfs(level) => &format!("zfs ({level})"),
> + // proxinstall::get_btrfs_raid_setup()
> + FsType::Btrfs(level) => &format!("btrfs ({level})"),
> + };
> +
> + serializer.collect_str(value)
> + }
> +}
Same as above but with Display and
serde_plain::derive_display_from_serialize
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [pve-devel] [PATCH installer 05/14] common: setup: serialize `target_hd` as string explicitly
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (3 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 04/14] common: simplify filesystem type serializing & Display trait impl Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 06/14] common: split out installer setup files loading functionality Christoph Heiss
` (10 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-auto-installer/src/utils.rs | 15 +++++++++++----
proxmox-installer-common/src/setup.rs | 23 ++---------------------
proxmox-tui-installer/src/setup.rs | 2 +-
3 files changed, 14 insertions(+), 26 deletions(-)
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 202ad41..9ff5e48 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -171,7 +171,7 @@ fn set_single_disk(
.iter()
.find(|item| item.path.ends_with(disk_name.as_str()));
match disk {
- Some(disk) => config.target_hd = Some(disk.clone()),
+ Some(disk) => config.target_hd = Some(disk.path.clone()),
None => bail!("disk in 'disk_selection' not found"),
}
}
@@ -181,10 +181,10 @@ fn set_single_disk(
.disks
.iter()
.find(|item| item.index == disk_index);
- config.target_hd = disk.cloned();
+ config.target_hd = disk.map(|d| d.path.clone());
}
}
- info!("Selected disk: {}", config.target_hd.clone().unwrap().path);
+ info!("Selected disk: {}", config.target_hd.clone().unwrap());
Ok(())
}
@@ -350,7 +350,14 @@ pub fn parse_answer(
set_disks(answer, udev_info, runtime_info, &mut config)?;
match &answer.disks.fs_options {
answer::FsOptions::LVM(lvm) => {
- config.hdsize = lvm.hdsize.unwrap_or(config.target_hd.clone().unwrap().size);
+ let disk = runtime_info
+ .disks
+ .iter()
+ .find(|d| Some(&d.path) == config.target_hd.as_ref());
+
+ config.hdsize = lvm
+ .hdsize
+ .unwrap_or_else(|| disk.map(|d| d.size).unwrap_or_default());
config.swapsize = lvm.swapsize;
config.maxroot = lvm.maxroot;
config.maxvz = lvm.maxvz;
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index c581100..9131ac9 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -458,16 +458,8 @@ pub struct InstallConfig {
#[serde(skip_serializing_if = "Option::is_none")]
pub zfs_opts: Option<InstallZfsOption>,
- #[serde(
- serialize_with = "serialize_disk_opt",
- skip_serializing_if = "Option::is_none",
- // only the 'path' property is serialized -> deserialization is problematic
- // The information would be present in the 'run-env-info-json', but for now there is no
- // need for it in any code that deserializes the low-level config. Therefore we are
- // currently skipping it on deserialization
- skip_deserializing
- )]
- pub target_hd: Option<Disk>,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub target_hd: Option<String>,
#[serde(skip_serializing_if = "BTreeMap::is_empty")]
pub disk_selection: BTreeMap<String, String>,
@@ -491,14 +483,3 @@ pub struct InstallConfig {
pub gateway: IpAddr,
pub dns: IpAddr,
}
-
-fn serialize_disk_opt<S>(value: &Option<Disk>, serializer: S) -> Result<S::Ok, S::Error>
-where
- S: Serializer,
-{
- if let Some(disk) = value {
- serializer.serialize_str(&disk.path)
- } else {
- serializer.serialize_none()
- }
-}
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index 8c01e42..3c93e37 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -42,7 +42,7 @@ impl From<InstallerOptions> for InstallConfig {
match &options.bootdisk.advanced {
AdvancedBootdiskOptions::Lvm(lvm) => {
config.hdsize = lvm.total_size;
- config.target_hd = Some(options.bootdisk.disks[0].clone());
+ config.target_hd = Some(options.bootdisk.disks[0].path.clone());
config.swapsize = lvm.swap_size;
config.maxroot = lvm.max_root_size;
config.minfree = lvm.min_lvm_free;
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 06/14] common: split out installer setup files loading functionality
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (4 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 05/14] common: setup: serialize `target_hd` as string explicitly Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-11 15:06 ` Stefan Hanreich
2024-07-10 13:27 ` [pve-devel] [PATCH installer 07/14] debian: strip unused library dependencies Christoph Heiss
` (9 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
This allows re-using this piece of code in e.g. the post hook, instead
of having to open-code it there.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-auto-installer/tests/parse-answer.rs | 40 +++++---------------
proxmox-installer-common/src/setup.rs | 13 +++++--
2 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 4014b6d..450915a 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
use serde_json::Value;
use std::fs;
@@ -8,13 +8,16 @@ use proxmox_auto_installer::answer::Answer;
use proxmox_auto_installer::udevinfo::UdevInfo;
use proxmox_auto_installer::utils::parse_answer;
-use proxmox_installer_common::setup::{read_json, LocaleInfo, RuntimeInfo, SetupInfo};
+use proxmox_installer_common::setup::{
+ load_installer_setup_files, read_json, LocaleInfo, RuntimeInfo, SetupInfo,
+};
fn get_test_resource_path() -> Result<PathBuf, String> {
Ok(std::env::current_dir()
.expect("current dir failed")
.join("tests/resources"))
}
+
fn get_answer(path: PathBuf) -> Result<Answer, String> {
let answer_raw = std::fs::read_to_string(path).unwrap();
let answer: answer::Answer = toml::from_str(&answer_raw)
@@ -24,46 +27,23 @@ fn get_answer(path: PathBuf) -> Result<Answer, String> {
Ok(answer)
}
-fn setup_test_basic(path: &PathBuf) -> (SetupInfo, LocaleInfo, RuntimeInfo, UdevInfo) {
- let installer_info: SetupInfo = {
- let mut path = path.clone();
- path.push("iso-info.json");
-
- read_json(&path)
- .map_err(|err| format!("Failed to retrieve setup info: {err}"))
- .unwrap()
- };
-
- let locale_info = {
- let mut path = path.clone();
- path.push("locales.json");
-
- read_json(&path)
- .map_err(|err| format!("Failed to retrieve locale info: {err}"))
- .unwrap()
- };
-
- let mut runtime_info: RuntimeInfo = {
- let mut path = path.clone();
- path.push("run-env-info.json");
-
- read_json(&path)
- .map_err(|err| format!("Failed to retrieve runtime environment info: {err}"))
- .unwrap()
- };
+pub fn setup_test_basic(path: &Path) -> (SetupInfo, LocaleInfo, RuntimeInfo, UdevInfo) {
+ let (installer_info, locale_info, mut runtime_info) = load_installer_setup_files(path).unwrap();
let udev_info: UdevInfo = {
- let mut path = path.clone();
+ let mut path = path.to_path_buf();
path.push("run-env-udev.json");
read_json(&path)
.map_err(|err| format!("Failed to retrieve udev info details: {err}"))
.unwrap()
};
+
runtime_info.disks.sort();
if runtime_info.disks.is_empty() {
panic!("disk list is empty!");
}
+
(installer_info, locale_info, runtime_info, udev_info)
}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 9131ac9..29137bf 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -163,24 +163,29 @@ pub fn installer_setup(in_test_mode: bool) -> Result<(SetupInfo, LocaleInfo, Run
} else {
crate::RUNTIME_DIR.to_owned()
};
- let path = PathBuf::from(base_path);
+ load_installer_setup_files(&PathBuf::from(base_path))
+}
+
+pub fn load_installer_setup_files(
+ base_path: &Path,
+) -> Result<(SetupInfo, LocaleInfo, RuntimeInfo), String> {
let installer_info: SetupInfo = {
- let mut path = path.clone();
+ let mut path = base_path.to_path_buf();
path.push("iso-info.json");
read_json(&path).map_err(|err| format!("Failed to retrieve setup info: {err}"))?
};
let locale_info = {
- let mut path = path.clone();
+ let mut path = base_path.to_path_buf();
path.push("locales.json");
read_json(&path).map_err(|err| format!("Failed to retrieve locale info: {err}"))?
};
let mut runtime_info: RuntimeInfo = {
- let mut path = path.clone();
+ let mut path = base_path.to_path_buf();
path.push("run-env-info.json");
read_json(&path)
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [pve-devel] [PATCH installer 06/14] common: split out installer setup files loading functionality
2024-07-10 13:27 ` [pve-devel] [PATCH installer 06/14] common: split out installer setup files loading functionality Christoph Heiss
@ 2024-07-11 15:06 ` Stefan Hanreich
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Hanreich @ 2024-07-11 15:06 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
On 7/10/24 15:27, Christoph Heiss wrote:
> diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
> index 9131ac9..29137bf 100644
> --- a/proxmox-installer-common/src/setup.rs
> +++ b/proxmox-installer-common/src/setup.rs
> @@ -163,24 +163,29 @@ pub fn installer_setup(in_test_mode: bool) -> Result<(SetupInfo, LocaleInfo, Run
> } else {
> crate::RUNTIME_DIR.to_owned()
> };
> - let path = PathBuf::from(base_path);
>
> + load_installer_setup_files(&PathBuf::from(base_path))
> +}
> +
> +pub fn load_installer_setup_files(
> + base_path: &Path,
Could use AsRef<Path> here since it's public API. In the test specific
code we could also use it for parameters, but there it's not really
important since it's not public API.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [pve-devel] [PATCH installer 07/14] debian: strip unused library dependencies
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (5 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 06/14] common: split out installer setup files loading functionality Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 08/14] fetch-answer: move http-related code to gated module in installer-common Christoph Heiss
` (8 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
Rust links in some dynamic libraries even if only used by a disabled
feature gate. This is needed due to the upcoming move of the HTTP
features into the `proxmox-installer-common` crate.
Very similar to how it is done in the `proxmox-backup` repository, which
needs the same for very similar reasons.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
debian/control | 1 +
debian/rules | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/debian/control b/debian/control
index eb4d3be..b7ddc11 100644
--- a/debian/control
+++ b/debian/control
@@ -26,6 +26,7 @@ Build-Depends: cargo:native,
librust-toml-0.7-dev,
librust-ureq-2.6-dev,
libtest-mockmodule-perl,
+ patchelf,
perl,
rustc:native,
shellcheck,
diff --git a/debian/rules b/debian/rules
index 1c03065..1c4eda8 100755
--- a/debian/rules
+++ b/debian/rules
@@ -10,3 +10,12 @@ export BUILD_MODE=release
override_dh_missing:
dh_missing --fail-missing
+
+override_dh_strip:
+ dh_strip
+ for f in $$(find debian/proxmox-installer debian/proxmox-auto-install-assistant -executable -type f); do \
+ if file -bi "$$f" | grep -qP '^application'; then \
+ echo "stripping unused dependencies from $$f"; \
+ ldd -u "$$f" | grep -oP '[^/:]+$$' | sed 's/^/--remove-needed /g' | xargs patchelf "$$f"; \
+ fi; \
+ done
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 08/14] fetch-answer: move http-related code to gated module in installer-common
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (6 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 07/14] debian: strip unused library dependencies Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 09/14] tree-wide: convert some more crates to use workspace dependencies Christoph Heiss
` (7 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Cargo.toml | 6 ++
proxmox-fetch-answer/Cargo.toml | 17 +--
.../src/fetch_plugins/http.rs | 100 +-----------------
proxmox-installer-common/Cargo.toml | 20 ++++
proxmox-installer-common/src/http.rs | 94 ++++++++++++++++
proxmox-installer-common/src/lib.rs | 3 +
6 files changed, 130 insertions(+), 110 deletions(-)
create mode 100644 proxmox-installer-common/src/http.rs
diff --git a/Cargo.toml b/Cargo.toml
index 1e730ce..1dcf669 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -9,3 +9,9 @@ members = [
"proxmox-tui-installer",
]
+[workspace.dependencies]
+anyhow = "1.0"
+log = "0.4.20"
+toml = "0.7"
+proxmox-auto-installer.path = "./proxmox-auto-installer"
+proxmox-installer-common.path = "./proxmox-installer-common"
diff --git a/proxmox-fetch-answer/Cargo.toml b/proxmox-fetch-answer/Cargo.toml
index 964682a..50f3da3 100644
--- a/proxmox-fetch-answer/Cargo.toml
+++ b/proxmox-fetch-answer/Cargo.toml
@@ -10,16 +10,9 @@ license = "AGPL-3"
exclude = [ "build", "debian" ]
homepage = "https://www.proxmox.com"
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
-
[dependencies]
-anyhow = "1.0"
-hex = "0.4"
-log = "0.4.20"
-native-tls = "0.2"
-proxmox-auto-installer = { path = "../proxmox-auto-installer" }
-rustls = { version = "0.20", features = [ "dangerous_configuration" ] }
-rustls-native-certs = "0.6"
-sha2 = "0.10"
-toml = "0.7"
-ureq = { version = "2.6", features = [ "native-certs", "native-tls" ] }
+anyhow.workspace = true
+log.workspace = true
+proxmox-auto-installer.workspace = true
+proxmox-installer-common = { workspace = true, features = ["http"] }
+toml.workspace = true
diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs
index 21bc6a2..a6a8de0 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
@@ -67,7 +67,8 @@ impl FetchFromHTTP {
info!("Gathering system information.");
let payload = SysInfo::as_json()?;
info!("Sending POST request to '{answer_url}'.");
- let answer = http_post::call(answer_url, fingerprint.as_deref(), payload)?;
+ let answer =
+ proxmox_installer_common::http::post(answer_url, fingerprint.as_deref(), payload)?;
Ok(answer)
}
@@ -179,100 +180,3 @@ impl FetchFromHTTP {
value.map(|value| String::from(&value[1..value.len() - 2]))
}
}
-
-mod http_post {
- use anyhow::Result;
- use rustls::ClientConfig;
- use sha2::{Digest, Sha256};
- use std::sync::Arc;
- use ureq::{Agent, AgentBuilder};
-
- /// Issues a POST request with the payload (JSON). Optionally a SHA256 fingerprint can be used to
- /// check the cert against it, instead of the regular cert validation.
- /// To gather the sha256 fingerprint you can use the following command:
- /// ```no_compile
- /// openssl s_client -connect <host>:443 < /dev/null 2>/dev/null | openssl x509 -fingerprint -sha256 -noout -in /dev/stdin
- /// ```
- ///
- /// # Arguemnts
- /// * `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 call(url: String, fingerprint: Option<&str>, payload: String) -> Result<String> {
- let answer;
-
- if let Some(fingerprint) = fingerprint {
- let tls_config = ClientConfig::builder()
- .with_safe_defaults()
- .with_custom_certificate_verifier(VerifyCertFingerprint::new(fingerprint)?)
- .with_no_client_auth();
-
- let agent: Agent = AgentBuilder::new().tls_config(Arc::new(tls_config)).build();
-
- answer = agent
- .post(&url)
- .set("Content-type", "application/json; charset=utf-8")
- .send_string(&payload)?
- .into_string()?;
- } else {
- let mut roots = rustls::RootCertStore::empty();
- for cert in rustls_native_certs::load_native_certs()? {
- roots.add(&rustls::Certificate(cert.0)).unwrap();
- }
-
- let tls_config = rustls::ClientConfig::builder()
- .with_safe_defaults()
- .with_root_certificates(roots)
- .with_no_client_auth();
-
- let agent = AgentBuilder::new()
- .tls_connector(Arc::new(native_tls::TlsConnector::new()?))
- .tls_config(Arc::new(tls_config))
- .build();
- answer = agent
- .post(&url)
- .set("Content-type", "application/json; charset=utf-8")
- .timeout(std::time::Duration::from_secs(60))
- .send_string(&payload)?
- .into_string()?;
- }
- Ok(answer)
- }
-
- struct VerifyCertFingerprint {
- cert_fingerprint: Vec<u8>,
- }
-
- impl VerifyCertFingerprint {
- fn new<S: AsRef<str>>(cert_fingerprint: S) -> Result<std::sync::Arc<Self>> {
- let cert_fingerprint = cert_fingerprint.as_ref();
- let sanitized = cert_fingerprint.replace(':', "");
- let decoded = hex::decode(sanitized)?;
- Ok(std::sync::Arc::new(Self {
- cert_fingerprint: decoded,
- }))
- }
- }
-
- impl rustls::client::ServerCertVerifier for VerifyCertFingerprint {
- fn verify_server_cert(
- &self,
- end_entity: &rustls::Certificate,
- _intermediates: &[rustls::Certificate],
- _server_name: &rustls::ServerName,
- _scts: &mut dyn Iterator<Item = &[u8]>,
- _ocsp_response: &[u8],
- _now: std::time::SystemTime,
- ) -> Result<rustls::client::ServerCertVerified, rustls::Error> {
- let mut hasher = Sha256::new();
- hasher.update(end_entity);
- let result = hasher.finalize();
-
- if result.as_slice() == self.cert_fingerprint {
- Ok(rustls::client::ServerCertVerified::assertion())
- } else {
- Err(rustls::Error::General("Fingerprint did not match!".into()))
- }
- }
- }
-}
diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
index 4b72041..d8e9e06 100644
--- a/proxmox-installer-common/Cargo.toml
+++ b/proxmox-installer-common/Cargo.toml
@@ -12,3 +12,23 @@ regex = "1.7"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
serde_plain = "1.0"
+
+# `http` feature
+anyhow = { workspace = true, optional = true }
+hex = { version = "0.4", optional = true }
+native-tls = { version = "0.2", optional = true }
+rustls = { version = "0.20", features = [ "dangerous_configuration" ], optional = true }
+rustls-native-certs = { version = "0.6", optional = true }
+sha2 = { version = "0.10", optional = true }
+ureq = { version = "2.6", features = [ "native-certs", "native-tls" ], optional = true }
+
+[features]
+http = [
+ "dep:anyhow",
+ "dep:hex",
+ "dep:native-tls",
+ "dep:rustls",
+ "dep:rustls-native-certs",
+ "dep:sha2",
+ "dep:ureq"
+]
diff --git a/proxmox-installer-common/src/http.rs b/proxmox-installer-common/src/http.rs
new file mode 100644
index 0000000..4a5d444
--- /dev/null
+++ b/proxmox-installer-common/src/http.rs
@@ -0,0 +1,94 @@
+use anyhow::Result;
+use rustls::ClientConfig;
+use sha2::{Digest, Sha256};
+use std::sync::Arc;
+use ureq::{Agent, AgentBuilder};
+
+/// Issues a POST request with the payload (JSON). Optionally a SHA256 fingerprint can be used to
+/// check the cert against it, instead of the regular cert validation.
+/// To gather the sha256 fingerprint you can use the following command:
+/// ```no_compile
+/// openssl s_client -connect <host>:443 < /dev/null 2>/dev/null | openssl x509 -fingerprint -sha256 -noout -in /dev/stdin
+/// ```
+///
+/// # Arguments
+/// * `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<String> {
+ let answer;
+
+ if let Some(fingerprint) = fingerprint {
+ let tls_config = ClientConfig::builder()
+ .with_safe_defaults()
+ .with_custom_certificate_verifier(VerifyCertFingerprint::new(fingerprint)?)
+ .with_no_client_auth();
+
+ let agent: Agent = AgentBuilder::new().tls_config(Arc::new(tls_config)).build();
+
+ answer = agent
+ .post(&url)
+ .set("Content-Type", "application/json; charset=utf-8")
+ .send_string(&payload)?
+ .into_string()?;
+ } else {
+ let mut roots = rustls::RootCertStore::empty();
+ for cert in rustls_native_certs::load_native_certs()? {
+ roots.add(&rustls::Certificate(cert.0)).unwrap();
+ }
+
+ let tls_config = rustls::ClientConfig::builder()
+ .with_safe_defaults()
+ .with_root_certificates(roots)
+ .with_no_client_auth();
+
+ let agent = AgentBuilder::new()
+ .tls_connector(Arc::new(native_tls::TlsConnector::new()?))
+ .tls_config(Arc::new(tls_config))
+ .build();
+ answer = agent
+ .post(&url)
+ .set("Content-Type", "application/json; charset=utf-8")
+ .timeout(std::time::Duration::from_secs(60))
+ .send_string(&payload)?
+ .into_string()?;
+ }
+ Ok(answer)
+}
+
+struct VerifyCertFingerprint {
+ cert_fingerprint: Vec<u8>,
+}
+
+impl VerifyCertFingerprint {
+ fn new<S: AsRef<str>>(cert_fingerprint: S) -> Result<std::sync::Arc<Self>> {
+ let cert_fingerprint = cert_fingerprint.as_ref();
+ let sanitized = cert_fingerprint.replace(':', "");
+ let decoded = hex::decode(sanitized)?;
+ Ok(std::sync::Arc::new(Self {
+ cert_fingerprint: decoded,
+ }))
+ }
+}
+
+impl rustls::client::ServerCertVerifier for VerifyCertFingerprint {
+ fn verify_server_cert(
+ &self,
+ end_entity: &rustls::Certificate,
+ _intermediates: &[rustls::Certificate],
+ _server_name: &rustls::ServerName,
+ _scts: &mut dyn Iterator<Item = &[u8]>,
+ _ocsp_response: &[u8],
+ _now: std::time::SystemTime,
+ ) -> Result<rustls::client::ServerCertVerified, rustls::Error> {
+ let mut hasher = Sha256::new();
+ hasher.update(end_entity);
+ let result = hasher.finalize();
+
+ if result.as_slice() == self.cert_fingerprint {
+ Ok(rustls::client::ServerCertVerified::assertion())
+ } else {
+ Err(rustls::Error::General("Fingerprint did not match!".into()))
+ }
+ }
+}
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index 850e825..ee9f2a8 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -3,4 +3,7 @@ pub mod options;
pub mod setup;
pub mod utils;
+#[cfg(feature = "http")]
+pub mod http;
+
pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 09/14] tree-wide: convert some more crates to use workspace dependencies
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (7 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 08/14] fetch-answer: move http-related code to gated module in installer-common Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 10/14] auto-installer: tests: replace left/right with got/expected in output Christoph Heiss
` (6 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Cargo.toml | 4 ++++
proxmox-auto-install-assistant/Cargo.toml | 14 +++++++-------
proxmox-auto-installer/Cargo.toml | 15 +++++++--------
proxmox-chroot/Cargo.toml | 8 ++++----
proxmox-installer-common/Cargo.toml | 7 +++----
proxmox-tui-installer/Cargo.toml | 8 ++++----
6 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index 1dcf669..94a4dec 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -12,6 +12,10 @@ members = [
[workspace.dependencies]
anyhow = "1.0"
log = "0.4.20"
+serde = { version = "1.0", features = ["derive"] }
+serde_json = "1.0"
+serde_plain = "1.0"
toml = "0.7"
+regex = "1.7"
proxmox-auto-installer.path = "./proxmox-auto-installer"
proxmox-installer-common.path = "./proxmox-installer-common"
diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml
index eaca7f8..1f46042 100644
--- a/proxmox-auto-install-assistant/Cargo.toml
+++ b/proxmox-auto-install-assistant/Cargo.toml
@@ -11,12 +11,12 @@ exclude = [ "build", "debian" ]
homepage = "https://www.proxmox.com"
[dependencies]
-anyhow = "1.0"
+anyhow.workspace = true
+log.workspace = true
+proxmox-auto-installer.workspace = true
+serde.workspace = true
+serde_json.workspace = true
+toml.workspace = true
+regex.workspace = true
clap = { version = "4.0", features = ["derive"] }
glob = "0.3"
-log = "0.4.20"
-proxmox-auto-installer = { path = "../proxmox-auto-installer" }
-regex = "1.7"
-serde = { version = "1.0", features = ["derive"] }
-serde_json = "1.0"
-toml = "0.7"
diff --git a/proxmox-auto-installer/Cargo.toml b/proxmox-auto-installer/Cargo.toml
index 4f54439..beec046 100644
--- a/proxmox-auto-installer/Cargo.toml
+++ b/proxmox-auto-installer/Cargo.toml
@@ -11,13 +11,12 @@ exclude = [ "build", "debian" ]
homepage = "https://www.proxmox.com"
[dependencies]
-anyhow = "1.0"
+anyhow.workspace = true
+log.workspace = true
+proxmox-installer-common.workspace = true
+serde.workspace = true
+serde_json.workspace = true
+serde_plain.workspace = true
+toml.workspace = true
clap = { version = "4.0", features = ["derive"] }
glob = "0.3"
-log = "0.4.20"
-proxmox-installer-common = { path = "../proxmox-installer-common" }
-regex = "1.7"
-serde = { version = "1.0", features = ["derive"] }
-serde_json = "1.0"
-serde_plain = "1.0"
-toml = "0.7"
diff --git a/proxmox-chroot/Cargo.toml b/proxmox-chroot/Cargo.toml
index 43b96ff..be5cc57 100644
--- a/proxmox-chroot/Cargo.toml
+++ b/proxmox-chroot/Cargo.toml
@@ -8,9 +8,9 @@ exclude = [ "build", "debian" ]
homepage = "https://www.proxmox.com"
[dependencies]
-anyhow = "1.0"
+anyhow.workspace = true
+proxmox-installer-common.workspace = true
+serde_json.workspace = true
+regex.workspace = true
clap = { version = "4.0", features = ["derive"] }
nix = "0.26.1"
-proxmox-installer-common = { path = "../proxmox-installer-common" }
-regex = "1.7"
-serde_json = "1.0"
diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
index d8e9e06..62a8622 100644
--- a/proxmox-installer-common/Cargo.toml
+++ b/proxmox-installer-common/Cargo.toml
@@ -8,10 +8,9 @@ exclude = [ "build", "debian" ]
homepage = "https://www.proxmox.com"
[dependencies]
-regex = "1.7"
-serde = { version = "1.0", features = ["derive"] }
-serde_json = "1.0"
-serde_plain = "1.0"
+serde.workspace = true
+serde_json.workspace = true
+serde_plain.workspace = true
# `http` feature
anyhow = { workspace = true, optional = true }
diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml
index fc653f0..ed38875 100644
--- a/proxmox-tui-installer/Cargo.toml
+++ b/proxmox-tui-installer/Cargo.toml
@@ -8,8 +8,8 @@ exclude = [ "build", "debian" ]
homepage = "https://www.proxmox.com"
[dependencies]
+proxmox-installer-common.workspace = true
+serde.workspace = true
+serde_json.workspace = true
+regex.workspace = true
cursive = { version = "0.20.0", default-features = false, features = ["termion-backend"] }
-serde = { version = "1.0", features = ["derive"] }
-serde_json = "1.0"
-regex = "1.7"
-proxmox-installer-common = { path = "../proxmox-installer-common" }
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 10/14] auto-installer: tests: replace left/right with got/expected in output
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (8 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 09/14] tree-wide: convert some more crates to use workspace dependencies Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-11 15:03 ` Stefan Hanreich
2024-07-10 13:27 ` [pve-devel] [PATCH installer 11/14] auto-installer: answer: add `posthook` section Christoph Heiss
` (5 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
Makes more sense and makes debugging easier.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-auto-installer/tests/parse-answer.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 450915a..81079b8 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -77,7 +77,7 @@ fn test_parse_answers() {
let compare: Value = serde_json::from_str(&compare_raw).unwrap();
if config != compare {
panic!(
- "Test {} failed:\nleft: {:#?}\nright: {:#?}\n",
+ "Test {} failed:\ngot: {:#?}\nexpected: {:#?}\n",
name, config, compare
);
}
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [pve-devel] [PATCH installer 10/14] auto-installer: tests: replace left/right with got/expected in output
2024-07-10 13:27 ` [pve-devel] [PATCH installer 10/14] auto-installer: tests: replace left/right with got/expected in output Christoph Heiss
@ 2024-07-11 15:03 ` Stefan Hanreich
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Hanreich @ 2024-07-11 15:03 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
On 7/10/24 15:27, Christoph Heiss wrote:
> Makes more sense and makes debugging easier.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> proxmox-auto-installer/tests/parse-answer.rs | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
> index 450915a..81079b8 100644
> --- a/proxmox-auto-installer/tests/parse-answer.rs
> +++ b/proxmox-auto-installer/tests/parse-answer.rs
> @@ -77,7 +77,7 @@ fn test_parse_answers() {
> let compare: Value = serde_json::from_str(&compare_raw).unwrap();
> if config != compare {
> panic!(
> - "Test {} failed:\nleft: {:#?}\nright: {:#?}\n",
> + "Test {} failed:\ngot: {:#?}\nexpected: {:#?}\n",
> name, config, compare
> );
> }
maybe use assert_eq!() here altogether?
Also above in the file use assert!(!runtime_info.disks.is_empty()) ?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [pve-devel] [PATCH installer 11/14] auto-installer: answer: add `posthook` section
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (9 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 10/14] auto-installer: tests: replace left/right with got/expected in output Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 12/14] fix #5536: add post-hook utility for sending notifications after auto-install Christoph Heiss
` (4 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
Adds a new global, optional section to the answerfile.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-auto-installer/src/answer.rs | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index aab7198..e27a321 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -16,6 +16,8 @@ pub struct Answer {
pub network: Network,
#[serde(rename = "disk-setup")]
pub disks: Disks,
+ #[serde(default)]
+ pub posthook: Option<PostNotificationHookInfo>,
}
#[derive(Clone, Deserialize, Debug)]
@@ -33,6 +35,15 @@ pub struct Global {
pub root_ssh_keys: Vec<String>,
}
+#[derive(Clone, Deserialize, Debug)]
+#[serde(deny_unknown_fields)]
+pub struct PostNotificationHookInfo {
+ /// URL to send a POST request to
+ pub url: String,
+ /// SHA256 cert fingerprint if certificate pinning should be used.
+ pub cert_fingerprint: Option<String>,
+}
+
#[derive(Clone, Deserialize, Debug, Default, PartialEq)]
#[serde(deny_unknown_fields)]
enum NetworkConfigMode {
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 12/14] fix #5536: add post-hook utility for sending notifications after auto-install
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (10 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 11/14] auto-installer: answer: add `posthook` section Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-11 15:54 ` Stefan Hanreich
2024-07-10 13:27 ` [pve-devel] [PATCH installer 13/14] fix #5536: post-hook: add some unit tests Christoph Heiss
` (3 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Cargo.toml | 1 +
Makefile | 8 +-
debian/install | 1 +
proxmox-auto-installer/src/answer.rs | 16 +-
.../src/bin/proxmox-auto-installer.rs | 13 +-
.../src/fetch_plugins/http.rs | 2 +-
.../src/fetch_plugins/partition.rs | 2 +-
proxmox-installer-common/src/http.rs | 6 +-
proxmox-installer-common/src/options.rs | 4 +
proxmox-installer-common/src/setup.rs | 6 +-
proxmox-installer-common/src/utils.rs | 2 +
proxmox-post-hook/Cargo.toml | 19 +
proxmox-post-hook/src/main.rs | 372 ++++++++++++++++++
13 files changed, 429 insertions(+), 23 deletions(-)
create mode 100644 proxmox-post-hook/Cargo.toml
create mode 100644 proxmox-post-hook/src/main.rs
diff --git a/Cargo.toml b/Cargo.toml
index 94a4dec..6d1e667 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -7,6 +7,7 @@ members = [
"proxmox-fetch-answer",
"proxmox-installer-common",
"proxmox-tui-installer",
+ "proxmox-post-hook",
]
[workspace.dependencies]
diff --git a/Makefile b/Makefile
index e96a0f2..9dc4c22 100644
--- a/Makefile
+++ b/Makefile
@@ -24,7 +24,8 @@ USR_BIN := \
proxmox-tui-installer\
proxmox-fetch-answer\
proxmox-auto-install-assistant \
- proxmox-auto-installer
+ proxmox-auto-installer \
+ proxmox-post-hook
COMPILED_BINS := \
$(addprefix $(CARGO_COMPILEDIR)/,$(USR_BIN))
@@ -59,6 +60,7 @@ $(BUILDDIR):
proxmox-chroot \
proxmox-tui-installer/ \
proxmox-installer-common/ \
+ proxmox-post-hook \
test/ \
$(SHELL_SCRIPTS) \
$@.tmp
@@ -132,7 +134,9 @@ cargo-build:
--package proxmox-auto-installer --bin proxmox-auto-installer \
--package proxmox-fetch-answer --bin proxmox-fetch-answer \
--package proxmox-auto-install-assistant --bin proxmox-auto-install-assistant \
- --package proxmox-chroot --bin proxmox-chroot $(CARGO_BUILD_ARGS)
+ --package proxmox-chroot --bin proxmox-chroot \
+ --package proxmox-post-hook --bin proxmox-post-hook \
+ $(CARGO_BUILD_ARGS)
%-banner.png: %-banner.svg
rsvg-convert -o $@ $<
diff --git a/debian/install b/debian/install
index bb91da7..b64c8ec 100644
--- a/debian/install
+++ b/debian/install
@@ -15,4 +15,5 @@ usr/bin/proxmox-chroot
usr/bin/proxmox-fetch-answer
usr/bin/proxmox-low-level-installer
usr/bin/proxmox-tui-installer
+usr/bin/proxmox-post-hook
var
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index e27a321..58c8136 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -1,10 +1,11 @@
+use anyhow::{format_err, Result};
use clap::ValueEnum;
use proxmox_installer_common::{
options::{BtrfsRaidLevel, FsType, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel},
utils::{CidrAddress, Fqdn},
};
use serde::{Deserialize, Serialize};
-use std::{collections::BTreeMap, net::IpAddr};
+use std::{collections::BTreeMap, io::BufRead, net::IpAddr};
// BTreeMap is used to store filters as the order of the filters will be stable, compared to
// storing them in a HashMap
@@ -20,6 +21,19 @@ pub struct Answer {
pub posthook: Option<PostNotificationHookInfo>,
}
+impl Answer {
+ pub fn from_reader(reader: impl BufRead) -> Result<Self> {
+ 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}"))
+ }
+}
+
#[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<PathBuf> {
- 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<String> {
+pub fn post(url: &str, fingerprint: Option<&str>, payload: String) -> Result<String> {
let answer;
if let Some(fingerprint) = fingerprint {
@@ -27,7 +27,7 @@ pub fn post(url: String, fingerprint: Option<&str>, payload: String) -> Result<S
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<S
.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 <c.heiss@proxmox.com>",
+ "Proxmox Support Team <support@proxmox.com>",
+]
+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<String, String>,
+}
+
+/// 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<String, String>,
+}
+
+/// 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<BootDiskInfo>,
+ /// 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<Self> {
+ 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))
+ .map_err(|err| anyhow!(err))?;
+
+ 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<Vec<BootDiskInfo>> {
+ 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())
+ } else {
+ Ok(config
+ .disk_selection
+ .values()
+ .filter_map(|index| Some(run_env.disks[index.parse::<usize>().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())
+ }
+ }
+
+ /// 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<NetworkInterfaceInfo> {
+ 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))?;
+
+ 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<String>,
+ ) -> Result<String> {
+ 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<PbsVersionInfo> = 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<String> {
+ for pkg in output.lines() {
+ let parts = pkg.split('|').collect::<Vec<&str>>();
+
+ 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<R, F: FnOnce(&str) -> Result<R>>(callback: F) -> Result<R> {
+ 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
+ }
+ }
+}
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [pve-devel] [PATCH installer 12/14] fix #5536: add post-hook utility for sending notifications after auto-install
2024-07-10 13:27 ` [pve-devel] [PATCH installer 12/14] fix #5536: add post-hook utility for sending notifications after auto-install Christoph Heiss
@ 2024-07-11 15:54 ` Stefan Hanreich
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Hanreich @ 2024-07-11 15:54 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
On 7/10/24 15:27, Christoph Heiss wrote:
> +impl Answer {
> + pub fn from_reader(reader: impl BufRead) -> Result<Self> {
> + 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<T: BufRead> TryFrom<T> 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<PathBuf> {
> - 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<String> {
> +pub fn post(url: &str, fingerprint: Option<&str>, payload: String) -> Result<String> {
> let answer;
>
> if let Some(fingerprint) = fingerprint {
> @@ -27,7 +27,7 @@ pub fn post(url: String, fingerprint: Option<&str>, payload: String) -> Result<S
> 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<S
> .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 <c.heiss@proxmox.com>",
> + "Proxmox Support Team <support@proxmox.com>",
> +]
> +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<String, String>,
> +}
> +
> +/// 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<String, String>,
> +}
> +
> +/// 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<BootDiskInfo>,
> + /// 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<Self> {
> + 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<Path> 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<Vec<BootDiskInfo>> {
> + 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::<usize>().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<Item = Result<>> can be
collected into a Result<Vec<>,..> 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<NetworkInterfaceInfo> {
> + 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<String>,
> + ) -> Result<String> {
> + 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<PbsVersionInfo> = 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<String> {
> + for pkg in output.lines() {
> + let parts = pkg.split('|').collect::<Vec<&str>>();
> +
> + 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<R, F: FnOnce(&str) -> Result<R>>(callback: F) -> Result<R> {
> + 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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [pve-devel] [PATCH installer 13/14] fix #5536: post-hook: add some unit tests
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (11 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 12/14] fix #5536: add post-hook utility for sending notifications after auto-install Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-10 13:27 ` [pve-devel] [PATCH installer 14/14] unconfigured.sh: run proxmox-post-hook after successful auto-install Christoph Heiss
` (2 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-post-hook/src/main.rs | 126 ++++++++++++++++++++++++++++++++++
1 file changed, 126 insertions(+)
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index 9e5680b..dc25a79 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -370,3 +370,129 @@ fn main() -> ExitCode {
}
}
}
+
+#[cfg(test)]
+mod tests {
+ use crate::PostHookInfo;
+ use proxmox_installer_common::setup::{ProxmoxProduct, SetupInfo};
+
+ #[test]
+ fn dpkg_query_parsing_returns_correct_kernel() {
+ const OUTPUT: &str = r#"
+ii |all|proxmox-kernel-6.8
+
+un ||proxmox-kernel-6.8.8-2-pve
+foobarinvalidentry
+ii |amd64|proxmox-kernel-6.8.8-2-pve-signed
+ "#;
+
+ assert_eq!(
+ PostHookInfo::parse_dpkg_query_kernel_output(OUTPUT, "amd64")
+ .as_deref()
+ .unwrap(),
+ "proxmox-kernel-6.8.8-2-pve-signed"
+ );
+ }
+
+ #[test]
+ fn dpkg_query_parsing_returns_error_on_invalid_arch() {
+ const OUTPUT: &str = r#"
+ii |all|proxmox-kernel-6.8
+
+un ||proxmox-kernel-6.8.8-2-pve
+foobarinvalidentry
+ii |amd64|proxmox-kernel-6.8.8-2-pve-signed
+ "#;
+
+ assert_eq!(
+ PostHookInfo::parse_dpkg_query_kernel_output(OUTPUT, "arm64")
+ .as_deref()
+ .unwrap_err()
+ .to_string(),
+ "failed to find kernel package"
+ );
+ }
+
+ #[test]
+ fn correctly_parses_pveversion_output() {
+ const OUTPUT: &str = "pve-manager/8.2.4/faa83925c9641325 (running kernel: 6.8.8-2-pve)\n";
+
+ assert_eq!(
+ PostHookInfo::gather_product_version(&SetupInfo::mocked(), &|_| Ok(OUTPUT.to_owned()))
+ .as_deref()
+ .unwrap(),
+ "pve-manager/8.2.4/faa83925c9641325"
+ );
+ }
+
+ #[test]
+ fn fails_to_parse_invalid_pveversion_output() {
+ assert_eq!(
+ PostHookInfo::gather_product_version(&SetupInfo::mocked(), &|_| Ok("".to_owned()))
+ .as_deref()
+ .unwrap_err()
+ .to_string(),
+ "failed to parse `pveversion` output"
+ );
+
+ assert_eq!(
+ PostHookInfo::gather_product_version(&SetupInfo::mocked(), &|_| Ok(
+ "invalid".to_owned()
+ ))
+ .as_deref()
+ .unwrap_err()
+ .to_string(),
+ "failed to parse `pveversion` output"
+ );
+ }
+
+ #[test]
+ fn correctly_parses_pmgversion_output() {
+ const OUTPUT: &str = "pmg-api/8.1.2/fd71566ae016 (running kernel: 6.8.8-1-pve)\n";
+
+ let mut setup_info = SetupInfo::mocked();
+ setup_info.config.product = ProxmoxProduct::PMG;
+ assert_eq!(
+ PostHookInfo::gather_product_version(&setup_info, &|_| Ok(OUTPUT.to_owned()))
+ .as_deref()
+ .unwrap(),
+ "pmg-api/8.1.2/fd71566ae016"
+ );
+ }
+
+ #[test]
+ fn correctly_parses_proxmox_backup_manager_version_output() {
+ const OUTPUT: &str = r#"[{"Arch":"amd64","Description":"Proxmox Backup Server daemon with tools and GUI\n This package contains the Proxmox Backup Server daemons and related\n tools. This includes a web-based graphical user interface.","ExtraInfo":"running version: 3.2.7","OldVersion":"3.2.7-1","Origin":"Proxmox","Package":"proxmox-backup-server","Priority":"optional","Section":"admin","Title":"Proxmox Backup Server daemon with tools and GUI","Version":"3.2.7-1"}]"#;
+
+ let mut setup_info = SetupInfo::mocked();
+ setup_info.config.product = ProxmoxProduct::PBS;
+
+ assert_eq!(
+ PostHookInfo::gather_product_version(&setup_info, &|_| Ok(OUTPUT.to_owned()))
+ .as_deref()
+ .unwrap(),
+ "proxmox-backup-server/3.2.7-1"
+ );
+ }
+
+ #[test]
+ fn fails_to_parse_invalid_proxmox_backup_manager_version_output() {
+ let mut setup_info = SetupInfo::mocked();
+ setup_info.config.product = ProxmoxProduct::PBS;
+
+ assert_eq!(
+ PostHookInfo::gather_product_version(&setup_info, &|_| Ok("".to_owned()))
+ .as_deref()
+ .unwrap_err()
+ .to_string(),
+ "failed to parse json output from 'proxmox-backup-manager'"
+ );
+ assert_eq!(
+ PostHookInfo::gather_product_version(&setup_info, &|_| Ok("invalid".to_owned()))
+ .as_deref()
+ .unwrap_err()
+ .to_string(),
+ "failed to parse json output from 'proxmox-backup-manager'"
+ );
+ }
+}
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* [pve-devel] [PATCH installer 14/14] unconfigured.sh: run proxmox-post-hook after successful auto-install
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (12 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 13/14] fix #5536: post-hook: add some unit tests Christoph Heiss
@ 2024-07-10 13:27 ` Christoph Heiss
2024-07-11 16:49 ` [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Stefan Hanreich
2024-07-15 10:42 ` Thomas Lamprecht
15 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-10 13:27 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
unconfigured.sh | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/unconfigured.sh b/unconfigured.sh
index a39e6ad..34951e7 100755
--- a/unconfigured.sh
+++ b/unconfigured.sh
@@ -253,7 +253,10 @@ elif [ $start_auto_installer -ne 0 ]; then
debugsh || true
fi
echo "Starting automatic installation"
- /usr/bin/proxmox-auto-installer </run/automatic-installer-answers
+
+ if /usr/bin/proxmox-auto-installer </run/automatic-installer-answers; then
+ /usr/bin/proxmox-post-hook </run/automatic-installer-answers
+ fi
else
echo "Starting the installer GUI - see tty2 (CTRL+ALT+F2) for any errors..."
xinit -- -dpi "$DPI" -s 0 >/dev/tty2 2>&1
@@ -262,7 +265,7 @@ fi
# just to be sure everything is on disk
sync
-if [ $proxdebug -ne 0 ]; then
+if [ $proxdebug -ne 0 ]; then
printf "\nDebug shell after installation exited (type exit or CTRL-D to reboot)\n"
debugsh || true
fi
--
2.45.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (13 preceding siblings ...)
2024-07-10 13:27 ` [pve-devel] [PATCH installer 14/14] unconfigured.sh: run proxmox-post-hook after successful auto-install Christoph Heiss
@ 2024-07-11 16:49 ` Stefan Hanreich
2024-07-12 9:11 ` Christoph Heiss
2024-07-15 10:42 ` Thomas Lamprecht
15 siblings, 1 reply; 25+ messages in thread
From: Stefan Hanreich @ 2024-07-11 16:49 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Did a quick smoke test of this series by creating an ISO with an answer
file baked in and checking the response via `nc -l`. Review is inline.
Consider this:
Tested-By: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-By: Stefan Hanreich <s.hanreich@proxmox.com>
On 7/10/24 15:27, Christoph Heiss wrote:
> This implements a mechanism for post-installation "notifications" via a
> POST request [0] when using the auto-installer.
>
> It's implemented as a separate, small utility to facilitate separation
> of concerns and make the information gathering easier by having it
> isolated in one place.
>
> Patches #1 through #10 are simply clean-ups, refactors, etc. that were
> done along the way, which do not impact functionality in any way.
>
> Most interesting here will be patch #12, which adds the actual
> implementation of the post-hook. (Bind-)mounting the installed host
> system is done using the existing `proxmox-chroot` utility, and the HTTP
> POST functionality can fortunately be re-used 1:1 from
> `proxmox-fetch-answer`.
>
> I've also included an example of how the JSON body (pretty-printed for
> readability) of such a post-installation request would look like below,
> for reference.
>
> Tested this with both PVE and PBS ISOs, PMG did not (yet) have a
> release with an auto-installation capable ISO. The only product-specific
> code is the version detection in `proxmox-post-hook`, which - since it
> works the same for PVE and PMG - be no obstacle.
>
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5536
>
> {
> "debian-version": "12.5",
> "product-version": "pve-manager/8.2.2/9355359cd7afbae4",
> "kernel-version": "proxmox-kernel-6.8.4-2-pve-signed",
> "boot-type": "bios",
> "filesystem": "zfs (RAID1)",
> "fqdn": "host.domain",
> "machine-id": "f4bf9711783248b7aaffe3ccbca3e3dc",
> "bootdisk": [
> {
> "size": 8589934592,
> "udev-properties": {
> "DEVNAME": "/dev/vda", [..]
> }
> },
> {
> "size": 8589934592,
> "udev-properties": {
> "DEVNAME": "/dev/vdb", [..]
> }
> }
> ],
> "management-nic": {
> "mac": "de:ad:f0:0d:12:34",
> "address": "10.0.0.10/24",
> "udev-properties": {
> "INTERFACE": "enp6s18", [..]
> }
> },
> "ssh-public-host-keys": {
> "ecdsa": "ecdsa-sha2-nistp256 [..] root@host",
> "ed25519": "ssh-ed25519 [..] root@host",
> "rsa": "ssh-rsa [..] root@host",
> }
> }
>
> Christoph Heiss (14): chroot: print full anyhow message
> tree-wide: fix some typos
> tree-wide: collect hardcoded installer runtime directory strings into
> constant
> common: simplify filesystem type serializing & Display trait impl
> common: setup: serialize `target_hd` as string explicitly
> common: split out installer setup files loading functionality
> debian: strip unused library dependencies
> fetch-answer: move http-related code to gated module in
> installer-common
> tree-wide: convert some more crates to use workspace dependencies
> auto-installer: tests: replace left/right with got/expected in output
> auto-installer: answer: add `posthook` section
> fix #5536: add post-hook utility for sending notifications after
> auto-install
> fix #5536: post-hook: add some unit tests
> unconfigured.sh: run proxmox-post-hook after successful auto-install
>
> Cargo.toml | 11 +
> Makefile | 8 +-
> debian/control | 1 +
> debian/install | 1 +
> debian/rules | 9 +
> proxmox-auto-install-assistant/Cargo.toml | 14 +-
> proxmox-auto-installer/Cargo.toml | 15 +-
> proxmox-auto-installer/src/answer.rs | 27 +-
> .../src/bin/proxmox-auto-installer.rs | 15 +-
> proxmox-auto-installer/src/sysinfo.rs | 10 +-
> proxmox-auto-installer/src/utils.rs | 15 +-
> proxmox-auto-installer/tests/parse-answer.rs | 42 +-
> proxmox-chroot/Cargo.toml | 8 +-
> proxmox-chroot/src/main.rs | 19 +-
> proxmox-fetch-answer/Cargo.toml | 17 +-
> .../src/fetch_plugins/http.rs | 100 +---
> .../src/fetch_plugins/partition.rs | 14 +-
> proxmox-installer-common/Cargo.toml | 26 +-
> proxmox-installer-common/src/http.rs | 94 ++++
> proxmox-installer-common/src/lib.rs | 5 +
> proxmox-installer-common/src/options.rs | 109 ++--
> proxmox-installer-common/src/setup.rs | 108 +---
> proxmox-installer-common/src/utils.rs | 2 +
> proxmox-post-hook/Cargo.toml | 19 +
> proxmox-post-hook/src/main.rs | 498 ++++++++++++++++++
> proxmox-tui-installer/Cargo.toml | 8 +-
> proxmox-tui-installer/src/setup.rs | 2 +-
> unconfigured.sh | 7 +-
> 28 files changed, 862 insertions(+), 342 deletions(-)
> create mode 100644 proxmox-installer-common/src/http.rs
> create mode 100644 proxmox-post-hook/Cargo.toml
> create mode 100644 proxmox-post-hook/src/main.rs
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism
2024-07-11 16:49 ` [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Stefan Hanreich
@ 2024-07-12 9:11 ` Christoph Heiss
0 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-12 9:11 UTC (permalink / raw)
To: Stefan Hanreich; +Cc: Proxmox VE development discussion
On Thu, Jul 11, 2024 at 06:49:28PM GMT, Stefan Hanreich wrote:
> Did a quick smoke test of this series by creating an ISO with an answer
> file baked in and checking the response via `nc -l`. Review is inline.
Thanks for the review & testing!
From a quick glance all the comments make sense, I'll address them in a
v2.
>
> Consider this:
>
> Tested-By: Stefan Hanreich <s.hanreich@proxmox.com>
> Reviewed-By: Stefan Hanreich <s.hanreich@proxmox.com>
>
>
> On 7/10/24 15:27, Christoph Heiss wrote:
> > This implements a mechanism for post-installation "notifications" via a
> > POST request [0] when using the auto-installer.
> >
> > It's implemented as a separate, small utility to facilitate separation
> > of concerns and make the information gathering easier by having it
> > isolated in one place.
> >
> > Patches #1 through #10 are simply clean-ups, refactors, etc. that were
> > done along the way, which do not impact functionality in any way.
> >
> > Most interesting here will be patch #12, which adds the actual
> > implementation of the post-hook. (Bind-)mounting the installed host
> > system is done using the existing `proxmox-chroot` utility, and the HTTP
> > POST functionality can fortunately be re-used 1:1 from
> > `proxmox-fetch-answer`.
> >
> > I've also included an example of how the JSON body (pretty-printed for
> > readability) of such a post-installation request would look like below,
> > for reference.
> >
> > Tested this with both PVE and PBS ISOs, PMG did not (yet) have a
> > release with an auto-installation capable ISO. The only product-specific
> > code is the version detection in `proxmox-post-hook`, which - since it
> > works the same for PVE and PMG - be no obstacle.
> >
> > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5536
> >
> > {
> > "debian-version": "12.5",
> > "product-version": "pve-manager/8.2.2/9355359cd7afbae4",
> > "kernel-version": "proxmox-kernel-6.8.4-2-pve-signed",
> > "boot-type": "bios",
> > "filesystem": "zfs (RAID1)",
> > "fqdn": "host.domain",
> > "machine-id": "f4bf9711783248b7aaffe3ccbca3e3dc",
> > "bootdisk": [
> > {
> > "size": 8589934592,
> > "udev-properties": {
> > "DEVNAME": "/dev/vda", [..]
> > }
> > },
> > {
> > "size": 8589934592,
> > "udev-properties": {
> > "DEVNAME": "/dev/vdb", [..]
> > }
> > }
> > ],
> > "management-nic": {
> > "mac": "de:ad:f0:0d:12:34",
> > "address": "10.0.0.10/24",
> > "udev-properties": {
> > "INTERFACE": "enp6s18", [..]
> > }
> > },
> > "ssh-public-host-keys": {
> > "ecdsa": "ecdsa-sha2-nistp256 [..] root@host",
> > "ed25519": "ssh-ed25519 [..] root@host",
> > "rsa": "ssh-rsa [..] root@host",
> > }
> > }
> >
> > Christoph Heiss (14): chroot: print full anyhow message
> > tree-wide: fix some typos
> > tree-wide: collect hardcoded installer runtime directory strings into
> > constant
> > common: simplify filesystem type serializing & Display trait impl
> > common: setup: serialize `target_hd` as string explicitly
> > common: split out installer setup files loading functionality
> > debian: strip unused library dependencies
> > fetch-answer: move http-related code to gated module in
> > installer-common
> > tree-wide: convert some more crates to use workspace dependencies
> > auto-installer: tests: replace left/right with got/expected in output
> > auto-installer: answer: add `posthook` section
> > fix #5536: add post-hook utility for sending notifications after
> > auto-install
> > fix #5536: post-hook: add some unit tests
> > unconfigured.sh: run proxmox-post-hook after successful auto-install
> >
> > Cargo.toml | 11 +
> > Makefile | 8 +-
> > debian/control | 1 +
> > debian/install | 1 +
> > debian/rules | 9 +
> > proxmox-auto-install-assistant/Cargo.toml | 14 +-
> > proxmox-auto-installer/Cargo.toml | 15 +-
> > proxmox-auto-installer/src/answer.rs | 27 +-
> > .../src/bin/proxmox-auto-installer.rs | 15 +-
> > proxmox-auto-installer/src/sysinfo.rs | 10 +-
> > proxmox-auto-installer/src/utils.rs | 15 +-
> > proxmox-auto-installer/tests/parse-answer.rs | 42 +-
> > proxmox-chroot/Cargo.toml | 8 +-
> > proxmox-chroot/src/main.rs | 19 +-
> > proxmox-fetch-answer/Cargo.toml | 17 +-
> > .../src/fetch_plugins/http.rs | 100 +---
> > .../src/fetch_plugins/partition.rs | 14 +-
> > proxmox-installer-common/Cargo.toml | 26 +-
> > proxmox-installer-common/src/http.rs | 94 ++++
> > proxmox-installer-common/src/lib.rs | 5 +
> > proxmox-installer-common/src/options.rs | 109 ++--
> > proxmox-installer-common/src/setup.rs | 108 +---
> > proxmox-installer-common/src/utils.rs | 2 +
> > proxmox-post-hook/Cargo.toml | 19 +
> > proxmox-post-hook/src/main.rs | 498 ++++++++++++++++++
> > proxmox-tui-installer/Cargo.toml | 8 +-
> > proxmox-tui-installer/src/setup.rs | 2 +-
> > unconfigured.sh | 7 +-
> > 28 files changed, 862 insertions(+), 342 deletions(-)
> > create mode 100644 proxmox-installer-common/src/http.rs
> > create mode 100644 proxmox-post-hook/Cargo.toml
> > create mode 100644 proxmox-post-hook/src/main.rs
> >
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism
2024-07-10 13:27 [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (14 preceding siblings ...)
2024-07-11 16:49 ` [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism Stefan Hanreich
@ 2024-07-15 10:42 ` Thomas Lamprecht
2024-07-15 14:31 ` Christoph Heiss
15 siblings, 1 reply; 25+ messages in thread
From: Thomas Lamprecht @ 2024-07-15 10:42 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Am 10/07/2024 um 15:27 schrieb Christoph Heiss:
> This implements a mechanism for post-installation "notifications" via a
> POST request [0] when using the auto-installer.
>
> It's implemented as a separate, small utility to facilitate separation
> of concerns and make the information gathering easier by having it
> isolated in one place.
>
> Patches #1 through #10 are simply clean-ups, refactors, etc. that were
> done along the way, which do not impact functionality in any way.
>
> Most interesting here will be patch #12, which adds the actual
> implementation of the post-hook. (Bind-)mounting the installed host
> system is done using the existing `proxmox-chroot` utility, and the HTTP
> POST functionality can fortunately be re-used 1:1 from
> `proxmox-fetch-answer`.
>
> I've also included an example of how the JSON body (pretty-printed for
> readability) of such a post-installation request would look like below,
> for reference.
>
> Tested this with both PVE and PBS ISOs, PMG did not (yet) have a
> release with an auto-installation capable ISO. The only product-specific
> code is the version detection in `proxmox-post-hook`, which - since it
> works the same for PVE and PMG - be no obstacle.
>
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5536
>
> {
> "debian-version": "12.5",
> "product-version": "pve-manager/8.2.2/9355359cd7afbae4",
> "kernel-version": "proxmox-kernel-6.8.4-2-pve-signed",
no hard feelings, but from a gut feeling I'd probably move this to a
"version" object and potentially use a more reduced, easier to parse, value.
Maybe also as an object so that both, a simple X.Y(.Z) release and a full
string are given, as we changed (kernel) packages name in the past, and I
could imagine that there will be a LTS and non LTS variant if we ever rework
on where we base our kernel on, so this might change in the future too.
While the simple X.Y.Z version will more likely stay as is.
And I do not want to move the goal post here to far, but isn't some of this
information potentially interesting to have sent to a metric server?
At least with a low frequency (or just once on every boot) so that one has a
somewhat recent externally saved set of information that can be used to
identify machines more easily and be aware of some changes to correlate
regressions or strange (load) metrics with.
No need to do that now, but maybe something to keep in mind to allow easier
reuse of this.
IMO it's a big plus if we manage to keep information laid out the same way,
or at list in a similar one, wherever it's included. And that doesn't have
to mean that the whole struct has to be shared, maybe it just could be just
a collection of types that stem from common crates outside the installer
(at least in the long run, as said, no need to completely block this on
scope extension now).
> "boot-type": "bios",
We call this "mode" in the product APIs [0], might potentially make sense
to use the same schema here? Else I'd at least name this boot-mode and use
the same keys.
[0]: https://pve.proxmox.com/pve-docs/api-viewer/index.html#/nodes/{node}/status
> "filesystem": "zfs (RAID1)",
> "fqdn": "host.domain",
> "machine-id": "f4bf9711783248b7aaffe3ccbca3e3dc",
> "bootdisk": [
could be also interesting to get all disks and flag the ones used for booting
with a boolean "bootdisk" flag. E.g. to make additional storage provisioning
later on slightly more convenient.
> {
> "size": 8589934592,
> "udev-properties": {
> "DEVNAME": "/dev/vda", [..]
> }
> },
> {
> "size": 8589934592,
> "udev-properties": {
> "DEVNAME": "/dev/vdb", [..]
> }
> }
> ],
> "management-nic": {
> "mac": "de:ad:f0:0d:12:34",
> "address": "10.0.0.10/24",
> "udev-properties": {
> "INTERFACE": "enp6s18", [..]
> }
> },
> "ssh-public-host-keys": {
> "ecdsa": "ecdsa-sha2-nistp256 [..] root@host",
> "ed25519": "ssh-ed25519 [..] root@host",
> "rsa": "ssh-rsa [..] root@host",
> }
> }
>
> Christoph Heiss (14): chroot: print full anyhow message
> tree-wide: fix some typos
> tree-wide: collect hardcoded installer runtime directory strings into
> constant
> common: simplify filesystem type serializing & Display trait impl
> common: setup: serialize `target_hd` as string explicitly
> common: split out installer setup files loading functionality
> debian: strip unused library dependencies
> fetch-answer: move http-related code to gated module in
> installer-common
> tree-wide: convert some more crates to use workspace dependencies
> auto-installer: tests: replace left/right with got/expected in output
> auto-installer: answer: add `posthook` section
> fix #5536: add post-hook utility for sending notifications after
> auto-install
> fix #5536: post-hook: add some unit tests
> unconfigured.sh: run proxmox-post-hook after successful auto-install
>
> Cargo.toml | 11 +
> Makefile | 8 +-
> debian/control | 1 +
> debian/install | 1 +
> debian/rules | 9 +
> proxmox-auto-install-assistant/Cargo.toml | 14 +-
> proxmox-auto-installer/Cargo.toml | 15 +-
> proxmox-auto-installer/src/answer.rs | 27 +-
> .../src/bin/proxmox-auto-installer.rs | 15 +-
> proxmox-auto-installer/src/sysinfo.rs | 10 +-
> proxmox-auto-installer/src/utils.rs | 15 +-
> proxmox-auto-installer/tests/parse-answer.rs | 42 +-
> proxmox-chroot/Cargo.toml | 8 +-
> proxmox-chroot/src/main.rs | 19 +-
> proxmox-fetch-answer/Cargo.toml | 17 +-
> .../src/fetch_plugins/http.rs | 100 +---
> .../src/fetch_plugins/partition.rs | 14 +-
> proxmox-installer-common/Cargo.toml | 26 +-
> proxmox-installer-common/src/http.rs | 94 ++++
> proxmox-installer-common/src/lib.rs | 5 +
> proxmox-installer-common/src/options.rs | 109 ++--
> proxmox-installer-common/src/setup.rs | 108 +---
> proxmox-installer-common/src/utils.rs | 2 +
> proxmox-post-hook/Cargo.toml | 19 +
> proxmox-post-hook/src/main.rs | 498 ++++++++++++++++++
> proxmox-tui-installer/Cargo.toml | 8 +-
> proxmox-tui-installer/src/setup.rs | 2 +-
> unconfigured.sh | 7 +-
> 28 files changed, 862 insertions(+), 342 deletions(-)
> create mode 100644 proxmox-installer-common/src/http.rs
> create mode 100644 proxmox-post-hook/Cargo.toml
> create mode 100644 proxmox-post-hook/src/main.rs
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism
2024-07-15 10:42 ` Thomas Lamprecht
@ 2024-07-15 14:31 ` Christoph Heiss
2024-07-16 16:09 ` Thomas Lamprecht
0 siblings, 1 reply; 25+ messages in thread
From: Christoph Heiss @ 2024-07-15 14:31 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: Proxmox VE development discussion
Thanks for the feedback!
Since this will be externally consumed API surface, changing it
afterwards will be a pain anyway - so comments on the JSON schema are
really appreciated too.
On Mon, Jul 15, 2024 at 12:42:41PM GMT, Thomas Lamprecht wrote:
> Am 10/07/2024 um 15:27 schrieb Christoph Heiss:
> > [..]
> > {
> > "debian-version": "12.5",
> > "product-version": "pve-manager/8.2.2/9355359cd7afbae4",
> > "kernel-version": "proxmox-kernel-6.8.4-2-pve-signed",
>
> no hard feelings, but from a gut feeling I'd probably move this to a
> "version" object and potentially use a more reduced, easier to parse, value.
> Maybe also as an object so that both, a simple X.Y(.Z) release and a full
> string are given, as we changed (kernel) packages name in the past, and I
> could imagine that there will be a LTS and non LTS variant if we ever rework
> on where we base our kernel on, so this might change in the future too.
> While the simple X.Y.Z version will more likely stay as is.
Yeah, that's fair. While writing this I've basically just looked around
what information could potentially be interesting for
users/administrators and then dumped them into a JSON.
Having an actual structured version object here makes sense.
>
> And I do not want to move the goal post here to far, but isn't some of this
> information potentially interesting to have sent to a metric server?
Yeah, at least Prometheus' node_exporter does send such information, so
its quite "standard" practice.
> At least with a low frequency (or just once on every boot) so that one has a
> somewhat recent externally saved set of information that can be used to
> identify machines more easily and be aware of some changes to correlate
> regressions or strange (load) metrics with.
Just to get this right: is the idea to (optionally) send some/all of
this information to a external metrics server, in addition to the
posthook target?
Or just generally to keep in mind where information like this could go
and why it should be uniform?
>
> No need to do that now, but maybe something to keep in mind to allow easier
> reuse of this.
>
> IMO it's a big plus if we manage to keep information laid out the same way,
> or at list in a similar one, wherever it's included. And that doesn't have
> to mean that the whole struct has to be shared, maybe it just could be just
> a collection of types that stem from common crates outside the installer
> (at least in the long run, as said, no need to completely block this on
> scope extension now).
>
> > "boot-type": "bios",
>
> We call this "mode" in the product APIs [0], might potentially make sense
> to use the same schema here? Else I'd at least name this boot-mode and use
> the same keys.
>
> [0]: https://pve.proxmox.com/pve-docs/api-viewer/index.html#/nodes/{node}/status
Sounds good! Looking at the API, I'd take the entire "boot-info" object
from there and have it be the same.
I will generally try to align the schema of this JSON a bit more with
that returned by the API mentioned above. E.g. also the (kernel) version
object above with the "current-kernel" structure, as far as possible.
And looking at it, we also could include the "cpuinfo" structure for
good measure.
Being consistent (where sensible, as you say) with our existing API
makes very much sense, didn't think of this!
>
> > "filesystem": "zfs (RAID1)",
> > "fqdn": "host.domain",
> > "machine-id": "f4bf9711783248b7aaffe3ccbca3e3dc",
> > "bootdisk": [
>
> could be also interesting to get all disks and flag the ones used for booting
> with a boolean "bootdisk" flag. E.g. to make additional storage provisioning
> later on slightly more convenient.
With that in mind it definitely could come in handy. Or maybe a separate
object "disks"/"other-disks"/etc. entirely? So as not have to filter out
the (non-)bootdisks again on the receiving end.
While at it, the same would IMO make sense for NICs too, since one might
want to set up additional network devices too.
>
> > {
> > "size": 8589934592,
> > "udev-properties": {
> > "DEVNAME": "/dev/vda", [..]
> > }
> > },
> > {
> > "size": 8589934592,
> > "udev-properties": {
> > "DEVNAME": "/dev/vdb", [..]
> > }
> > }
> > ],
> > "management-nic": {
> > "mac": "de:ad:f0:0d:12:34",
> > "address": "10.0.0.10/24",
> > "udev-properties": {
> > "INTERFACE": "enp6s18", [..]
> > }
> > },
> > "ssh-public-host-keys": {
> > "ecdsa": "ecdsa-sha2-nistp256 [..] root@host",
> > "ed25519": "ssh-ed25519 [..] root@host",
> > "rsa": "ssh-rsa [..] root@host",
> > }
> > }
> > [..]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism
2024-07-15 14:31 ` Christoph Heiss
@ 2024-07-16 16:09 ` Thomas Lamprecht
2024-07-17 7:25 ` Christoph Heiss
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Lamprecht @ 2024-07-16 16:09 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Am 15/07/2024 um 16:31 schrieb Christoph Heiss:
> With that in mind it definitely could come in handy. Or maybe a separate
> object "disks"/"other-disks"/etc. entirely? So as not have to filter out
> the (non-)bootdisks again on the receiving end.
Could be fine too, albeit I'd slightly prefer a single disk property, as it
feels more naturally to me to filter on properties compared to checking, or
having to combine, different array sets. But not too hard feelings here,
maybe someone else got some (stronger) opinion.
> While at it, the same would IMO make sense for NICs too, since one might
> want to set up additional network devices too.
good point.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism
2024-07-16 16:09 ` Thomas Lamprecht
@ 2024-07-17 7:25 ` Christoph Heiss
0 siblings, 0 replies; 25+ messages in thread
From: Christoph Heiss @ 2024-07-17 7:25 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: Proxmox VE development discussion
On Tue, Jul 16, 2024 at 06:09:17PM GMT, Thomas Lamprecht wrote:
> Am 15/07/2024 um 16:31 schrieb Christoph Heiss:
> > With that in mind it definitely could come in handy. Or maybe a separate
> > object "disks"/"other-disks"/etc. entirely? So as not have to filter out
> > the (non-)bootdisks again on the receiving end.
>
> Could be fine too, albeit I'd slightly prefer a single disk property, as it
> feels more naturally to me to filter on properties compared to checking, or
> having to combine, different array sets. But not too hard feelings here,
> maybe someone else got some (stronger) opinion.
If nobody else voices their opinion over that I'll go ahead with your
idea, a single property. Both have merits, but having them a single
property is more universal I guess.
>
> > While at it, the same would IMO make sense for NICs too, since one might
> > want to set up additional network devices too.
>
> good point.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 25+ messages in thread