From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
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) server-digest SHA256)
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id D6ED69A8B0
for <pve-devel@lists.proxmox.com>; Wed, 10 May 2023 10:19:04 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id B625325E02
for <pve-devel@lists.proxmox.com>; Wed, 10 May 2023 10:18:34 +0200 (CEST)
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) server-digest SHA256)
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS
for <pve-devel@lists.proxmox.com>; Wed, 10 May 2023 10:18:33 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 9E09E47F3D
for <pve-devel@lists.proxmox.com>; Wed, 10 May 2023 10:18:33 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Wed, 10 May 2023 10:18:29 +0200
Message-Id: <20230510081829.752401-1-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.30.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.015 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 -
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [sectionconfig.pm]
Subject: [pve-devel] [RFC PATCH common] section config: implement array
support
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>
X-List-Received-Date: Wed, 10 May 2023 08:19:04 -0000
enables section configs in the style of:
----
type: id
property value
property value2
property value3
----
can be combined with property strings
the provided createSchema just uses the name of the property but the
schema of the inner item
the api call is supposed to check if the overall entry of the section
config already exists and if yes, insert a new entry into the array
the updateSchema works very similar, but has the following addition:
if it's not a property string, there is an extra 'X-index' parameter
that indicates the index of the element to be updated (for property
strings, the api call has to figure out which entry to update, but there
we have most likely something resembling an id anyway)
for deletion, we don't provide a schema generator, but a good approach
would be to have optional parameters for the array id/index and only
deleting the relevant array entries then
also adds a test case for such array fields
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
another idea i had with the schemas was to extract the seperate options
of the property string into the api, but that was weird for the
following reasons:
* it's asymmetric if we don't automatically use print_property_string or
* we'd have to parse those directly with parse_property_string
(that would be a major change, since all users that use non-array
property strings don't expect that)
* or have it inconsistent between non-array and array property
strings...
this way we get a property string from the api and the config parsing,
and the config writer expects property strings, so easy to implement and
consistent, for the cost of having all api calls use parse/print
manually (which won't be that many hopefully)
src/PVE/SectionConfig.pm | 88 ++++++++++++++++++++++++++++++-------
test/section_config_test.pl | 26 +++++++++++
2 files changed, 98 insertions(+), 16 deletions(-)
diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
index f36cede..baace66 100644
--- a/src/PVE/SectionConfig.pm
+++ b/src/PVE/SectionConfig.pm
@@ -51,6 +51,17 @@ sub plugindata {
return {};
}
+my $copy_property = sub {
+ my ($src) = @_;
+
+ my $res = {};
+ foreach my $k (keys %$src) {
+ $res->{$k} = $src->{$k};
+ }
+
+ return $res;
+};
+
sub createSchema {
my ($class, $skip_type) = @_;
@@ -60,20 +71,15 @@ sub createSchema {
my $props = {};
- my $copy_property = sub {
- my ($src) = @_;
-
- my $res = {};
- foreach my $k (keys %$src) {
- $res->{$k} = $src->{$k};
- }
-
- return $res;
- };
-
foreach my $p (keys %$propertyList) {
next if $skip_type && $p eq 'type';
+ if ($propertyList->{$p}->{type} eq 'array') {
+ $props->{$p} = $copy_property->($propertyList->{$p}->{items});
+ $props->{$p}->{optional} = $propertyList->{$p}->{optional} // 0;
+ next;
+ }
+
if (!$propertyList->{$p}->{optional}) {
$props->{$p} = $propertyList->{$p};
next;
@@ -124,6 +130,24 @@ sub updateSchema {
next if defined($filter_type) && !defined($copts->{$p});
+ if ($propertyList->{$p}->{type} eq 'array') {
+ $props->{$p} = $copy_property->($propertyList->{$p}->{items});
+ $props->{$p}->{optional} = $propertyList->{$p}->{optional} // 0;
+
+ # add index for all non property strings
+ if (!defined($propertyList->{$p}->{items}->{format})) {
+ $props->{$p}->{requires} = "$p-index";
+ $props->{"$p-index"} = {
+ type => 'integer',
+ minimum => 0,
+ description => "Determines which array element to update (0 is the first element).",
+ optional => $propertyList->{$p}->{optional} // 0,
+ },
+ }
+
+ next;
+ }
+
if (!$propertyList->{$p}->{optional}) {
$props->{$p} = $propertyList->{$p};
next;
@@ -254,7 +278,15 @@ sub check_value {
if (!$skipSchemaCheck) {
my $errors = {};
- PVE::JSONSchema::check_prop($value, $schema, '', $errors);
+
+ my $checkschema = $schema;
+
+ if ($ct eq 'array') {
+ die "no item schema for array" if !defined($schema->{items});
+ $checkschema = $schema->{items};
+ }
+
+ PVE::JSONSchema::check_prop($value, $checkschema, '', $errors);
if (scalar(keys %$errors)) {
die "$errors->{$key}\n" if $errors->{$key};
die "$errors->{_root}\n" if $errors->{_root};
@@ -311,6 +343,15 @@ sub parse_config {
}
};
+ my $is_array = sub {
+ my ($type, $key) = @_;
+
+ my $schema = $pdata->{propertyList}->{$key};
+ die "unknown property type\n" if !$schema;
+
+ return $schema->{type} eq 'array';
+ };
+
my $errors = [];
while (@lines) {
my $line = $nextline->();
@@ -352,11 +393,19 @@ sub parse_config {
my ($k, $v) = ($1, $3);
eval {
- die "duplicate attribute\n" if defined($config->{$k});
- if (!$unknown) {
- $v = $plugin->check_value($type, $k, $v, $sectionId);
+ if ($is_array->($type, $k)) {
+ if (!$unknown) {
+ $v = $plugin->check_value($type, $k, $v, $sectionId);
+ }
+ $config->{$k} = [] if !defined($config->{$k});
+ push $config->{$k}->@*, $v;
+ } else {
+ die "duplicate attribute\n" if defined($config->{$k});
+ if (!$unknown) {
+ $v = $plugin->check_value($type, $k, $v, $sectionId);
+ }
+ $config->{$k} = $v;
}
- $config->{$k} = $v;
};
if (my $err = $@) {
warn "$errprefix (section '$sectionId') - unable to parse value of '$k': $err";
@@ -448,6 +497,13 @@ my $format_config_line = sub {
if ($ct eq 'boolean') {
return "\t$key " . ($value ? 1 : 0) . "\n"
if defined($value);
+ } elsif ($ct eq 'array') {
+ die "property '$key' is not an array" if ref($value) ne 'ARRAY';
+ my $result = '';
+ for my $line ($value->@*) {
+ $result .= "\t$key $line\n" if $value ne '';
+ }
+ return $result;
} else {
return "\t$key $value\n" if "$value" ne '';
}
diff --git a/test/section_config_test.pl b/test/section_config_test.pl
index 22a9643..fc20e4c 100755
--- a/test/section_config_test.pl
+++ b/test/section_config_test.pl
@@ -105,6 +105,25 @@ sub properties {
minimum => 3,
maximum => 9,
},
+ arrayfield => {
+ descritpion => "Array Field with property string",
+ type => 'array',
+ items => {
+ type => 'string',
+ descritpion => 'a property string',
+ format => {
+ subfield1 => {
+ type => 'string',
+ descritpion => 'first subfield'
+ },
+ subfield2 => {
+ type => 'integer',
+ minimum => 0,
+ optional => 1,
+ },
+ },
+ },
+ },
};
}
@@ -113,6 +132,7 @@ sub options {
common => { optional => 1 },
field2 => {},
another => {},
+ arrayfield => { optional => 1 },
};
}
@@ -190,6 +210,10 @@ my $with_unknown_data = {
type => 'two',
field2 => 5,
another => 'even more text',
+ arrayfield => [
+ 'subfield1=test,subfield2=2',
+ 'subfield1=test2',
+ ],
},
invalid => {
type => 'bad',
@@ -214,6 +238,8 @@ bad: invalid
two: t3
field2 5
another even more text
+ arrayfield subfield1=test,subfield2=2
+ arrayfield subfield1=test2
EOF
Conf->expect_fail('unknown-forbidden', $with_unknown_data, $with_unknown_text);
--
2.30.2