[pve-devel] [PATCH manager+common 0/2] drop custom node config parser

[pve-devel] [PATCH manager+common 0/2] drop custom node config parser

[Wolfgang Bumiller]

This extends PVE::JSONSchema::parse_config to have an optional "comment
key" to collect comments into and drops the node config parser and its
custom check_type helper to instead use the common parser.

And while I'm at it, I also add the return schema for get_config, since
I see no obvious reason not to.
We do have the 'property' parameter to filter out a specific config
property, but given that the fields are all optional anyway the schema
still matches regardless.

[pve-devel] [PATCH manager 1/2] nodeconfig: use common config parser

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 PVE/NodeConfig.pm | 75 +++++++----------------------------------------
 1 file changed, 10 insertions(+), 65 deletions(-)

diff --git a/PVE/NodeConfig.pm b/PVE/NodeConfig.pm
index df44410f..941e6009 100644
--- a/PVE/NodeConfig.pm
+++ b/PVE/NodeConfig.pm
@@ -48,7 +48,7 @@ sub load_config {
     my $raw = eval { PVE::Tools::file_get_contents($filename); };
     return {} if !$raw;
 
-    return parse_node_config($raw);
+    return parse_node_config($raw, $filename);
 }
 
 sub write_config {
@@ -153,74 +153,19 @@ for my $i (0..$MAXDOMAINS) {
     };
 };
 
-sub check_type {
-    my ($key, $value) = @_;
-
-    die "unknown setting '$key'\n" if !$confdesc->{$key};
-
-    my $type = $confdesc->{$key}->{type};
-
-    if (!defined($value)) {
-	die "got undefined value\n";
-    }
-
-    if ($value =~ m/[\n\r]/) {
-	die "property contains a line feed\n";
-    }
-
-    if ($type eq 'boolean') {
-	return 1 if ($value eq '1') || ($value =~ m/^(on|yes|true)$/i);
-	return 0 if ($value eq '0') || ($value =~ m/^(off|no|false)$/i);
-	die "type check ('boolean') failed - got '$value'\n";
-    } elsif ($type eq 'integer') {
-	return int($1) if $value =~ m/^(\d+)$/;
-	die "type check ('integer') failed - got '$value'\n";
-    } elsif ($type eq 'number') {
-	return $value if $value =~ m/^(\d+)(\.\d+)?$/;
-	die "type check ('number') failed - got '$value'\n";
-    } elsif ($type eq 'string') {
-	if (my $fmt = $confdesc->{$key}->{format}) {
-	    PVE::JSONSchema::check_format($fmt, $value);
-	    return $value;
-	} elsif (my $pattern = $confdesc->{$key}->{pattern}) {
-	    if ($value !~ m/^$pattern$/) {
-		die "value does not match the regex pattern\n";
-	    }
-	}
-	return $value;
-    } else {
-	die "internal error"
-    }
-}
+my $conf_schema = {
+    type => 'object',
+    properties => $confdesc,
+};
 
-sub parse_node_config {
-    my ($content) = @_;
+sub parse_node_config : prototype($$) {
+    my ($content, $filename) = @_;
 
     return undef if !defined($content);
+    my $digest = Digest::SHA::sha1_hex($content);
 
-    my $conf = {
-	digest => Digest::SHA::sha1_hex($content),
-    };
-    my $descr = '';
-
-    my @lines = split(/\n/, $content);
-    foreach my $line (@lines) {
-	if ($line =~ /^\#(.*)\s*$/ || $line =~ /^description:\s*(.*\S)\s*$/) {
-	    $descr .= PVE::Tools::decode_text($1) . "\n";
-	    next;
-	}
-	if ($line =~ /^([a-z][a-z-_]*\d*):\s*(\S.*)\s*$/) {
-	    my $key = $1;
-	    my $value = $2;
-	    $value = eval { check_type($key, $value) };
-	    die "cannot parse value of '$key' in node config: $@" if $@;
-	    $conf->{$key} = $value;
-	} else {
-	    warn "cannot parse line '$line' in node config\n";
-	}
-    }
-
-    $conf->{description} = $descr if $descr;
+    my $conf = PVE::JSONSchema::parse_config($conf_schema, $filename, $content, 'description');
+    $conf->{digest} = $digest;
 
     return $conf;
 }
-- 
2.30.2

[pve-devel] applied: [PATCH manager 1/2] nodeconfig: use common config parser

[Thomas Lamprecht]

On 17.03.22 11:26, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  PVE/NodeConfig.pm | 75 +++++++----------------------------------------
>  1 file changed, 10 insertions(+), 65 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH manager 2/2] api: fill in nodeconfig schema

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 PVE/API2/NodeConfig.pm | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/NodeConfig.pm b/PVE/API2/NodeConfig.pm
index 9c48bcdb..961cd345 100644
--- a/PVE/API2/NodeConfig.pm
+++ b/PVE/API2/NodeConfig.pm
@@ -11,25 +11,25 @@ use base qw(PVE::RESTHandler);
 
 my $node_config_schema = PVE::NodeConfig::get_nodeconfig_schema();
 my $node_config_keys = [ sort keys %$node_config_schema ];
-my $node_config_properties = {
-    delete => {
-	type => 'string', format => 'pve-configid-list',
-	description => "A list of settings you want to delete.",
-	optional => 1,
-    },
+my $node_config_return_properties = {
     digest => {
 	type => 'string',
 	description => 'Prevent changes if current configuration file has different SHA1 digest. This can be used to prevent concurrent modifications.',
 	maxLength => 40,
 	optional => 1,
     },
+    %$node_config_schema,
+};
+my $node_config_properties = {
+    delete => {
+	type => 'string', format => 'pve-configid-list',
+	description => "A list of settings you want to delete.",
+	optional => 1,
+    },
     node => get_standard_option('pve-node'),
+    %$node_config_return_properties,
 };
 
-foreach my $opt (keys %{$node_config_schema}) {
-    $node_config_properties->{$opt} = $node_config_schema->{$opt};
-}
-
 __PACKAGE__->register_method({
     name => 'get_config',
     path => '',
@@ -54,7 +54,7 @@ __PACKAGE__->register_method({
     },
     returns => {
 	type => "object",
-	properties => {},
+	properties => $node_config_return_properties,
     },
     code => sub {
 	my ($param) = @_;
-- 
2.30.2

[pve-devel] applied: [PATCH manager 2/2] api: fill in nodeconfig schema

[Thomas Lamprecht]

On 17.03.22 11:26, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  PVE/API2/NodeConfig.pm | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH common] schema: parse_config: optionally collect comments

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/JSONSchema.pm | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 65055e0..2caf109 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -1823,8 +1823,8 @@ sub get_options {
 }
 
 # A way to parse configuration data by giving a json schema
-sub parse_config {
-    my ($schema, $filename, $raw) = @_;
+sub parse_config : prototype($$$;$) {
+    my ($schema, $filename, $raw, $comment_key) = @_;
 
     # do fast check (avoid validate_schema($schema))
     die "got strange schema" if !$schema->{type} ||
@@ -1832,10 +1832,24 @@ sub parse_config {
 
     my $cfg = {};
 
+    my $comment_data;
+    my $handle_comment = sub { $_[0] =~ /^#/ };
+    if (defined($comment_key)) {
+	$comment_data = '';
+	my $comment_re = qr/^\Q$comment_key\E:\s*(.*\S)\s*$/;
+	$handle_comment = sub {
+	    if ($_[0] =~ /^\#(.*)\s*$/ || $_[0] =~ $comment_re) {
+		$comment_data .= PVE::Tools::decode_text($1) . "\n";
+		return 1;
+	    }
+	    return undef;
+	};
+    }
+
     while ($raw =~ /^\s*(.+?)\s*$/gm) {
 	my $line = $1;
 
-	next if $line =~ /^#/;
+	next if $handle_comment->($line);
 
 	if ($line =~ m/^(\S+?):\s*(.*)$/) {
 	    my $key = $1;
@@ -1851,6 +1865,10 @@ sub parse_config {
 	}
     }
 
+    if (defined($comment_data)) {
+	$cfg->{$comment_key} = $comment_data;
+    }
+
     my $errors = {};
     check_prop($cfg, $schema, '', $errors);
 
-- 
2.30.2

[pve-devel] applied: [PATCH common] schema: parse_config: optionally collect comments

[Thomas Lamprecht]

On 17.03.22 11:26, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  src/PVE/JSONSchema.pm | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
>

applied, thanks!