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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id B898493C67 for ; Tue, 11 Apr 2023 13:04:08 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9EBDD3485E for ; Tue, 11 Apr 2023 13:04:08 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 11 Apr 2023 13:04:07 +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 4DD2C46E12 for ; Tue, 11 Apr 2023 13:04:07 +0200 (CEST) Message-ID: Date: Tue, 11 Apr 2023 13:04:06 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 To: Thomas Lamprecht , pmg-devel@lists.proxmox.com References: <20230407134258.199691-1-l.nunner@proxmox.com> <745e3d07-95e1-a649-7832-1127f6e0c921@proxmox.com> Content-Language: en-US From: Leo Nunner In-Reply-To: <745e3d07-95e1-a649-7832-1127f6e0c921@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.490 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 NICE_REPLY_A -3.246 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Apr 2023 11:04:08 -0000 On 2023-04-11 11:52, Thomas Lamprecht wrote: > For now some higher level review inline from my side, not sure if I get to > a more in-depth for each patch soonish. > > Am 07/04/2023 um 15:42 schrieb Leo Nunner: >> This series introduces two new features for the PMG rule system: object >> negation, and match groups. Negation allows the user to negate objects >> inside a rule, meaning that they will evaluate to true if their >> condition is NOT fulfilled. > Do we really don't have any test system for the rule system, if that'd be some > technical debt to address probably rather sooner than later. We have regression tests for PMG, which should already cover this AFAIK. >> The second feature, match groups, allows objects to be chained together. >> A match group functions as a container for multiple other objects; it >> will only evaluate to true if all its members evaluate to true. Match groups >> are connected via logical 'or' to all other objects inside the rule; >> take the following rule system: >> >> - Rule >> - Match Group >> - Object 1 >> - Object 2 >> - Object 3 >> >> It will match if either (Object 1 && Object 2), or if Object 3 >> evaluate to true. > Why not allow or matches? Most mail filter tools (like e.g., the x-sieve > fronted of open-xchange) allow to configure if all or any of a group's matching > rules must trigger for the whole group to be true. > > I mean, OR matching can be workarounded by duplicated rules but if we add > groups with AND combination, then OR is something that I'd think is also > expected to be there. Not sure if I follow… - Rule     - Who Objects         - Object 1     - When Objects         - Object 2 You would want something like this to match if either Who *or* When produce a match? IIRC those are connected via AND right now. >> To properly achieve match groups inside the GUI, the rule overview was >> reworked as a tree, instead of a simple list. It can now be >> collapsed/expanded, and each object type (except actions) has a single >> subfolder called 'Match Group'. >> >> In combination, these features should cover quite a few use cases, and >> make it possible to model more complex rule systems. >> >> pmg-api: >> >> Leo Nunner (8): > can we please drop the weird feature prefix, doesn't really add anything and is > relatively hard to parse as the subsystem is missing – PMG is somewhat small > enough in scope, so one can guess that; but I'd rather have that explicit. > > Also, order and separation is a bit odd as some things that are implementing a > single semantic work are split, and others like the api expand+add are merged, > also making the whole thing work only after exposing it via the API just feels > plain wrong and is actually risky if someone overlooks this and applies the > first few patches already. > > So, order/separation could be IMO better if: > > commit 2 ("parse negation value into objects"), and 4 ("implement matching > logic") are squashed into a "rule system: implement match negation" as they > implement the internal/backend functionallity, and then exposing that via the > api could be done in the next commit. The one adding the field to the rule db > schema could be also squashed into the commit 2 & 4, but less hard feelings > there, albeit I'd note in the commit message that it's a preparation for the > next commits. > > […] I'll change the commits around accordingly (and keep the formatting in mind for future patches), thanks!