[PATCH proxmox v4 06/30] installer-types: bump fetch schema for bond and VLAN answer options

[Christoph Heiss <c.heiss@proxmox.com>]

Installers reject answers containing options they do not know. So an
answer server like Proxmox Datacenter Manager needs to know whether an
installer accepts the new management bond and VLAN options before it
sends an answer using them, as the installation fails otherwise. So far
all installers report version 1.0 for the schema of the data they send
when fetching an answer.

Bump that version to 1.1, which installers built against this crate
version then report, and document that its minor version also increases
when installers start to accept new answer options. Add helpers to parse
the reported version and to get the version an answer requires, so that
answer servers can compare the two.

Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v3 -> v4:
  * added new tests and adjusted existing ones for new bond member
    selection

Changes v2 -> v3:
  * new patch

 proxmox-installer-types/src/answer.rs | 175 ++++++++++++++++++++++++--
 1 file changed, 167 insertions(+), 8 deletions(-)

diff --git a/proxmox-installer-types/src/answer.rs b/proxmox-installer-types/src/answer.rs
index c97f287a..02558e39 100644
--- a/proxmox-installer-types/src/answer.rs
+++ b/proxmox-installer-types/src/answer.rs
@@ -85,12 +85,22 @@ pub mod fetch {
         /// major: Incremented for incompatible/breaking API changes, e.g. removing an existing
         /// field.
         /// minor: Incremented when adding functionality in a backwards-compatible matter, e.g.
-        /// adding a new field.
+        /// adding a new field or accepting new answer file options.
         pub version: String,
     }
 
     impl AnswerFetchDataSchema {
-        const SCHEMA_VERSION: &str = "1.0";
+        const SCHEMA_VERSION: &str = "1.1";
+
+        /// Returns the version as `(major, minor)` tuple, or `None` if it is malformed.
+        ///
+        /// Answer servers can compare it with
+        /// [`super::AutoInstallerConfig::required_fetch_schema_version`] to tell whether the
+        /// installer accepts all options used in an answer.
+        pub fn version(&self) -> Option<(u32, u32)> {
+            let (major, minor) = self.version.split_once('.')?;
+            Some((major.parse().ok()?, minor.parse().ok()?))
+        }
     }
 
     impl Default for AnswerFetchDataSchema {
@@ -113,7 +123,8 @@ pub mod fetch {
     /// Data sent in the body of POST request when retrieving the answer file via HTTP(S).
     ///
     /// NOTE: The format is versioned through `schema.version` (`$schema.version` in the
-    /// resulting JSON), ensure you update it when this struct or any of its members gets modified.
+    /// resulting JSON), ensure you update it when this struct or any of its members gets modified,
+    /// or when the installer starts to accept new answer file options.
     pub struct AnswerFetchData {
         /// Metadata for the answer file fetch payload
         // This field is prefixed by `$` on purpose, to indicate that it is document metadata and not
@@ -150,6 +161,22 @@ pub struct AutoInstallerConfig {
     pub first_boot: Option<FirstBootHookInfo>,
 }
 
+impl AutoInstallerConfig {
+    /// Returns the answer fetch schema version, as `(major, minor)` tuple, an installer has to
+    /// report at least to accept all options used in this answer.
+    ///
+    /// Installers reject answers with options they do not know, so answer servers can compare
+    /// this with [`fetch::AnswerFetchDataSchema::version()`] of the request to avoid sending an
+    /// answer the installer cannot use.
+    pub fn required_fetch_schema_version(&self) -> (u32, u32) {
+        if self.network.bond().is_some() || self.network.vlan().is_some() {
+            return (1, 1);
+        }
+
+        (1, 0)
+    }
+}
+
 /// Machine root password schema.
 #[cfg(feature = "api-types")]
 pub const ROOT_PASSWORD_SCHEMA: proxmox_schema::Schema = StringSchema::new("Root Password.")
@@ -1595,6 +1622,138 @@ pub fn email_validate(email: &str) -> Result<()> {
 mod tests {
     use super::*;
 
+    const MINIMAL_ANSWER: &str = r#"
+        [global]
+        country = "at"
+        keyboard = "de"
+        mailto = "root@example.invalid"
+        timezone = "Europe/Vienna"
+        root-password = "123456789"
+        fqdn = "pve.example.invalid"
+
+        [network]
+        source = "from-dhcp"
+
+        [disk-setup]
+        filesystem = "ext4"
+        disk-list = ["sda"]
+    "#;
+
+    #[test]
+    fn fetch_schema_version_is_parsed() {
+        let schema = |version: &str| fetch::AnswerFetchDataSchema {
+            version: version.to_owned(),
+        };
+
+        assert_eq!(
+            fetch::AnswerFetchDataSchema::default().version(),
+            Some((1, 1))
+        );
+        assert_eq!(schema("1.0").version(), Some((1, 0)));
+        assert_eq!(schema("2.10").version(), Some((2, 10)));
+
+        for invalid in ["", "1", "1.", ".1", "1.x", "1.1.1", "-1.0"] {
+            assert_eq!(schema(invalid).version(), None, "version '{invalid}'");
+        }
+    }
+
+    #[test]
+    fn required_fetch_schema_version_follows_used_options() {
+        let answer: AutoInstallerConfig = toml::from_str(MINIMAL_ANSWER).unwrap();
+        assert_eq!(answer.required_fetch_schema_version(), (1, 0));
+
+        let static_network = r#"
+            source = "from-answer"
+            cidr = "192.0.2.10/24"
+            dns = "192.0.2.1"
+            gateway = "192.0.2.1"
+            filter.ID_NET_NAME = "enp1s0"
+        "#;
+        let with_network =
+            |network: &str| MINIMAL_ANSWER.replace(r#"source = "from-dhcp""#, network);
+
+        let answer: AutoInstallerConfig = toml::from_str(&with_network(static_network)).unwrap();
+        assert_eq!(answer.required_fetch_schema_version(), (1, 0));
+
+        let vlan = format!("{static_network}\nvlan.vid = 10");
+        let answer: AutoInstallerConfig = toml::from_str(&with_network(&vlan)).unwrap();
+        assert_eq!(answer.required_fetch_schema_version(), (1, 1));
+
+        let bond = r#"
+            source = "from-dhcp"
+
+            [network.bond]
+            members-list = ["aa:bb:cc:dd:ee:01", "aa:bb:cc:dd:ee:02"]
+            mode = "active-backup"
+        "#;
+        let answer: AutoInstallerConfig = toml::from_str(&with_network(bond)).unwrap();
+        assert_eq!(answer.required_fetch_schema_version(), (1, 1));
+    }
+
+    #[test]
+    fn network_bond_are_parsed_correctly() {
+        let with_network =
+            |network: &str| MINIMAL_ANSWER.replace(r#"source = "from-dhcp""#, network);
+
+        let static_members = r#"
+            source = "from-dhcp"
+
+            [network.bond]
+            members-list = ["aa:bb:cc:dd:ee:01", "aa:bb:cc:dd:ee:02"]
+            mode = "active-backup"
+        "#;
+        let answer: AutoInstallerConfig = toml::from_str(&with_network(static_members)).unwrap();
+        assert_eq!(
+            answer.network.bond().unwrap().members().unwrap(),
+            Some(NetworkBondMemberSelection::Fixed(vec![
+                "aa:bb:cc:dd:ee:01".parse().unwrap(),
+                "aa:bb:cc:dd:ee:02".parse().unwrap()
+            ]))
+        );
+
+        let dynamic_members = r#"
+            source = "from-dhcp"
+
+            [network.bond]
+            members-filter.ID_NET_DRIVER = "igc"
+            members-filter.ID_PATH = "pci-0000:04:*"
+            members-filter-match = "all"
+            mode = "active-backup"
+        "#;
+        let answer: AutoInstallerConfig = toml::from_str(&with_network(dynamic_members)).unwrap();
+
+        let mut expected_filter = BTreeMap::new();
+        expected_filter.insert("ID_NET_DRIVER".to_owned(), "igc".to_owned());
+        expected_filter.insert("ID_PATH".to_owned(), "pci-0000:04:*".to_owned());
+
+        assert_eq!(
+            answer.network.bond().unwrap().members().unwrap(),
+            Some(NetworkBondMemberSelection::Filter(expected_filter))
+        );
+        assert_eq!(
+            answer.network.bond().unwrap().members_filter_match,
+            Some(FilterMatch::All)
+        );
+
+        let dynamic_members = r#"
+            source = "from-dhcp"
+
+            [network.bond]
+            members-list = ["aa:bb:cc:dd:ee:01", "aa:bb:cc:dd:ee:02"]
+            members-filter.ID_NET_DRIVER = "igc"
+            members-filter.ID_PATH = "pci-0000:04:*"
+            mode = "active-backup"
+        "#;
+        let answer: AutoInstallerConfig = toml::from_str(&with_network(dynamic_members)).unwrap();
+
+        let members = answer.network.bond().unwrap().members();
+        assert!(members.is_err());
+        assert_eq!(
+            members.unwrap_err().to_string(),
+            "Bond options 'members-list' and 'members-filter' are mutual exclusive"
+        );
+    }
+
     fn disks(n: usize) -> Vec<String> {
         (0..n).map(|i| format!("sd{i}")).collect()
     }
@@ -1700,10 +1859,10 @@ mod tests {
         assert!(res.is_err());
         assert_eq!(
             res.unwrap_err().to_string(),
-            "bond setup requires at least 2 interfaces, got 0"
+            "bond setup requires either fixed list of members or udev filter list"
         );
 
-        bond.members = vec!["12:34:56:ab:cd:df".parse().unwrap()];
+        bond.members_list = vec!["12:34:56:ab:cd:df".parse().unwrap()];
 
         let res = bond.verify();
         assert!(res.is_err());
@@ -1712,7 +1871,7 @@ mod tests {
             "bond setup requires at least 2 interfaces, got 1"
         );
 
-        bond.members = vec![
+        bond.members_list = vec![
             "12:34:56:ab:cd:df".parse().unwrap(),
             "ab:cd:df:12:34:56".parse().unwrap(),
         ];
@@ -1722,7 +1881,7 @@ mod tests {
     #[test]
     fn network_bond_primary_if_only_with_active_backup() {
         let mut bond = NetworkBondOptions::disabled();
-        bond.members = vec![
+        bond.members_list = vec![
             "12:34:56:ab:cd:df".parse().unwrap(),
             "ab:cd:df:12:34:56".parse().unwrap(),
         ];
@@ -1747,7 +1906,7 @@ mod tests {
     #[test]
     fn network_bond_primary_if_must_be_member() {
         let mut bond = NetworkBondOptions::disabled();
-        bond.members = vec![
+        bond.members_list = vec![
             "12:34:56:ab:cd:df".parse().unwrap(),
             "ab:cd:df:12:34:56".parse().unwrap(),
         ];
-- 
2.55.0