public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism
@ 2024-08-21  9:40 Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 01/20] tree-wide: fix some typos Christoph Heiss
                   ` (19 more replies)
  0 siblings, 20 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

This implements a mechanism for post-installation "notifications" via a
POST request [0] when using the auto-installer.

It's implemented as a separate, small utility to facilitate separation
of concerns and make the information gathering easier by having it
isolated in one place.

Patches #1 through #5 are simply clean-ups, refactors, etc. that were
done along the way and can be applied independently.

Most interesting here will be patch #16, which adds the actual
implementation of the post-hook. (Bind-)mounting the installed host
system is done using the existing `proxmox-chroot` utility, and the HTTP
POST functionality can fortunately be re-used 1:1 from
`proxmox-fetch-answer`.

I've also included an example of how the JSON body (pretty-printed and
reduced some things for readability) of such a post-installation request
would look like below, for reference. 
Where applicable (and sensible), I have tried to align the format as
much as possible with 1) the format as used in the `fetch-answer` POST
request and 2) PVE's /nodes/<host>/status API endpoint.

Feedback on the post-hook information schema is of course also very much
appreciated!

It should be noted that some information like DMI is generally very
depended on the motherboard/firmware, on what information is actually
available and filled-in. So the contents are expected to vary wildly
between machines and may also be empty, as in the example below from a
VM.

Tested this with both PVE and PBS ISOs, with BIOS, UEFI w/ and w/o
SecureBoot. PMG did not (yet) have a auto-installation-capable release.
The only really product-specific code is the version detection in
`proxmox-post-hook`, which already handles all three products, so it
should work OOTB too.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=5536

History
-------

v2: https://lists.proxmox.com/pipermail/pve-devel/2024-July/064764.html
v1: https://lists.proxmox.com/pipermail/pve-devel/2024-July/064580.html

Notable changes v2 -> v3:
  * dropped patch #11 "auto-installer: tests: replace manual panic!() 
    with assert_eq!()"
  * split out some preparatory changes into separate patches, based on
    Aaron's feedback
  * fixed bug in run env serialization w.r.t. secureboot state

Notable changes v1 -> v2:
  * dropped already applied patches & rebased on master
  * new fields; now includes ISO version, SecureBoot state, CPU and DMI 
    info 
  * product information was split into separate fields & expanded
  * 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
  * all disks and NICs are now included, a field indicates whether they
    are boot disk or management interface, respectively
  * some new cleanup/refactoring patches as noted on v1 
    (thanks Stefan for the in-depth review!)

Post notification example json
------------------------------

{
  "debian-version": "12.5",
  "product": {
    "fullname": "Proxmox VE",
    "short": "pve",
    "version": "8.2.2"
  },
  "iso": {
    "release": "8.2",
    "isorelease": "1"
  },
  "kernel-version": {
    "sysname": "Linux",
    "release": "6.8.4-2-pve",
    "version": "#1 SMP PREEMPT_DYNAMIC PMX 6.8.4-2 (2024-04-10T17:36Z)",
    "machine": "x86_64"
  },
  "boot-info": {
    "mode": "efi",
    "secureboot": true
  },
  "cpu-info": {
    "cores": 4,
    "cpus": 4,
    "flags": "fpu vme [..]",
    "hvm": true,
    "model": "AMD Ryzen 7 3700X 8-Core Processor",
    "sockets": 1
  },
  "dmi": {
    "system": {
      "serial": "",
      "sku": "",
      "uuid": "b2fd1aa2-dc6e-4d8f-ad67-6dcc31984938",
      "name": "Standard PC (Q35 + ICH9, 2009)"
    },
    "baseboard": {},
    "chassis": {
      "asset_tag": "",
      "serial": ""
    }
  },
  "filesystem": "ext4",
  "fqdn": "host.domain",
  "machine-id": "b8737afea804482697ffe04db69c73d1",
  "disks": [
    {
      "size": 8589934592,
      "is-bootdisk": true,
      "udev-properties": {
        "DEVNAME": "/dev/vda",
	[..]
      }
    },
    {
      "size": 8589934592,
      "udev-properties": {
        "DEVNAME": "/dev/vdb",
	[..]
      }
    }
  ],
  "network-interfaces": [
    {
      "mac": "de:ad:ff:c2:63:5e",
      "address": "10.0.0.27/24",
      "is-management": true,
      "udev-properties": {
        "INTERFACE": "enp6s18",
	[..]
      }
    },
    {
      "mac": "de:ad:ff:1c:c9:01",
      "udev-properties": {
        "INTERFACE": "enp6s19",
	[..]
      }
    }
  ],
  "ssh-public-host-keys": {
    "ecdsa": "ecdsa-sha2-nistp256 [..] root@host.domain",
    "ed25519": "ssh-ed25519 [..] root@host.domain",
    "rsa": "ssh-rsa [..] root@host.domain"
  }
}

Git diffstat
------------

Christoph Heiss (20):
  tree-wide: fix some typos
  fetch-answer: partition: fix clippy warning
  low level: run env: ensure `secure_boot` property is dumped as int
  common: simplify filesystem type serializing & Display trait impl
  common: setup: serialize `target_hd` as string explicitly
  common: split out installer setup files loading functionality
  common: setup: deserialize `secure_boot` property from runtime env
  common: http: pass url by reference
  debian: strip unused library dependencies
  fetch-answer: move http-related code to gated module in
    installer-common
  tree-wide: convert some more crates to use workspace dependencies
  auto-install-assistant: replace `PathBuf` parameters with
    `AsRef<Path>`
  auto-installer: tests: simplify empty disks check
  auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>`
  auto-installer: move `SystemDMI` struct to common crate
  auto-installer: answer: factor out answer file reading into function
  auto-installer: udevinfo: introduce type alias for udev properties
  fix #5536: auto-installer: answer: add `posthook` section
  fix #5536: post-hook: add utility for sending notifications after
    auto-install
  unconfigured.sh: run proxmox-post-hook after successful auto-install

 Cargo.toml                                    |  11 +
 Makefile                                      |   8 +-
 Proxmox/Install/RunEnv.pm                     |   3 +-
 debian/control                                |   1 +
 debian/install                                |   1 +
 debian/rules                                  |  13 +
 proxmox-auto-install-assistant/Cargo.toml     |  14 +-
 proxmox-auto-install-assistant/src/main.rs    |  27 +-
 proxmox-auto-installer/Cargo.toml             |  15 +-
 proxmox-auto-installer/src/answer.rs          |  27 +-
 .../src/bin/proxmox-auto-installer.rs         |  13 +-
 proxmox-auto-installer/src/sysinfo.rs         |  51 +-
 proxmox-auto-installer/src/udevinfo.rs        |   8 +-
 proxmox-auto-installer/src/utils.rs           |  15 +-
 proxmox-auto-installer/tests/parse-answer.rs  |  47 +-
 proxmox-chroot/Cargo.toml                     |   8 +-
 proxmox-fetch-answer/Cargo.toml               |  17 +-
 .../src/fetch_plugins/http.rs                 | 100 +--
 .../src/fetch_plugins/partition.rs            |   2 +-
 proxmox-installer-common/Cargo.toml           |  26 +-
 proxmox-installer-common/src/disk_checks.rs   |   2 +-
 proxmox-installer-common/src/http.rs          |  94 +++
 proxmox-installer-common/src/lib.rs           |   4 +
 proxmox-installer-common/src/options.rs       | 121 +--
 proxmox-installer-common/src/setup.rs         | 109 +--
 proxmox-installer-common/src/sysinfo.rs       |  52 ++
 proxmox-installer-common/src/utils.rs         |   2 +
 proxmox-post-hook/Cargo.toml                  |  18 +
 proxmox-post-hook/src/main.rs                 | 784 ++++++++++++++++++
 proxmox-tui-installer/Cargo.toml              |   8 +-
 proxmox-tui-installer/src/setup.rs            |   2 +-
 unconfigured.sh                               |  11 +-
 32 files changed, 1219 insertions(+), 395 deletions(-)
 create mode 100644 proxmox-installer-common/src/http.rs
 create mode 100644 proxmox-installer-common/src/sysinfo.rs
 create mode 100644 proxmox-post-hook/Cargo.toml
 create mode 100644 proxmox-post-hook/src/main.rs

-- 
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] 21+ messages in thread

* [pve-devel] [PATCH installer v3 01/20] tree-wide: fix some typos
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 02/20] fetch-answer: partition: fix clippy warning Christoph Heiss
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * fix some more typos

Changes v1 -> v2:
  * new patch

 proxmox-auto-install-assistant/src/main.rs     | 4 ++--
 proxmox-fetch-answer/src/fetch_plugins/http.rs | 2 +-
 proxmox-installer-common/src/disk_checks.rs    | 2 +-
 unconfigured.sh                                | 4 ++--
 4 files changed, 6 insertions(+), 6 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-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs
index 21bc6a2..f5af226 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
@@ -194,7 +194,7 @@ mod http_post {
     /// openssl s_client -connect <host>:443 < /dev/null 2>/dev/null | openssl x509 -fingerprint -sha256  -noout -in /dev/stdin
     /// ```
     ///
-    /// # Arguemnts
+    /// # 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.
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 {
diff --git a/unconfigured.sh b/unconfigured.sh
index a39e6ad..b1c980a 100755
--- a/unconfigured.sh
+++ b/unconfigured.sh
@@ -166,7 +166,7 @@ export PREVLEVEL=N
 mkdir -p /dev/shm
 mount -t tmpfs tmpfs /dev/shm
 
-# allow pseudo terminals for debuggin in X
+# allow pseudo terminals for debugging in X
 mkdir -p /dev/pts
 mount -vt devpts devpts /dev/pts -o gid=5,mode=620
 
@@ -190,7 +190,7 @@ if command -v dbus-daemon; then
     mkdir /run/dbus
     dbus-daemon --system --syslog-only
 
-    if [ $proxdebug -ne 0 ]; then # FIXME: better intergration, e.g., use iwgtk?
+    if [ $proxdebug -ne 0 ]; then # FIXME: better integration, e.g., use iwgtk?
         handle_wireless # no-op if not wireless dev is found
     fi
 fi
-- 
2.45.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 02/20] fetch-answer: partition: fix clippy warning
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 01/20] tree-wide: fix some typos Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 03/20] low level: run env: ensure `secure_boot` property is dumped as int Christoph Heiss
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 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 v2 -> v3:
  * no changes

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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 03/20] low level: run env: ensure `secure_boot` property is dumped as int
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 01/20] tree-wide: fix some typos Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 02/20] fetch-answer: partition: fix clippy warning Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 04/20] common: simplify filesystem type serializing & Display trait impl Christoph Heiss
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Previously, in the "false" case, it would serialize to an empty string.
In the future this property will be deserialized by (type-safe) Rust
code.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * new patch, found by Aaron

 Proxmox/Install/RunEnv.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index 7eaf96a..13ee8d8 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -291,7 +291,7 @@ sub query_installation_environment : prototype() {
 	    log_warn("Failed to read secure boot state: $@\n");
 	} else {
 	    my @secureboot = unpack("CCCCC", $content);
-	    $output->{secure_boot} = $secureboot[4] == 1;
+	    $output->{secure_boot} = $secureboot[4] == 1 ? 1 : 0;
 	}
     }
 
-- 
2.45.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 04/20] common: simplify filesystem type serializing & Display trait impl
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (2 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 03/20] low level: run env: ensure `secure_boot` property is dumped as int Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 05/20] common: setup: serialize `target_hd` as string explicitly Christoph Heiss
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 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 v2 -> v3:
  * no changes

Changes v1 -> v2:
  * implement FromStr instead of Deserialize and use
    serde_plain::derive_deserialize_from_fromstr!()

 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 9375ded..7a733e5 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 e4e609b..ad77308 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,
@@ -457,10 +452,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")]
@@ -519,51 +510,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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 05/20] common: setup: serialize `target_hd` as string explicitly
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (3 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 04/20] common: simplify filesystem type serializing & Display trait impl Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 06/20] common: split out installer setup files loading functionality Christoph Heiss
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * no changes

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 45ad222..86a02eb 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -173,7 +173,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"),
             }
         }
@@ -183,10 +183,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(())
 }
 
@@ -367,7 +367,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 ad77308..4be2f43 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -466,16 +466,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>,
 
@@ -499,14 +491,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 ee4e1c7..e3497b8 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -48,7 +48,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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 06/20] common: split out installer setup files loading functionality
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (4 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 05/20] common: setup: serialize `target_hd` as string explicitly Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 07/20] common: setup: deserialize `secure_boot` property from runtime env Christoph Heiss
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 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 v2 -> v3:
  * no changes

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 4be2f43..51d8711 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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 07/20] common: setup: deserialize `secure_boot` property from runtime env
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (5 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 06/20] common: split out installer setup files loading functionality Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 08/20] common: http: pass url by reference Christoph Heiss
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 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 v2 -> v3:
  * no changes

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 13ee8d8..e6f5893 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 51d8711..d5bdc9e 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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 08/20] common: http: pass url by reference
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (6 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 07/20] common: setup: deserialize `secure_boot` property from runtime env Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 09/20] debian: strip unused library dependencies Christoph Heiss
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * new patch, split out from later patch

 proxmox-fetch-answer/src/fetch_plugins/http.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs
index f5af226..5e10f6a 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
@@ -67,7 +67,7 @@ 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 = http_post::call(&answer_url, fingerprint.as_deref(), payload)?;
         Ok(answer)
     }
 
@@ -198,7 +198,7 @@ mod http_post {
     /// * `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> {
+    pub fn call(url: &str, fingerprint: Option<&str>, payload: String) -> Result<String> {
         let answer;
 
         if let Some(fingerprint) = fingerprint {
@@ -210,7 +210,7 @@ mod http_post {
             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()?;
@@ -230,7 +230,7 @@ mod http_post {
                 .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)?
-- 
2.45.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 09/20] debian: strip unused library dependencies
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (7 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 08/20] common: http: pass url by reference Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 10/20] fetch-answer: move http-related code to gated module in installer-common Christoph Heiss
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 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 v2 -> v3:
  * no changes

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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 10/20] fetch-answer: move http-related code to gated module in installer-common
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (8 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 09/20] debian: strip unused library dependencies Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 11/20] tree-wide: convert some more crates to use workspace dependencies Christoph Heiss
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 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 v2 -> v3:
  * no changes

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 5e10f6a..4317430 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
-    /// ```
-    ///
-    /// # 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 call(url: &str, 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..b754ed8
--- /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: &str, 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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 11/20] tree-wide: convert some more crates to use workspace dependencies
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (9 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 10/20] fetch-answer: move http-related code to gated module in installer-common Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 12/20] auto-install-assistant: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * no changes

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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 12/20] auto-install-assistant: replace `PathBuf` parameters with `AsRef<Path>`
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (10 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 11/20] tree-wide: convert some more crates to use workspace dependencies Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 13/20] auto-installer: tests: simplify empty disks check Christoph Heiss
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * no changes

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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 13/20] auto-installer: tests: simplify empty disks check
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (11 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 12/20] auto-install-assistant: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 14/20] auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * no changes

Changes v1 -> v2:
  * new patch, suggestion 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 450915a..72bddaa 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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 14/20] auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>`
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (12 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 13/20] auto-installer: tests: simplify empty disks check Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 15/20] auto-installer: move `SystemDMI` struct to common crate Christoph Heiss
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * no changes

Changes v1 -> v2:
  * new patch, suggestion 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 72bddaa..0e5d6e7 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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 15/20] auto-installer: move `SystemDMI` struct to common crate
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (13 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 14/20] auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 16/20] auto-installer: answer: factor out answer file reading into function Christoph Heiss
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 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 v2 -> v3:
  * no changes

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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 16/20] auto-installer: answer: factor out answer file reading into function
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (14 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 15/20] auto-installer: move `SystemDMI` struct to common crate Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 17/20] auto-installer: udevinfo: introduce type alias for udev properties Christoph Heiss
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * new patch, split out from later patch

 proxmox-auto-installer/src/answer.rs             | 16 +++++++++++++++-
 .../src/bin/proxmox-auto-installer.rs            | 11 +----------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index d691da1..bada8a5 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
@@ -18,6 +19,19 @@ pub struct Answer {
     pub disks: Disks,
 }
 
+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..b29228f 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))
 }
 
-- 
2.45.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 17/20] auto-installer: udevinfo: introduce type alias for udev properties
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (15 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 16/20] auto-installer: answer: factor out answer file reading into function Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 18/20] fix #5536: auto-installer: answer: add `posthook` section Christoph Heiss
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Makes handling & passing them around a bit more convenient. Will be used
in the upcoming proxmox-post-hook utility.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * new patch, split out from later patch

 proxmox-auto-installer/src/udevinfo.rs | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

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>,
 }
-- 
2.45.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 18/20] fix #5536: auto-installer: answer: add `posthook` section
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (16 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 17/20] auto-installer: udevinfo: introduce type alias for udev properties Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 19/20] fix #5536: post-hook: add utility for sending notifications after auto-install Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 20/20] unconfigured.sh: run proxmox-post-hook after successful auto-install Christoph Heiss
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * no changes

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 bada8a5..36f9276 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -17,6 +17,8 @@ pub struct Answer {
     pub network: Network,
     #[serde(rename = "disk-setup")]
     pub disks: Disks,
+    #[serde(default)]
+    pub posthook: Option<PostNotificationHookInfo>,
 }
 
 impl Answer {
@@ -48,6 +50,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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 19/20] fix #5536: post-hook: add utility for sending notifications after auto-install
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (17 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 18/20] fix #5536: auto-installer: answer: add `posthook` section Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 20/20] unconfigured.sh: run proxmox-post-hook after successful auto-install Christoph Heiss
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 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>
---
Since it was discussed a bit in the last revision [0 ff], some changes
to other crates were split out, while keeping all small trivial changes
-- which are really not worth separate commits and are confined in
impact to this tool -- changes as part of this patch.

[0] https://lists.proxmox.com/pipermail/pve-devel/2024-July/064858.html

Changes v2 -> v3:
  * split out two prepratory changes into separate changes
  * fix outdated doc-comment for `struct DiskInfo`
  * align continuation dots with rest of messages

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()

 Cargo.toml                                    |   1 +
 Makefile                                      |   8 +-
 debian/install                                |   1 +
 .../src/bin/proxmox-auto-installer.rs         |   2 -
 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 ++++++++++++++++++
 9 files changed, 818 insertions(+), 5 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/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index b29228f..6c78d5f 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -82,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-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 7a733e5..3f71840 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 d5bdc9e..f91a939 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..8558da2
--- /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,
+}
+
+/// Holds information about a single disk in the system.
+#[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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [pve-devel] [PATCH installer v3 20/20] unconfigured.sh: run proxmox-post-hook after successful auto-install
  2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
                   ` (18 preceding siblings ...)
  2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 19/20] fix #5536: post-hook: add utility for sending notifications after auto-install Christoph Heiss
@ 2024-08-21  9:40 ` Christoph Heiss
  19 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2024-08-21  9:40 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * no changes

Changes v1 -> v2:
  * no changes

 unconfigured.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/unconfigured.sh b/unconfigured.sh
index b1c980a..51b7974 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.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2024-08-21  9:48 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-21  9:40 [pve-devel] [PATCH installer v3 00/20] fix #5536: implement post-(auto-)installation notification mechanism Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 01/20] tree-wide: fix some typos Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 02/20] fetch-answer: partition: fix clippy warning Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 03/20] low level: run env: ensure `secure_boot` property is dumped as int Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 04/20] common: simplify filesystem type serializing & Display trait impl Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 05/20] common: setup: serialize `target_hd` as string explicitly Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 06/20] common: split out installer setup files loading functionality Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 07/20] common: setup: deserialize `secure_boot` property from runtime env Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 08/20] common: http: pass url by reference Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 09/20] debian: strip unused library dependencies Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 10/20] fetch-answer: move http-related code to gated module in installer-common Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 11/20] tree-wide: convert some more crates to use workspace dependencies Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 12/20] auto-install-assistant: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 13/20] auto-installer: tests: simplify empty disks check Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 14/20] auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>` Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 15/20] auto-installer: move `SystemDMI` struct to common crate Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 16/20] auto-installer: answer: factor out answer file reading into function Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 17/20] auto-installer: udevinfo: introduce type alias for udev properties Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 18/20] fix #5536: auto-installer: answer: add `posthook` section Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 19/20] fix #5536: post-hook: add utility for sending notifications after auto-install Christoph Heiss
2024-08-21  9:40 ` [pve-devel] [PATCH installer v3 20/20] unconfigured.sh: run proxmox-post-hook after successful auto-install Christoph Heiss

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal