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 C57041FF09B for ; Mon, 12 Oct 2026 15:21:12 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 6820221C49; Mon, 12 Oct 2026 15:16:16 +0200 (CEST) From: Lukas Wagner To: pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com Subject: [PATCH manager v4 38/39] api: notification: reject matchers with expression if not supported by all nodes Date: Mon, 12 Oct 2026 15:14:12 +0200 Message-ID: <20261012131413.314088-39-l.wagner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261012131413.314088-1-l.wagner@proxmox.com> References: <20261012131413.314088-1-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791810865650 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.283 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_MED -2.3 Sender listed at https://www.dnswl.org/, medium 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: GZPH6RGTJWXQ57QTZG4R23CL3G4CR42N X-Message-ID-Hash: GZPH6RGTJWXQ57QTZG4R23CL3G4CR42N X-MailFrom: l.wagner@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 X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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. PVE/API2/Cluster/Notifications.pm | 54 +++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm index bd3c86cc..ab06b739 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.21"; +my $matcher_expressions_version_array = [9, 2, 21]; + +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 => '', @@ -524,6 +563,7 @@ __PACKAGE__->register_method({ $config->add_sendmail_endpoint($param); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -583,6 +623,7 @@ __PACKAGE__->register_method({ $name, $param, $delete, $digest, ); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -619,6 +660,7 @@ __PACKAGE__->register_method({ PVE::Notify::lock_config(sub { my $config = PVE::Notify::read_config(); $config->delete_sendmail_endpoint($name); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -779,6 +821,7 @@ __PACKAGE__->register_method({ }, ); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -845,6 +888,7 @@ __PACKAGE__->register_method({ $digest, ); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -881,6 +925,7 @@ __PACKAGE__->register_method({ PVE::Notify::lock_config(sub { my $config = PVE::Notify::read_config(); $config->delete_gotify_endpoint($name); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1090,6 +1135,7 @@ __PACKAGE__->register_method({ }, ); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1157,6 +1203,7 @@ __PACKAGE__->register_method({ $digest, ); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1193,6 +1240,7 @@ __PACKAGE__->register_method({ PVE::Notify::lock_config(sub { my $config = PVE::Notify::read_config(); $config->delete_smtp_endpoint($name); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1368,6 +1416,7 @@ __PACKAGE__->register_method({ $param, ); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1429,6 +1478,7 @@ __PACKAGE__->register_method({ $digest, ); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1465,6 +1515,7 @@ __PACKAGE__->register_method({ PVE::Notify::lock_config(sub { my $config = PVE::Notify::read_config(); $config->delete_webhook_endpoint($name); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1666,6 +1717,7 @@ __PACKAGE__->register_method({ $config->add_matcher($param); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1716,6 +1768,7 @@ __PACKAGE__->register_method({ $name, $param, $delete, $digest, ); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; @@ -1752,6 +1805,7 @@ __PACKAGE__->register_method({ PVE::Notify::lock_config(sub { my $config = PVE::Notify::read_config(); $config->delete_matcher($name); + ensure_expression_supported_if_used($config); PVE::Notify::write_config($config); }); }; -- 2.47.3