Looked at the patch series and the implementation generally looks fine from my POV. We have a bit of a hodgepodge of types for the firewall now though. In PDM we use the auto-generated types from pve-api-types, then we now have the ones from this series and the ones in ve-rs that are used by the custom firewall configuration parser and subsequently the firewall daemon.
PDM shouldn't be hard to migrate over to the types in this series, since they should be interchangeable - although @HannesL might be able to say more w.r.t that topic. With this set of types, the firewall configuration parser + daemon shouldn't be too much work to move over either afaict. We should consolidate everything sooner rather than later imo, since otherwise introducing new features in the firewall could become quite cumbersome. If we don't want to introduce the more complex 'rusty' types to the API types, then we could just keep the more specialized ones around for the firewall daemon and convert from the API types there. The firewall configuration parser should then be adapted to return / accept the API types when reading / writing the configuration. The firewall daemon does further processing of those internally then - if required. On 2/16/26 11:45 AM, Dietmar Maurer wrote: > The current PVE firewall implementation is written in Perl, and Rust type > definitions can be auto-generated from its API schemas. However, many of the > more complex types are represented as opaque strings, which limits type > safety. > > Verifiers for complex types like ports and address matches cannot be generated > automatically, so we need to implement them manually anyway. > > To address this, the crate provides hand-crafted Rust types that parse and > validate these > string-encoded values into proper enums and structs, while remaining fully > compatible with the existing API wire format. The initial type definitions > were > seeded from the auto-generated `pve-api-types` crate and then refined by hand. > > Types from proxmox-ve-rs/proxmox-ve-config/src/firewall/ are not really > designed > to be used directly, as they are not fully compatible with the API wire > format. they > also depends on system crates (nix, proxmox-sys, etc.) which we want to avoid > for this crate. > I tried to reuse some of those types, but in many cases it was easier to > use types generated from the perl API schemas as a starting point and then > modify them > as needed. > > Dependencies are minimal, so that we can use this crate for wasm targets > (GUI). > > > This series depends on the CommaSeparatedList patch send recently. > > > Dietmar Maurer (22): > firewall-api-types: add new crate for firewall api types > firewall-api-types: add README.md > firewall-api-types: add firewall policy types > firewall-api-types: add logging types > firewall-api-types: add FirewallClusterOptions > firewall-api-types: add FirewallGuestOptions > firewall-api-types: add FirewallConntrackHelper enum > firewall-api-types: add FirewallNodeOptions struct > firewall-api-types: add FirewallRef type > firewall-api-types: add FirewallPortList types > firewall-api-types: add FirewallIcmpType > firewall-api-types: add FirewallIpsetReference type > firewall-api-types: add FirewallAliasReference type > firewall-api-types: add firewall address types > firewall-api-types: add FirewallRule type > firewall-api-types: use ConfigDigest from proxmox-config-digest crate > firewall-api-types: use COMMENT_SCHEMA from proxmox-schema crate > firewall-api-types: add FirewallRuleUpdater type > firewall-api-types: refactor FirewallRule and add > FirewallRuleListEntry > firewall-api-types: add DeletableFirewallRuleProperty enum > firewall-api-types: add FirewallAliasEntry API type > firewall-api-types: add FirewallIpsetListEntry and FirewallIpsetEntry > api types > > Cargo.toml | 1 + > proxmox-firewall-api-types/Cargo.toml | 30 + > proxmox-firewall-api-types/README.md | 54 ++ > proxmox-firewall-api-types/debian/changelog | 5 + > proxmox-firewall-api-types/debian/control | 52 ++ > proxmox-firewall-api-types/debian/copyright | 18 + > .../debian/debcargo.toml | 7 + > proxmox-firewall-api-types/src/address.rs | 229 +++++++ > proxmox-firewall-api-types/src/alias.rs | 181 ++++++ > .../src/cluster_options.rs | 61 ++ > proxmox-firewall-api-types/src/conntrack.rs | 52 ++ > .../src/firewall_ref.rs | 62 ++ > .../src/guest_options.rs | 97 +++ > proxmox-firewall-api-types/src/icmp_type.rs | 559 ++++++++++++++++++ > proxmox-firewall-api-types/src/ipset.rs | 254 ++++++++ > proxmox-firewall-api-types/src/lib.rs | 46 ++ > proxmox-firewall-api-types/src/log.rs | 312 ++++++++++ > .../src/node_options.rs | 240 ++++++++ > proxmox-firewall-api-types/src/policy.rs | 151 +++++ > proxmox-firewall-api-types/src/port.rs | 177 ++++++ > proxmox-firewall-api-types/src/rule.rs | 351 +++++++++++ > 21 files changed, 2939 insertions(+) > create mode 100644 proxmox-firewall-api-types/Cargo.toml > create mode 100644 proxmox-firewall-api-types/README.md > create mode 100644 proxmox-firewall-api-types/debian/changelog > create mode 100644 proxmox-firewall-api-types/debian/control > create mode 100644 proxmox-firewall-api-types/debian/copyright > create mode 100644 proxmox-firewall-api-types/debian/debcargo.toml > create mode 100644 proxmox-firewall-api-types/src/address.rs > create mode 100644 proxmox-firewall-api-types/src/alias.rs > create mode 100644 proxmox-firewall-api-types/src/cluster_options.rs > create mode 100644 proxmox-firewall-api-types/src/conntrack.rs > create mode 100644 proxmox-firewall-api-types/src/firewall_ref.rs > create mode 100644 proxmox-firewall-api-types/src/guest_options.rs > create mode 100644 proxmox-firewall-api-types/src/icmp_type.rs > create mode 100644 proxmox-firewall-api-types/src/ipset.rs > create mode 100644 proxmox-firewall-api-types/src/lib.rs > create mode 100644 proxmox-firewall-api-types/src/log.rs > create mode 100644 proxmox-firewall-api-types/src/node_options.rs > create mode 100644 proxmox-firewall-api-types/src/policy.rs > create mode 100644 proxmox-firewall-api-types/src/port.rs > create mode 100644 proxmox-firewall-api-types/src/rule.rs >
