* [pve-devel] [PATCH installer v2 01/17] tree-wide: fix some typos
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 02/17] fetch-answer: partition: fix clippy warning Christoph Heiss
` (17 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch
---
proxmox-auto-install-assistant/src/main.rs | 4 ++--
proxmox-installer-common/src/disk_checks.rs | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index 1447175..c6f1ec8 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -55,7 +55,7 @@ struct CommandDeviceInfo {
/// Filters support the following syntax:
/// ? Match a single character
/// * Match any number of characters
-/// [a], [0-9] Specifc character or range of characters
+/// [a], [0-9] Specific character or range of characters
/// [!a] Negate a specific character of range
///
/// To avoid globbing characters being interpreted by the shell, use single quotes.
@@ -419,7 +419,7 @@ fn get_iso_uuid(iso: &PathBuf) -> Result<String> {
uuid = line
.split(' ')
.last()
- .ok_or_else(|| format_err!("xorriso did behave unexpextedly"))?
+ .ok_or_else(|| format_err!("xorriso did behave unexpectedly"))?
.replace('\'', "")
.trim()
.into();
diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
index 89b300c..ecc43bd 100644
--- a/proxmox-installer-common/src/disk_checks.rs
+++ b/proxmox-installer-common/src/disk_checks.rs
@@ -24,7 +24,7 @@ pub fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
///
/// # Arguments
///
-/// * `disks` - A list of disks to check the lenght of.
+/// * `disks` - A list of disks to check the length of.
/// * `min` - Minimum number of disks
pub fn check_raid_min_disks(disks: &[Disk], min: usize) -> Result<(), String> {
if disks.len() < min {
--
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] 34+ messages in thread* [pve-devel] [PATCH installer v2 02/17] fetch-answer: partition: fix clippy warning
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 01/17] tree-wide: fix some typos Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 03/17] common: simplify filesystem type serializing & Display trait impl Christoph Heiss
` (16 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
warning: the borrowed expression implements the required traits
--> proxmox-fetch-answer/src/fetch_plugins/partition.rs:34:44
|
34 | let path = Path::new(search_path).join(&file_name);
| ^^^^^^^^^^ help: change this to: `file_name`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
= note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch, split out from unrelated patch
---
proxmox-fetch-answer/src/fetch_plugins/partition.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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),
--
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] 34+ messages in thread* [pve-devel] [PATCH installer v2 03/17] common: simplify filesystem type serializing & Display trait impl
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 01/17] tree-wide: fix some typos Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 02/17] fetch-answer: partition: fix clippy warning Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 04/17] common: setup: serialize `target_hd` as string explicitly Christoph Heiss
` (15 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 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>
---
Changes v1 -> v2:
* implement FromStr instead of Deserialize and use
serde_plain::derive_deserialize_from_fromstr!()
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-installer-common/Cargo.toml | 1 +
proxmox-installer-common/src/options.rs | 116 +++++++++++++-----------
proxmox-installer-common/src/setup.rs | 59 +-----------
3 files changed, 67 insertions(+), 109 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..32c62a7 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -1,5 +1,6 @@
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
use std::net::{IpAddr, Ipv4Addr};
+use std::str::FromStr;
use std::{cmp, fmt};
use crate::setup::{
@@ -7,55 +8,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,
@@ -71,16 +50,59 @@ impl FsType {
impl fmt::Display for FsType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- use FsType::*;
+ // Values displayed to the user in the installer UI
match self {
- Ext4 => write!(f, "ext4"),
- Xfs => write!(f, "XFS"),
- Zfs(level) => write!(f, "ZFS ({level})"),
- Btrfs(level) => write!(f, "Btrfs ({level})"),
+ FsType::Ext4 => write!(f, "ext4"),
+ FsType::Xfs => write!(f, "XFS"),
+ FsType::Zfs(level) => write!(f, "ZFS ({level})"),
+ FsType::Btrfs(level) => write!(f, "Btrfs ({level})"),
+ }
+ }
+}
+
+impl Serialize for FsType {
+ fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+ where
+ S: serde::Serializer,
+ {
+ // These values must match exactly what the low-level installer expects
+ 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)
+ }
+}
+
+impl FromStr for FsType {
+ type Err = String;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ match s {
+ "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(format!("Could not find file system: {s}")),
}
}
}
+serde_plain::derive_deserialize_from_fromstr!(FsType, "valid filesystem");
+
#[derive(Clone, Debug)]
pub struct LvmBootdiskOptions {
pub total_size: f64,
@@ -119,8 +141,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 +154,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 +167,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 +176,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 804da1a..e0ac8d6 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] 34+ messages in thread* [pve-devel] [PATCH installer v2 04/17] common: setup: serialize `target_hd` as string explicitly
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (2 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 03/17] common: simplify filesystem type serializing & Display trait impl Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 05/17] common: split out installer setup files loading functionality Christoph Heiss
` (14 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
---
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 cc47f5f..fefcac9 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 e0ac8d6..c0d701e 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 02d9ece..033d642 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] 34+ messages in thread* [pve-devel] [PATCH installer v2 05/17] common: split out installer setup files loading functionality
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (3 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 04/17] common: setup: serialize `target_hd` as string explicitly Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 06/17] common: setup: deserialize `secure_boot` property from runtime env Christoph Heiss
` (13 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 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>
---
Changes v1 -> v2:
* change `&PathBuf` parameter to `impl AsRef<Path>`
---
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 c0d701e..ee3d0c9 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(base_path)
+}
+
+pub fn load_installer_setup_files(
+ base_path: impl AsRef<Path>,
+) -> Result<(SetupInfo, LocaleInfo, RuntimeInfo), String> {
let installer_info: SetupInfo = {
- let mut path = path.clone();
+ let mut path = base_path.as_ref().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.as_ref().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.as_ref().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] 34+ messages in thread* [pve-devel] [PATCH installer v2 06/17] common: setup: deserialize `secure_boot` property from runtime env
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (4 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 05/17] common: split out installer setup files loading functionality Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-23 14:31 ` Aaron Lauterer
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 07/17] debian: strip unused library dependencies Christoph Heiss
` (12 subsequent siblings)
18 siblings, 1 reply; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
Needed for the post-hook functionality, which sends this information as
part of its information set.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch
---
Proxmox/Install/RunEnv.pm | 1 +
proxmox-installer-common/src/setup.rs | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index 7eaf96a..bb60080 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -236,6 +236,7 @@ my sub detect_country_tracing_to : prototype($$) {
# kernel_cmdline = <contents of /proc/cmdline>,
# total_memory = <memory size in MiB>,
# hvm_supported = <1 if the CPU supports hardware-accelerated virtualization>,
+# secure_boot = <1 if SecureBoot is enabled>,
# boot_type = <either 'efi' or 'bios'>,
# disks => <see Proxmox::Sys::Block::hd_list()>,
# network => {
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index ee3d0c9..2ca9641 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -236,6 +236,14 @@ where
Ok(val != 0)
}
+fn deserialize_bool_from_int_maybe<'de, D>(deserializer: D) -> Result<Option<bool>, D::Error>
+where
+ D: Deserializer<'de>,
+{
+ let val: Option<u32> = Deserialize::deserialize(deserializer)?;
+ Ok(val.map(|v| v != 0))
+}
+
fn deserialize_cczones_map<'de, D>(
deserializer: D,
) -> Result<HashMap<String, Vec<String>>, D::Error>
@@ -333,6 +341,10 @@ pub struct RuntimeInfo {
/// Whether the CPU supports hardware-accelerated virtualization
#[serde(deserialize_with = "deserialize_bool_from_int")]
pub hvm_supported: bool,
+
+ /// Whether the system was booted with SecureBoot enabled
+ #[serde(default, deserialize_with = "deserialize_bool_from_int_maybe")]
+ pub secure_boot: Option<bool>,
}
#[derive(Copy, Clone, Eq, Deserialize, PartialEq)]
--
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] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 06/17] common: setup: deserialize `secure_boot` property from runtime env
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 06/17] common: setup: deserialize `secure_boot` property from runtime env Christoph Heiss
@ 2024-07-23 14:31 ` Aaron Lauterer
2024-08-05 13:12 ` Christoph Heiss
2024-08-19 10:32 ` Christoph Heiss
0 siblings, 2 replies; 34+ messages in thread
From: Aaron Lauterer @ 2024-07-23 14:31 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
In my tests, with secure boot disabled, it failed to parse the
run-env-info.json because the Perl code wrote it this way:
"secure_boot":""
And it currently cannot parse a string. Setting it manually to:
"secure_boot":0
helped. The question is, if we want the parser to be more flexible or
fix the Perl code that dumps that info.
On 2024-07-18 15:48, Christoph Heiss wrote:
> Needed for the post-hook functionality, which sends this information as
> part of its information set.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v1 -> v2:
> * new patch
> ---
> Proxmox/Install/RunEnv.pm | 1 +
> proxmox-installer-common/src/setup.rs | 12 ++++++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
> index 7eaf96a..bb60080 100644
> --- a/Proxmox/Install/RunEnv.pm
> +++ b/Proxmox/Install/RunEnv.pm
> @@ -236,6 +236,7 @@ my sub detect_country_tracing_to : prototype($$) {
> # kernel_cmdline = <contents of /proc/cmdline>,
> # total_memory = <memory size in MiB>,
> # hvm_supported = <1 if the CPU supports hardware-accelerated virtualization>,
> +# secure_boot = <1 if SecureBoot is enabled>,
> # boot_type = <either 'efi' or 'bios'>,
> # disks => <see Proxmox::Sys::Block::hd_list()>,
> # network => {
> diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
> index ee3d0c9..2ca9641 100644
> --- a/proxmox-installer-common/src/setup.rs
> +++ b/proxmox-installer-common/src/setup.rs
> @@ -236,6 +236,14 @@ where
> Ok(val != 0)
> }
>
> +fn deserialize_bool_from_int_maybe<'de, D>(deserializer: D) -> Result<Option<bool>, D::Error>
> +where
> + D: Deserializer<'de>,
> +{
> + let val: Option<u32> = Deserialize::deserialize(deserializer)?;
> + Ok(val.map(|v| v != 0))
> +}
> +
> fn deserialize_cczones_map<'de, D>(
> deserializer: D,
> ) -> Result<HashMap<String, Vec<String>>, D::Error>
> @@ -333,6 +341,10 @@ pub struct RuntimeInfo {
> /// Whether the CPU supports hardware-accelerated virtualization
> #[serde(deserialize_with = "deserialize_bool_from_int")]
> pub hvm_supported: bool,
> +
> + /// Whether the system was booted with SecureBoot enabled
> + #[serde(default, deserialize_with = "deserialize_bool_from_int_maybe")]
> + pub secure_boot: Option<bool>,
> }
>
> #[derive(Copy, Clone, Eq, Deserialize, PartialEq)]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 06/17] common: setup: deserialize `secure_boot` property from runtime env
2024-07-23 14:31 ` Aaron Lauterer
@ 2024-08-05 13:12 ` Christoph Heiss
2024-08-19 10:32 ` Christoph Heiss
1 sibling, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-08-05 13:12 UTC (permalink / raw)
To: Aaron Lauterer; +Cc: Proxmox VE development discussion
On Tue, Jul 23, 2024 at 04:31:35PM GMT, Aaron Lauterer wrote:
> In my tests, with secure boot disabled, it failed to parse the
> run-env-info.json because the Perl code wrote it this way:
>
> "secure_boot":""
>
> And it currently cannot parse a string. Setting it manually to:
>
> "secure_boot":0
>
> helped. The question is, if we want the parser to be more flexible or fix
> the Perl code that dumps that info.
Good catch!
I'd go the route of fixing the Perl side, such that it is always either
0 or 1 for consistency.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH installer v2 06/17] common: setup: deserialize `secure_boot` property from runtime env
2024-07-23 14:31 ` Aaron Lauterer
2024-08-05 13:12 ` Christoph Heiss
@ 2024-08-19 10:32 ` Christoph Heiss
2024-08-20 14:56 ` Christoph Heiss
1 sibling, 1 reply; 34+ messages in thread
From: Christoph Heiss @ 2024-08-19 10:32 UTC (permalink / raw)
To: Aaron Lauterer; +Cc: Proxmox VE development discussion
On Tue, Jul 23, 2024 at 04:31:35PM GMT, Aaron Lauterer wrote:
> In my tests, with secure boot disabled, it failed to parse the
> run-env-info.json because the Perl code wrote it this way:
>
> "secure_boot":""
>
> And it currently cannot parse a string. Setting it manually to:
>
> "secure_boot":0
>
> helped. The question is, if we want the parser to be more flexible or fix
> the Perl code that dumps that info.
FWIW: Testing this, I couldn't actually reproduce this.
Re-reading the dumping code in Proxmox/Install/RunEnv.pm, I'd also
expect this. In the non-secureboot case, the hash member isn't set, so
the key should not be dumped at all. Otherwise, 1 as value gets dumped.
So I'm not sure how the perl code would even get to dumping an empty
string ..
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH installer v2 06/17] common: setup: deserialize `secure_boot` property from runtime env
2024-08-19 10:32 ` Christoph Heiss
@ 2024-08-20 14:56 ` Christoph Heiss
0 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-08-20 14:56 UTC (permalink / raw)
To: Proxmox VE development discussion
On Mon, Aug 19, 2024 at 12:32:09PM GMT, Christoph Heiss wrote:
> On Tue, Jul 23, 2024 at 04:31:35PM GMT, Aaron Lauterer wrote:
> > In my tests, with secure boot disabled, it failed to parse the
> > run-env-info.json because the Perl code wrote it this way:
> >
> > "secure_boot":""
> >
> > And it currently cannot parse a string. Setting it manually to:
> >
> > "secure_boot":0
> >
> > helped. The question is, if we want the parser to be more flexible or fix
> > the Perl code that dumps that info.
>
> FWIW: Testing this, I couldn't actually reproduce this.
Well, I've got to correct myself on that .. for whatever reason, I
seem to have tested this with a SeaBIOS machine, not a OVMF machine with
SB disabled.
In the former case, it works by having the member simply not present. In
the latter, it indeed dumped it as "secure_boot": ""
In any case - thanks for the testing, fixed it for the next revision!
>
> Re-reading the dumping code in Proxmox/Install/RunEnv.pm, I'd also
> expect this. In the non-secureboot case, the hash member isn't set, so
> the key should not be dumped at all. Otherwise, 1 as value gets dumped.
>
> So I'm not sure how the perl code would even get to dumping an empty
> string ..
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH installer v2 07/17] debian: strip unused library dependencies
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (5 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 06/17] common: setup: deserialize `secure_boot` property from runtime env Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 08/17] fetch-answer: move http-related code to gated module in installer-common Christoph Heiss
` (11 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
Rust links in some dynamic libraries even if only used by a disabled
feature gate.
This will be needed due to moving http-related code into the
proxmox-installer-common crate and thus pulling it in at more places.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* print libraries being stripped from each binary
---
debian/control | 1 +
debian/rules | 13 +++++++++++++
2 files changed, 14 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..3cebdff 100755
--- a/debian/rules
+++ b/debian/rules
@@ -10,3 +10,16 @@ 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 \
+ deps="$$(ldd -u "$$f" | grep -oP '[^/:]+$$')"; \
+ if [ ! -z "$$deps" ]; then \
+ printf "stripping unused dependencies from $$f: "; \
+ echo "$$deps" | tr '\n' ' '; echo; \
+ echo "$$deps" | sed 's/^/--remove-needed /g' | xargs patchelf "$$f"; \
+ fi \
+ 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] 34+ messages in thread* [pve-devel] [PATCH installer v2 08/17] fetch-answer: move http-related code to gated module in installer-common
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (6 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 07/17] debian: strip unused library dependencies Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 09/17] tree-wide: convert some more crates to use workspace dependencies Christoph Heiss
` (10 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
This enable reusage of this code in other crates. Needed esp. by the
upcoming post-installation notification hook functionality.
No functional changes overall.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
---
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] 34+ messages in thread* [pve-devel] [PATCH installer v2 09/17] tree-wide: convert some more crates to use workspace dependencies
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (7 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 08/17] fetch-answer: move http-related code to gated module in installer-common Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 10/17] auto-install-assistant: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
` (9 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
---
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] 34+ messages in thread* [pve-devel] [PATCH installer v2 10/17] auto-install-assistant: replace `PathBuf` parameters with `AsRef<Path>`
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (8 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 09/17] tree-wide: convert some more crates to use workspace dependencies Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!() Christoph Heiss
` (8 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch, suggestion by Stefan
---
proxmox-auto-install-assistant/src/main.rs | 23 +++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index c6f1ec8..3cf42f6 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -5,7 +5,7 @@ use regex::Regex;
use serde::Serialize;
use std::{
collections::BTreeMap,
- fs,
+ fmt, fs,
io::{self, Read},
path::{Path, PathBuf},
process::{Command, Stdio},
@@ -377,7 +377,12 @@ fn final_iso_location(args: &CommandPrepareISO) -> PathBuf {
target.to_path_buf()
}
-fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str, uuid: &String) -> Result<()> {
+fn inject_file_to_iso(
+ iso: impl AsRef<Path> + fmt::Debug,
+ file: &PathBuf,
+ location: &str,
+ uuid: &String,
+) -> Result<()> {
let result = Command::new("xorriso")
.arg("-boot_image")
.arg("any")
@@ -386,7 +391,7 @@ fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str, uuid: &Stri
.arg("uuid")
.arg(uuid)
.arg("-dev")
- .arg(iso)
+ .arg(iso.as_ref())
.arg("-map")
.arg(file)
.arg(location)
@@ -400,10 +405,10 @@ fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str, uuid: &Stri
Ok(())
}
-fn get_iso_uuid(iso: &PathBuf) -> Result<String> {
+fn get_iso_uuid(iso: impl AsRef<Path>) -> Result<String> {
let result = Command::new("xorriso")
.arg("-dev")
- .arg(iso)
+ .arg(iso.as_ref())
.arg("-report_system_area")
.arg("cmd")
.output()?;
@@ -530,11 +535,11 @@ fn get_nics() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
Ok(nics)
}
-fn get_udev_properties(path: &PathBuf) -> Result<String> {
+fn get_udev_properties(path: impl AsRef<Path> + fmt::Debug) -> Result<String> {
let udev_output = Command::new("udevadm")
.arg("info")
.arg("--path")
- .arg(path)
+ .arg(path.as_ref())
.arg("--query")
.arg("all")
.output()?;
@@ -544,8 +549,8 @@ fn get_udev_properties(path: &PathBuf) -> Result<String> {
Ok(String::from_utf8(udev_output.stdout)?)
}
-fn parse_answer(path: &PathBuf) -> Result<Answer> {
- let mut file = match fs::File::open(path) {
+fn parse_answer(path: impl AsRef<Path> + fmt::Debug) -> Result<Answer> {
+ let mut file = match fs::File::open(&path) {
Ok(file) => file,
Err(err) => bail!("Opening answer file {path:?} failed: {err}"),
};
--
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] 34+ messages in thread* [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!()
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (9 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 10/17] auto-install-assistant: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-23 10:39 ` Aaron Lauterer
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 12/17] auto-installer: tests: simplify empty disks check Christoph Heiss
` (7 subsequent siblings)
18 siblings, 1 reply; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch, suggested by Stefan
---
proxmox-auto-installer/tests/parse-answer.rs | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 450915a..1fc515e 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -75,12 +75,8 @@ fn test_parse_answers() {
path.push(format!("{name}.json"));
let compare_raw = std::fs::read_to_string(&path).unwrap();
let compare: Value = serde_json::from_str(&compare_raw).unwrap();
- if config != compare {
- panic!(
- "Test {} failed:\nleft: {:#?}\nright: {:#?}\n",
- name, config, compare
- );
- }
+
+ assert_eq!(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] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!()
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!() Christoph Heiss
@ 2024-07-23 10:39 ` Aaron Lauterer
2024-07-23 10:40 ` Aaron Lauterer
2024-07-23 10:46 ` Christoph Heiss
0 siblings, 2 replies; 34+ messages in thread
From: Aaron Lauterer @ 2024-07-23 10:39 UTC (permalink / raw)
To: pve-devel
Do we still see which test case actually failed? IIRC I used the panic
so I can print the needed info, mainly the name of the current test
scenario so it is easier to find out which failed.
On 2024-07-18 15:48, Christoph Heiss wrote:
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v1 -> v2:
> * new patch, suggested by Stefan
> ---
> proxmox-auto-installer/tests/parse-answer.rs | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
> index 450915a..1fc515e 100644
> --- a/proxmox-auto-installer/tests/parse-answer.rs
> +++ b/proxmox-auto-installer/tests/parse-answer.rs
> @@ -75,12 +75,8 @@ fn test_parse_answers() {
> path.push(format!("{name}.json"));
> let compare_raw = std::fs::read_to_string(&path).unwrap();
> let compare: Value = serde_json::from_str(&compare_raw).unwrap();
> - if config != compare {
> - panic!(
> - "Test {} failed:\nleft: {:#?}\nright: {:#?}\n",
> - name, config, compare
> - );
> - }
> +
> + assert_eq!(config, compare);
> }
> }
> }
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!()
2024-07-23 10:39 ` Aaron Lauterer
@ 2024-07-23 10:40 ` Aaron Lauterer
2024-07-23 10:46 ` Christoph Heiss
1 sibling, 0 replies; 34+ messages in thread
From: Aaron Lauterer @ 2024-07-23 10:40 UTC (permalink / raw)
To: pve-devel
switching these tests over to something like https://insta.rs could be
something we might want to do mid-/long term.
On 2024-07-23 12:39, Aaron Lauterer wrote:
> Do we still see which test case actually failed? IIRC I used the panic
> so I can print the needed info, mainly the name of the current test
> scenario so it is easier to find out which failed.
>
> On 2024-07-18 15:48, Christoph Heiss wrote:
>> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
>> ---
>> Changes v1 -> v2:
>> * new patch, suggested by Stefan
>> ---
>> proxmox-auto-installer/tests/parse-answer.rs | 8 ++------
>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/proxmox-auto-installer/tests/parse-answer.rs
>> b/proxmox-auto-installer/tests/parse-answer.rs
>> index 450915a..1fc515e 100644
>> --- a/proxmox-auto-installer/tests/parse-answer.rs
>> +++ b/proxmox-auto-installer/tests/parse-answer.rs
>> @@ -75,12 +75,8 @@ fn test_parse_answers() {
>> path.push(format!("{name}.json"));
>> let compare_raw = std::fs::read_to_string(&path).unwrap();
>> let compare: Value =
>> serde_json::from_str(&compare_raw).unwrap();
>> - if config != compare {
>> - panic!(
>> - "Test {} failed:\nleft: {:#?}\nright: {:#?}\n",
>> - name, config, compare
>> - );
>> - }
>> +
>> + assert_eq!(config, compare);
>> }
>> }
>> }
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!()
2024-07-23 10:39 ` Aaron Lauterer
2024-07-23 10:40 ` Aaron Lauterer
@ 2024-07-23 10:46 ` Christoph Heiss
2024-07-23 11:04 ` Aaron Lauterer
1 sibling, 1 reply; 34+ messages in thread
From: Christoph Heiss @ 2024-07-23 10:46 UTC (permalink / raw)
To: Aaron Lauterer; +Cc: Proxmox VE development discussion
On Tue, Jul 23, 2024 at 12:39:20PM GMT, Aaron Lauterer wrote:
> Do we still see which test case actually failed? IIRC I used the panic so I
> can print the needed info, mainly the name of the current test scenario so
> it is easier to find out which failed.
Yes, since it printed earlier in the code, at the start of the
`if extension == "toml" { .. }` block.
>
> On 2024-07-18 15:48, Christoph Heiss wrote:
> > Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> > ---
> > Changes v1 -> v2:
> > * new patch, suggested by Stefan
> > ---
> > proxmox-auto-installer/tests/parse-answer.rs | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
> > index 450915a..1fc515e 100644
> > --- a/proxmox-auto-installer/tests/parse-answer.rs
> > +++ b/proxmox-auto-installer/tests/parse-answer.rs
> > @@ -75,12 +75,8 @@ fn test_parse_answers() {
> > path.push(format!("{name}.json"));
> > let compare_raw = std::fs::read_to_string(&path).unwrap();
> > let compare: Value = serde_json::from_str(&compare_raw).unwrap();
> > - if config != compare {
> > - panic!(
> > - "Test {} failed:\nleft: {:#?}\nright: {:#?}\n",
> > - name, config, compare
> > - );
> > - }
> > +
> > + assert_eq!(config, compare);
> > }
> > }
> > }
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!()
2024-07-23 10:46 ` Christoph Heiss
@ 2024-07-23 11:04 ` Aaron Lauterer
2024-07-23 11:37 ` Christoph Heiss
0 siblings, 1 reply; 34+ messages in thread
From: Aaron Lauterer @ 2024-07-23 11:04 UTC (permalink / raw)
To: Christoph Heiss; +Cc: Proxmox VE development discussion
On 2024-07-23 12:46, Christoph Heiss wrote:
> On Tue, Jul 23, 2024 at 12:39:20PM GMT, Aaron Lauterer wrote:
>> Do we still see which test case actually failed? IIRC I used the panic so I
>> can print the needed info, mainly the name of the current test scenario so
>> it is easier to find out which failed.
>
> Yes, since it printed earlier in the code, at the start of the
> `if extension == "toml" { .. }` block.
ah okay.
I quickly compared both variants and realized again, that with the
panic, we can pretty debug print the structs, making it quite a bit
easier to compare the expected result to the actual one.
With the output we get from `assert_eq` it can be very cumbersome to
actually see the diff and why the test failed since the data structures
can be quite large.
Instead of hacking or own pretty print, we could maybe think about using
https://crates.io/crates/pretty_assertions
>
>>
>> On 2024-07-18 15:48, Christoph Heiss wrote:
>>> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
>>> ---
>>> Changes v1 -> v2:
>>> * new patch, suggested by Stefan
>>> ---
>>> proxmox-auto-installer/tests/parse-answer.rs | 8 ++------
>>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
>>> index 450915a..1fc515e 100644
>>> --- a/proxmox-auto-installer/tests/parse-answer.rs
>>> +++ b/proxmox-auto-installer/tests/parse-answer.rs
>>> @@ -75,12 +75,8 @@ fn test_parse_answers() {
>>> path.push(format!("{name}.json"));
>>> let compare_raw = std::fs::read_to_string(&path).unwrap();
>>> let compare: Value = serde_json::from_str(&compare_raw).unwrap();
>>> - if config != compare {
>>> - panic!(
>>> - "Test {} failed:\nleft: {:#?}\nright: {:#?}\n",
>>> - name, config, compare
>>> - );
>>> - }
>>> +
>>> + assert_eq!(config, compare);
>>> }
>>> }
>>> }
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!()
2024-07-23 11:04 ` Aaron Lauterer
@ 2024-07-23 11:37 ` Christoph Heiss
2024-07-24 7:54 ` Thomas Lamprecht
0 siblings, 1 reply; 34+ messages in thread
From: Christoph Heiss @ 2024-07-23 11:37 UTC (permalink / raw)
To: Aaron Lauterer; +Cc: Proxmox VE development discussion
On Tue, Jul 23, 2024 at 01:04:06PM GMT, Aaron Lauterer wrote:
>
> I quickly compared both variants and realized again, that with the panic, we
> can pretty debug print the structs, making it quite a bit easier to compare
> the expected result to the actual one.
Yep, that's right. Seems I've missed that.
>
> With the output we get from `assert_eq` it can be very cumbersome to
> actually see the diff and why the test failed since the data structures can
> be quite large.
>
> Instead of hacking or own pretty print, we could maybe think about using
> https://crates.io/crates/pretty_assertions
As discussed offline, I think that this is definitely the way to go.
Especially when keeping in mind that these tests are going to grow with
~every auto-installer feature.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!()
2024-07-23 11:37 ` Christoph Heiss
@ 2024-07-24 7:54 ` Thomas Lamprecht
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Lamprecht @ 2024-07-24 7:54 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss, Aaron Lauterer
Am 23/07/2024 um 13:37 schrieb Christoph Heiss:
> On Tue, Jul 23, 2024 at 01:04:06PM GMT, Aaron Lauterer wrote:
>> Instead of hacking or own pretty print, we could maybe think about using
>> https://crates.io/crates/pretty_assertions
>
> As discussed offline, I think that this is definitely the way to go.
> Especially when keeping in mind that these tests are going to grow with
> ~every auto-installer feature.
fine by me, it's already packaged and as long as it's used as dev-dependency
and inside `#[cfg(test)]` code it's low in impact anyway.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH installer v2 12/17] auto-installer: tests: simplify empty disks check
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (10 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 11/17] auto-installer: tests: replace manual panic!() with assert_eq!() Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 13/17] auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
` (6 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch, suggested by Stefan
---
proxmox-auto-installer/tests/parse-answer.rs | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 1fc515e..ff25a65 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -40,9 +40,7 @@ pub fn setup_test_basic(path: &Path) -> (SetupInfo, LocaleInfo, RuntimeInfo, Ude
};
runtime_info.disks.sort();
- if runtime_info.disks.is_empty() {
- panic!("disk list is empty!");
- }
+ assert!(!runtime_info.disks.is_empty(), "disk list cannot be empty");
(installer_info, locale_info, runtime_info, udev_info)
}
--
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] 34+ messages in thread* [pve-devel] [PATCH installer v2 13/17] auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>`
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (11 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 12/17] auto-installer: tests: simplify empty disks check Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 14/17] auto-installer: move `SystemDMI` struct to common crate Christoph Heiss
` (5 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch, suggested by Stefan
---
proxmox-auto-installer/tests/parse-answer.rs | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index ff25a65..53ead8c 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -18,7 +18,7 @@ fn get_test_resource_path() -> Result<PathBuf, String> {
.join("tests/resources"))
}
-fn get_answer(path: PathBuf) -> Result<Answer, String> {
+fn get_answer(path: impl AsRef<Path>) -> Result<Answer, String> {
let answer_raw = std::fs::read_to_string(path).unwrap();
let answer: answer::Answer = toml::from_str(&answer_raw)
.map_err(|err| format!("error parsing answer.toml: {err}"))
@@ -27,11 +27,12 @@ fn get_answer(path: PathBuf) -> Result<Answer, String> {
Ok(answer)
}
-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();
+pub fn setup_test_basic(path: impl AsRef<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.to_path_buf();
+ let mut path = path.as_ref().to_path_buf();
path.push("run-env-udev.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] 34+ messages in thread* [pve-devel] [PATCH installer v2 14/17] auto-installer: move `SystemDMI` struct to common crate
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (12 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 13/17] auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
@ 2024-07-18 13:48 ` Christoph Heiss
2024-07-18 13:49 ` [pve-devel] [PATCH installer v2 15/17] fix #5536: auto-installer: answer: add `posthook` section Christoph Heiss
` (4 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:48 UTC (permalink / raw)
To: pve-devel
This functionality will be reused by the post-hook, which sends this
data as part of its information set.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch
---
proxmox-auto-installer/src/sysinfo.rs | 51 +-----------------------
proxmox-installer-common/src/lib.rs | 1 +
proxmox-installer-common/src/sysinfo.rs | 52 +++++++++++++++++++++++++
3 files changed, 55 insertions(+), 49 deletions(-)
create mode 100644 proxmox-installer-common/src/sysinfo.rs
diff --git a/proxmox-auto-installer/src/sysinfo.rs b/proxmox-auto-installer/src/sysinfo.rs
index 112e898..0a6aaf2 100644
--- a/proxmox-auto-installer/src/sysinfo.rs
+++ b/proxmox-auto-installer/src/sysinfo.rs
@@ -1,15 +1,14 @@
use anyhow::{bail, Result};
use proxmox_installer_common::{
setup::{IsoInfo, ProductConfig, SetupInfo},
+ sysinfo::SystemDMI,
RUNTIME_DIR,
};
use serde::Serialize;
-use std::{collections::HashMap, fs, io, path::PathBuf};
+use std::{fs, io, path::PathBuf};
use crate::utils::get_nic_list;
-const DMI_PATH: &str = "/sys/devices/virtual/dmi/id";
-
#[derive(Debug, Serialize)]
pub struct SysInfo {
product: ProductConfig,
@@ -70,49 +69,3 @@ impl NetdevWithMac {
Ok(result)
}
}
-
-#[derive(Debug, Serialize)]
-struct SystemDMI {
- system: HashMap<String, String>,
- baseboard: HashMap<String, String>,
- chassis: HashMap<String, String>,
-}
-
-impl SystemDMI {
- pub(crate) fn get() -> Result<Self> {
- let system_files = [
- "product_serial",
- "product_sku",
- "product_uuid",
- "product_name",
- ];
- let baseboard_files = ["board_asset_tag", "board_serial", "board_name"];
- let chassis_files = ["chassis_serial", "chassis_sku", "chassis_asset_tag"];
-
- Ok(Self {
- system: Self::get_dmi_infos(&system_files)?,
- baseboard: Self::get_dmi_infos(&baseboard_files)?,
- chassis: Self::get_dmi_infos(&chassis_files)?,
- })
- }
-
- fn get_dmi_infos(files: &[&str]) -> Result<HashMap<String, String>> {
- let mut res: HashMap<String, String> = HashMap::new();
-
- for file in files {
- let path = format!("{DMI_PATH}/{file}");
- let content = match fs::read_to_string(&path) {
- Err(ref err) if err.kind() == std::io::ErrorKind::NotFound => continue,
- Err(ref err) if err.kind() == std::io::ErrorKind::PermissionDenied => {
- bail!("Could not read data. Are you running as root or with sudo?")
- }
- Err(err) => bail!("Error: '{err}' on '{path}'"),
- Ok(content) => content.trim().into(),
- };
- let key = file.splitn(2, '_').last().unwrap();
- res.insert(key.into(), content);
- }
-
- Ok(res)
- }
-}
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index ee9f2a8..59f1ab1 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -1,6 +1,7 @@
pub mod disk_checks;
pub mod options;
pub mod setup;
+pub mod sysinfo;
pub mod utils;
#[cfg(feature = "http")]
diff --git a/proxmox-installer-common/src/sysinfo.rs b/proxmox-installer-common/src/sysinfo.rs
new file mode 100644
index 0000000..9746cb2
--- /dev/null
+++ b/proxmox-installer-common/src/sysinfo.rs
@@ -0,0 +1,52 @@
+use std::{collections::HashMap, fs};
+
+use anyhow::{bail, Result};
+use serde::Serialize;
+
+const DMI_PATH: &str = "/sys/devices/virtual/dmi/id";
+
+#[derive(Debug, Serialize)]
+pub struct SystemDMI {
+ system: HashMap<String, String>,
+ baseboard: HashMap<String, String>,
+ chassis: HashMap<String, String>,
+}
+
+impl SystemDMI {
+ pub fn get() -> Result<Self> {
+ let system_files = [
+ "product_serial",
+ "product_sku",
+ "product_uuid",
+ "product_name",
+ ];
+ let baseboard_files = ["board_asset_tag", "board_serial", "board_name"];
+ let chassis_files = ["chassis_serial", "chassis_sku", "chassis_asset_tag"];
+
+ Ok(Self {
+ system: Self::get_dmi_infos(&system_files)?,
+ baseboard: Self::get_dmi_infos(&baseboard_files)?,
+ chassis: Self::get_dmi_infos(&chassis_files)?,
+ })
+ }
+
+ fn get_dmi_infos(files: &[&str]) -> Result<HashMap<String, String>> {
+ let mut res: HashMap<String, String> = HashMap::new();
+
+ for file in files {
+ let path = format!("{DMI_PATH}/{file}");
+ let content = match fs::read_to_string(&path) {
+ Err(ref err) if err.kind() == std::io::ErrorKind::NotFound => continue,
+ Err(ref err) if err.kind() == std::io::ErrorKind::PermissionDenied => {
+ bail!("Could not read data. Are you running as root or with sudo?")
+ }
+ Err(err) => bail!("Error: '{err}' on '{path}'"),
+ Ok(content) => content.trim().into(),
+ };
+ let key = file.splitn(2, '_').last().unwrap();
+ res.insert(key.into(), content);
+ }
+
+ Ok(res)
+ }
+}
--
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] 34+ messages in thread* [pve-devel] [PATCH installer v2 15/17] fix #5536: auto-installer: answer: add `posthook` section
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (13 preceding siblings ...)
2024-07-18 13:48 ` [pve-devel] [PATCH installer v2 14/17] auto-installer: move `SystemDMI` struct to common crate Christoph Heiss
@ 2024-07-18 13:49 ` Christoph Heiss
2024-07-18 13:49 ` [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install Christoph Heiss
` (3 subsequent siblings)
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:49 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
---
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] 34+ messages in thread* [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (14 preceding siblings ...)
2024-07-18 13:49 ` [pve-devel] [PATCH installer v2 15/17] fix #5536: auto-installer: answer: add `posthook` section Christoph Heiss
@ 2024-07-18 13:49 ` Christoph Heiss
2024-07-23 14:57 ` Aaron Lauterer
2024-07-24 11:21 ` Aaron Lauterer
2024-07-18 13:49 ` [pve-devel] [PATCH installer v2 17/17] unconfigured.sh: run proxmox-post-hook after successful auto-install Christoph Heiss
` (2 subsequent siblings)
18 siblings, 2 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:49 UTC (permalink / raw)
To: pve-devel
This utility can be called with the low-level install config after a
successful installation to send a notification via a HTTP POST request,
if the user has configured an endpoint for that in the answer file.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* squash implementation and unit tests into one patch
* simplify udev property retrieving by introducing proper helpers on
`UdevInfo` itself
* rename Answer::from_reader() -> Answer::try_from_reader to better
reflect it returns a Result<>
* improved error message in some places
* added new fields; now includes ISO version, SecureBoot state, CPU
and DMI info
* product information was split into separate fields
* boot mode information was split into separate fields
* product version is now retrieved from the package using dpkg-query
directly
* kernel version was split into separate fields, retrieving version
string from the image directly
* all disks and NICs are now included, a field indicates whether they
are boot disk or management interface, respectively
* move with_chroot() invocation out of PostHookInfo::gather()
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 +-
proxmox-auto-installer/src/udevinfo.rs | 8 +-
.../src/fetch_plugins/http.rs | 2 +-
proxmox-installer-common/src/http.rs | 6 +-
proxmox-installer-common/src/options.rs | 5 +
proxmox-installer-common/src/setup.rs | 2 +-
proxmox-installer-common/src/utils.rs | 2 +
proxmox-post-hook/Cargo.toml | 18 +
proxmox-post-hook/src/main.rs | 784 ++++++++++++++++++
13 files changed, 843 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..2670735 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 try_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..6c78d5f 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::try_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-auto-installer/src/udevinfo.rs b/proxmox-auto-installer/src/udevinfo.rs
index a6b61b5..677f3f6 100644
--- a/proxmox-auto-installer/src/udevinfo.rs
+++ b/proxmox-auto-installer/src/udevinfo.rs
@@ -1,9 +1,11 @@
use serde::Deserialize;
use std::collections::BTreeMap;
+/// Uses a BTreeMap to have the keys sorted
+pub type UdevProperties = BTreeMap<String, String>;
+
#[derive(Clone, Deserialize, Debug)]
pub struct UdevInfo {
- // use BTreeMap to have keys sorted
- pub disks: BTreeMap<String, BTreeMap<String, String>>,
- pub nics: BTreeMap<String, BTreeMap<String, String>>,
+ pub disks: BTreeMap<String, UdevProperties>,
+ pub nics: BTreeMap<String, UdevProperties>,
}
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-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 32c62a7..5c3471d 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -46,6 +46,11 @@ impl FsType {
pub fn is_btrfs(&self) -> bool {
matches!(self, FsType::Btrfs(_))
}
+
+ /// Returns true if the filesystem is used on top of LVM, e.g. ext4 or XFS.
+ 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 2ca9641..ca03e07 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -347,7 +347,7 @@ pub struct RuntimeInfo {
pub secure_boot: Option<bool>,
}
-#[derive(Copy, Clone, Eq, Deserialize, PartialEq)]
+#[derive(Copy, Clone, Eq, Deserialize, PartialEq, Serialize)]
#[serde(rename_all = "lowercase")]
pub enum BootType {
Bios,
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..3acea6c
--- /dev/null
+++ b/proxmox-post-hook/Cargo.toml
@@ -0,0 +1,18 @@
+[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
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
new file mode 100644
index 0000000..d3e5b5c
--- /dev/null
+++ b/proxmox-post-hook/src/main.rs
@@ -0,0 +1,784 @@
+//! 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::HashSet,
+ ffi::CStr,
+ fs::{self, File},
+ io::BufReader,
+ os::unix::fs::FileExt,
+ path::PathBuf,
+ process::{Command, ExitCode},
+};
+
+use anyhow::{anyhow, bail, Context, Result};
+use proxmox_auto_installer::{
+ answer::{Answer, PostNotificationHookInfo},
+ udevinfo::{UdevInfo, UdevProperties},
+};
+use proxmox_installer_common::{
+ options::{Disk, FsType},
+ setup::{
+ load_installer_setup_files, BootType, InstallConfig, IsoInfo, ProxmoxProduct, RuntimeInfo,
+ SetupInfo,
+ },
+ sysinfo::SystemDMI,
+ utils::CidrAddress,
+};
+use serde::Serialize;
+
+/// Information about the system boot status.
+#[derive(Serialize)]
+struct BootInfo {
+ /// Whether the system is booted using UEFI or legacy BIOS.
+ mode: BootType,
+ /// Whether SecureBoot is enabled for the installation.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ secureboot: Option<bool>,
+}
+
+/// 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(Serialize)]
+#[serde(rename_all = "kebab-case")]
+struct DiskInfo {
+ /// Size in bytes
+ size: usize,
+ /// Set to true if the disk is used for booting.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ is_bootdisk: Option<bool>,
+ /// Properties about the device as given by udev.
+ udev_properties: UdevProperties,
+}
+
+/// 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
+ #[serde(skip_serializing_if = "Option::is_none")]
+ address: Option<CidrAddress>,
+ /// Set to true if the interface is the chosen management interface during
+ /// installation.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ is_management: Option<bool>,
+ /// Properties about the device as given by udev.
+ udev_properties: UdevProperties,
+}
+
+/// Information about the installed product itself.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+struct ProductInfo {
+ /// Full name of the product
+ fullname: String,
+ /// Product abbreviation
+ short: ProxmoxProduct,
+ /// Version of the installed product
+ version: String,
+}
+
+/// The current kernel version.
+/// Aligns with the format as used by the /nodes/<node>/status API of each product.
+#[derive(Serialize)]
+struct KernelVersionInformation {
+ /// The systemname/nodename
+ pub sysname: String,
+ /// The kernel release number
+ pub release: String,
+ /// The kernel version
+ pub version: String,
+ /// The machine architecture
+ pub machine: String,
+}
+
+/// Information about the CPU(s) installed in the system
+#[derive(Serialize)]
+struct CpuInfo {
+ /// Number of physical CPU cores.
+ cores: usize,
+ /// Number of logical CPU cores aka. threads.
+ cpus: usize,
+ /// CPU feature flag set as a space-delimited list.
+ flags: String,
+ /// Whether hardware-accelerated virtualization is supported.
+ hvm: bool,
+ /// Reported model of the CPU(s)
+ model: String,
+ /// Number of physical CPU sockets
+ sockets: usize,
+}
+
+/// 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: ProductInfo,
+ /// Release information for the ISO used for the installation.
+ iso: IsoInfo,
+ /// Installed kernel version
+ kernel_version: KernelVersionInformation,
+ /// Describes the boot mode of the machine and the SecureBoot status.
+ boot_info: BootInfo,
+ /// Information about the installed CPU(s)
+ cpu_info: CpuInfo,
+ /// DMI information about the system
+ dmi: SystemDMI,
+ /// 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,
+ /// All disks detected on the system.
+ disks: Vec<DiskInfo>,
+ /// All network interfaces detected on the system.
+ network_interfaces: Vec<NetworkInterfaceInfo>,
+ /// Public parts of SSH host keys of the installed system
+ ssh_public_host_keys: SshPublicHostKeys,
+}
+
+/// 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 sending
+ /// it to a specified server.
+ ///
+ /// # Arguments
+ ///
+ /// * `target_path` - Path to where the chroot environment root is mounted
+ /// * `answer` - Answer file as provided by the user
+ fn gather(target_path: &str, 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(proxmox_installer_common::RUNTIME_DIR)
+ .map_err(|err| anyhow!("Failed to load setup files: {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)?))?
+ };
+
+ // Opens a file, specified by an absolute path _inside_ the chroot
+ // from the target.
+ let open_file = |path: &str| {
+ File::open(format!("{}/{}", target_path, path))
+ .with_context(|| format!("failed to open '{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(|| format!("failed to read '{path}'"))
+ };
+
+ // 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)?))
+ };
+
+ Ok(Self {
+ debian_version: read_file("/etc/debian_version")?,
+ product: Self::gather_product_info(&setup_info, &run_cmd)?,
+ iso: setup_info.iso_info.clone(),
+ kernel_version: Self::gather_kernel_version(&run_cmd, &open_file)?,
+ boot_info: BootInfo {
+ mode: run_env.boot_type,
+ secureboot: run_env.secure_boot,
+ },
+ cpu_info: Self::gather_cpu_info(&run_env)?,
+ dmi: SystemDMI::get()?,
+ filesystem: answer.disks.fs_type,
+ fqdn: answer.global.fqdn.to_string(),
+ machine_id: read_file("/etc/machine-id")?,
+ disks: Self::gather_disks(&config, &run_env, &udev)?,
+ network_interfaces: 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<DiskInfo>> {
+ let get_udev_properties = |disk: &Disk| {
+ udev.disks
+ .get(&disk.index)
+ .with_context(|| {
+ format!("could not find udev information for disk '{}'", disk.path)
+ })
+ .cloned()
+ };
+
+ let disks = if config.filesys.is_lvm() {
+ // If the filesystem is LVM, there is only boot disk. The path (aka. /dev/..)
+ // can be found in `config.target_hd`.
+ run_env
+ .disks
+ .iter()
+ .flat_map(|disk| {
+ let is_bootdisk = config
+ .target_hd
+ .as_ref()
+ .and_then(|hd| (*hd == disk.path).then_some(true));
+
+ anyhow::Ok(DiskInfo {
+ size: (config.hdsize * (SIZE_GIB as f64)) as usize,
+ is_bootdisk,
+ udev_properties: get_udev_properties(disk)?,
+ })
+ })
+ .collect()
+ } else {
+ // If the filesystem is not LVM-based (thus Btrfs or ZFS), `config.disk_selection`
+ // contains a list of indices identifiying the boot disks, as given by udev.
+ let selected_disks_indices: Vec<&String> = config.disk_selection.values().collect();
+
+ run_env
+ .disks
+ .iter()
+ .flat_map(|disk| {
+ let is_bootdisk = selected_disks_indices
+ .contains(&&disk.index)
+ .then_some(true);
+
+ anyhow::Ok(DiskInfo {
+ size: (config.hdsize * (SIZE_GIB as f64)) as usize,
+ is_bootdisk,
+ udev_properties: get_udev_properties(disk)?,
+ })
+ })
+ .collect()
+ };
+
+ Ok(disks)
+ }
+
+ /// 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<Vec<NetworkInterfaceInfo>> {
+ Ok(run_env
+ .network
+ .interfaces
+ .values()
+ .flat_map(|nic| {
+ let udev_properties = udev
+ .nics
+ .get(&nic.name)
+ .with_context(|| {
+ format!("could not find udev information for NIC '{}'", nic.name)
+ })?
+ .clone();
+
+ if config.mngmt_nic == nic.name {
+ // Use the actual IP address from the low-level install config, as the runtime info
+ // contains the original IP address from DHCP.
+ anyhow::Ok(NetworkInterfaceInfo {
+ mac: nic.mac.clone(),
+ address: Some(config.cidr.clone()),
+ is_management: Some(true),
+ udev_properties,
+ })
+ } else {
+ anyhow::Ok(NetworkInterfaceInfo {
+ mac: nic.mac.clone(),
+ address: None,
+ is_management: None,
+ udev_properties,
+ })
+ }
+ })
+ .collect())
+ }
+
+ /// 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_info(
+ setup_info: &SetupInfo,
+ run_cmd: &dyn Fn(&[&str]) -> Result<String>,
+ ) -> Result<ProductInfo> {
+ let package = match setup_info.config.product {
+ ProxmoxProduct::PVE => "pve-manager",
+ ProxmoxProduct::PMG => "pmg-api",
+ ProxmoxProduct::PBS => "proxmox-backup-server",
+ };
+
+ let version = run_cmd(&[
+ "dpkg-query",
+ "--showformat",
+ "${Version}",
+ "--show",
+ package,
+ ])
+ .with_context(|| format!("failed to retrieve version of {package}"))?;
+
+ Ok(ProductInfo {
+ fullname: setup_info.config.fullname.clone(),
+ short: setup_info.config.product,
+ version,
+ })
+ }
+
+ /// Extracts the version string from the *installed* kernel image.
+ ///
+ /// First, it determines the exact path to the kernel image (aka. `/boot/vmlinuz-<version>`)
+ /// by looking at the installed kernel package, then reads the string directly from the image
+ /// from the well-defined kernel header. See also [0] for details.
+ ///
+ /// [0] https://www.kernel.org/doc/html/latest/arch/x86/boot.html
+ ///
+ /// # Arguments
+ ///
+ /// * `run_cmd` - Callback to run a command inside the target chroot.
+ /// * `open_file` - Callback to open a file inside the target chroot.
+ #[cfg(target_arch = "x86_64")]
+ fn gather_kernel_version(
+ run_cmd: &dyn Fn(&[&str]) -> Result<String>,
+ open_file: &dyn Fn(&str) -> Result<File>,
+ ) -> Result<KernelVersionInformation> {
+ let file = open_file(&Self::find_kernel_image_path(run_cmd)?)?;
+
+ // Read the 2-byte `kernel_version` field at offset 0x20e [0] from the file ..
+ // https://www.kernel.org/doc/html/latest/arch/x86/boot.html#the-real-mode-kernel-header
+ let mut buffer = [0u8; 2];
+ file.read_exact_at(&mut buffer, 0x20e)
+ .context("could not read kernel_version offset from image")?;
+
+ // .. which gives us the offset of the kernel version string inside the image, minus 0x200.
+ // https://www.kernel.org/doc/html/latest/arch/x86/boot.html#details-of-header-fields
+ let offset = u16::from_le_bytes(buffer) + 0x200;
+
+ // The string is usually somewhere around 80-100 bytes long, so 256 bytes is more than
+ // enough to cover all cases.
+ let mut buffer = [0u8; 256];
+ file.read_exact_at(&mut buffer, offset.into())
+ .context("could not read kernel version string from image")?;
+
+ // Now just consume the buffer until the NUL byte
+ let kernel_version = CStr::from_bytes_until_nul(&buffer)
+ .context("did not find a NUL-terminator in kernel version string")?
+ .to_str()
+ .context("could not convert kernel version string")?;
+
+ // The version string looks like:
+ // 6.8.4-2-pve (build@proxmox) #1 SMP PREEMPT_DYNAMIC PMX 6.8.4-2 (2024-04-10T17:36Z) x86_64 GNU/Linux
+ //
+ // Thus split it into three parts, as we are interested in the release version
+ // and everything starting at the build number
+ let parts: Vec<&str> = kernel_version.splitn(3, ' ').collect();
+
+ if parts.len() != 3 {
+ bail!("failed to split kernel version string");
+ }
+
+ Ok(KernelVersionInformation {
+ machine: std::env::consts::ARCH.to_owned(),
+ sysname: "Linux".to_owned(),
+ release: parts
+ .first()
+ .context("kernel release not found")?
+ .to_string(),
+ version: parts
+ .get(2)
+ .context("kernel version not found")?
+ .to_string(),
+ })
+ }
+
+ /// Retrieves the absolute path to the kernel image (aka. `/boot/vmlinuz-<version>`)
+ /// inside the chroot by looking at the file list installed by the kernel package.
+ ///
+ /// # Arguments
+ ///
+ /// * `run_cmd` - Callback to run a command inside the target chroot.
+ fn find_kernel_image_path(run_cmd: &dyn Fn(&[&str]) -> Result<String>) -> Result<String> {
+ let pkg_name = Self::find_kernel_package_name(run_cmd)?;
+
+ let all_files = run_cmd(&["dpkg-query", "--listfiles", &pkg_name])?;
+ for file in all_files.lines() {
+ if file.starts_with("/boot/vmlinuz-") {
+ return Ok(file.to_owned());
+ }
+ }
+
+ bail!("failed to find installed kernel image path")
+ }
+
+ /// Retrieves the full name of the kernel package installed inside the chroot.
+ ///
+ /// # Arguments
+ ///
+ /// * `run_cmd` - Callback to run a command inside the target chroot.
+ fn find_kernel_package_name(run_cmd: &dyn Fn(&[&str]) -> Result<String>) -> Result<String> {
+ let dpkg_arch = run_cmd(&["dpkg", "--print-architecture"])?
+ .trim()
+ .to_owned();
+
+ let kernel_pkgs = run_cmd(&[
+ "dpkg-query",
+ "--showformat",
+ "${db:Status-Abbrev}|${Architecture}|${Package}\\n",
+ "--show",
+ "proxmox-kernel-[0-9]*",
+ ])?;
+
+ // 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
+ for pkg in kernel_pkgs.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 installed kernel package")
+ }
+
+ /// Retrieves some basic information about the CPU in the running system,
+ /// reading them from /proc/cpuinfo.
+ ///
+ /// # Arguments
+ ///
+ /// * `run_env` - Runtime envirornment information gathered by the installer at the start
+ fn gather_cpu_info(run_env: &RuntimeInfo) -> Result<CpuInfo> {
+ let mut result = CpuInfo {
+ cores: 0,
+ cpus: 0,
+ flags: String::new(),
+ hvm: run_env.hvm_supported,
+ model: String::new(),
+ sockets: 0,
+ };
+ let mut sockets = HashSet::new();
+ let mut cores = HashSet::new();
+
+ // Does not matter if we read the file from inside the chroot or directly on the host.
+ let cpuinfo = fs::read_to_string("/proc/cpuinfo")?;
+ for line in cpuinfo.lines() {
+ match line.split_once(':') {
+ Some((key, _)) if key.trim() == "processor" => {
+ result.cpus += 1;
+ }
+ Some((key, value)) if key.trim() == "core id" => {
+ cores.insert(value);
+ }
+ Some((key, value)) if key.trim() == "physical id" => {
+ sockets.insert(value);
+ }
+ Some((key, value)) if key.trim() == "flags" => {
+ value.trim().clone_into(&mut result.flags);
+ }
+ Some((key, value)) if key.trim() == "model name" => {
+ value.trim().clone_into(&mut result.model);
+ }
+ _ => {}
+ }
+ }
+
+ result.cores = cores.len();
+ result.sockets = sockets.len();
+
+ Ok(result)
+ }
+}
+
+/// 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.
+///
+/// # Arguments
+///
+/// * `callback` - Callback to call with the absolute path where the chroot environment root is
+/// mounted.
+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 w.r.t to the path, which is hard-coded there
+ let result = callback("/target");
+
+ let ec = Command::new("proxmox-chroot").arg("cleanup").status();
+ // We do not want to necessarily fail here, as the install environment is about
+ // to be teared down completely anyway.
+ if ec.is_err() || !ec.map(|ec| ec.success()).unwrap_or(false) {
+ eprintln!("failed to clean up chroot for installed system");
+ }
+
+ result
+}
+
+/// Reads the answer file from stdin, checks for a configured post-hook URL (+ optional certificate
+/// fingerprint for HTTPS). If configured, retrieves all relevant information about the installed
+/// system and sends them to the given endpoint.
+fn do_main() -> Result<()> {
+ let answer = Answer::try_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 = with_chroot(|target_path| PostHookInfo::gather(target_path, &answer))?;
+
+ proxmox_installer_common::http::post(
+ url,
+ cert_fingerprint.as_deref(),
+ serde_json::to_string(&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
+ }
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use crate::PostHookInfo;
+
+ #[test]
+ fn finds_correct_kernel_package_name() {
+ let mocked_run_cmd = |cmd: &[&str]| {
+ if cmd[0] == "dpkg" {
+ assert_eq!(cmd, &["dpkg", "--print-architecture"]);
+ Ok("amd64\n".to_owned())
+ } else {
+ assert_eq!(
+ cmd,
+ &[
+ "dpkg-query",
+ "--showformat",
+ "${db:Status-Abbrev}|${Architecture}|${Package}\\n",
+ "--show",
+ "proxmox-kernel-[0-9]*",
+ ]
+ );
+ Ok(r#"ii |all|proxmox-kernel-6.8
+un ||proxmox-kernel-6.8.8-2-pve
+ii |amd64|proxmox-kernel-6.8.8-2-pve-signed
+ "#
+ .to_owned())
+ }
+ };
+
+ assert_eq!(
+ PostHookInfo::find_kernel_package_name(&mocked_run_cmd).unwrap(),
+ "proxmox-kernel-6.8.8-2-pve-signed"
+ );
+ }
+
+ #[test]
+ fn find_kernel_package_name_fails_on_wrong_architecture() {
+ let mocked_run_cmd = |cmd: &[&str]| {
+ if cmd[0] == "dpkg" {
+ assert_eq!(cmd, &["dpkg", "--print-architecture"]);
+ Ok("arm64\n".to_owned())
+ } else {
+ assert_eq!(
+ cmd,
+ &[
+ "dpkg-query",
+ "--showformat",
+ "${db:Status-Abbrev}|${Architecture}|${Package}\\n",
+ "--show",
+ "proxmox-kernel-[0-9]*",
+ ]
+ );
+ Ok(r#"ii |all|proxmox-kernel-6.8
+un ||proxmox-kernel-6.8.8-2-pve
+ii |amd64|proxmox-kernel-6.8.8-2-pve-signed
+ "#
+ .to_owned())
+ }
+ };
+
+ assert_eq!(
+ PostHookInfo::find_kernel_package_name(&mocked_run_cmd)
+ .unwrap_err()
+ .to_string(),
+ "failed to find installed kernel package"
+ );
+ }
+
+ #[test]
+ fn find_kernel_package_name_fails_on_missing_package() {
+ let mocked_run_cmd = |cmd: &[&str]| {
+ if cmd[0] == "dpkg" {
+ assert_eq!(cmd, &["dpkg", "--print-architecture"]);
+ Ok("amd64\n".to_owned())
+ } else {
+ assert_eq!(
+ cmd,
+ &[
+ "dpkg-query",
+ "--showformat",
+ "${db:Status-Abbrev}|${Architecture}|${Package}\\n",
+ "--show",
+ "proxmox-kernel-[0-9]*",
+ ]
+ );
+ Ok(r#"ii |all|proxmox-kernel-6.8
+un ||proxmox-kernel-6.8.8-2-pve
+ "#
+ .to_owned())
+ }
+ };
+
+ assert_eq!(
+ PostHookInfo::find_kernel_package_name(&mocked_run_cmd)
+ .unwrap_err()
+ .to_string(),
+ "failed to find installed kernel package"
+ );
+ }
+
+ #[test]
+ fn finds_correct_absolute_kernel_image_path() {
+ let mocked_run_cmd = |cmd: &[&str]| {
+ if cmd[0] == "dpkg" {
+ assert_eq!(cmd, &["dpkg", "--print-architecture"]);
+ Ok("amd64\n".to_owned())
+ } else if cmd[0..=1] == ["dpkg-query", "--showformat"] {
+ assert_eq!(
+ cmd,
+ &[
+ "dpkg-query",
+ "--showformat",
+ "${db:Status-Abbrev}|${Architecture}|${Package}\\n",
+ "--show",
+ "proxmox-kernel-[0-9]*",
+ ]
+ );
+ Ok(r#"ii |all|proxmox-kernel-6.8
+un ||proxmox-kernel-6.8.8-2-pve
+ii |amd64|proxmox-kernel-6.8.8-2-pve-signed
+ "#
+ .to_owned())
+ } else {
+ assert_eq!(
+ cmd,
+ [
+ "dpkg-query",
+ "--listfiles",
+ "proxmox-kernel-6.8.8-2-pve-signed"
+ ]
+ );
+ Ok(r#"
+/.
+/boot
+/boot/System.map-6.8.8-2-pve
+/boot/config-6.8.8-2-pve
+/boot/vmlinuz-6.8.8-2-pve
+/lib
+/lib/modules
+/lib/modules/6.8.8-2-pve
+/lib/modules/6.8.8-2-pve/kernel
+/lib/modules/6.8.8-2-pve/kernel/arch
+/lib/modules/6.8.8-2-pve/kernel/arch/x86
+/lib/modules/6.8.8-2-pve/kernel/arch/x86/crypto
+/lib/modules/6.8.8-2-pve/kernel/arch/x86/crypto/aegis128-aesni.ko
+/lib/modules/6.8.8-2-pve/kernel/arch/x86/crypto/aesni-intel.ko
+/lib/modules/6.8.8-2-pve/kernel/arch/x86/crypto/aria-aesni-avx-x86_64.ko
+/lib/modules/6.8.8-2-pve/kernel/arch/x86/crypto/aria-aesni-avx2-x86_64.ko
+/lib/modules/6.8.8-2-pve/kernel/arch/x86/crypto/aria-gfni-avx512-x86_64.ko
+/lib/modules/6.8.8-2-pve/kernel/arch/x86/crypto/blowfish-x86_64.ko
+/lib/modules/6.8.8-2-pve/kernel/arch/x86/crypto/camellia-aesni-avx-x86_64.ko
+ "#
+ .to_owned())
+ }
+ };
+
+ assert_eq!(
+ PostHookInfo::find_kernel_image_path(&mocked_run_cmd).unwrap(),
+ "/boot/vmlinuz-6.8.8-2-pve"
+ );
+ }
+}
--
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] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install
2024-07-18 13:49 ` [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install Christoph Heiss
@ 2024-07-23 14:57 ` Aaron Lauterer
2024-07-24 8:24 ` Thomas Lamprecht
2024-07-24 11:21 ` Aaron Lauterer
1 sibling, 1 reply; 34+ messages in thread
From: Aaron Lauterer @ 2024-07-23 14:57 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
There are quite a few preparation changes in other sub-crates
(auto-installer, installer-common).
I've only gotten through them for now and haven't looked at the actual
post-hook crate stuff.
Wouldn't it be nicer to split the preparation patches into their own
commit? It would make the patch smaller and the history would be a bit
clearer as we don't mix adding the post-hook crate with all the
preparation that is needed.
I plan to look at the actual post-hook stuff tomorrow
On 2024-07-18 15:49, Christoph Heiss wrote:
> This utility can be called with the low-level install config after a
> successful installation to send a notification via a HTTP POST request,
> if the user has configured an endpoint for that in the answer file.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v1 -> v2:
> * squash implementation and unit tests into one patch
> * simplify udev property retrieving by introducing proper helpers on
> `UdevInfo` itself
> * rename Answer::from_reader() -> Answer::try_from_reader to better
> reflect it returns a Result<>
> * improved error message in some places
> * added new fields; now includes ISO version, SecureBoot state, CPU
> and DMI info
> * product information was split into separate fields
> * boot mode information was split into separate fields
> * product version is now retrieved from the package using dpkg-query
> directly
> * kernel version was split into separate fields, retrieving version
> string from the image directly
> * all disks and NICs are now included, a field indicates whether they
> are boot disk or management interface, respectively
> * move with_chroot() invocation out of PostHookInfo::gather()
>
> 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 +-
> proxmox-auto-installer/src/udevinfo.rs | 8 +-
> .../src/fetch_plugins/http.rs | 2 +-
> proxmox-installer-common/src/http.rs | 6 +-
> proxmox-installer-common/src/options.rs | 5 +
> proxmox-installer-common/src/setup.rs | 2 +-
> proxmox-installer-common/src/utils.rs | 2 +
> proxmox-post-hook/Cargo.toml | 18 +
> proxmox-post-hook/src/main.rs | 784 ++++++++++++++++++
> 13 files changed, 843 insertions(+), 23 deletions(-)
> 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] 34+ messages in thread
* Re: [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install
2024-07-23 14:57 ` Aaron Lauterer
@ 2024-07-24 8:24 ` Thomas Lamprecht
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Lamprecht @ 2024-07-24 8:24 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer, Christoph Heiss
Am 23/07/2024 um 16:57 schrieb Aaron Lauterer:
> There are quite a few preparation changes in other sub-crates
> (auto-installer, installer-common).
> I've only gotten through them for now and haven't looked at the actual
> post-hook crate stuff.
>
> Wouldn't it be nicer to split the preparation patches into their own
> commit? It would make the patch smaller and the history would be a bit
> clearer as we don't mix adding the post-hook crate with all the
> preparation that is needed.
This isn't a binary thing, sometimes it can be nicer to have some
preparatory stuff up-front, most often if it's not only affecting new code
and code changes limited to the thing a patch wants to do, otherwise it can
also be nice to see what is needed to make it work, especially as often
preparatory patches cannot be reviewed in isolation anyway and one needs to
look into the patches that they're preparing for anyway to actually be able
to determine if the thing as a whole makes sense the way it's implemented,
split and used.
As mentioned, this is often subjective, both have some advantages and
disadvantages, IMO the best heuristic is the mentioned "how much is existing
code impacted and how much is this related to the semantic thing the patch
wants to do", all else is a bit of a balance that different reviewers might
also disagree a bit with, so it's also fine to reject a split or merge if
you, as patch author, really think it would make it worse – ideally with
some argumentation and avoiding hard lines to avoid "kerfuffle" or getting
less reviews in the future ;-)
unrelated and definitively not only applying just to you, only as a
gentle reminder: I would like to see that replies on the mailing list get
unrelated stuff trimmed out and use of inline/bottom posting, that makes
it quicker to read and reply and also to digest (longer) threads.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install
2024-07-18 13:49 ` [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install Christoph Heiss
2024-07-23 14:57 ` Aaron Lauterer
@ 2024-07-24 11:21 ` Aaron Lauterer
2024-08-05 13:17 ` Christoph Heiss
1 sibling, 1 reply; 34+ messages in thread
From: Aaron Lauterer @ 2024-07-24 11:21 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Two few small things inline
On 2024-07-18 15:49, Christoph Heiss wrote:
> This utility can be called with the low-level install config after a
> successful installation to send a notification via a HTTP POST request,
> if the user has configured an endpoint for that in the answer file.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v1 -> v2:
> * squash implementation and unit tests into one patch
> * simplify udev property retrieving by introducing proper helpers on
> `UdevInfo` itself
> * rename Answer::from_reader() -> Answer::try_from_reader to better
> reflect it returns a Result<>
> * improved error message in some places
> * added new fields; now includes ISO version, SecureBoot state, CPU
> and DMI info
> * product information was split into separate fields
> * boot mode information was split into separate fields
> * product version is now retrieved from the package using dpkg-query
> directly
> * kernel version was split into separate fields, retrieving version
> string from the image directly
> * all disks and NICs are now included, a field indicates whether they
> are boot disk or management interface, respectively
> * move with_chroot() invocation out of PostHookInfo::gather()
>
> 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 +-
> proxmox-auto-installer/src/udevinfo.rs | 8 +-
> .../src/fetch_plugins/http.rs | 2 +-
> proxmox-installer-common/src/http.rs | 6 +-
> proxmox-installer-common/src/options.rs | 5 +
> proxmox-installer-common/src/setup.rs | 2 +-
> proxmox-installer-common/src/utils.rs | 2 +
> proxmox-post-hook/Cargo.toml | 18 +
> proxmox-post-hook/src/main.rs | 784 ++++++++++++++++++
> 13 files changed, 843 insertions(+), 23 deletions(-)
> create mode 100644 proxmox-post-hook/Cargo.toml
> create mode 100644 proxmox-post-hook/src/main.rs
>
[…]
> diff --git a/proxmox-post-hook/Cargo.toml b/proxmox-post-hook/Cargo.toml
> new file mode 100644
> index 0000000..3acea6c
> --- /dev/null
> +++ b/proxmox-post-hook/Cargo.toml
> @@ -0,0 +1,18 @@
> +[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
> diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
> new file mode 100644
> index 0000000..d3e5b5c
> --- /dev/null
> +++ b/proxmox-post-hook/src/main.rs
> @@ -0,0 +1,784 @@
> +//! 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::HashSet,
> + ffi::CStr,
> + fs::{self, File},
> + io::BufReader,
> + os::unix::fs::FileExt,
> + path::PathBuf,
> + process::{Command, ExitCode},
> +};
> +
> +use anyhow::{anyhow, bail, Context, Result};
> +use proxmox_auto_installer::{
> + answer::{Answer, PostNotificationHookInfo},
> + udevinfo::{UdevInfo, UdevProperties},
> +};
> +use proxmox_installer_common::{
> + options::{Disk, FsType},
> + setup::{
> + load_installer_setup_files, BootType, InstallConfig, IsoInfo, ProxmoxProduct, RuntimeInfo,
> + SetupInfo,
> + },
> + sysinfo::SystemDMI,
> + utils::CidrAddress,
> +};
> +use serde::Serialize;
> +
> +/// Information about the system boot status.
> +#[derive(Serialize)]
> +struct BootInfo {
> + /// Whether the system is booted using UEFI or legacy BIOS.
> + mode: BootType,
> + /// Whether SecureBoot is enabled for the installation.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + secureboot: Option<bool>,
> +}
> +
> +/// 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.
Is this comment valid this way? AFAIU there was a dedicated struct for
boot disks in the previous version, but now this struct is for all
disks, optionally marking it as boot disk.
> +#[derive(Serialize)]
> +#[serde(rename_all = "kebab-case")]
> +struct DiskInfo {
> + /// Size in bytes
> + size: usize,
> + /// Set to true if the disk is used for booting.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + is_bootdisk: Option<bool>,
> + /// Properties about the device as given by udev.
> + udev_properties: UdevProperties,
> +}
> +
> +/// 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
> + #[serde(skip_serializing_if = "Option::is_none")]
> + address: Option<CidrAddress>,
> + /// Set to true if the interface is the chosen management interface during
> + /// installation.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + is_management: Option<bool>,
> + /// Properties about the device as given by udev.
> + udev_properties: UdevProperties,
> +}
> +
> +/// Information about the installed product itself.
> +#[derive(Serialize)]
> +#[serde(rename_all = "kebab-case")]
> +struct ProductInfo {
> + /// Full name of the product
> + fullname: String,
> + /// Product abbreviation
> + short: ProxmoxProduct,
> + /// Version of the installed product
> + version: String,
> +}
> +
> +/// The current kernel version.
> +/// Aligns with the format as used by the /nodes/<node>/status API of each product.
> +#[derive(Serialize)]
> +struct KernelVersionInformation {
> + /// The systemname/nodename
> + pub sysname: String,
> + /// The kernel release number
> + pub release: String,
> + /// The kernel version
> + pub version: String,
> + /// The machine architecture
> + pub machine: String,
> +}
> +
> +/// Information about the CPU(s) installed in the system
> +#[derive(Serialize)]
> +struct CpuInfo {
> + /// Number of physical CPU cores.
> + cores: usize,
> + /// Number of logical CPU cores aka. threads.
> + cpus: usize,
> + /// CPU feature flag set as a space-delimited list.
> + flags: String,
> + /// Whether hardware-accelerated virtualization is supported.
> + hvm: bool,
> + /// Reported model of the CPU(s)
> + model: String,
> + /// Number of physical CPU sockets
> + sockets: usize,
> +}
> +
> +/// 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: ProductInfo,
> + /// Release information for the ISO used for the installation.
> + iso: IsoInfo,
> + /// Installed kernel version
> + kernel_version: KernelVersionInformation,
> + /// Describes the boot mode of the machine and the SecureBoot status.
> + boot_info: BootInfo,
> + /// Information about the installed CPU(s)
> + cpu_info: CpuInfo,
> + /// DMI information about the system
> + dmi: SystemDMI,
> + /// 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,
> + /// All disks detected on the system.
> + disks: Vec<DiskInfo>,
> + /// All network interfaces detected on the system.
> + network_interfaces: Vec<NetworkInterfaceInfo>,
> + /// Public parts of SSH host keys of the installed system
> + ssh_public_host_keys: SshPublicHostKeys,
> +}
> +
> +/// 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 sending
> + /// it to a specified server.
> + ///
> + /// # Arguments
> + ///
> + /// * `target_path` - Path to where the chroot environment root is mounted
> + /// * `answer` - Answer file as provided by the user
> + fn gather(target_path: &str, answer: &Answer) -> Result<Self> {
> + println!("Gathering installed system data ..");
The three dots at the end can most likely be either 3 (or UTF-8 3-dots)
or just one?
> +
> + let config: InstallConfig =
> + serde_json::from_reader(BufReader::new(File::open("/tmp/low-level-config.json")?))?;
> +
[…]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install
2024-07-24 11:21 ` Aaron Lauterer
@ 2024-08-05 13:17 ` Christoph Heiss
0 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-08-05 13:17 UTC (permalink / raw)
To: Aaron Lauterer; +Cc: Proxmox VE development discussion
Thanks for the in-depth review!
On Wed, Jul 24, 2024 at 01:21:11PM GMT, Aaron Lauterer wrote:
> Two few small things inline
>
> On 2024-07-18 15:49, Christoph Heiss wrote:
> > [..]
> > diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
> > new file mode 100644
> > index 0000000..d3e5b5c
> > --- /dev/null
> > +++ b/proxmox-post-hook/src/main.rs
> > @@ -0,0 +1,784 @@
> > [..]
> > +/// 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.
> Is this comment valid this way? AFAIU there was a dedicated struct for boot
> disks in the previous version, but now this struct is for all disks,
> optionally marking it as boot disk.
No, that's really just a leftover from the previous revision, where it
really was just for boot disks. I'll change it appropriately for the
next version.
> > [..]
> > +impl PostHookInfo {
> > + /// Gathers all needed information about the newly installed system for sending
> > + /// it to a specified server.
> > + ///
> > + /// # Arguments
> > + ///
> > + /// * `target_path` - Path to where the chroot environment root is mounted
> > + /// * `answer` - Answer file as provided by the user
> > + fn gather(target_path: &str, answer: &Answer) -> Result<Self> {
> > + println!("Gathering installed system data ..");
> The three dots at the end can most likely be either 3 (or UTF-8 3-dots) or
> just one?
I'd go with three dots, based on that is what we use everywhere else (as
opposed to the UTF-8 ellipsis) and being the same style we use
everywhere else (most notably unconfigured.sh and the
auto-install-assistant) for such "in progress" tasks.
> > +
> > + let config: InstallConfig =
> > + serde_json::from_reader(BufReader::new(File::open("/tmp/low-level-config.json")?))?;
> > +
> […]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [pve-devel] [PATCH installer v2 17/17] unconfigured.sh: run proxmox-post-hook after successful auto-install
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (15 preceding siblings ...)
2024-07-18 13:49 ` [pve-devel] [PATCH installer v2 16/17] fix #5536: post-hook: add utility for sending notifications after auto-install Christoph Heiss
@ 2024-07-18 13:49 ` Christoph Heiss
2024-07-24 11:34 ` [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Aaron Lauterer
2024-08-21 9:41 ` Christoph Heiss
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-07-18 13:49 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
---
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] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (16 preceding siblings ...)
2024-07-18 13:49 ` [pve-devel] [PATCH installer v2 17/17] unconfigured.sh: run proxmox-post-hook after successful auto-install Christoph Heiss
@ 2024-07-24 11:34 ` Aaron Lauterer
2024-08-21 9:41 ` Christoph Heiss
18 siblings, 0 replies; 34+ messages in thread
From: Aaron Lauterer @ 2024-07-24 11:34 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
I tested the patch series with the HTTP auto install variant and logged
the POST hook request.
Overall it works as advertised.
Patch 6/17 <common: setup: deserialize `secure_boot` property from
runtime env> runs into a problem if the system isn't using secure boot.
But I think we should fix that in Proxmox/Install/RunEnv.pm to store it
in a way that the JSON stores it not in the form `"secure_boot":""` but
rather as `"secure_boot":0`.
Patch 11/17 <auto-installer: tests: replace manual panic!() with
assert_eq!()> should be dropped, as it will reduce the usefulness of the
output to find out why the test failed. The plan here is to switch that
to, most likely, pretty_assertions.
With this, and the smaller comments on the individual patches, considers
this series:
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism
2024-07-18 13:48 [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
` (17 preceding siblings ...)
2024-07-24 11:34 ` [pve-devel] [PATCH installer v2 00/17] fix #5536: implement post-(auto-)installation notification mechanism Aaron Lauterer
@ 2024-08-21 9:41 ` Christoph Heiss
18 siblings, 0 replies; 34+ messages in thread
From: Christoph Heiss @ 2024-08-21 9:41 UTC (permalink / raw)
To: Proxmox VE development discussion
v3 out: https://lists.proxmox.com/pipermail/pve-devel/2024-August/065163.html
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 34+ messages in thread