[PATCH installer v5 19/23] tui: network: add vlan setup options to advanced network dialog

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

Fixes #2164 [0].

Adds a third panel to the advanced network options dialog, allowing the
user to set a VLAN ID for the management interface.

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

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
  * also toggle form labels with the state

Changes v3 -> v4:
  * no changes

Changes v2 -> v3:
  * no changes

Changes v1 -> v2:
  * new patch

 proxmox-tui-installer/src/main.rs          |   1 +
 proxmox-tui-installer/src/options.rs       |   6 +-
 proxmox-tui-installer/src/setup.rs         |   5 +
 proxmox-tui-installer/src/views/mod.rs     |  35 +++++--
 proxmox-tui-installer/src/views/network.rs | 116 ++++++++++++++++++++-
 5 files changed, 151 insertions(+), 12 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 6e7c691..cd3b0ec 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -178,6 +178,7 @@ fn main() {
                 // We enable network interface pinning by default in the TUI
                 Some(&NetworkInterfacePinningOptions::default()),
                 None,
+                None,
             ),
             autoreboot: true,
         },
diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs
index 6fcdecb..fc54b99 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -42,7 +42,7 @@ impl InstallerOptions {
             .map(|l| &l.name)
             .unwrap_or(&self.timezone.kb_layout);
 
-        let mngmt_interface = if let Some(opts) = &self.network.bond_opts {
+        let mut mngmt_interface = if let Some(opts) = &self.network.bond_opts {
             let members = self
                 .resolve_bond_member_names(opts, &runtime_info.network)
                 .join(" | ");
@@ -52,6 +52,10 @@ impl InstallerOptions {
             self.network.ifname.to_owned()
         };
 
+        if let Some(opts) = &self.network.vlan_opts {
+            mngmt_interface += &format!(", VLAN {}", opts.vid);
+        }
+
         vec![
             SummaryOption::new("Bootdisk filesystem", self.bootdisk.fstype.to_string()),
             SummaryOption::new(
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index aecbe10..9410bfe 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -48,6 +48,11 @@ impl From<InstallerOptions> for InstallConfig {
                     }
                 })
                 .unwrap_or_default(),
+            mngmt_vlan: options
+                .network
+                .vlan_opts
+                .map(|opts| opts.into())
+                .unwrap_or_default(),
             network_interface_pin_map: pinning_opts.map(|o| o.mapping.clone()).unwrap_or_default(),
 
             hostname: options.network.fqdn.host().to_owned(),
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 3a6a00d..760b467 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -31,6 +31,7 @@ pub use timezone::*;
 
 pub struct NumericEditView<T> {
     view: LinearLayout,
+    min_value: Option<T>,
     max_value: Option<T>,
     placeholder: Option<T>,
     max_content_width: Option<usize>,
@@ -44,6 +45,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
 
         Self {
             view,
+            min_value: None,
             max_value: None,
             placeholder: None,
             max_content_width: None,
@@ -63,6 +65,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
 
         Self {
             view,
+            min_value: None,
             max_value: None,
             placeholder: None,
             max_content_width: None,
@@ -70,6 +73,16 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
         }
     }
 
+    pub fn enabled(mut self, enabled: bool) -> Self {
+        self.inner_mut().set_enabled(enabled);
+        self
+    }
+
+    pub fn min_value(mut self, min: T) -> Self {
+        self.min_value = Some(min);
+        self
+    }
+
     pub fn max_value(mut self, max: T) -> Self {
         self.max_value = Some(max);
         self
@@ -139,13 +152,23 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
         self.max_value = Some(max);
     }
 
+    pub fn set_enabled(&mut self, enabled: bool) {
+        self.inner_mut().set_enabled(enabled);
+    }
+
+    /// Checks if the new value is actually valid according to the min and max value, if set
     fn check_bounds(&mut self, original: Arc<String>, result: EventResult) -> EventResult {
-        // Check if the new value is actually valid according to the max value, if set
-        if let Some(max) = self.max_value
-            && let Ok(val) = self.get_content()
-            && result.is_consumed()
-            && val > max
-        {
+        let above_min = self
+            .min_value
+            .map(|min| self.get_content().map(|val| val >= min).unwrap_or(true))
+            .unwrap_or(true);
+
+        let below_max = self
+            .max_value
+            .map(|max| self.get_content().map(|val| val <= max).unwrap_or(true))
+            .unwrap_or(true);
+
+        if result.is_consumed() && (!above_min || !below_max) {
             // Restore the original value, before the insert
             let cb = self.inner_mut().set_content((*original).clone());
             return EventResult::with_cb_once(move |siv| {
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 785f957..929cb9c 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -18,15 +18,16 @@ use proxmox_installer_common::{
     options::{NetworkInterfacePinningOptions, NetworkOptions},
     setup::{Interface, NetworkInfo},
 };
-use proxmox_installer_types::answer::NetworkBondOptions;
+use proxmox_installer_types::answer::{NetworkBondOptions, NetworkVlanOptions};
 use proxmox_network_api::{BondXmitHashPolicy, LinuxBondMode};
 use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
 
-use super::{CheckboxGroup, CidrAddressEditView, FormView};
+use super::{CheckboxGroup, CidrAddressEditView, FormView, IntegerEditView};
 
 struct NetworkViewOptions {
     pinning_enabled: bool,
     bond_enabled: bool,
+    vlan_enabled: bool,
     interfaces: Vec<Interface>,
     /// Last selected interface (by its MAC address) in the [SelectView].
     /// Keeps track of the selected interface across enabling/disabling bonding, which replaces the
@@ -36,6 +37,7 @@ struct NetworkViewOptions {
     // by the user - to preserve the state between toggling the checkbox
     pinning_options: NetworkInterfacePinningOptions,
     bond_options: NetworkBondOptions,
+    vlan_options: NetworkVlanOptions,
 }
 
 /// Convenience wrapper when needing to take a (interior-mutable) reference to
@@ -55,6 +57,7 @@ impl NetworkOptionsView {
 
     const INTERFACE_PINNING_VIEW_NAME: &str = "network-name-pinning-view";
     const BOND_OPTIONS_VIEW_NAME: &str = "network-bond-options-view";
+    const VLAN_OPTIONS_VIEW_NAME: &str = "network-vlan-options-view";
 
     pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
         let mut interfaces = network_info
@@ -80,10 +83,15 @@ impl NetworkOptionsView {
         let options_ref = Arc::new(Mutex::new(NetworkViewOptions {
             pinning_enabled: options.pinning_opts.is_some(),
             bond_enabled: options.bond_opts.is_some(),
+            vlan_enabled: options.vlan_opts.is_some(),
             interfaces,
             last_selected_iface: selected_mac,
             pinning_options: options.pinning_opts.clone().unwrap_or_default(),
             bond_options: options.bond_opts.clone().unwrap_or_default(),
+            vlan_options: options
+                .vlan_opts
+                .clone()
+                .unwrap_or(NetworkVlanOptions { vid: 0 }),
         }));
 
         let iface_selection = Self::build_mgmt_ifname_selectview(options_ref.clone());
@@ -178,6 +186,7 @@ impl NetworkOptionsView {
             .then_some(options.pinning_options.clone());
 
         let bond_opts = options.bond_enabled.then_some(options.bond_options.clone());
+        let vlan_opts = options.vlan_enabled.then_some(options.vlan_options.clone());
 
         let ifname = if options.bond_enabled {
             "bond0".to_owned()
@@ -208,6 +217,7 @@ impl NetworkOptionsView {
                 dns_server,
                 pinning_opts,
                 bond_opts,
+                vlan_opts,
             })
         }
     }
@@ -222,6 +232,10 @@ impl NetworkOptionsView {
                 .child(Panel::new(
                     BondOptionsView::new(options_ref.clone())
                         .with_name(Self::BOND_OPTIONS_VIEW_NAME),
+                ))
+                .child(Panel::new(
+                    VlanOptionsView::new(options_ref.clone())
+                        .with_name(Self::VLAN_OPTIONS_VIEW_NAME),
                 )),
         );
 
@@ -230,11 +244,12 @@ impl NetworkOptionsView {
             .button("Ok", {
                 move |siv| {
                     match Self::get_view_values(siv, options_ref.clone()) {
-                        Ok((pin_opts, bond_opts)) => {
+                        Ok((pin_opts, bond_opts, vlan_opts)) => {
                             siv.pop_layer();
                             let options = &mut options_ref.lock().expect("unpoisoned lock");
                             options.pinning_options = pin_opts;
                             options.bond_options = bond_opts;
+                            options.vlan_options = vlan_opts;
                         }
                         Err(err) => {
                             siv.add_layer(Dialog::info(err.to_string()));
@@ -251,7 +266,11 @@ impl NetworkOptionsView {
     fn get_view_values(
         siv: &mut Cursive,
         options_ref: NetworkViewOptionsRef,
-    ) -> Result<(NetworkInterfacePinningOptions, NetworkBondOptions)> {
+    ) -> Result<(
+        NetworkInterfacePinningOptions,
+        NetworkBondOptions,
+        NetworkVlanOptions,
+    )> {
         let pin_opts = siv
             .call_on_name(
                 Self::INTERFACE_PINNING_VIEW_NAME,
@@ -273,7 +292,19 @@ impl NetworkOptionsView {
             bond_opts.verify()?;
         }
 
-        Ok((pin_opts, bond_opts))
+        let vlan_opts = siv
+            .call_on_name(Self::VLAN_OPTIONS_VIEW_NAME, VlanOptionsView::get_values)
+            .unwrap_or_else(|| bail!("failed to retrieve management network VLAN options"))?;
+
+        if options_ref
+            .lock()
+            .map(|opts| opts.vlan_enabled)
+            .unwrap_or(false)
+        {
+            vlan_opts.verify()?;
+        }
+
+        Ok((pin_opts, bond_opts, vlan_opts))
     }
 
     fn refresh_ifname_view(siv: &mut Cursive, options_ref: NetworkViewOptionsRef) {
@@ -747,3 +778,78 @@ impl BondOptionsView {
 impl ViewWrapper for BondOptionsView {
     cursive::wrap_impl!(self.view: LinearLayout);
 }
+
+struct VlanOptionsView {
+    view: LinearLayout,
+}
+
+impl VlanOptionsView {
+    const FORM_NAME: &str = "network-vlan-form-view";
+
+    fn new(options_ref: NetworkViewOptionsRef) -> Self {
+        let options = &options_ref.lock().expect("unpoisoned lock");
+
+        let view = LinearLayout::vertical()
+            .child(
+                LinearLayout::horizontal()
+                    .child(
+                        Checkbox::new()
+                            .with_checked(options.vlan_enabled)
+                            .on_change({
+                                let options_ref = options_ref.clone();
+                                move |siv, enabled| {
+                                    options_ref.lock().expect("unpoisoned lock").vlan_enabled =
+                                        enabled;
+                                    Self::toggle_all(siv, enabled);
+                                }
+                            }),
+                    )
+                    .child(TextView::new(" Enable VLAN setup on management interface").no_wrap()),
+            )
+            .child(DummyView::new())
+            .child(
+                FormView::<()>::new()
+                    .child(
+                        "VLAN ID",
+                        IntegerEditView::new()
+                            .min_value(1)
+                            .max_value(4094)
+                            .content(options.vlan_options.vid as usize)
+                            .enabled(options.vlan_enabled),
+                    )
+                    .with_name(Self::FORM_NAME),
+            );
+
+        Self { view }
+    }
+
+    fn toggle_all(siv: &mut Cursive, enabled: bool) {
+        siv.call_on_name(Self::FORM_NAME, |form: &mut FormView| {
+            form.set_labels_enabled(enabled);
+            if let Some(v) = form.get_child_mut::<IntegerEditView>(0) {
+                v.set_enabled(enabled);
+            }
+        });
+    }
+
+    fn get_values(&mut self) -> Result<NetworkVlanOptions> {
+        let form = self
+            .view
+            .get_child_mut(2)
+            .and_then(|v| v.downcast_mut::<NamedView<FormView<()>>>())
+            .ok_or(anyhow!("failed to retrieve bond options form"))?
+            .get_mut();
+
+        let vid = form
+            .get_value::<IntegerEditView, _>(0)
+            .ok_or(anyhow!("failed to retrieve VLAN ID"))?;
+
+        Ok(NetworkVlanOptions {
+            vid: vid.try_into()?,
+        })
+    }
+}
+
+impl ViewWrapper for VlanOptionsView {
+    cursive::wrap_impl!(self.view: LinearLayout);
+}
-- 
2.56.0