Re: [pbs-devel] [PATCH proxmox{, -widget-toolkit, -backup} 0/4] notifications: add support for nested matchers

[Lukas Wagner <l.wagner@proxmox.com>]

On  2025-05-21 16:23, Lukas Wagner wrote:
> This series adds support for nested notification matchers. A new match rule is
> added, 'eval-matcher', which allows to evaluate other matchers and use their
> result in the current matcher.
> 
> Any matcher that shall be used as a nested matcher must have the (new) `nested`
> flag set to true. These matchers are only evaluated if they are referenced
> by another matcher. Any target configured for a nested matcher is ignored,
> only the 'top-level'/'outermost' matcher decides which targets to notify.
> 
> This patch series includes:
>   - patches for proxmox-notify, which add the new config entries for matchers,
>     API methods and nested matcher evaluation
>   - patches for proxmox-backup, which contain documentation for the new
>     feature
>   - patches for proxmox-widget-toolkit, containing the GUI changes needed
>     for this feature.
> 
> Small config example:
> 
> matcher: top-level
>   target mail-to-root
>   mode any
>   eval-matcher a
>   eval-matcher b
> 
> matcher: a
>   nested true
>   mode all
>   match-field exact:datastore=store
>   match-field exact:type=gc
>   match-severity error
> 
> matcher: b
>   nested true
>   mode all
>   match-field exact:datastore=store
>   match-field exact:type=prune
>   match-severity error
> 

I think I might need to NACK my own patch series here. After some
experimentation and more careful testing, I've become quite unhappy with the
overall UX of these new nested matchers.

For context, my original plan when I first created the notification stack was
to eventually allow to create nested rules by adding new "any/all" sub-nodes in
the match rule tree of a *single* matcher. Each of these new nodes could then
have further 'match-*' rules as their children, etc.

In this series however, I proposed a different approach. Here you could
reference *other* matchers from a matcher's rule tree; the nested matcher
must be created first from the regular matcher overview.

The reason why I went with the current approach were technical
limitations of our tech stack. Neither section-config nor the our POST/PUT
x-www-form-urlencoded data work particularly well for highly nested data
structures, like match rule nodes of a tree.
The current approach was the most straight-forward one to implement,
without requiring major changes to any component in the stack,
neither the UI, nor the API, nor the config format.

I see the following problems with my current approach:

  - The matcher overview becomes cluttered quite quickly. For example,
    implementing the (legacy) notification policy of a single datastore
    (always, never, error) x (GC, Prune, Verify, Sync) results in up to 5
    matchers (1 top-level matching 'any' of 4 nested matches, one for each
    event type/severity combination). Having multiple datastores with different
    policies (e.g. different admins, or production/non-production), makes it
    even more cluttered and requires conscious effort by the admin to keep
    things easy to understand (e.g. good naming/comments for nested matchers)

  - A nested matcher must already exist when being added to a top-level
    matcher. Creating matchers 'bottom-up' feels unnatural and might force
    users to jump around different matcher config windows to produce the
    desired policy.

  - Understanding a nested matcher structure requires opening multiple modal
    dialogs, hampering usability. There is no way to see the full matcher
    hierarchy at once.

I think this should be implemented in a way that aligns more with my
original idea. The core takeaway of these UX problems is that these (nested)
rules should be edited from a single place in the UI, such as the match rule
tree that already exists in the matcher edit window.

I think the most realistic option, keeping the limitations of our API and
section config in mind, is to find a backwards-compatible way to flatten these
nested rules from the tree and store them all in a single 'matcher' section.

Alternatively, we could keep the proposed nested/eval-matcher structure in
the config file and then provide some API to update multiple
matchers in one go (either multiple POST/PUT wrapped in a transaction, or by
having a single endpoint which then 'de-multiplexes' into multiple
matcher sections.

I really wish we could just send a JSON body in the POST/PUT and store the
rules in a config format with proper nesting, this would make all of this
*much* easier, both for API consumers as well as the API/config
implementation.

-- 
- Lukas



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel