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 1FBCF9072D for ; Tue, 2 Apr 2024 16:55:47 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 097ED805F for ; Tue, 2 Apr 2024 16:55:47 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 2 Apr 2024 16:55:45 +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 7350F446D3 for ; Tue, 2 Apr 2024 16:55:45 +0200 (CEST) From: Max Carrara To: pve-devel@lists.proxmox.com Date: Tue, 2 Apr 2024 16:55:14 +0200 Message-Id: <20240402145523.683008-3-m.carrara@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240402145523.683008-1-m.carrara@proxmox.com> References: <20240402145523.683008-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.022 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 Subject: [pve-devel] [PATCH v5 pve-storage 02/11] test: add tests for 'ceph.conf' parser and writer 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: Tue, 02 Apr 2024 14:55:47 -0000 These tests attempt to cover most of Ceph's config parser's grammar, including all of its syntax quirks [0]. Each case is tested against two testing subroutines: 1. The parser's output is compared with the expected output. 2. The writer's output is parsed again ant then compared with the expected output. [0]: https://git.proxmox.com/?p=ceph.git;a=blob;f=ceph/src/common/ConfUtils.cc;h=2f78fd02bf9e27467275752e6f3bca0c5e3946ce;hb=e9fe820e7fffd1b7cde143a9f77653b73fcec748#l144 Signed-off-by: Max Carrara --- Changes v3 --> v4: * new Changes v4 --> v5: * refactor loop over test cases - each test is now represented by its own subroutine - put loop into new `main` subroutine - make it easier to add new testing subroutines by looping over them and calling them by reference on each case * add many new test cases * improve diagnostic output on test failure * reword commit message, omitting paragraph about quoted / unquoted string handling * reorder commit (was previously added after parser / writer changes) src/Makefile | 1 + src/PVE/Makefile | 4 + src/PVE/test/Makefile | 9 + src/PVE/test/ceph_conf_parse_write_test.pl | 794 +++++++++++++++++++++ 4 files changed, 808 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..1d5e506 --- /dev/null +++ b/src/PVE/test/ceph_conf_parse_write_test.pl @@ -0,0 +1,794 @@ +#!/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_cfg => the hash that parse_ceph_config should return +# raw => the raw content of the file to test +my $tests = [ + { + description => 'empty file', + expected_cfg => {}, + raw => <<~EOF, + EOF + }, + { + description => 'file without section', + expected_cfg => {}, + raw => <<~EOF, + While Ceph's format doesn't allow this, we do, because it makes things simpler + foo = bar + arbitrary text can go here + + Rust is better than Perl + EOF + }, + { + description => 'single section', + expected_cfg => { + foo => { + bar => 'baz', + }, + }, + raw => <<~EOF, + [foo] + bar = baz + EOF + }, + { + description => 'single section, no key-value pairs', + expected_cfg => { + foo => {}, + }, + raw => <<~EOF, + [foo] + EOF + }, + { + description => 'single section, whitespace before key', + expected_cfg => { + foo => { + bar => 'baz', + }, + }, + raw => <<~EOF, + [foo] + \t bar = baz + EOF + }, + { + description => 'single section, section header with preceding whitespace', + expected_cfg => { + foo => { + bar => 'baz', + }, + }, + raw => <<~EOF, + \t [foo] + bar = baz + EOF + }, + { + description => 'single section, section header with comment', + expected_cfg => { + foo => { + bar => 'baz', + }, + }, + raw => <<~EOF, + [foo] # some comment + bar = baz + EOF + }, + { + description => 'single section, section header ' . + 'with preceding whitespace and comment', + expected_cfg => { + foo => { + bar => 'baz', + }, + }, + raw => <<~EOF, + \t [foo] ; some comment + bar = baz + EOF + }, + { + description => 'single section, arbitrary text before section', + expected_cfg => { + 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_cfg => { + 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_cfg => { + foo => { + one => 1, + two => 2, + three => 3, + }, + }, + raw => <<~EOF, + [foo] + one = 1 + two = 2 + + three = 3 + EOF + }, + { + description => 'multiple sections with whitespace in section headers', + expected_cfg => { + 'foo bar' => {}, + ' quo qux ' => {}, + }, + raw => <<~EOF, + [foo bar] + [ quo qux ] + EOF + }, + { + description => 'single section with whitespace in section header ' + . 'and multiple key-value pair', + expected_cfg => { + 'foo bar' => { + one => 1, + two => 2, + }, + ' quo ' => { + three => 3, + }, + }, + raw => <<~EOF, + [foo bar] + one = 1 + two = 2 + + [ quo ] + three = 3 + EOF + }, + { + description => 'single section, numeric key-value pairs', + expected_cfg => { + foo => { + '0' => 0, + '1' => 1, + '2' => 0, + '3' => 1, + '3.14' => 1.414, + }, + }, + raw => <<~EOF, + [foo] + 0 = 0 + 1 = 1 + 2 = 0 + 3 = 1 + 3.14 = 1.414 + EOF + }, + { + description => 'single section, keys with single-quoted values', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [foo] + bar = 'baz' + quo = 'qux' + EOF + }, + { + description => 'single section, keys with double-quoted values', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [foo] + bar = "baz" + quo = "qux" + EOF + }, + { + description => 'single section, keys with quoted values, ' + . 'comment literals within quotes', + expected_cfg => { + foo => {}, + }, + raw => <<~EOF, + [foo] + one = "1;1" + two = "2#2" + three = '3;3' + four = '4#4' + EOF + }, + { + description => 'single section, keys with quoted values, ' + . 'escaped comment literals within quotes', + expected_cfg => { + foo => { + one => '1;1', + two => '2#2', + three => '3;3', + four => '4#4', + }, + }, + raw => <<~EOF, + [foo] + one = "1\\;1" + two = "2\\#2" + three = '3\\;3' + four = '4\\#4' + EOF + }, + { + description => 'single section, keys with quoted values, ' + . 'comments after values', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [foo] + bar = "baz" ; some comment + quo = 'qux' # another comment + EOF + }, + { + description => 'single section, keys with quoted values, ' + . 'continued lines after quotes', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [foo] + bar = "baz"\\ + + quo = 'qux'\\ + + EOF + }, + { + description => 'single section, keys with quoted values, ' + . 'continued lines with comments after quotes', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [foo] + bar = "baz"\\ + # believe it or not, this is valid syntax + + quo = 'qux'\\ + \\ + \\ + \\ + \t \t ; and it "supports" trailing whitespace for some reason + EOF + }, + { + description => 'single section, key-value pairs with whitespace', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [foo] + \t bar \t =\t \tbaz\t + + quo\t=\tqux + EOF + }, + { + description => 'single section, key-value pairs without whitespace', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [foo] + bar=baz + quo=qux + EOF + }, + { + description => 'single section, key-value pairs with repeated whitespace ' + . 'in key names', + expected_cfg => { + foo => { + 'one_space' => 1, + 'two_spaces' => 2, + 'three_spaces' => 3, + }, + }, + raw => <<~EOF, + [foo] + one space = 1 + two spaces = 2 + three spaces = 3 + EOF + }, + { + description => 'single section, key-value pairs with whitespace and ' + . 'complex whitespace in key names', + expected_cfg => { + foo => { + 'one_two' => 'foo', + 'three_four' => 'bar', + 'five_six' => 'baz', + }, + }, + raw => <<~EOF, + [foo] + \t one two \t =\t \tfoo\t + + three \t four\t=\tbar\t + five six=baz + EOF + }, + { + description => 'single section, key-value pairs with repeated whitespace ' + . 'and underlines in key names', + expected_cfg => { + foo => { + 'one_ul' => 2, + 'two_ul' => 0, + 'two__ul' => 0, + 'odd___name' => 4, + }, + }, + raw => <<~EOF, + [foo] + # these are equivalent + one ul = 0 + one_ul = 1 + one ul = 2 + + # these are not + two ul = 0 + two__ul = 0 + + # these are equivalent + odd _ name = 0 + odd _ name = 1 + odd__ name = 2 + odd __name = 3 + odd___name = 4 + + EOF + }, + { + description => 'single section with line continuations, multiple key-value pairs', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [\\ + f\\ + o\\ + o\\ + ]\\ + + bar = baz + quo = qux + EOF + }, + { + description => 'single section, key-value pairs with continued lines in keys', + expected_cfg => { + foo => { + bar => 'baz', + quo => 'qux', + }, + }, + raw => <<~EOF, + [foo] + bar\\ + = baz + \\ + quo\\ + \\ + \\ + \\ + \\ + \\ + \\ + = qux + EOF + }, + { + description => 'multiple sections with escaped comment literals, ' + . 'multiple key-value pairs', + expected_cfg => { + 'f;oo' => { + one => 1, + two => 2, + }, + 'b#ar' => { + three => 3, + four => 4, + }, + '###' => { + five => 5, + }, + ';;;' => { + six => 6, + }, + }, + raw => <<~EOF, + [f\\;oo] + one = 1 + two = 2 + + [b\\#ar] + three = 3 + four = 4 + + [\\#\\#\\#] + five = 5 + + [\\;\\;\\;] + six = 6 + EOF + }, + { + description => 'single section, key-value pairs with comments', + expected_cfg => { + 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_cfg => { + 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_cfg => { + 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 in values', + expected_cfg => { + 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 literals in values', + expected_cfg => { + foo => { + bar => 'baz#escaped', + quo => 'qux;escaped continued# escaped done', + }, + }, + raw => <<~EOF, + [foo] + bar = baz\\#escaped + + quo = qux\\;escaped\\ + continued\\# escaped \\ + done + EOF + }, + { + description => 'single section, key-value pairs with escaped comment ' + . 'literals in key names', + expected_cfg => { + foo => { + 'b#a#r' => 'baz', + ';q;uo' => 'qux', + '#' => 1, + '##' => 2, + '###' => 3, + ';' => 1, + ';;' => 2, + ';;;' => 3, + }, + }, + raw => <<~EOF, + [foo] + b\\#a\\#r = baz + \\;q\\;uo = qux + + \\# = 1 + \\#\\# = 2 + \\#\\#\\# = 3 + + \\; = 1 + \\;\\; = 2 + \\;\\;\\; = 3 + EOF + }, + { + description => 'multiple sections, multiple key-value pairs', + expected_cfg => { + 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', + # NOTE: We don't use '/etc/pve/priv/$cluster.$name.keyring' as value for + # 'keyring' below, because `ceph-conf` will actually substitute those. + # Because we don't care for that (not the parser's or writer's job) we + # just omit the dollar signs. + expected_cfg => { + 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', + }, + 'some arbitrary section' => { + some_key => 'foo;bar;baz', + }, + }, + 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 + + [some arbitrary section] + some key = "foo\\;bar\\;baz" # I am a comment + EOF + }, +]; + +sub test_parse_ceph_config { + my ($case) = @_; + + my $desc = "parse_ceph_config: $case->{description}"; + + my $parse_result = eval { PVE::CephConfig::parse_ceph_config(undef, $case->{raw}) }; + if ($@) { + fail($desc); + diag('Failed to parse config:'); + diag($@); + return; + } + + if (!is_deeply($parse_result, $case->{expected_cfg}, $desc)) { + diag("=== Expected ==="); + diag(explain($case->{expected_cfg})); + diag("=== Got ==="); + diag(explain($parse_result)); + } +} + +sub test_write_ceph_config { + my ($case) = @_; + + my $desc = "write_ceph_config: $case->{description}"; + + my $write_result = eval { PVE::CephConfig::write_ceph_config(undef, $case->{expected_cfg}) }; + if ($@) { + fail($desc); + diag('Failed to write config:'); + diag($@); + return; + } + + my $parse_result = eval { PVE::CephConfig::parse_ceph_config(undef, $write_result) }; + if ($@) { + fail($desc); + diag('Failed to parse previously written config:'); + diag($@); + return; + } + + if (!is_deeply($parse_result, $case->{expected_cfg}, $desc)) { + diag("=== Expected ==="); + diag(explain($case->{expected_cfg})); + diag("=== Got ==="); + diag(explain($parse_result)); + diag("=== Write Output ==="); + diag($write_result); + } +} + +sub main { + my $test_subs = [ + \&test_parse_ceph_config, + \&test_write_ceph_config, + ]; + + plan(tests => scalar($tests->@*) * scalar($test_subs->@*)); + + for my $case ($tests->@*) { + for my $test_sub ($test_subs->@*) { + eval { + # suppress warnings here to make output less noisy for certain tests + local $SIG{__WARN__} = sub {}; + $test_sub->($case); + }; + warn "$@\n" if $@; + }; + } + + done_testing(); +} + +main(); -- 2.39.2