From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 210251FF0EC for ; Fri, 24 Jul 2026 08:53:41 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id B1D9E214A1; Fri, 24 Jul 2026 08:53:40 +0200 (CEST) Date: Fri, 24 Jul 2026 08:53:35 +0200 From: Arthur Bied-Charreton To: Lukas Wagner Subject: Re: [PATCH many 00/29] notifications: add nested match expressions Message-ID: References: <20260709115716.299836-1-l.wagner@proxmox.com> <7ozvhk2v3zjg62jhrosyurbus2yjajtglsmlj4gfswr43ekoub@rghnsykkbv3p> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7ozvhk2v3zjg62jhrosyurbus2yjajtglsmlj4gfswr43ekoub@rghnsykkbv3p> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784875986029 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.986 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: W3WEYU2OGDGHRK44ABG5EOCRW3J5UR2L X-Message-ID-Hash: W3WEYU2OGDGHRK44ABG5EOCRW3J5UR2L X-MailFrom: a.bied-charreton@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri, Jul 24, 2026 at 08:44:54AM +0200, Arthur Bied-Charreton wrote: > On Thu, Jul 09, 2026 at 01:56:47PM +0200, Lukas Wagner wrote: > > NOTE: I have a couple of other patches planned for the notification stack (**); > > so I guess it would make sense wait a bit before applying this -- just to avoid > > too many version bumps and too much maintainer churn. > > > > This resolves a long-standing limitation of notification matchers. Up until > > now, notification match rules could only be one level deep, so one top-level > > combinator (all/any) and an arbitrary number of match rules as direct children. > > This severely limits which kind of matching behaviors can be represented. > > > > The notification stack was always designed with the possibility of arbitrarily > > nested match rules, however, due to limitations in our API and config formats > > (lacking support for nested data structures), this was not implemented yet. > > > > A prior attempt to resolve the limitations imposed by the lack of nesting was > > to add 'sub-matchers', where one matcher could evaluate the result of another > > match. After some consideration, I decided not to pursue this approach further, > > due to concerns about the UX of that approach [1]. > > > > This new series chooses a different approach. Nested match rules can now be > > represented by a single matcher in the config. The lack of support for nested > > data structures in the API was circumvented by storing a serialized form of the > > 'rule tree' as a JSON blob in a single new configuration parameter > > 'expression'. If this new key is set, the old 'match-*', 'invert-match' and > > 'mode' keys are ignored; 'expression' is intended as a replacement for these > > keys. I have not yet marked the old keys as deprecated. Theoretically these can > > stay supported for now, however when editing a matcher via the UI, the old keys > > will always be translated into an equivalent 'expression'. For any new products > > receiving the notification stack, we can drop the support for the old keys by > > compiling proxmox-notify without the 'legacy-matchers' flag. > > > > The expression evaluation logic was implemented in a new generic crate > > 'proxmox-match-expression'. 'proxmox-notify' is the first user of this crate, > > however it is generic enough to be potentially useful in other contexts as > > well. > > > > Bumps: > > - pve-rs needs proxmox-notify bumped > > - pve-manager needs pve-rs and widget-toolkit bumped > > - proxmox-backup needs widget-toolkit and proxmox-notify bumped > > - proxmox-notify needs a first release of proxmox-match-expression > > > > - proxmox-mail-forward needs to be rebuilt against bumped proxmox-notify (unless > > it has already been changed to not call into proxmox-notify anymore, see below) > > > > [1] https://lore.proxmox.com/pbs-devel/20250521142309.264719-1-l.wagner@proxmox.com/ > > > > > > (**): Some of the things I want to improve/implement: > > - Migrate PVE to use a worker-based approach, same as in PBS > > -> once that is done, we can fully migrate proxmox-mail-forward > > to use the worker/spool dir approach, instead of using proxmox-notify > > directly > > - Introduce a notification history > > - Refactor error types in proxmox-notify a bit > > > > Also, Arthur's XOAUTH2 patch series should probably be applied in one go with > > my efforts. He probably needs to rebase on top of my changes anyway. > > > [...] > > thanks a lot for working on this! this is a very nice improvement to > the notification stack UX. > > i tested backwards compatibility with older configs and the migration to > the new expression format on R/W cycles, it works really well and the > generated new format is semantically equivalent fwict. old configs that > have not yet been migrated still work as expected. > > broken matcher expressions only abort evaluation for that matcher, not > for the whole notification. > > old rules are ignored if 'expression' is set, as advertised. > > i replied to some patches with some comments/questions, overall i think > this is already in great shape, nice work!! > > [0] https://pve.proxmox.com/pve-docs/chapter-notifications.html#notification_matchers_calendar > [1] https://lore.proxmox.com/pve-devel/20260723114258.316106-1-a.bied-charreton@proxmox.com/T/#u > [2] https://lore.proxmox.com/pbs-devel/20260723114548.319543-1-a.bied-charreton@proxmox.com/T/#u forgot to remove those references after editing the message, might still be interesting additional info regarding matcher docs updates