From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3560F99C6F for ; Thu, 16 Nov 2023 16:22:28 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F402C16A6A for ; Thu, 16 Nov 2023 16:21:57 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 16 Nov 2023 16:21:55 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 9BA6E43973 for ; Thu, 16 Nov 2023 16:21:55 +0100 (CET) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Thu, 16 Nov 2023 16:21:50 +0100 Message-Id: <20231116152152.1371406-5-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20231116152152.1371406-1-d.csapak@proxmox.com> References: <20231116152152.1371406-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.017 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pve-devel] [PATCH common v4 4/5] section config: allow separated property lists for plugins X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Nov 2023 15:22:28 -0000 when using 'init(1)'. This saves the property lists per type instead of a big one, and using create/updateSchema creates a new schema with the options as 'oneOf' and/or 'instance-types' (depending if the schemas match). with that, we change how we work with the options hash: it's not needed anymore to specify options that are specified in the type specific propertyList, except if it's 'fixed => 1' (since that does not exist in the schema) Signed-off-by: Dominik Csapak --- src/PVE/SectionConfig.pm | 312 ++++++++++++++++++++++++++++++++------- 1 file changed, 255 insertions(+), 57 deletions(-) diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm index 5690476..43575ff 100644 --- a/src/PVE/SectionConfig.pm +++ b/src/PVE/SectionConfig.pm @@ -1,3 +1,56 @@ +# This pacakge provides a way to have multiple (often similar) types of entries +# in the same config file, each in its own section, thus Section Config +# +# The intended structure is to have a single 'base' plugin that inherits from +# this class and provides meaningful defaults in its '$defaultData', e.g. a +# default list of properties in its propertyList (most often only 'id' and +# 'type') +# +# Each 'real' plugin then has it's own package that should inherit from the +# 'base' plugin and returns it's wanted properties in the 'properties' method, +# its type in the 'type' method and the used options in the 'options' method. +# The options method declares if the property is 'optional' or 'fixed' like so: +# +# ```` +# sub options { +# return { +# optional1 => { optional => 1 }, +# fixed1 => { fixed => 1 }, +# requiredButNotFixed => {}, +# }; +# } +# ``` +# +# 'fixed' options can be set on create, but not changed afterwards. +# +# To actually use it, you have to register the plugins and init the 'base' +# plugin, like so: +# +# ``` +# PVE::Dummy::Plugin1->register(); +# PVE::Dummy::Plugin2->register(); +# PVE::Dummy::Plugin3->register(); +# PVE::Dummy::BasePlugin->init(); +# ``` +# +# There are two modes in how to use it, the 'unified' mode and the 'isolated' +# mode. In the default unified mode, there is only a global list of properties +# which the plugins can use, so you cannot define the same propertyname twice +# in different plugins. Reason for this is to force the use of identical +# properties for multiple plugins. +# +# The second way is to use the 'isolated' mode, which can be achieved by +# calling init with `1` as its parameter like this: +# +# ``` +# PVE::Dummy::BasePlugin->init(1); +# ``` +# +# With this, each plugin get's their own isolated list of properties which it +# can use. Note that in this mode, you only have to specify the property in the +# options method when it is either 'fixed' or comes from the global list of +# properties. All locally defined ones get automatically added to the schema +# for that plugin. package PVE::SectionConfig; use strict; @@ -8,6 +61,7 @@ use Digest::SHA; use PVE::Exception qw(raise_param_exc); use PVE::JSONSchema qw(get_standard_option); +use PVE::Tools; my $defaultData = { options => {}, @@ -51,6 +105,62 @@ sub plugindata { return {}; } +sub has_isolated_properties { + my ($class) = @_; + + my $isolatedPropertyList = $class->private()->{isolatedPropertyList}; + + return defined($isolatedPropertyList) && scalar(keys $isolatedPropertyList->%*) > 0; +} + +my sub compare_property { + my ($a, $b, $skip_opts) = @_; + + my $merged = {$a->%*, $b->%*}; + delete $merged->{$_} for $skip_opts->@*; + + for my $opt (keys $merged->%*) { + return 0 if !PVE::Tools::is_deeply($a->{$opt}, $b->{$opt}); + } + + return 1; +}; + +my sub add_property { + my ($props, $key, $prop, $type) = @_; + + if (!defined($props->{$key})) { + $props->{$key} = $prop; + return; + } + + if (!defined($props->{$key}->{oneOf})) { + if (compare_property($props->{$key}, $prop, ['instance-types'])) { + push $props->{$key}->{'instance-types'}->@*, $type; + } else { + my $new_prop = delete $props->{$key}; + delete $new_prop->{'type-property'}; + delete $prop->{'type-property'}; + $props->{$key} = { + 'type-property' => 'type', + oneOf => [ + $new_prop, + $prop, + ], + }; + } + } else { + for my $existing_prop ($props->{$key}->{oneOf}->@*) { + if (compare_property($existing_prop, $prop, ['instance-types', 'type-property'])) { + push $existing_prop->{'instance-types'}->@*, $type; + return; + } + } + + push $props->{$key}->{oneOf}->@*, $prop; + } +}; + sub createSchema { my ($class, $skip_type, $base) = @_; @@ -60,42 +170,61 @@ sub createSchema { my $props = $base || {}; - my $copy_property = sub { - my ($src) = @_; + if (!$class->has_isolated_properties()) { + foreach my $p (keys %$propertyList) { + next if $skip_type && $p eq 'type'; - my $res = {}; - foreach my $k (keys %$src) { - $res->{$k} = $src->{$k}; - } + if (!$propertyList->{$p}->{optional}) { + $props->{$p} = $propertyList->{$p}; + next; + } - return $res; - }; + my $required = 1; - foreach my $p (keys %$propertyList) { - next if $skip_type && $p eq 'type'; + my $copts = $class->options(); + $required = 0 if defined($copts->{$p}) && $copts->{$p}->{optional}; - if (!$propertyList->{$p}->{optional}) { - $props->{$p} = $propertyList->{$p}; - next; - } - - my $required = 1; - - my $copts = $class->options(); - $required = 0 if defined($copts->{$p}) && $copts->{$p}->{optional}; + foreach my $t (keys %$plugins) { + my $opts = $pdata->{options}->{$t} || {}; + $required = 0 if !defined($opts->{$p}) || $opts->{$p}->{optional}; + } - foreach my $t (keys %$plugins) { - my $opts = $pdata->{options}->{$t} || {}; - $required = 0 if !defined($opts->{$p}) || $opts->{$p}->{optional}; + if ($required) { + # make a copy, because we modify the optional property + my $res = {$propertyList->{$p}->%*}; # shallow copy + $res->{optional} = 0; + $props->{$p} = $res; + } else { + $props->{$p} = $propertyList->{$p}; + } } - - if ($required) { - # make a copy, because we modify the optional property - my $res = &$copy_property($propertyList->{$p}); - $res->{optional} = 0; - $props->{$p} = $res; - } else { - $props->{$p} = $propertyList->{$p}; + } else { + for my $type (sort keys %$plugins) { + my $opts = $pdata->{options}->{$type} || {}; + for my $key (sort keys $opts->%*) { + my $schema = $class->get_property_schema($type, $key); + my $prop = {$schema->%*}; + $prop->{'instance-types'} = [$type]; + $prop->{'type-property'} = 'type'; + $prop->{optional} = 1 if $opts->{$key}->{optional}; + + add_property($props, $key, $prop, $type); + } + } + # add remaining global properties + for my $opt (keys $propertyList->%*) { + next if $props->{$opt}; + $props->{$opt} = {$propertyList->{$opt}->%*}; + } + for my $opt (keys $props->%*) { + if (my $necessaryTypes = $props->{$opt}->{'instance-types'}) { + if ($necessaryTypes->@* == scalar(keys $plugins->%*)) { + delete $props->{$opt}->{'instance-types'}; + delete $props->{$opt}->{'type-property'}; + } else { + $props->{$opt}->{optional} = 1; + } + } } } @@ -117,30 +246,61 @@ sub updateSchema { my $filter_type = $single_class ? $class->type() : undef; - foreach my $p (keys %$propertyList) { - next if $p eq 'type'; + if (!$class->has_isolated_properties()) { + foreach my $p (keys %$propertyList) { + next if $p eq 'type'; - my $copts = $class->options(); + my $copts = $class->options(); - next if defined($filter_type) && !defined($copts->{$p}); + next if defined($filter_type) && !defined($copts->{$p}); - if (!$propertyList->{$p}->{optional}) { - $props->{$p} = $propertyList->{$p}; - next; - } + if (!$propertyList->{$p}->{optional}) { + $props->{$p} = $propertyList->{$p}; + next; + } - my $modifyable = 0; + my $modifyable = 0; - $modifyable = 1 if defined($copts->{$p}) && !$copts->{$p}->{fixed}; + $modifyable = 1 if defined($copts->{$p}) && !$copts->{$p}->{fixed}; - foreach my $t (keys %$plugins) { - my $opts = $pdata->{options}->{$t} || {}; - next if !defined($opts->{$p}); - $modifyable = 1 if !$opts->{$p}->{fixed}; + foreach my $t (keys %$plugins) { + my $opts = $pdata->{options}->{$t} || {}; + next if !defined($opts->{$p}); + $modifyable = 1 if !$opts->{$p}->{fixed}; + } + next if !$modifyable; + + $props->{$p} = $propertyList->{$p}; + } + } else { + for my $type (sort keys %$plugins) { + my $opts = $pdata->{options}->{$type} || {}; + for my $key (sort keys $opts->%*) { + next if $opts->{$key}->{fixed}; + + my $schema = $class->get_property_schema($type, $key); + my $prop = {$schema->%*}; + $prop->{'instance-types'} = [$type]; + $prop->{'type-property'} = 'type'; + $prop->{optional} = 1; + + add_property($props, $key, $prop, $type); + } + } + + for my $opt (keys $propertyList->%*) { + next if $props->{$opt}; + $props->{$opt} = {$propertyList->{$opt}->%*}; } - next if !$modifyable; - $props->{$p} = $propertyList->{$p}; + for my $opt (keys $props->%*) { + if (my $necessaryTypes = $props->{$opt}->{'instance-types'}) { + if ($necessaryTypes->@* == scalar(keys $plugins->%*)) { + delete $props->{$opt}->{'instance-types'}; + delete $props->{$opt}->{'type-property'}; + } + } + } } $props->{digest} = get_standard_option('pve-config-digest'); @@ -160,22 +320,28 @@ sub updateSchema { } sub init { - my ($class) = @_; + my ($class, $separate_properties) = @_; my $pdata = $class->private(); - foreach my $k (qw(options plugins plugindata propertyList)) { + foreach my $k (qw(options plugins plugindata propertyList isolatedPropertyList)) { $pdata->{$k} = {} if !$pdata->{$k}; } my $plugins = $pdata->{plugins}; my $propertyList = $pdata->{propertyList}; + my $isolatedPropertyList = $pdata->{isolatedPropertyList}; foreach my $type (keys %$plugins) { my $props = $plugins->{$type}->properties(); foreach my $p (keys %$props) { - die "duplicate property '$p'" if defined($propertyList->{$p}); - my $res = $propertyList->{$p} = {}; + my $res; + if ($separate_properties) { + $res = $isolatedPropertyList->{$type}->{$p} = {}; + } else { + die "duplicate property '$p'" if defined($propertyList->{$p}); + $res = $propertyList->{$p} = {}; + } my $data = $props->{$p}; for my $a (keys %$data) { $res->{$a} = $data->{$a}; @@ -187,8 +353,23 @@ sub init { foreach my $type (keys %$plugins) { my $opts = $plugins->{$type}->options(); foreach my $p (keys %$opts) { - die "undefined property '$p'" if !$propertyList->{$p}; + my $prop; + if ($separate_properties) { + $prop = $isolatedPropertyList->{$type}->{$p}; + } + $prop //= $propertyList->{$p}; + die "undefined property '$p'" if !$prop; + } + + # automatically the properties to options (if not specified explicitely) + if ($separate_properties) { + foreach my $p (keys $isolatedPropertyList->{$type}->%*) { + next if $opts->{$p}; + $opts->{$p} = {}; + $opts->{$p}->{optional} = 1 if $isolatedPropertyList->{$type}->{$p}->{optional}; + } } + $pdata->{options}->{$type} = $opts; } @@ -241,7 +422,7 @@ sub check_value { die "unexpected property '$key'\n" if !defined($opts->{$key}); - my $schema = $pdata->{propertyList}->{$key}; + my $schema = $class->get_property_schema($type, $key); die "unknown property type\n" if !$schema; my $ct = $schema->{type}; @@ -295,6 +476,20 @@ sub format_section_header { return "$type: $sectionId\n"; } +sub get_property_schema { + my ($class, $type, $key) = @_; + + my $pdata = $class->private(); + my $opts = $pdata->{options}->{$type}; + + my $schema; + if ($class->has_isolated_properties()) { + $schema = $pdata->{isolatedPropertyList}->{$type}->{$key}; + } + $schema //= $pdata->{propertyList}->{$key}; + + return $schema; +} sub parse_config { my ($class, $filename, $raw, $allow_unknown) = @_; @@ -322,7 +517,7 @@ sub parse_config { my $is_array = sub { my ($type, $key) = @_; - my $schema = $pdata->{propertyList}->{$key}; + my $schema = $class->get_property_schema($type, $key); die "unknown property type\n" if !$schema; return $schema->{type} eq 'array'; @@ -494,7 +689,6 @@ sub write_config { my ($class, $filename, $cfg, $allow_unknown) = @_; my $pdata = $class->private(); - my $propertyList = $pdata->{propertyList}; my $out = ''; @@ -516,6 +710,7 @@ sub write_config { my $scfg = $ids->{$sectionId}; my $type = $scfg->{type}; my $opts = $pdata->{options}->{$type}; + my $global_opts = $pdata->{options}->{__global}; die "unknown section type '$type'\n" if !$opts && !$allow_unknown; @@ -545,7 +740,8 @@ sub write_config { if ($scfg->{comment} && !$done_hash->{comment}) { my $k = 'comment'; my $v = $class->encode_value($type, $k, $scfg->{$k}); - $data .= &$format_config_line($propertyList->{$k}, $k, $v); + my $prop = $class->get_property_schema($type, $k); + $data .= &$format_config_line($prop, $k, $v); } $data .= "\tdisable\n" if $scfg->{disable} && !$done_hash->{disable}; @@ -562,7 +758,8 @@ sub write_config { die "section '$sectionId' - missing value for required option '$k'\n" if !defined ($v); $v = $class->encode_value($type, $k, $v); - $data .= &$format_config_line($propertyList->{$k}, $k, $v); + my $prop = $class->get_property_schema($type, $k); + $data .= &$format_config_line($prop, $k, $v); } foreach my $k (@option_keys) { @@ -570,7 +767,8 @@ sub write_config { my $v = $scfg->{$k}; next if !defined($v); $v = $class->encode_value($type, $k, $v); - $data .= &$format_config_line($propertyList->{$k}, $k, $v); + my $prop = $class->get_property_schema($type, $k); + $data .= &$format_config_line($prop, $k, $v); } $out .= "$data\n"; -- 2.30.2