From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 1786B949D5 for ; Thu, 11 Apr 2024 09:55:45 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EB3171BD4A for ; Thu, 11 Apr 2024 09:55:44 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 11 Apr 2024 09:55:44 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 06A5F4460E for ; Thu, 11 Apr 2024 09:55:44 +0200 (CEST) Message-ID: <4029a14e-25ca-4da3-9db8-d1e27c6b3540@proxmox.com> Date: Thu, 11 Apr 2024 09:55:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Thomas Lamprecht , Proxmox VE development discussion , Lukas Wagner References: <20240402171629.536804-1-s.hanreich@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.562 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [RFC container/firewall/manager/proxmox-firewall/qemu-server 00/37] proxmox firewall nftables implementation X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Apr 2024 07:55:45 -0000 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.