public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: pdm-devel@lists.proxmox.com
Subject: [pdm-devel] [PATCH proxmox-yew-comp v2 1/1] firewall: rules: make security group entries expandable
Date: Wed, 17 Dec 2025 17:18:02 +0100	[thread overview]
Message-ID: <20251217161803.214102-5-h.laimer@proxmox.com> (raw)
In-Reply-To: <20251217161803.214102-1-h.laimer@proxmox.com>

With this is is possible to see what rules a security group contains
within the list of normal firewall rules.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/firewall/context.rs |  12 ++
 src/firewall/rules.rs   | 384 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 369 insertions(+), 27 deletions(-)

diff --git a/src/firewall/context.rs b/src/firewall/context.rs
index 6495fa3..79aa69b 100644
--- a/src/firewall/context.rs
+++ b/src/firewall/context.rs
@@ -82,6 +82,18 @@ impl FirewallContext {
         }
     }
 
+    pub fn group_rules_url(&self, group: &str) -> String {
+        match self {
+            Self::Cluster { remote } | Self::Node { remote, .. } | Self::Guest { remote, .. } => {
+                format!(
+                    "/pve/remotes/{}/firewall/groups/{}",
+                    percent_encode_component(remote),
+                    percent_encode_component(group)
+                )
+            }
+        }
+    }
+
     pub fn options_url(&self) -> String {
         match self {
             Self::Cluster { remote } => {
diff --git a/src/firewall/rules.rs b/src/firewall/rules.rs
index c40fab7..9ee7f56 100644
--- a/src/firewall/rules.rs
+++ b/src/firewall/rules.rs
@@ -1,12 +1,17 @@
+use std::collections::HashMap;
+use std::collections::HashSet;
 use std::rc::Rc;
 
 use yew::html::{IntoEventCallback, IntoPropValue};
 use yew::virtual_dom::{Key, VComp, VNode};
 
 use pwt::prelude::*;
-use pwt::state::{Loader, LoaderState, SharedStateObserver, Store};
-use pwt::widget::data_table::{DataTable, DataTableColumn, DataTableHeader};
-use pwt::widget::{Container, Fa};
+use pwt::props::ExtractPrimaryKey;
+use pwt::state::{Loader, LoaderState, SharedStateObserver, SlabTree, TreeStore};
+use pwt::widget::data_table::{
+    DataTable, DataTableCellRenderArgs, DataTableCellRenderer, DataTableColumn, DataTableHeader,
+};
+use pwt::widget::{Container, Fa, Row};
 use pwt_macros::builder;
 
 use super::context::FirewallContext;
@@ -76,16 +81,111 @@ impl FirewallRules {
     }
 }
 
+#[derive(Clone, Debug, PartialEq)]
+pub enum FirewallRuleEntry {
+    Root,
+    Rule {
+        rule: pve_api_types::FirewallRule,
+        parent_pos: Option<i64>,
+    },
+    Placeholder {
+        kind: PlaceholderKind,
+        parent_pos: i64,
+    },
+}
+
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum PlaceholderKind {
+    Loading,
+    Empty,
+    Error,
+}
+
+impl FirewallRuleEntry {
+    fn rule(&self) -> Option<&pve_api_types::FirewallRule> {
+        match self {
+            FirewallRuleEntry::Rule { rule, .. } => Some(rule),
+            FirewallRuleEntry::Root | FirewallRuleEntry::Placeholder { .. } => None,
+        }
+    }
+
+    fn placeholder_kind(&self) -> Option<PlaceholderKind> {
+        match self {
+            FirewallRuleEntry::Placeholder { kind, .. } => Some(*kind),
+            FirewallRuleEntry::Root | FirewallRuleEntry::Rule { .. } => None,
+        }
+    }
+
+    fn is_child_row(&self) -> bool {
+        matches!(self, FirewallRuleEntry::Placeholder { .. })
+            || matches!(
+                self,
+                FirewallRuleEntry::Rule {
+                    parent_pos: Some(_),
+                    ..
+                }
+            )
+    }
+
+    fn is_group_row(&self) -> bool {
+        matches!(
+            self,
+            FirewallRuleEntry::Rule { rule, parent_pos: None } if rule.ty == "group"
+        )
+    }
+}
+
+impl ExtractPrimaryKey for FirewallRuleEntry {
+    fn extract_key(&self) -> Key {
+        match self {
+            FirewallRuleEntry::Root => Key::from("root"),
+            FirewallRuleEntry::Placeholder { kind, parent_pos } => {
+                let kind = match kind {
+                    PlaceholderKind::Loading => "loading",
+                    PlaceholderKind::Empty => "empty",
+                    PlaceholderKind::Error => "error",
+                };
+                Key::from(format!("group-{parent_pos}-__{kind}__"))
+            }
+            FirewallRuleEntry::Rule {
+                rule, parent_pos, ..
+            } => {
+                if let Some(parent_pos) = parent_pos {
+                    Key::from(format!("group-{parent_pos}-{}", rule.pos))
+                } else {
+                    Key::from(format!("top-{}", rule.pos))
+                }
+            }
+        }
+    }
+}
+
 pub enum FirewallMsg {
     DataChange,
+    ToggleGroup(String, i64),
+    GroupRulesLoaded(
+        String,
+        i64,
+        Result<Vec<pve_api_types::FirewallRule>, anyhow::Error>,
+    ),
+}
+
+#[derive(Clone, Debug)]
+enum GroupLoadState {
+    Loading,
+    Loaded(Vec<pve_api_types::FirewallRule>),
+    Error,
 }
 
 #[doc(hidden)]
 pub struct ProxmoxFirewallRules {
-    store: Store<pve_api_types::FirewallRule>,
+    store: TreeStore<FirewallRuleEntry>,
     loader: Loader<Vec<pve_api_types::FirewallRule>>,
     _listener: SharedStateObserver<LoaderState<Vec<pve_api_types::FirewallRule>>>,
-    columns: Rc<Vec<DataTableHeader<pve_api_types::FirewallRule>>>,
+    columns: Rc<Vec<DataTableHeader<FirewallRuleEntry>>>,
+    expanded_groups: HashSet<i64>,
+    group_state: HashMap<String, GroupLoadState>,
+    base_rules: Vec<pve_api_types::FirewallRule>,
 }
 
 fn pill(text: impl Into<AttrValue>) -> Container {
@@ -148,50 +248,201 @@ fn format_firewall_rule(rule: &pve_api_types::FirewallRule) -> Html {
         .collect::<Html>()
 }
 
+impl Default for FirewallRuleEntry {
+    fn default() -> Self {
+        Self::Root
+    }
+}
+
 impl ProxmoxFirewallRules {
-    fn update_data(&mut self) {
-        if let Some(Ok(data)) = &self.loader.read().data {
-            self.store.set_data((**data).clone());
+    fn rebuild_tree(&mut self) {
+        let mut tree = SlabTree::new();
+        let mut root = tree.set_root(FirewallRuleEntry::default());
+
+        for rule in self.base_rules.iter() {
+            let is_group = rule.ty == "group";
+            let parent_pos = rule.pos;
+            let group_name = rule.action.as_str();
+
+            let entry = FirewallRuleEntry::Rule {
+                rule: rule.clone(),
+                parent_pos: None,
+            };
+
+            let mut node = root.append(entry);
+
+            if is_group && self.expanded_groups.contains(&parent_pos) {
+                node.set_expanded(true);
+
+                match self.group_state.get(group_name) {
+                    Some(GroupLoadState::Loaded(rules)) => {
+                        if rules.is_empty() {
+                            node.append(Self::placeholder_entry(
+                                PlaceholderKind::Empty,
+                                parent_pos,
+                            ));
+                        } else {
+                            for rule in rules.iter().cloned() {
+                                node.append(FirewallRuleEntry::Rule {
+                                    rule,
+                                    parent_pos: Some(parent_pos),
+                                });
+                            }
+                        }
+                    }
+                    Some(GroupLoadState::Error) => {
+                        node.append(Self::placeholder_entry(PlaceholderKind::Error, parent_pos));
+                    }
+                    Some(GroupLoadState::Loading) | None => {
+                        node.append(Self::placeholder_entry(
+                            PlaceholderKind::Loading,
+                            parent_pos,
+                        ));
+                    }
+                }
+            }
         }
+
+        self.store.set_data(tree);
+    }
+
+    fn update_data(&mut self) {
+        let data = match &self.loader.read().data {
+            Some(Ok(data)) => (**data).clone(),
+            _ => return,
+        };
+
+        self.base_rules = data;
+
+        self.expanded_groups.clear();
+        self.group_state.clear();
+
+        self.rebuild_tree();
+    }
+
+    fn placeholder_entry(kind: PlaceholderKind, parent_pos: i64) -> FirewallRuleEntry {
+        FirewallRuleEntry::Placeholder { kind, parent_pos }
     }
 
-    fn build_columns() -> Rc<Vec<DataTableHeader<pve_api_types::FirewallRule>>> {
+    fn build_columns(
+        on_toggle: Callback<(String, i64)>,
+    ) -> Rc<Vec<DataTableHeader<FirewallRuleEntry>>> {
         Rc::new(vec![
             DataTableColumn::new("")
-                .width("30px")
-                .justify("right")
+                .width("28px")
+                .justify("start")
                 .show_menu(false)
                 .resizable(false)
-                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.pos})
+                .render_cell(DataTableCellRenderer::new(move |args: &mut DataTableCellRenderArgs<FirewallRuleEntry>| {
+                    let on_toggle = on_toggle.clone();
+                    let record = args.record();
+                    let (is_group, group_name, group_pos) = match record {
+                        FirewallRuleEntry::Rule { rule, .. } if rule.ty == "group" => {
+                            (true, rule.action.clone(), rule.pos)
+                        }
+                        _ => (false, String::new(), 0),
+                    };
+
+                    let expander = if is_group {
+                        let caret = if args.is_expanded() {
+                            "pwt-tree-expander fa fa-fw fa-caret-down"
+                        } else {
+                            "pwt-tree-expander fa fa-fw fa-caret-right"
+                        };
+
+                        let onclick = move |event: MouseEvent| {
+                            event.stop_propagation();
+                            on_toggle.emit((group_name.clone(), group_pos));
+                        };
+
+                        html! { <i role="none" style="cursor: pointer;" class={caret} {onclick} /> }
+                    } else {
+                        html! { }
+                    };
+
+                    Row::new()
+                        .class(pwt::css::AlignItems::Baseline)
+                        .with_child(expander)
+                        .into()
+                }))
+                .into(),
+            DataTableColumn::new("")
+                .width("40px")
+                .justify("start")
+                .show_menu(false)
+                .resizable(false)
+                .render(|entry: &FirewallRuleEntry| {
+                    match entry {
+                        FirewallRuleEntry::Rule { rule, parent_pos: Some(parent_pos), .. } => {
+                            html! {
+                                <span style="font-size: 0.9em; opacity: 0.9;">
+                                    { format!("{parent_pos}.{}", rule.pos) }
+                                </span>
+                            }
+                        }
+                        FirewallRuleEntry::Rule { rule, .. } => html! { {rule.pos} },
+                        FirewallRuleEntry::Root | FirewallRuleEntry::Placeholder { .. } => "".into(),
+                    }
+                })
                 .into(),
             DataTableColumn::new(tr!("On"))
                 .width("40px")
                 .justify("center")
                 .resizable(false)
-                .render(
-                    |rule: &pve_api_types::FirewallRule| match rule.enable {
+                .render(|entry: &FirewallRuleEntry| {
+                    let Some(rule) = entry.rule() else {
+                        return "".into();
+                    };
+
+                    match rule.enable {
                         Some(1) => Fa::new("check").into(),
                         Some(0) | None => Fa::new("minus").into(),
                         _ => "-".into(),
-                    },
-                )
+                    }
+                })
                 .into(),
             DataTableColumn::new(tr!("Type"))
                 .width("80px")
-                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.ty})
+                .render(|entry: &FirewallRuleEntry| match entry.rule() {
+                    Some(rule) => html! {&rule.ty},
+                    None => html! {""},
+                })
                 .into(),
             DataTableColumn::new(tr!("Action"))
                 .width("100px")
-                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.action})
+                .render(|entry: &FirewallRuleEntry| match entry {
+                    FirewallRuleEntry::Rule { rule, .. } => html! {&rule.action},
+                    FirewallRuleEntry::Root | FirewallRuleEntry::Placeholder { .. } => "".into(),
+                })
                 .into(),
             DataTableColumn::new(tr!("Rule"))
                 .flex(1)
-                .render(|rule: &pve_api_types::FirewallRule| format_firewall_rule(rule))
+                .render(|entry: &FirewallRuleEntry| {
+                    if let Some(kind) = entry.placeholder_kind() {
+                        let text = match kind {
+                            PlaceholderKind::Loading => tr!("Loading…"),
+                            PlaceholderKind::Empty => tr!("No rules in this group"),
+                            PlaceholderKind::Error => {
+                                format!(
+                                    "{} ({})",
+                                    tr!("Failed to load group rules"),
+                                    tr!("collapse/expand to retry")
+                                )
+                            }
+                        };
+                        return html! { <span>{text}</span> };
+                    }
+                    match entry.rule() {
+                        Some(rule) => format_firewall_rule(rule),
+                        None => "".into(),
+                    }
+                })
                 .into(),
             DataTableColumn::new(tr!("Comment"))
                 .width("150px")
-                .render(|rule: &pve_api_types::FirewallRule| {
-                    rule.comment.as_deref().unwrap_or("-").into()
+                .render(|entry: &FirewallRuleEntry| match entry.rule() {
+                    Some(rule) => rule.comment.as_deref().unwrap_or("-").into(),
+                    None => "".into(),
                 })
                 .into(),
         ])
@@ -207,9 +458,7 @@ impl Component for ProxmoxFirewallRules {
 
         let url: AttrValue = props.context.rules_url().into();
 
-        let store = Store::with_extract_key(|item: &pve_api_types::FirewallRule| {
-            Key::from(item.pos.to_string())
-        });
+        let store = TreeStore::new().view_root(false);
 
         let loader = Loader::new().loader({
             let url = url.clone();
@@ -224,12 +473,20 @@ impl Component for ProxmoxFirewallRules {
         loader.load();
 
         let mut me = Self {
-            store,
+            store: store.clone(),
             loader,
             _listener,
-            columns: Self::build_columns(),
+            columns: Rc::new(Vec::new()), // Initial empty columns, will be set below
+            expanded_groups: HashSet::new(),
+            group_state: HashMap::new(),
+            base_rules: Vec::new(),
         };
 
+        me.columns = Self::build_columns(
+            ctx.link()
+                .callback(|(group, pos)| FirewallMsg::ToggleGroup(group, pos)),
+        );
+
         me.update_data();
         me
     }
@@ -241,12 +498,56 @@ impl Component for ProxmoxFirewallRules {
         true
     }
 
-    fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool {
+    fn update(&mut self, ctx: &Context<Self>, msg: Self::Message) -> bool {
         match msg {
             FirewallMsg::DataChange => {
                 self.update_data();
                 true
             }
+            FirewallMsg::ToggleGroup(group_name, group_pos) => {
+                if self.expanded_groups.contains(&group_pos) {
+                    self.expanded_groups.remove(&group_pos);
+                    self.rebuild_tree();
+                    return true;
+                }
+
+                self.expanded_groups.insert(group_pos);
+
+                let should_load = match self.group_state.get(&group_name) {
+                    Some(GroupLoadState::Loaded(_)) | Some(GroupLoadState::Loading) => false,
+                    Some(GroupLoadState::Error) | None => true,
+                };
+
+                if should_load {
+                    self.group_state
+                        .insert(group_name.clone(), GroupLoadState::Loading);
+                    let url = ctx.props().context.group_rules_url(&group_name);
+                    ctx.link().send_future(async move {
+                        let result = crate::http_get(url, None).await;
+                        FirewallMsg::GroupRulesLoaded(group_name, group_pos, result)
+                    });
+                }
+
+                self.rebuild_tree();
+                true
+            }
+            FirewallMsg::GroupRulesLoaded(group_name, group_pos, result) => {
+                match result {
+                    Ok(mut rules) => {
+                        rules.sort_by_key(|r| r.pos);
+                        self.group_state
+                            .insert(group_name, GroupLoadState::Loaded(rules));
+                    }
+                    Err(err) => {
+                        log::warn!(
+                            "failed to load firewall security group rules for {group_pos}: {err:#}"
+                        );
+                        self.group_state.insert(group_name, GroupLoadState::Error);
+                    }
+                }
+                self.rebuild_tree();
+                true
+            }
         }
     }
 
@@ -261,6 +562,35 @@ impl Component for ProxmoxFirewallRules {
                 DataTable::new(self.columns.clone(), self.store.clone())
                     .show_header(true)
                     .striped(true)
+                    .row_render_callback({
+                        let store = self.store.clone();
+                        move |args: &mut pwt::widget::data_table::DataTableRowRenderArgs<
+                            FirewallRuleEntry,
+                        >| {
+                            let mut style = String::new();
+
+                            if args.record().is_child_row() {
+                                style.push_str(
+                                    "background-color: var(--pwt-color-neutral-container);",
+                                );
+                            }
+
+                            if args.record().is_group_row() {
+                                style.push_str("font-weight: 600;");
+                                if let Some(node) = store.read().lookup_node(args.key()) {
+                                    if node.expanded() {
+                                        style.push_str(
+                                            "box-shadow: inset 0 -2px 0 var(--pwt-color-border);",
+                                        );
+                                    }
+                                }
+                            }
+
+                            if !style.is_empty() {
+                                args.set_attribute("style", Some(style));
+                            }
+                        }
+                    })
                     .into()
             }
         })
-- 
2.47.3



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

      parent reply	other threads:[~2025-12-17 16:17 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-17 16:17 [pdm-devel] [PATCH proxmox{, -datacenter-manager, -yew-comp} v2 0/4] make security groups expandable in firewall rules list Hannes Laimer
2025-12-17 16:17 ` [pdm-devel] [PATCH proxmox v2 1/2] pve-api-types: add security group GET endpoints Hannes Laimer
2025-12-17 16:18 ` [pdm-devel] [PATCH proxmox v2 2/2] pve-api-types: regenerate Hannes Laimer
2025-12-17 16:18 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 1/1] api: firewall: add pve firewall security group GET endpoints Hannes Laimer
2025-12-17 16:18 ` Hannes Laimer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251217161803.214102-5-h.laimer@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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