[pve-devel] [PATCH installer v2 4/4] fetch-answer: use partition label from fetch config instead of hardcoded

[Christoph Heiss]

This has been requested by at least one user one user [0] and definitely
makes sense, esp. for BMCs/IPMIs where one might not be able to control
the partition label.

[0] https://forum.proxmox.com/threads/proxmox-ais-question-request.153043/post-695689

Tested-By: Aaron Lauterer <a.lauterer at proxmox.com>
Reviewed-By: Aaron Lauterer <a.lauterer at proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss at proxmox.com>
---
Changes v1 -> v2:
  * addressed suggestions made by Aaron

 proxmox-auto-install-assistant/src/main.rs    |  2 ++
 .../src/fetch_plugins/partition.rs            | 16 ++++++-------
 proxmox-fetch-answer/src/main.rs              | 23 +++++++++++++------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index 5d11882..9f0fbec 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -146,6 +146,8 @@ struct CommandPrepareISO {
 
     /// Can be used in combination with `--fetch-from partition` to set the partition label
     /// the auto-installer will search for.
+    // FAT can only handle 11 characters (per specification at least, drivers might allow more),
+    // so shorten "Automated Installer Source" to "AIS" to be safe.
     #[arg(long, default_value_t = { "proxmox-ais".to_owned() } )]
     partition_label: String,
 }
diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
index cbfe2d5..f4f8bcc 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -8,18 +8,16 @@ use std::{
 
 static ANSWER_FILE: &str = "answer.toml";
 static ANSWER_MP: &str = "/mnt/answer";
-// FAT can only handle 11 characters, so shorten Automated Installer Source to AIS
-static PARTLABEL: &str = "proxmox-ais";
 static DISK_BY_ID_PATH: &str = "/dev/disk/by-label";
 
 pub struct FetchFromPartition;
 
 impl FetchFromPartition {
     /// Returns the contents of the answer file
-    pub fn get_answer() -> Result<String> {
+    pub fn get_answer(part_label: &str) -> Result<String> {
         info!("Checking for answer file on partition.");
 
-        let mut mount_path = PathBuf::from(mount_proxmoxinst_part()?);
+        let mut mount_path = PathBuf::from(mount_proxmoxinst_part(part_label)?);
         mount_path.push(ANSWER_FILE);
         let answer = fs::read_to_string(mount_path)
             .map_err(|err| format_err!("failed to read answer file - {err}"))?;
@@ -74,14 +72,16 @@ fn scan_partlabels(partlabel: &str, search_path: &str) -> Result<PathBuf> {
     bail!("Could not find partition for label '{partlabel}'");
 }
 
-/// Will search and mount a partition/FS labeled PARTLABEL (proxmox-ais) in lower or uppercase
-/// to ANSWER_MP
-fn mount_proxmoxinst_part() -> Result<String> {
+/// Searches for a partition/filesystem labeled `part_label` mounts it to `ANSWER_MP`, if found.
+///
+/// # Arguments
+///   * `partlabel` - Partition Label, used for matching, in the exact, upper and lower case
+fn mount_proxmoxinst_part(part_label: &str) -> Result<String> {
     if let Ok(true) = check_if_mounted(ANSWER_MP) {
         info!("Skipping: '{ANSWER_MP}' is already mounted.");
         return Ok(ANSWER_MP.into());
     }
-    let part_path = scan_partlabels(PARTLABEL, DISK_BY_ID_PATH)?;
+    let part_path = scan_partlabels(part_label, DISK_BY_ID_PATH)?;
     info!("Mounting partition at {ANSWER_MP}");
     // create dir for mountpoint
     create_dir_all(ANSWER_MP)?;
diff --git a/proxmox-fetch-answer/src/main.rs b/proxmox-fetch-answer/src/main.rs
index 753ab29..d56f8b9 100644
--- a/proxmox-fetch-answer/src/main.rs
+++ b/proxmox-fetch-answer/src/main.rs
@@ -26,6 +26,7 @@ Commands:
   http        Fetch the answer file via HTTP(S)
               Additional parameters: [<http-url>] [<tls-cert-fingerprint>]
   partition   Fetch the answer file from a mountable partition
+              Additional parameters: [<partition-label>]
 
 Options:
   -h, --help  Print this help menu
@@ -49,10 +50,12 @@ fn fetch_answer(install_settings: &AutoInstSettings) -> Result<String> {
                 Err(err) => info!("Fetching answer file from ISO failed: {err}"),
             }
         }
-        FetchAnswerFrom::Partition => match FetchFromPartition::get_answer() {
-            Ok(answer) => return Ok(answer),
-            Err(err) => info!("Fetching answer file from partition failed: {err}"),
-        },
+        FetchAnswerFrom::Partition => {
+            match FetchFromPartition::get_answer(&install_settings.partition_label) {
+                Ok(answer) => return Ok(answer),
+                Err(err) => info!("Fetching answer file from partition failed: {err}"),
+            }
+        }
         FetchAnswerFrom::Http => match FetchFromHTTP::get_answer(&install_settings.http) {
             Ok(answer) => return Ok(answer),
             Err(err) => info!("Fetching answer file via HTTP failed: {err}"),
@@ -74,18 +77,24 @@ fn settings_from_cli_args(args: &[String]) -> Result<AutoInstSettings> {
     };
 
     match mode {
-        FetchAnswerFrom::Iso | FetchAnswerFrom::Partition if args.len() > 2 => {
-            bail!("'iso' and 'partition' modes do not take any additional arguments")
+        FetchAnswerFrom::Iso if args.len() > 2 => {
+            bail!("'iso' mode does not take any additional arguments")
         }
         FetchAnswerFrom::Http if args.len() > 4 => {
             bail!("'http' mode takes at most 2 additional arguments")
         }
+        FetchAnswerFrom::Partition if args.len() > 3 => {
+            bail!("'partition' mode takes at most 1 additional argument")
+        }
         _ => {}
     };
 
     Ok(AutoInstSettings {
         mode,
-        partition_label: "proxmox-ais".to_owned(),
+        partition_label: args
+            .get(2)
+            .ok_or(format_err!("partition label expected"))
+            .cloned()?,
         http: HttpOptions {
             url: args.get(2).cloned(),
             cert_fingerprint: args.get(3).cloned(),
-- 
2.47.0