[pve-devel] [PATCH v4 pve-storage 13/16] test: add tests for 'ceph.conf' parser and writer

[Max Carrara <m.carrara@proxmox.com>]

These tests attempt to cover all syntax quirks that the format of
'ceph.conf' currently allows.

One known exception however is the handling of "quoted" and "unquoted"
strings, as Ceph's own parser appears to not actually differ between
them either. Curiously, if a "quoted string" isn't able to be
parsed by Ceph's implementation, it goes on to parse it as an
"unquoted string" anyway. [0] In both cases, the result is the same -
the string is parsed with quotes.

Each test case is first tested against the parser - if the resulting
hash matches the expected hash, it is consequently passed into the
writer. The writer's result is then parsed another time and compared
against the expected hash once more.

[0]: https://git.proxmox.com/?p=ceph.git;a=blob;f=ceph/src/common/ConfUtils.cc;h=2f78fd02bf9e27467275752e6f3bca0c5e3946ce;hb=refs/heads/master#l189

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v3 --> v4:
  * new

 src/Makefile                               |   1 +
 src/PVE/Makefile                           |   4 +
 src/PVE/test/Makefile                      |   9 +
 src/PVE/test/ceph_conf_parse_write_test.pl | 402 +++++++++++++++++++++
 4 files changed, 416 insertions(+)
 create mode 100644 src/PVE/test/Makefile
 create mode 100755 src/PVE/test/ceph_conf_parse_write_test.pl

diff --git a/src/Makefile b/src/Makefile
index 1eba51e..a322f46 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -15,6 +15,7 @@ install: PVE bin udev-rbd
 test:
 	perl -I. -T -e "use PVE::CLI::pvesm; PVE::CLI::pvesm->verify_api();"
 	$(MAKE) -C test
+	$(MAKE) -C PVE test
 
 .PHONY: clean
 clean:
diff --git a/src/PVE/Makefile b/src/PVE/Makefile
index 5fe4a0a..d438804 100644
--- a/src/PVE/Makefile
+++ b/src/PVE/Makefile
@@ -9,4 +9,8 @@ install:
 	make -C API2 install
 	make -C CLI install
 
+.PHONY: test
+test:
+	$(MAKE) -C test test
+
 clean:
diff --git a/src/PVE/test/Makefile b/src/PVE/test/Makefile
new file mode 100644
index 0000000..f9c6c79
--- /dev/null
+++ b/src/PVE/test/Makefile
@@ -0,0 +1,9 @@
+all:
+
+.PHONY: test
+test: test_ceph_conf_parse_write
+
+.PHONY: test_ceph_conf_parse_write
+test_ceph_conf_parse_write: ceph_conf_parse_write_test.pl
+	./ceph_conf_parse_write_test.pl
+
diff --git a/src/PVE/test/ceph_conf_parse_write_test.pl b/src/PVE/test/ceph_conf_parse_write_test.pl
new file mode 100755
index 0000000..f76a877
--- /dev/null
+++ b/src/PVE/test/ceph_conf_parse_write_test.pl
@@ -0,0 +1,402 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use lib qw(../..);
+
+use Test::More;
+
+use PVE::CephConfig;
+
+
+# An array of test cases.
+# Each test case is comprised of the following keys:
+#   description => to identify a single test
+#   expected    => the hash that parse_ceph_config should return
+#   raw		=> the raw content of the file to test
+#
+# A case is tested as follows:
+#   1. The 'raw' key is fed into parse_ceph_config
+#   2. The result is compared with 'expected'
+#   3. If correct, the resulting hash is fed into write_ceph_config
+#   4. It's output is then fed to parse_ceph_config again for one last
+#      comparison with 'expected'
+my $tests = [
+    {
+	description => 'empty file',
+	expected => {},
+	raw => <<~EOF,
+	EOF
+    },
+    {
+	description => 'file without section',
+	expected => {},
+	raw => <<~EOF,
+	foo = bar
+	arbitrary text can go here
+
+	Rust is better than Perl
+	EOF
+    },
+    {
+	description => 'single section',
+	expected => {
+	    foo => {
+		bar => 'baz',
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	bar = baz
+	EOF
+    },
+    {
+	description => 'single section, no key-value pairs',
+	expected => {
+	    foo => {},
+	},
+	raw => <<~EOF,
+	[foo]
+	EOF
+    },
+    {
+	description => 'single section, whitespace before key',
+	expected => {
+	    foo => {
+		bar => 'baz',
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	     \t     bar = baz
+	EOF
+    },
+    {
+	description => 'single section, section header with preceding whitespace',
+	expected => {
+	    foo => {
+		bar => 'baz',
+	    },
+	},
+	raw => <<~EOF,
+	  \t    [foo]
+	bar = baz
+	EOF
+    },
+    {
+	description => 'single section, section header with comment',
+	expected => {
+	    foo => {
+		bar => 'baz',
+	    },
+	},
+	raw => <<~EOF,
+	[foo] # some comment
+	bar = baz
+	EOF
+    },
+    {
+	description => 'single section, section header ' .
+	    'with preceding whitespace and comment',
+	expected => {
+	    foo => {
+		bar => 'baz',
+	    },
+	},
+	raw => <<~EOF,
+	  \t  [foo] ; some comment
+	bar = baz
+	EOF
+    },
+    {
+	description => 'single section, arbitrary text before section',
+	expected => {
+	    foo => {
+		bar => 'baz',
+	    },
+	},
+	raw => <<~EOF,
+	Rust is better than Perl
+
+	This text is ignored by our parser, because it makes things simpler
+	[foo]
+	bar = baz
+	EOF
+    },
+    {
+	description => 'single section, invalid key-value pairs',
+	expected => {
+	    foo => {
+		bar => 'baz',
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	this here will cause a warning and is ignored
+	bar = baz
+	as well as this
+	EOF
+    },
+    {
+	description => 'single section, multiple key-value pairs',
+	expected => {
+	    foo => {
+		one => 1,
+		two => 2,
+		three => 3,
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	one = 1
+	two = 2
+
+	three = 3
+	EOF
+    },
+    {
+	description => 'single section, key-value pairs with whitespace',
+	expected => {
+	    foo => {
+		bar => 'baz',
+		quo => 'qux',
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	       \t  bar  \t   =\t \tbaz
+
+	quo\t=\tqux
+	EOF
+    },
+    {
+	description => 'single section, key-value pairs with comments',
+	expected => {
+	    foo => {
+		bar => 'baz',
+		quo => 'qux',
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	; preceding comment
+	bar = baz # some comment
+	### comment inbetween
+	;; another one for good measure
+	quo = qux ; another comment
+	# trailing comment
+	EOF
+    },
+    {
+	description => 'single section, key-value pairs with continued lines',
+	expected => {
+	    foo => {
+		bar => 'baz continued baz',
+		quo => "qux continued \tqux",
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	bar = baz \\
+	continued baz
+
+	quo =\\
+	qux \\
+	continued \\
+	\tqux
+	EOF
+    },
+    {
+	description => 'single section, key-value pairs with ' .
+	    'continued lines and comments',
+	expected => {
+	    foo => {
+		bar => 'baz continued baz',
+		quo => 'qux continued qux',
+		key => 'value',
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	bar = baz \\
+	continued baz # comments are allowed here
+
+	quo =\\
+	qux \\
+	continued \\
+	qux # but this continuation will be ignored, because it's in a comment: \\
+	key = value\\
+	# really weird comment
+	EOF
+    },
+    {
+	description => 'single section, key-value pairs with ' .
+	    'escaped commment literals',
+	expected => {
+	    foo => {
+		bar => 'baz#escaped',
+		quo => 'qux;escaped',
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	bar = baz\\#escaped
+	quo = qux\\;escaped
+	EOF
+    },
+    {
+	description => 'single section, key-value pairs with ' .
+	    'continued lines and escaped commment literal',
+	expected => {
+	    foo => {
+		bar => 'baz#escaped',
+		quo => 'qux;escaped continued# escaped done',
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	bar = baz\\#escaped
+
+	quo = qux\\;escaped\\
+	 continued\\# escaped \\
+	done
+	EOF
+    },
+    {
+	description => 'multiple sections, multiple key-value pairs',
+	expected => {
+	    foo => {
+		one => 1,
+		two => 2,
+	    },
+	    bar => {
+		three => 3,
+		four => 4,
+	    },
+	},
+	raw => <<~EOF,
+	[foo]
+	one = 1
+	two = 2
+	[bar]
+	three = 3
+	four = 4
+	EOF
+    },
+    {
+	description => 'multiple sections, multiple key-value pairs, '
+	    . 'comments inline and inbetween, escaped comment literals, '
+	    . 'continued lines, arbitrary whitespace',
+	expected => {
+	    global => {
+		auth_client_required => 'cephx',
+		auth_cluster_required => 'cephx',
+		auth_service_required => 'cephx',
+		cluster_network => '172.16.65.0/24',
+		fsid => '0e2f72eb-ffff-ffff-ffff-f480790a5b07',
+		mon_allow_pool_delete => 'true',
+		mon_host => '172.16.65.12 172.16.65.13 172.16.65.11',
+		ms_bind_ipv4 => 'true',
+		osd_pool_default_min_size => '2',
+		osd_pool_default_size => '3',
+		public_network => '172.16.65.0/24',
+	    },
+	    client => {
+		keyring => '/etc/pve/priv/$cluster.$name.keyring',
+	    },
+	    'mon.ceph-01' => {
+		public_addr => '172.16.65.11',
+	    },
+	    'mon.ceph-02' => {
+		public_addr => '172.16.65.12',
+	    },
+	    'mon.ceph-03' => {
+		public_addr => '172.16.65.13',
+	    },
+	},
+	raw => <<~EOF,
+	[global]
+	auth_client_required = cephx
+		auth_cluster_required = \\
+	cephx
+	auth_service_required =           cephx
+			cluster_network =        172.16.65.0/24
+	fsid = 0e2f72eb-ffff-ffff-ffff-f480790a5b07
+	    mon_allow_pool_delete = true
+	mon_host = \\
+	172.16.65.12 \\
+	172.16.65.13 \\
+	172.16.65.11 # why is this one last? nobody knows for sure!
+
+	ms_bind_ipv4 = true
+	osd_pool_default_min_size =\\
+	\\
+	\\
+	\\
+	\\
+	\\
+	\\
+	\\
+	\\
+	2
+	osd_pool_default_size =\\
+					    3
+	public_network = 172.16.65.0/24 # some comment
+
+	[client] ### another comment
+	keyring = /etc/pve/priv/\$cluster.\$name.keyring# cheeky trailing comment
+
+	## mon config ##
+	[mon.ceph-01]
+	public_addr = 172.16.65.11 ; foo
+
+	;; another arbitrary comment ;;
+	[mon.ceph-02] ;; very important comment here
+	public_addr = 172.16.65.12 # bar
+
+	[mon.ceph-03]
+	public_addr = 172.16.65.13			# baz
+	EOF
+    },
+];
+
+plan(tests => scalar($tests->@*) * 2);
+
+for my $tt ($tests->@*) {
+
+    my $has_parse_err = 0;
+    my $parse_res = eval {
+	# suppress warnings here to make output less noisy for certain tests
+	local $SIG{__WARN__} = sub {};
+
+	PVE::CephConfig::parse_ceph_config(undef, $tt->{raw})
+    };
+
+    if ($@) {
+	$parse_res = $@;
+	$has_parse_err = 1;
+    }
+
+    my $parse_desc = "parse: $tt->{description}";
+    is_deeply($parse_res, $tt->{expected}, $parse_desc || diag(explain($parse_res)));
+
+
+    my $second_parse_res = eval {
+	local $SIG{__WARN__} = sub {};
+
+	die "cannot execute test due to previous error\n" if $has_parse_err;
+
+	my $write_res = PVE::CephConfig::write_ceph_config(undef, $parse_res);
+	PVE::CephConfig::parse_ceph_config(undef, $write_res)
+    };
+
+    $second_parse_res = $@ if $@;
+
+    my $write_desc = "write: $tt->{description}";
+    is_deeply($second_parse_res, $tt->{expected}, $write_desc || diag(explain($second_parse_res)));
+}
+
+done_testing();
+
+1;
-- 
2.39.2