public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer 1/2] assistant: validate: warn if answer file contains old snake_case keys
@ 2025-04-30 11:25 Christoph Heiss
  2025-04-30 11:25 ` [pve-devel] [RFC PATCH for-9.0 installer 2/2] auto: answer: drop deprecated " Christoph Heiss
  0 siblings, 1 reply; 2+ messages in thread
From: Christoph Heiss @ 2025-04-30 11:25 UTC (permalink / raw)
  To: pve-devel

Parses the TOML file as a `Value` without structure and then just
iterates over all keys. An exception is made for filter values, as these
contain raw udev key names.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Essentially a bit more robust version of

https://lore.proxmox.com/pve-devel/20250217121748.117222-6-d.kral@proxmox.com/

by actually parsing the TOML and iterating over the keys.

 proxmox-auto-install-assistant/src/main.rs    | 73 +++++++++++++++++--
 proxmox-auto-installer/src/answer.rs          |  4 +-
 proxmox-auto-installer/src/utils.rs           | 12 +--
 .../tests/resources/parse_answer/btrfs.toml   |  4 +-
 .../btrfs_raid_level_uppercase.toml           |  4 +-
 .../resources/parse_answer/disk_match.toml    |  6 +-
 .../parse_answer/disk_match_all.toml          |  6 +-
 .../parse_answer/disk_match_any.toml          |  6 +-
 .../resources/parse_answer/first_boot.toml    |  4 +-
 .../parse_answer/fqdn_from_dhcp.toml          |  4 +-
 ...cp_no_dhcp_domain_with_default_domain.toml |  4 +-
 ...ll_fqdn_from_dhcp_with_default_domain.toml |  4 +-
 .../parse_answer/hashed_root_password.toml    |  4 +-
 .../tests/resources/parse_answer/minimal.toml |  4 +-
 .../resources/parse_answer/nic_matching.toml  |  4 +-
 .../resources/parse_answer/specific_nic.toml  |  5 +-
 .../tests/resources/parse_answer/zfs.toml     |  6 +-
 .../zfs_raid_level_uppercase.toml             |  6 +-
 .../both_password_and_hashed_set.json         |  2 +-
 .../both_password_and_hashed_set.toml         |  6 +-
 .../fqdn_from_dhcp_no_default_domain.toml     |  4 +-
 .../parse_answer_fail/fqdn_hostname_only.toml |  4 +-
 .../parse_answer_fail/no_fqdn_from_dhcp.toml  |  4 +-
 .../no_root_password_set.json                 |  2 +-
 .../no_root_password_set.toml                 |  2 +-
 .../parse_answer_fail/short_password.json     |  2 +-
 .../parse_answer_fail/short_password.toml     |  4 +-
 27 files changed, 124 insertions(+), 66 deletions(-)

diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index b64623b..add5053 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -1,4 +1,4 @@
-use anyhow::{Result, bail, format_err};
+use anyhow::{Context, Result, bail, format_err};
 use clap::{Args, Parser, Subcommand, ValueEnum};
 use glob::Pattern;
 use serde::Serialize;
@@ -280,12 +280,72 @@ fn match_filter(args: &CommandDeviceMatch) -> Result<()> {
     Ok(())
 }
 
-fn validate_answer(args: &CommandValidateAnswer) -> Result<()> {
-    let answer = parse_answer(&args.path)?;
-    if args.debug {
-        println!("Parsed data from answer file:\n{:#?}", answer);
+fn validate_answer_file_keys(path: impl AsRef<Path> + fmt::Debug) -> Result<bool> {
+    let mut file =
+        fs::File::open(&path).with_context(|| format!("Opening answer file {path:?} failed"))?;
+
+    let mut contents = String::new();
+    file.read_to_string(&mut contents)
+        .with_context(|| format!("Reading from file {path:?} failed"))?;
+
+    fn validate(section: &[&str], v: toml::Value) -> bool {
+        // These sections only hold udev properties, so don't validate them
+        if section == ["network", "filter"] || section == ["disk-setup", "filter"] {
+            return true;
+        }
+
+        let mut valid = true;
+        if let toml::Value::Table(t) = v {
+            for (k, v) in t {
+                if k.contains('_') {
+                    eprintln!(
+                        "Warning: Section [{}] contains deprecated key `{k}`, use `{}` instead.",
+                        section.join("."),
+                        k.replace("_", "-")
+                    );
+                    valid = false;
+                }
+                valid &= validate(&[section, &[&k]].concat(), v);
+            }
+        }
+
+        valid
+    }
+
+    let answers: toml::Value = toml::from_str(&contents).context("Error parsing answer file")?;
+
+    if validate(&[], answers) {
+        Ok(true)
+    } else {
+        eprintln!(
+            "Warning: Answer file is using deprecated underscore keys. \
+            Since PVE 8.4-1 and PBS 3.4-1, kebab-case style keys are now preferred."
+        );
+        Ok(false)
+    }
+}
+
+fn validate_answer(args: &CommandValidateAnswer) -> Result<()> {
+    let mut valid = validate_answer_file_keys(&args.path)?;
+
+    match parse_answer(&args.path) {
+        Ok(answer) => {
+            if args.debug {
+                println!("Parsed data from answer file:\n{:#?}", answer);
+            }
+        }
+        Err(err) => {
+            eprintln!("{err:#}");
+            valid = false;
+        }
+    }
+
+    if valid {
+        println!("The answer file was parsed successfully, no errors found!");
+        Ok(())
+    } else {
+        bail!("Found issues in the answer file.");
     }
-    Ok(())
 }
 
 fn show_system_info(_args: &CommandSystemInfo) -> Result<()> {
@@ -599,7 +659,6 @@ fn parse_answer(path: impl AsRef<Path> + fmt::Debug) -> Result<Answer> {
             verify_locale_settings(&answer, &serde_json::from_str(LOCALE_INFO)?)?;
             verify_first_boot_settings(&answer)?;
             verify_email_and_root_password_settings(&answer)?;
-            println!("The answer file was parsed successfully, no errors found!");
             Ok(answer)
         }
         Err(err) => bail!("Error parsing answer file: {err}"),
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index 34065e8..d0d5b78 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -286,10 +286,10 @@ impl TryFrom<DiskSetup> for Disks {
 
     fn try_from(source: DiskSetup) -> Result<Self, Self::Error> {
         if source.disk_list.is_empty() && source.filter.is_none() {
-            return Err("Need either 'disk_list' or 'filter' set");
+            return Err("Need either 'disk-list' or 'filter' set");
         }
         if !source.disk_list.is_empty() && source.filter.is_some() {
-            return Err("Cannot use both, 'disk_list' and 'filter'");
+            return Err("Cannot use both, 'disk-list' and 'filter'");
         }
 
         let disk_selection = if !source.disk_list.is_empty() {
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 365e01a..185905c 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -209,7 +209,7 @@ fn set_single_disk(
                 .find(|item| item.path.ends_with(disk_name.as_str()));
             match disk {
                 Some(disk) => config.target_hd = Some(disk.path.clone()),
-                None => bail!("disk in 'disk_selection' not found"),
+                None => bail!("disk in 'disk-selection' not found"),
             }
         }
         answer::DiskSelection::Filter(filter) => {
@@ -352,8 +352,8 @@ pub fn verify_locale_settings(answer: &Answer, locales: &LocaleInfo) -> Result<(
 
 /// Validates the following options of an user-provided answer:
 ///
-/// - `global.root_password`
-/// - `global.root_password_hashed`
+/// - `global.root-password`
+/// - `global.root-password-hashed`
 /// - `global.mailto`
 ///
 /// Ensures that the provided email-address is of valid format and that one
@@ -369,15 +369,15 @@ pub fn verify_email_and_root_password_settings(answer: &Answer) -> Result<()> {
     ) {
         (Some(_), Some(_)) => {
             bail!(
-                "`global.root_password` and `global.root_password_hashed` cannot be set at the same time"
+                "`global.root-password` and `global.root-password-hashed` cannot be set at the same time"
             );
         }
         (None, None) => {
-            bail!("One of `global.root_password` or `global.root_password_hashed` must be set");
+            bail!("One of `global.root-password` or `global.root-password-hashed` must be set");
         }
         (Some(password), None) if password.len() < ROOT_PASSWORD_MIN_LENGTH => {
             bail!(
-                "`global.root_password` must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long"
+                "`global.root-password` must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long"
             );
         }
         _ => Ok(()),
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
index 9071f7f..ee04b72 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
@@ -14,4 +14,4 @@ filesystem = "btrfs"
 btrfs.raid = "raid1"
 btrfs.compress = "zlib"
 btrfs.hdsize = 80
-disk_list = ["sda", "sdb"]
+disk-list = ["sda", "sdb"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/btrfs_raid_level_uppercase.toml b/proxmox-auto-installer/tests/resources/parse_answer/btrfs_raid_level_uppercase.toml
index f8f7aa9..422edec 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/btrfs_raid_level_uppercase.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/btrfs_raid_level_uppercase.toml
@@ -4,12 +4,12 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "btrfs"
-disk_list = ["sda", "sdb"]
+disk-list = ["sda", "sdb"]
 btrfs.raid = "RAID1"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml
index 0351f5c..9db80d5 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
@@ -12,6 +12,6 @@ source = "from-dhcp"
 [disk-setup]
 filesystem = "zfs"
 zfs.raid = "raid10"
-zfs.arc_max = 2048
-#disk_list = ['sda']
+zfs.arc-max = 2048
+#disk-list = ['sda']
 filter.ID_SERIAL = "*MZ7KM240HAGR*"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml
index ed38d20..ce4c53f 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
@@ -12,7 +12,7 @@ source = "from-dhcp"
 [disk-setup]
 filesystem = "zfs"
 zfs.raid = "raid0"
-zfs.arc_max = 2048
-filter_match = "all"
+zfs.arc-max = 2048
+filter-match = "all"
 filter.ID_SERIAL = "*MZ7KM240HAGR*"
 filter.ID_SERIAL_SHORT = "S2HRNX0J403419"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml
index 8bd8b86..e972377 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
@@ -12,7 +12,7 @@ source = "from-dhcp"
 [disk-setup]
 filesystem = "zfs"
 zfs.raid = "raid10"
-zfs.arc_max = 2048
-filter_match = "any"
+zfs.arc-max = 2048
+filter-match = "any"
 filter.ID_SERIAL = "*MZ7KM240HAGR*"
 filter.ID_MODEL = "Micron_9300*"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/first_boot.toml b/proxmox-auto-installer/tests/resources/parse_answer/first_boot.toml
index 720cd9c..f2d7402 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/first_boot.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/first_boot.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [first-boot]
 source = "from-iso"
@@ -15,4 +15,4 @@ source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp.toml b/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp.toml
index 9d13a5f..169026a 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp.toml
@@ -4,11 +4,11 @@ country = "at"
 fqdn.source = "from-dhcp"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_no_dhcp_domain_with_default_domain.toml b/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_no_dhcp_domain_with_default_domain.toml
index 8fbbede..59f795a 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_no_dhcp_domain_with_default_domain.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_no_dhcp_domain_with_default_domain.toml
@@ -3,7 +3,7 @@ keyboard = "de"
 country = "at"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [global.fqdn]
 source = "from-dhcp"
@@ -14,4 +14,4 @@ source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/full_fqdn_from_dhcp_with_default_domain.toml b/proxmox-auto-installer/tests/resources/parse_answer/full_fqdn_from_dhcp_with_default_domain.toml
index 8fbbede..59f795a 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/full_fqdn_from_dhcp_with_default_domain.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/full_fqdn_from_dhcp_with_default_domain.toml
@@ -3,7 +3,7 @@ keyboard = "de"
 country = "at"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [global.fqdn]
 source = "from-dhcp"
@@ -14,4 +14,4 @@ source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml
index ed4d2e5..004fbc1 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml
@@ -4,11 +4,11 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password_hashed = "$y$j9T$VgMv8lsz/TEvzesCZU3xD.$SK.h4QW51Jr/EmjuaTz5Bt4kYiX2Iezz6omzoqVEwj9"
+root-password-hashed = "$y$j9T$VgMv8lsz/TEvzesCZU3xD.$SK.h4QW51Jr/EmjuaTz5Bt4kYiX2Iezz6omzoqVEwj9"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/minimal.toml b/proxmox-auto-installer/tests/resources/parse_answer/minimal.toml
index 16f355c..f6de242 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/minimal.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/minimal.toml
@@ -4,11 +4,11 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml
index eb6130a..901f894 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-answer"
@@ -16,4 +16,4 @@ filter.ID_NET_NAME_MAC = "*a0369f0ab382"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml b/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml
index 4ea49bc..9a9d54f 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-answer"
@@ -13,7 +13,6 @@ dns = "10.10.10.1"
 gateway = "10.10.10.1"
 filter.ID_NET_NAME = "enp129s0f1np1"
 
-
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/zfs.toml b/proxmox-auto-installer/tests/resources/parse_answer/zfs.toml
index 971b463..10178e9 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/zfs.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/zfs.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
@@ -17,5 +17,5 @@ zfs.checksum = "on"
 zfs.compress = "lz4"
 zfs.copies = 2
 zfs.hdsize = 80
-zfs.arc_max = 2048
-disk_list = ["sda", "sdb"]
+zfs.arc-max = 2048
+disk-list = ["sda", "sdb"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/zfs_raid_level_uppercase.toml b/proxmox-auto-installer/tests/resources/parse_answer/zfs_raid_level_uppercase.toml
index 0b0d6cb..5e6ccae 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/zfs_raid_level_uppercase.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/zfs_raid_level_uppercase.toml
@@ -4,13 +4,13 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "zfs"
-disk_list = ["sda", "sdb", "sdc"]
+disk-list = ["sda", "sdb", "sdc"]
 zfs.raid = "RAIDZ-1"
-zfs.arc_max = 2048
+zfs.arc-max = 2048
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/both_password_and_hashed_set.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/both_password_and_hashed_set.json
index fd1213e..0cfaf0c 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/both_password_and_hashed_set.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/both_password_and_hashed_set.json
@@ -1,3 +1,3 @@
 {
-  "error": "`global.root_password` and `global.root_password_hashed` cannot be set at the same time"
+  "error": "`global.root-password` and `global.root-password-hashed` cannot be set at the same time"
 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/both_password_and_hashed_set.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/both_password_and_hashed_set.toml
index 0a56fc9..665d440 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/both_password_and_hashed_set.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/both_password_and_hashed_set.toml
@@ -4,12 +4,12 @@ country = "at"
 fqdn = "both-password-and-hashed-set.fail.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
-root_password_hashed = "$y$j9T$343s9MNhV4xZhW1Be6J6H1$rIxofnXWmp0FQGGIPO3BRwb1jK4ZXWaxT7OjhHJmum0"
+root-password = "12345678"
+root-password-hashed = "$y$j9T$343s9MNhV4xZhW1Be6J6H1$rIxofnXWmp0FQGGIPO3BRwb1jK4ZXWaxT7OjhHJmum0"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.toml
index 9d13a5f..169026a 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.toml
@@ -4,11 +4,11 @@ country = "at"
 fqdn.source = "from-dhcp"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_hostname_only.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_hostname_only.toml
index a0a22ef..871c0f5 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_hostname_only.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_hostname_only.toml
@@ -4,11 +4,11 @@ country = "at"
 fqdn = "foobar"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.toml
index 9d13a5f..169026a 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.toml
@@ -4,11 +4,11 @@ country = "at"
 fqdn.source = "from-dhcp"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345678"
+root-password = "12345678"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_root_password_set.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_root_password_set.json
index 6d75755..885fa21 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_root_password_set.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_root_password_set.json
@@ -1,3 +1,3 @@
 {
-  "error": "One of `global.root_password` or `global.root_password_hashed` must be set"
+  "error": "One of `global.root-password` or `global.root-password-hashed` must be set"
 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_root_password_set.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_root_password_set.toml
index 454e0b6..824d593 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_root_password_set.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_root_password_set.toml
@@ -10,4 +10,4 @@ source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/short_password.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/short_password.json
index c424b0b..d888f6b 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/short_password.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/short_password.json
@@ -1,3 +1,3 @@
 {
-    "error": "`global.root_password` must be at least 8 characters long"
+    "error": "`global.root-password` must be at least 8 characters long"
 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/short_password.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/short_password.toml
index a0eb1ec..0a56017 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/short_password.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/short_password.toml
@@ -4,11 +4,11 @@ country = "at"
 fqdn = "short-password.fail.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "12345"
+root-password = "12345"
 
 [network]
 source = "from-dhcp"
 
 [disk-setup]
 filesystem = "ext4"
-disk_list = ["sda"]
+disk-list = ["sda"]
-- 
2.49.0



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


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

* [pve-devel] [RFC PATCH for-9.0 installer 2/2] auto: answer: drop deprecated snake_case keys
  2025-04-30 11:25 [pve-devel] [PATCH installer 1/2] assistant: validate: warn if answer file contains old snake_case keys Christoph Heiss
@ 2025-04-30 11:25 ` Christoph Heiss
  0 siblings, 0 replies; 2+ messages in thread
From: Christoph Heiss @ 2025-04-30 11:25 UTC (permalink / raw)
  To: pve-devel

This has been deprecated in PVE 8.4-1 and PBS 3.4-1, respectively. No
PMG release to date with this change.

The kebab-case style is preferred for all new things.

Migration for users is pretty simple, for both hand-written answer files
as well as on-the-fly generated ones by automated provisioning tools and
such - basically a s/_/-/.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Would be nice to drop them with the next major release, but if there are
reasons for keeping it around for another major, disregard this patch.

 proxmox-auto-install-assistant/src/main.rs      |  6 +++---
 proxmox-auto-installer/src/answer.rs            | 17 ++++-------------
 proxmox-auto-installer/tests/parse-answer.rs    |  3 ++-
 .../parse_answer_fail/underscore_keys.json      |  3 +++
 .../parse_answer_fail/underscore_keys.toml      | 14 ++++++++++++++
 5 files changed, 26 insertions(+), 17 deletions(-)
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/underscore_keys.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/underscore_keys.toml

diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index add5053..fc8e9b2 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -299,7 +299,7 @@ fn validate_answer_file_keys(path: impl AsRef<Path> + fmt::Debug) -> Result<bool
             for (k, v) in t {
                 if k.contains('_') {
                     eprintln!(
-                        "Warning: Section [{}] contains deprecated key `{k}`, use `{}` instead.",
+                        "Error: Section [{}] contains key `{k}` which has been removed, use `{}` instead.",
                         section.join("."),
                         k.replace("_", "-")
                     );
@@ -318,8 +318,8 @@ fn validate_answer_file_keys(path: impl AsRef<Path> + fmt::Debug) -> Result<bool
         Ok(true)
     } else {
         eprintln!(
-            "Warning: Answer file is using deprecated underscore keys. \
-            Since PVE 8.4-1 and PBS 3.4-1, kebab-case style keys are now preferred."
+            "Error: Answer file is using underscore keys, which have been removed. \
+            Since PVE 8.4-1 and PBS 3.4-1, kebab-case style keys are now preferred and mandantory since PVE 9.0-1."
         );
         Ok(false)
     }
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index d0d5b78..c631b1b 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -10,9 +10,6 @@ use proxmox_installer_common::{
 use serde::{Deserialize, Serialize};
 use std::{collections::BTreeMap, io::BufRead, net::IpAddr};
 
-// NOTE New answer file properties must use kebab-case, but should allow snake_case for backwards
-// compatibility. TODO Remove the snake_cased variants in a future major version (e.g. PVE 10).
-
 // BTreeMap is used to store filters as the order of the filters will be stable, compared to
 // storing them in a HashMap
 
@@ -51,15 +48,13 @@ pub struct Global {
     pub keyboard: KeyboardLayout,
     pub mailto: String,
     pub timezone: String,
-    #[serde(alias = "root_password")]
     pub root_password: Option<String>,
-    #[serde(alias = "root_password_hashed")]
     pub root_password_hashed: Option<String>,
-    #[serde(alias = "reboot_on_error", default)]
+    #[serde(default)]
     pub reboot_on_error: bool,
-    #[serde(alias = "reboot_mode", default)]
+    #[serde(default)]
     pub reboot_mode: RebootMode,
-    #[serde(alias = "root_ssh_keys", default)]
+    #[serde(default)]
     pub root_ssh_keys: Vec<String>,
 }
 
@@ -110,7 +105,6 @@ pub struct PostNotificationHookInfo {
     /// URL to send a POST request to
     pub url: String,
     /// SHA256 cert fingerprint if certificate pinning should be used.
-    #[serde(alias = "cert_fingerprint")]
     pub cert_fingerprint: Option<String>,
 }
 
@@ -168,7 +162,6 @@ pub struct FirstBootHookInfo {
     /// Retrieve the post-install script from a URL, if source == "from-url".
     pub url: Option<String>,
     /// SHA256 cert fingerprint if certificate pinning should be used, if source == "from-url".
-    #[serde(alias = "cert_fingerprint")]
     pub cert_fingerprint: Option<String>,
 }
 
@@ -262,10 +255,9 @@ pub struct NetworkManual {
 #[serde(rename_all = "kebab-case", deny_unknown_fields)]
 pub struct DiskSetup {
     pub filesystem: Filesystem,
-    #[serde(alias = "disk_list", default)]
+    #[serde(default)]
     pub disk_list: Vec<String>,
     pub filter: Option<BTreeMap<String, String>>,
-    #[serde(alias = "filter_match")]
     pub filter_match: Option<FilterMatch>,
     pub zfs: Option<ZfsOptions>,
     pub lvm: Option<LvmOptions>,
@@ -385,7 +377,6 @@ pub enum Filesystem {
 pub struct ZfsOptions {
     pub raid: Option<ZfsRaidLevel>,
     pub ashift: Option<usize>,
-    #[serde(alias = "arc_max")]
     pub arc_max: Option<usize>,
     pub checksum: Option<ZfsChecksumOption>,
     pub compress: Option<ZfsCompressOption>,
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 34bc969..f4c3ae8 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -146,7 +146,8 @@ mod tests {
             fqdn_hostname_only,
             no_fqdn_from_dhcp,
             no_root_password_set,
-            short_password
+            short_password,
+            underscore_keys
         );
     }
 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/underscore_keys.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/underscore_keys.json
new file mode 100644
index 0000000..484279d
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/underscore_keys.json
@@ -0,0 +1,3 @@
+{
+    "parse-error": "error parsing answer.toml: unknown field `root_password`, expected one of `country`, `fqdn`, `keyboard`, `mailto`, `timezone`, `root-password`, `root-password-hashed`, `reboot-on-error`, `reboot-mode`, `root-ssh-keys`"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/underscore_keys.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/underscore_keys.toml
new file mode 100644
index 0000000..16f355c
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/underscore_keys.toml
@@ -0,0 +1,14 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root_password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "ext4"
+disk_list = ["sda"]
-- 
2.49.0



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


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

end of thread, other threads:[~2025-04-30 11:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-30 11:25 [pve-devel] [PATCH installer 1/2] assistant: validate: warn if answer file contains old snake_case keys Christoph Heiss
2025-04-30 11:25 ` [pve-devel] [RFC PATCH for-9.0 installer 2/2] auto: answer: drop deprecated " 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