From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id A9F031FF165
for <inbox@lore.proxmox.com>; Thu, 24 Apr 2025 15:03:43 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id C0D7E797D;
Thu, 24 Apr 2025 15:03:49 +0200 (CEST)
Message-ID: <c9a5bd93-751f-4861-89ee-5e5bb1cb1c80@proxmox.com>
Date: Thu, 24 Apr 2025 15:03:45 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Daniel Kral <d.kral@proxmox.com>
References: <20250325151254.193177-1-d.kral@proxmox.com>
<20250325151254.193177-6-d.kral@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <20250325151254.193177-6-d.kral@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.036 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [rules.pm]
Subject: Re: [pve-devel] [PATCH ha-manager 04/15] add rules section config
base plugin
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Am 25.03.25 um 16:12 schrieb Daniel Kral:
> Add a rules section config base plugin to allow users to specify
> different kinds of rules in a single configuration file.
>
> The interface is designed to allow sub plugins to implement their own
> {decode,encode}_value() methods and also offer a canonicalized version
It's not "allow" them to implement, but actually requires them to
implement it. Otherwise, it would be infinite recursion.
> of their rules with canonicalize(), i.e. with any inconsistencies
> removed and ambiguities resolved. There is also a are_satisfiable()
> method for anticipation of the verification of additions or changes to
> the rules config via the API.
---snip 8<---
> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
> new file mode 100644
> index 0000000..bff3375
> --- /dev/null
> +++ b/src/PVE/HA/Rules.pm
> @@ -0,0 +1,118 @@
> +package PVE::HA::Rules;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::SectionConfig;
Missing include of PVE::Tools.
Nit: I'd put a blank here to separate modules from different packages
and modules from the same package.
> +use PVE::HA::Tools;
> +
> +use base qw(PVE::SectionConfig);
> +
> +# TODO Add descriptions, completions, etc.
> +my $defaultData = {
> + propertyList => {
> + type => { description => "Rule type." },
> + ruleid => get_standard_option('pve-ha-rule-id'),
> + comment => {
> + type => 'string',
> + maxLength => 4096,
> + description => "Rule description.",
> + },
Oh good, so there already is a comment property :)
---snip 8<---
> +sub foreach_service_rule {
> + my ($rules, $func, $opts) = @_;
> +
> + my $sid = $opts->{sid};
> + my $type = $opts->{type};
> +
> + my @ruleids = sort {
> + $rules->{order}->{$a} <=> $rules->{order}->{$b}
> + } keys %{$rules->{ids}};
> +
> + for my $ruleid (@ruleids) {
> + my $rule = $rules->{ids}->{$ruleid};
> +
> + next if !$rule; # invalid rules are kept undef in section config, delete them
s/delete/skip/ ?
> + next if $type && $rule->{type} ne $type;
> + next if $sid && !defined($rule->{services}->{$sid});
Style nit: I'd prefer defined($type) and defined($sid) in the above
expressions
> +
> + $func->($rule, $ruleid);
> + }
> +}
> +
> +sub canonicalize {
> + my ($class, $rules, $groups, $services) = @_;
> +
> + die "implement in subclass";
> +}
> +
> +sub are_satisfiable {
> + my ($class, $rules, $groups, $services) = @_;
> +
> + die "implement in subclass";
> +}
This might not be possible to implement in just the subclasses. E.g.
services 1 and 2 have strict colocation with each other, but 1 has
restricted location on node A and 2 has restricted location on node B.
I don't think it hurts to rather put the implementation here with
knowledge of all rule types and what inter-dependencies they entail. And
maybe have it be a function rather than a method then?
> +sub checked_config {
> + my ($rules, $groups, $services) = @_;
> +
> + my $types = __PACKAGE__->lookup_types();
> +
> + for my $type (@$types) {
> + my $plugin = __PACKAGE__->lookup($type);
> +
> + $plugin->canonicalize($rules, $groups, $services);
Shouldn't we rather only pass the rules that belong to the specific
plugin rather than always all?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel