[PATCH ha-manager 0/2] fix #7399: check if required rule props are set

[PATCH ha-manager 0/2] fix #7399: check if required rule props are set

[Daniel Kral]

The API endpoints allow users to set empty 'nodes' and 'resources'
properties for HA rules, which are not handled by the api schema
verification nor the api handlers themselves.

PATCH 1 fixes the

    got unexpected error - cannot lookup undefined type! at
    /usr/share/perl5/PVE/HA/Config.pm line 232.

error message for existing configs with invalid sections, which prevents
the CRM from doing any other progress. It will only keep logging the
hint that a section misses a required option.

PATCH 2 prevents users from passing empty strings for the 'nodes' and
'resources' parameters, which are catched before these are written to
the rules config itself.

A more general solution is proposed in [0], which fixes this behavior in
PVE::SectionConfig itself.

[0] https://lore.proxmox.com/pve-devel/20260311122659.250421-1-d.kral@proxmox.com/

Daniel Kral (2):
  rules: ensure rule is defined and has type in set_rule_defaults
  api: rules: check for non-empty nodes and resources properties

 src/PVE/API2/HA/Rules.pm | 16 ++++++++++------
 src/PVE/HA/Rules.pm      |  3 ++-
 2 files changed, 12 insertions(+), 7 deletions(-)

-- 
2.47.3

[PATCH ha-manager 1/2] rules: ensure rule is defined and has type in set_rule_defaults

[Daniel Kral]

If the rule is not defined or does not have a type set, there are no
optional property defaults to set.

This prevents users of set_rule_defaults(...) to have to deal with
unnecessary error handling.

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7399
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 src/PVE/HA/Rules.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 38c57548..3a14eeb3 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -274,7 +274,8 @@ haven't been explicitly set yet.
 sub set_rule_defaults {
     my ($class, $rule) = @_;
 
-    if (my $plugin = $class->lookup($rule->{type})) {
+    if ($rule && $rule->{type}) {
+        my $plugin = $class->lookup($rule->{type});
         my $properties = $plugin->properties();
 
         for my $prop (keys %$properties) {
-- 
2.47.3

Re: [PATCH ha-manager 1/2] rules: ensure rule is defined and has type in set_rule_defaults

[Fiona Ebner]

Just a note for the future: please also include the bug number in the
commit title, so it can also be seen in one-line git history. From the
developer documentation [0]:
If the commit fixes a bug start with that information in this form:
fix #1234: summary here

[0]:
https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages

Am 11.03.26 um 1:50 PM schrieb Daniel Kral:
> If the rule is not defined or does not have a type set, there are no
> optional property defaults to set.
> 
> This prevents users of set_rule_defaults(...) to have to deal with
> unnecessary error handling.
> 
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7399
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
>  src/PVE/HA/Rules.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
> index 38c57548..3a14eeb3 100644
> --- a/src/PVE/HA/Rules.pm
> +++ b/src/PVE/HA/Rules.pm
> @@ -274,7 +274,8 @@ haven't been explicitly set yet.
>  sub set_rule_defaults {
>      my ($class, $rule) = @_;
>  
> -    if (my $plugin = $class->lookup($rule->{type})) {
> +    if ($rule && $rule->{type}) {
> +        my $plugin = $class->lookup($rule->{type});
>          my $properties = $plugin->properties();
>  
>          for my $prop (keys %$properties) {

[PATCH ha-manager 2/2] api: rules: check for non-empty nodes and resources properties

[Daniel Kral]

The 'resources' property is required (i.e., not optional) for all rule
types and the 'nodes' property is required for node affinity rules.

Though setting these as required does not prevent users from setting
them to an empty string as non-optional properties are only checked for
definedness in PVE::JSONSchema at the moment.

This allows users to write invalid rule sections to the rules config as
subsequent config reads will complain about the missing required options
not being set as PVE::SectionConfig::write_config(...) will drop these
if their values evaluate to empty strings.

Therefore, add an additional check to prevent users from being able to
pass empty nodes and/or resource lists when creating or modifying
existing HA rules.

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7399
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 src/PVE/API2/HA/Rules.pm | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/PVE/API2/HA/Rules.pm b/src/PVE/API2/HA/Rules.pm
index 046a4a9b..f09ab362 100644
--- a/src/PVE/API2/HA/Rules.pm
+++ b/src/PVE/API2/HA/Rules.pm
@@ -42,9 +42,11 @@ my $get_api_ha_rule = sub {
     return $cfg;
 };
 
-my $assert_resources_are_configured = sub {
+my $assert_valid_resources_param = sub {
     my ($resources) = @_;
 
+    die "no resources were specified\n" if !%$resources;
+
     my $unconfigured_resources = [];
 
     for my $resource (sort keys %$resources) {
@@ -56,9 +58,11 @@ my $assert_resources_are_configured = sub {
         if @$unconfigured_resources;
 };
 
-my $assert_nodes_do_exist = sub {
+my $assert_valid_nodes_param = sub {
     my ($nodes) = @_;
 
+    die "no nodes were specified\n" if !%$nodes;
+
     my $nonexistent_nodes = [];
     my $localnode = PVE::INotify::nodename();
 
@@ -281,8 +285,8 @@ __PACKAGE__->register_method({
 
                 die "HA rule '$ruleid' already defined\n" if $rules->{ids}->{$ruleid};
 
-                $assert_resources_are_configured->($opts->{resources});
-                $assert_nodes_do_exist->($opts->{nodes}) if $opts->{nodes};
+                $assert_valid_resources_param->($opts->{resources});
+                $assert_valid_nodes_param->($opts->{nodes}) if $opts->{nodes};
 
                 $rules->{order}->{$ruleid} = PVE::HA::Rules::get_next_ordinal($rules);
                 $rules->{ids}->{$ruleid} = $opts;
@@ -339,8 +343,8 @@ __PACKAGE__->register_method({
                 my $plugin = PVE::HA::Rules->lookup($type);
                 my $opts = $plugin->check_config($ruleid, $param, 0, 1);
 
-                $assert_resources_are_configured->($opts->{resources});
-                $assert_nodes_do_exist->($opts->{nodes}) if $opts->{nodes};
+                $assert_valid_resources_param->($opts->{resources});
+                $assert_valid_nodes_param->($opts->{nodes}) if $opts->{nodes};
 
                 my $options = $plugin->private()->{options}->{$type};
                 PVE::SectionConfig::delete_from_config($rule, $options, $opts, $delete);
-- 
2.47.3

applied: [PATCH ha-manager 0/2] fix #7399: check if required rule props are set

[Thomas Lamprecht]

On Wed, 11 Mar 2026 13:49:34 +0100, Daniel Kral wrote:
> The API endpoints allow users to set empty 'nodes' and 'resources'
> properties for HA rules, which are not handled by the api schema
> verification nor the api handlers themselves.
> 
> PATCH 1 fixes the
> 
>     got unexpected error - cannot lookup undefined type! at
>     /usr/share/perl5/PVE/HA/Config.pm line 232.
> 
> [...]

Applied, thanks.

Tiny nit: A short hint in 1/2 that not guarding the result of
$class->lookup($rule->{type}) anymore is fine, as lookup dies
anyway if the plugin cannot be found would have been nice here,
but just a detail.

[1/2] rules: ensure rule is defined and has type in set_rule_defaults
      commit: 00a71f147a203d15e952ca016c188e3e8a0cb518
[2/2] api: rules: check for non-empty nodes and resources properties
      commit: 41603823b12cdc10bb6041b002544795ddd0f91b

Re: applied: [PATCH ha-manager 0/2] fix #7399: check if required rule props are set

[Daniel Kral]

On Wed Mar 11, 2026 at 6:06 PM CET, Thomas Lamprecht wrote:
> On Wed, 11 Mar 2026 13:49:34 +0100, Daniel Kral wrote:
>> The API endpoints allow users to set empty 'nodes' and 'resources'
>> properties for HA rules, which are not handled by the api schema
>> verification nor the api handlers themselves.
>> 
>> PATCH 1 fixes the
>> 
>>     got unexpected error - cannot lookup undefined type! at
>>     /usr/share/perl5/PVE/HA/Config.pm line 232.
>> 
>> [...]
>
> Applied, thanks.
>
> Tiny nit: A short hint in 1/2 that not guarding the result of
> $class->lookup($rule->{type}) anymore is fine, as lookup dies
> anyway if the plugin cannot be found would have been nice here,
> but just a detail.

Good point, the helper does rely on the fact that we don't allow unknown
section types in the rules config and therefore $rule and $rule->{type}
not being set, but still existent in $rules->{ids}.

I'll keep that in mind in the future and address this in a patch series
which should reduce the amount of die's in the Manager.pm to only the
debug assertions!

>
> [1/2] rules: ensure rule is defined and has type in set_rule_defaults
>       commit: 00a71f147a203d15e952ca016c188e3e8a0cb518
> [2/2] api: rules: check for non-empty nodes and resources properties
>       commit: 41603823b12cdc10bb6041b002544795ddd0f91b