[PATCH manager v6 41/43] api: notification: reject matchers with expression if not supported by all nodes

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

The new matcher UI will automatically migrate any existing matcher to
one with an 'expression', and new matchers will also only use this new
parameter. If any of the cluster nodes uses a version of pve-manager
that does not yet support the new parameter, any matcher written by an
updated node will break notifications on the older nodes.

To avoid this, any time before we write the notification config, we walk
the matchers and check if any of them has 'expression' set.
If it is set, we check published pve-manager version of all cluster
nodes and reject the update to the config if any node is too old.

While this still leaves the possibility of an outdated, offline node
coming back online and then failing to parse the migrated notification
config, it would be unreasonable to disallow editing the notification
configuration if any node is offline.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
---

Notes:
    New in v3.
    
    Note to maintainers: Please change the version guard to the version
    of pve-manager that actually includes the change.
    
    Change since v3:
      - ran `make tidy`
      - Instead of only when creating/updating matchers, *all* calls that
        potentially write the notification config check for the new
        parameter. This is due to 'expression' being used in the new builtin
        'default-matcher' implementation, which will be written to the
        config on *any* change.
        In v3, if there was no config file there before, creating *any*
        new config entity would cause 'default-matcher' to be serialized,
        breaking notifications on outdated nodes.
    
    Changes since v5:
      - The 'default-matcher' remains now unchanged (without an expression),
        so we only need to check when creating/updating matchers, not one
        every single write of the notification config.

 PVE/API2/Cluster/Notifications.pm | 41 +++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index bd3c86cc..abbd00d0 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -6,11 +6,13 @@ use strict;
 use Storable qw(dclone);
 use JSON;
 
+use PVE::Cluster;
 use PVE::Exception qw(raise_param_exc);
 use PVE::Tools qw(extract_param);
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::RESTHandler;
 use PVE::Notify;
+use PVE::SafeSyslog;
 
 use base qw(PVE::RESTHandler);
 
@@ -56,6 +58,43 @@ sub raise_api_error {
     die $exc;
 }
 
+my $matcher_expressions_version = "9.2.25";
+my $matcher_expressions_version_array = [9, 2, 25];
+
+my sub matcher_expression_supported_by_all_nodes {
+    my $version_info = PVE::Cluster::get_node_kv('version-info');
+    for my $node (sort keys $version_info->%*) {
+        my $node_info = eval { JSON::decode_json($version_info->{$node}); };
+        if (my $err = $@) { 
+            # warn, but continue.
+            # Rejecting writes to notifications.cfg if a cluster node is
+            # not available or if has not yet published its version through
+            # pvestatd would be unreasonable.
+            syslog('warn', "cannot parse version info for $node as JSON - $err");
+        } elsif (!PVE::Cluster::pvecfg_min_version(
+            $node_info->{version},
+            @$matcher_expressions_version_array,
+        )) {
+            return 0;
+        }
+    }
+
+    return 1;
+}
+
+# FIXME: MAJOR VERSION: This check can be dropped at the next major version bump.
+my sub ensure_expression_supported_if_used {
+    my ($config) = @_;
+
+    my $matchers = $config->get_matchers();
+    my $has_expression = grep { defined($_->{expression}) } $matchers->@*;
+
+    if ($has_expression && !matcher_expression_supported_by_all_nodes()) {
+        die "At least one cluster node does not support matcher expressions."
+            . " Please upgrade pve-manager to at least $matcher_expressions_version.";
+    }
+}
+
 __PACKAGE__->register_method({
     name => 'index',
     path => '',
@@ -1666,6 +1705,7 @@ __PACKAGE__->register_method({
 
                 $config->add_matcher($param);
 
+                ensure_expression_supported_if_used($config);
                 PVE::Notify::write_config($config);
             });
         };
@@ -1716,6 +1756,7 @@ __PACKAGE__->register_method({
                     $name, $param, $delete, $digest,
                 );
 
+                ensure_expression_supported_if_used($config);
                 PVE::Notify::write_config($config);
             });
         };
-- 
2.47.3