gave this another test and review - looks good to me - thanks for looking into this!
found out while testing that the comment '0' doesn't get saved due to falsiness checks instead of definedness check - nothing to do with your patch series. Tested-by: Stefan Hanreich <[email protected]> Reviewed-by: Stefan Hanreich <[email protected]> On 12/15/25 4:10 PM, Robert Obkircher wrote: > Pass firewall rule comments from the UI to the underlying firewall > configuration. > > Combines and changes two previously separate patches: > New: > - added a preserve_comments firewall option to api and UI > [1] fix #7068: show rule comments in iptables output > - shortened PVECOMMENT: prefix to PVE: > - fixed escape logic regex > - added tests for print_ipt_command function > [2] fix #7068: show rule comments in nftables output > - test truncation logic > > Note that for testing you likely want the fix for '#' symbols inside > comments [3] and the fix for "make check" in pve-firewall [4]. > > At some point I ran into a strange issue, where the options grid > displayed "Yes" for boolean options which were clearly 0 in the network > response, while the edit dialog displayed the correct value. I'm not > sure what caused this but system updates and rebuilding pve-firewall > +pve-manager fixed it. > > [1] > https://lore.proxmox.com/pve-devel/[email protected]/#r > [2] > https://lore.proxmox.com/pve-devel/[email protected]/#r > [3] > https://lore.proxmox.com/pve-devel/[email protected]/T/#u > [4] > https://lore.proxmox.com/pve-devel/[email protected]/ > > *** MURPP HERE *** > > pve-firewall: > > Robert Obkircher (2): > api: firewall: add option to preserve comments > fix #7068: show rule comments in iptables output > > src/PVE/Firewall.pm | 36 +++++++++++++++++- > test/Makefile | 1 + > test/test_comments.pl | 86 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 121 insertions(+), 2 deletions(-) > create mode 100755 test/test_comments.pl > > > pve-manager: > > Robert Obkircher (1): > ui: firewall: add preserve comments option > > www/manager6/grid/FirewallOptions.js | 1 + > 1 file changed, 1 insertion(+) > > > proxmox-ve-rs: > > Robert Obkircher (1): > firewall: parse preserve_comments host firewall option > > proxmox-ve-config/src/firewall/host.rs | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > > proxmox-firewall: > > Robert Obkircher (2): > fix #7068: show rule comments in nftables output > firewall: add rule comments to snapshot tests > > proxmox-firewall/src/rule.rs | 56 ++++++++++++++++++- > proxmox-firewall/tests/input/host.fw | 4 +- > .../integration_tests__firewall.snap | 44 ++++++++++++++- > 3 files changed, 100 insertions(+), 4 deletions(-) > > > Summary over all repositories: > 8 files changed, 236 insertions(+), 6 deletions(-) > _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
