Re: [pdm-devel] [PATCH proxmox{, -yew-comp, -datacenter-manager} v4 00/12] add basic integration of PVE firewall

["Michael Köppl" <m.koeppl@proxmox.com>]

Gave this another spin. Tested the following:
- Checked that firewall status is correctly displayed for both clusters
  and individual nodes (also if the node has firewall enabled, but the
  cluster does not)
- Checked that number of rules, status of rules (enabled/disabled) and
  details are displayed correctly
- Enabling/disabling firewalls of clusters and nodes
- Editing input, output, forward policies (also default values)
- Editing log rate limit
- Checked that the filter for the tree works

Noticed that:
- When there is no /etc/pve/nodes/<node>/host.fw for a node yet, the
  edit dialog displays the firewall as enabled, whereas the tree view
  does not.
- If a node is not reachable and the edit dialog is opened, the error
  message is a bit weird:
  api error (status = 400: api error (status = 595: ))
- When adding a firewall rule in PVE and then refreshing the list in
  PDM (not reloading the page), it shows "0 of 1 rules enabled", but
  will not show the firewall rules in the right panel unless another
  entry in the tree is selected first and then the original one is
  selected again.
- When entering log rate limit values that are invalid (like -1), the
  update button can still be pressed if any other value is changed. The
  default value seems to be used in this case. The node firewall dialog
  does not seem to allow this, whereas the cluster firewall dialog does.
- It was not entirely clear to me what the Filter field was for. FWICT
  based on the implementation it only filters guests? Is that on
  purpose?

On Thu Nov 20, 2025 at 1:55 PM CET, Hannes Laimer wrote:
> This adds a basic UI for displaying the status of the firewall on remotes,
> nodes and guests in a tree. Status includes whether the firewall is
> enabled and the count of enabled rules. These rules are also shown in a
> panel once an enetity in the tree is selected. Firewall options can be
> edited, most useful is probably enable/disable, but generally all
> options are exposed(since we had the types anyway).
>
> Generally loading the status involves 2 requests per entity, so the PDM
> server has to do quite a bit of work collecting all the relevant data.
> That is the reason we have multiple status endpoints
>  - for all pve remotes
>  - for a specific remote
>  - for a specific node
> a bit more context on the commit adding these endpoints. With these we
> can limit the number of requests the PDM potentially has to do. In this
> context a cache could also make sense, should be somewhat straight
> forward integrating something like Dominik proposed in [1]. But since
> these are configs, caches would have to be really short lived, but still,
> they could help with different useres requesting the same data at close
> to the same time.
>
> Firewall options edit form and the firewall rules tables were added to
> yew-comp as they are not necesarrily PDM specific. I tried having them
> in a way so it would not be too complicated reusing them in other places
> at some point.
>
> This also includes an updated pve-api.json, some api endpoint specs did
> require minor adjustments so they'd work with the type generator.
> Generally this is build with the latest master of proxmox-yew-comp and
> proxmox-yew-widget-toolkit.
>
> Notes: node or guest firewalls could be enabled, but end up being masked
> by the cluster setting. I tried visualizing that by having the checkmark
> normal if masked and green if not.
>
> [1] https://lore.proxmox.com/pdm-devel/20251017120315.2723235-1-d.csapak@proxmox.com/
>
>
> v4, thanks @Stefan
>  * UI:
>    - fix sorting, now stable
>    - fix problem with dirty-detection for the log-ratelimit field(s)
>    - keep rules selection during tree refresh
>    - properly show errors
>    - minior code improvements
>    - cache columns for rules tablem don't re-create them every time
>  * handfull of non-specific, small code improvments I've noticed throughout
>    the series
>  * still needs [2] for the reason mentioned in v3
>
>
> v3, thanks @Lukas and @Michael
>  * UI:
>    - fixed(as in !dynamic and as in !problem anymore) rule status text col width
>    - align option edit form fields with the ones present in the current
>      PVE ui
>    - set defaults for checkboxes, note: this needs [2], without it
>      having a default defined for checkboxes leads to the form being
>      marked as dirty immediately
>    - add missing .max(99) to field
>    - only show remotes of type `pve` in remote filter
>  * add doc strings to pub stuff, also made some thing private
>  * drop default value for firewall IO policy and fix default for forward
>    policy
>  * fixed problem with how cluster firewall enable field was mapped from
>    int to a bool
>  
> [2] https://lore.proxmox.com/yew-devel/20251110161831.261526-1-h.laimer@proxmox.com/T/#u
>
>
> v2, thanks a lot @Dominik, @Lukas and @Thomas
>  * rebased onto master
>  * UI improvements
>    - move filters into tree panel
>    - shrink status tree panel
>    - the firewall rules table now doesn't always show all the columns,
>      instead we have a new column that shows only the things that are
>      set. We save a lot of space like that, also, most of the columns are
>      empty.
>    - added toggle button that collapses the status tree and shows the
>      rules tables "full-screen". With the current UI changes this should
>      not really be needed unless a really small screen is used.
>      Nontheless it may be useful, so I kept it it.
>    - for the cluster options form I put a border around the log ratelimit
>      fields, that should help separating them from the rest of the
>      options.
>  * concurrently fetch status data for `all remotes` and `single remote`,
>    was sequential in v1
> (* this doesn't include [3] anymore, since it was applied already )
>
> [3] https://git.proxmox.com/?p=proxmox.git;a=commit;h=eb41684db1a6d13f4ae3d95761e40db5a7c333ce
>
>
> proxmox:
>
> Hannes Laimer (4):
>   pve-api-types: update pve-api.json
>   pve-api-types: add get/update firewall options endpoints
>   pve-api-types: add list firewall rules endpoints
>   pve-api-types: regenerate
>
>  pve-api-types/generate.pl            |   53 +
>  pve-api-types/pve-api.json           |  362 +------
>  pve-api-types/src/generated/code.rs  |  206 +++-
>  pve-api-types/src/generated/types.rs | 1365 ++++++++++++++++++++++++--
>  4 files changed, 1588 insertions(+), 398 deletions(-)
>
>
> proxmox-yew-comp:
>
> Hannes Laimer (4):
>   form: add helpers for extractig data out of schemas
>   firewall: add FirewallContext
>   firewall: add options edit form
>   firewall: add rules table
>
>  src/firewall/context.rs             | 142 ++++++++
>  src/firewall/log_ratelimit_field.rs | 334 +++++++++++++++++++
>  src/firewall/mod.rs                 |  11 +
>  src/firewall/options_edit.rs        | 495 ++++++++++++++++++++++++++++
>  src/firewall/rules.rs               | 264 +++++++++++++++
>  src/form/mod.rs                     |  70 ++++
>  src/lib.rs                          |   3 +
>  7 files changed, 1319 insertions(+)
>  create mode 100644 src/firewall/context.rs
>  create mode 100644 src/firewall/log_ratelimit_field.rs
>  create mode 100644 src/firewall/mod.rs
>  create mode 100644 src/firewall/options_edit.rs
>  create mode 100644 src/firewall/rules.rs
>
>
> proxmox-datacenter-manager:
>
> Hannes Laimer (4):
>   pdm-api-types: add firewall status types
>   api: firewall: add option, rules and status endpoints
>   pdm-client: add api methods for firewall options, rules and status
>     endpoints
>   ui: add firewall status tree
>
>  lib/pdm-api-types/src/firewall.rs     | 173 ++++++
>  lib/pdm-api-types/src/lib.rs          |   2 +
>  lib/pdm-client/src/lib.rs             | 133 ++++
>  server/src/api/pve/firewall.rs        | 857 ++++++++++++++++++++++++++
>  server/src/api/pve/lxc.rs             |   1 +
>  server/src/api/pve/mod.rs             |   3 +
>  server/src/api/pve/node.rs            |   1 +
>  server/src/api/pve/qemu.rs            |   1 +
>  ui/src/remotes/firewall/columns.rs    | 153 +++++
>  ui/src/remotes/firewall/mod.rs        |  30 +
>  ui/src/remotes/firewall/tree.rs       | 673 ++++++++++++++++++++
>  ui/src/remotes/firewall/types.rs      | 284 +++++++++
>  ui/src/remotes/firewall/ui_helpers.rs | 166 +++++
>  ui/src/remotes/mod.rs                 |  10 +
>  14 files changed, 2487 insertions(+)
>  create mode 100644 lib/pdm-api-types/src/firewall.rs
>  create mode 100644 server/src/api/pve/firewall.rs
>  create mode 100644 ui/src/remotes/firewall/columns.rs
>  create mode 100644 ui/src/remotes/firewall/mod.rs
>  create mode 100644 ui/src/remotes/firewall/tree.rs
>  create mode 100644 ui/src/remotes/firewall/types.rs
>  create mode 100644 ui/src/remotes/firewall/ui_helpers.rs
>
>
> Summary over all repositories:
>   25 files changed, 5394 insertions(+), 398 deletions(-)



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