Re: [pve-devel] [RFC container/firewall/manager/proxmox-firewall/qemu-server 00/37] proxmox firewall nftables implementation

[Stefan Hanreich <s.hanreich@proxmox.com>]

On 4/11/24 09:34, Thomas Lamprecht wrote:
> Am 11/04/2024 um 07:21 schrieb Stefan Hanreich:
>>> Since `Command` is serializable anyway, we could have a nice test suite of
>>> firewall/VM config files and expected commands as JSON dumps. 
>>> This will be tedious to setup at first, but will help to detect any unwanted
>>> regressions in the long-term.
>>
>> Yes, that is certainly something that is on the menu, as we've already
>> talked off-list using something like insta[1], which is already
>> packaged, would be a good approach to this imo.
>>
>> [1] https://github.com/mitsuhiko/insta
> 
> Does a simple serialize config and then diff that to the reference
> really needs that elaborate crate that comes with its own cargo sub
> command? I mean it looks Like I do not need to use the latter for
> running the tests, so I guess if it's packaged in Debian we could
> try it if you really think it provides that much convenience.

Imo the main upside would be that it also takes care of managing all the
reference values, which I think could get quite unwieldy in the future
when we make changes to the way rules are generated.

A quick check of the generated JSON shows that for a relatively small
firewall configuration we already generate 36K worth of JSON (which is
mostly due to the overhead of generating the chains for the options and
so on).

With insta we could generate the reference values for the first run,
check the output, and then regenerate the JSON in case of changes and
then only review the diff (which is conveniently displayed via the
inbuilt tool).

I wanted to evaluate it at least, because I think this could greatly
simplify updating the test cases in the case of changing rule outputs -
which might otherwise turn out quite cumbersome. Particularly since
there are still some low-hanging fruits wrt optimization of generated
rules I'd imagine this would reduce the churn of updating all the tests
when I introduce such optimizations.