From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 525821FF0A7 for ; Tue, 18 Aug 2026 15:35:29 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5D7C32152F; Tue, 18 Aug 2026 15:34:25 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH pve-firewall v2 6/9] firewall: tests: add tests for object reference update logic Date: Tue, 18 Aug 2026 15:34:10 +0200 Message-ID: <20260818133413.450776-7-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260818133413.450776-1-a.bied-charreton@proxmox.com> References: <20260818133413.450776-1-a.bied-charreton@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -0.604 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: FSTEEHM2YOFE2MPETBRIPYKLT2XSRVG6 X-Message-ID-Hash: FSTEEHM2YOFE2MPETBRIPYKLT2XSRVG6 X-MailFrom: abied-charreton@jett.proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The reference updating logic has quite a few edge cases, especially regarding the fact that cluster objects may be shadowed by guest objects. Add a few tests to consolidate the intended functionality. Note that these tests do not cover the actual iteration over the different configs in the cluster, rather they are focused on the core reference rewriting logic. Signed-off-by: Arthur Bied-Charreton --- test/Makefile | 1 + test/referenceupdatetests.pl | 178 +++++++++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+) create mode 100755 test/referenceupdatetests.pl diff --git a/test/Makefile b/test/Makefile index fea9c21..15d2ae3 100644 --- a/test/Makefile +++ b/test/Makefile @@ -4,6 +4,7 @@ all: .PHONY: check check: ./fwtester.pl + ./referenceupdatetests.pl .PHONY: install install: check diff --git a/test/referenceupdatetests.pl b/test/referenceupdatetests.pl new file mode 100755 index 0000000..e80cb50 --- /dev/null +++ b/test/referenceupdatetests.pl @@ -0,0 +1,178 @@ +#!/usr/bin/perl + +# tests for the alias/ipset reference updating logic + +use lib '../src'; + +use v5.36; + +use Test::More; + +use PVE::Firewall::Helpers; + +my $ipset_spec = PVE::Firewall::Helpers::get_object_spec('ipset'); +my $alias_spec = PVE::Firewall::Helpers::get_object_spec('aliases'); + +sub rewrite($conf, $spec, $old, $new, $env, $is_guest) { + return PVE::Firewall::Helpers::rewrite_refs_in_conf($conf, $spec, $old, $new, $env, $is_guest); +} + +subtest 'ipset: cluster rename rewrites dc/ and bare refs' => sub { + my $conf = { + rules => [ + { source => '+dc/foo' }, + { dest => '+foo' }, + { source => '+bar' }, + { source => '+dc/bar' }, + ], + groups => { + grp => [{ source => '+dc/foo' }, { dest => '+foo' }], + }, + ipset => { + set => [{ cidr => '10.0.0.0/8' }], + }, + }; + + my $modified = rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 0); + + ok($modified, 'reports modified'); + is($conf->{rules}->[0]->{source}, '+dc/baz', 'dc/ ref rewritten'); + is($conf->{rules}->[1]->{dest}, '+baz', 'bare ref rewritten'); + is($conf->{rules}->[2]->{source}, '+bar', 'unrelated bare ref kept'); + is($conf->{rules}->[3]->{source}, '+dc/bar', 'unrelated dc/ ref kept'); + is($conf->{groups}->{grp}->[0]->{source}, '+dc/baz', 'group dc/ ref rewritten'); + is($conf->{groups}->{grp}->[1]->{dest}, '+baz', 'group bare ref rewritten'); + is($conf->{ipset}->{set}->[0]->{cidr}, '10.0.0.0/8', 'ipset member not touched'); +}; + +subtest 'ipset: cluster delete drops matching rules' => sub { + my $conf = { + rules => [ + { source => '+dc/foo' }, { source => '+foo' }, { source => '+other' }, + ], + groups => { grp => [{ dest => '+dc/foo' }, { dest => '+keep' }] }, + }; + + my $modified = rewrite($conf, $ipset_spec, 'foo', undef, 'cluster', 0); + + ok($modified, 'reports modified'); + is(scalar($conf->{rules}->@*), 1, 'matching rules dropped'); + is($conf->{rules}->[0]->{source}, '+other', 'unrelated rule kept'); + is(scalar($conf->{groups}->{grp}->@*), 1, 'matching group rule dropped'); + is($conf->{groups}->{grp}->[0]->{dest}, '+keep', 'unrelated group rule kept'); +}; + +subtest 'ipset: guest shadows cluster object' => sub { + my $conf = { + rules => [ + { source => '+foo' }, { dest => '+dc/foo' }, + ], + ipset => { foo => [] }, + }; + + my $modified = rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 1); + + ok($modified, 'reports modified'); + is($conf->{rules}->[0]->{source}, '+foo', 'shadowed bare ref not touched'); + is($conf->{rules}->[1]->{dest}, '+dc/baz', 'explicit cluster ref rewritten'); +}; + +subtest 'ipset: guest without own object' => sub { + my $conf = { + rules => [{ source => '+foo' }, { dest => '+dc/foo' }], + ipset => {}, + }; + + rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 1); + + is($conf->{rules}->[0]->{source}, '+baz', 'implicit cluster ref rewritten'); + is($conf->{rules}->[1]->{dest}, '+dc/baz', 'explicit cluster ref rewritten'); +}; + +subtest 'ipset: guest-level rename' => sub { + my $conf = { + rules => [ + { source => '+foo' }, { dest => '+guest/foo' }, { source => '+dc/foo' }, + ], + }; + + rewrite($conf, $ipset_spec, 'foo', 'baz', 'vm', 0); + + is($conf->{rules}->[0]->{source}, '+baz', 'implicit guest ref rewritten'); + is($conf->{rules}->[1]->{dest}, '+guest/baz', 'guest/ ref rewritten'); + is($conf->{rules}->[2]->{source}, '+dc/foo', 'dc/ ref not touched in guest env'); +}; + +subtest 'ipset: not confused with alias' => sub { + my $conf = { rules => [{ source => 'foo' }, { dest => '+foo' }] }; + + my $modified = rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 0); + + ok($modified, 'reports modified'); + is($conf->{rules}->[0]->{source}, 'foo', 'bare alias ref not touched'); + is($conf->{rules}->[1]->{dest}, '+baz', 'ipset ref rewritten'); +}; + +subtest 'alias: cluster rename rewrites rules and ipset members' => sub { + my $conf = { + rules => [{ source => 'al' }, { dest => 'dc/al' }, { source => 'other' }], + groups => { grp => [{ source => 'dc/al' }] }, + ipset => { + set => [{ cidr => 'al' }, { cidr => 'dc/al' }, { cidr => '10.0.0.1' }], + }, + }; + + my $modified = rewrite($conf, $alias_spec, 'al', 'new', 'cluster', 0); + + ok($modified, 'reports modified'); + is($conf->{rules}->[0]->{source}, 'new', 'bare alias in rule rewritten'); + is($conf->{rules}->[1]->{dest}, 'dc/new', 'dc/ alias in rule rewritten'); + is($conf->{rules}->[2]->{source}, 'other', 'unrelated rule kept'); + is($conf->{groups}->{grp}->[0]->{source}, 'dc/new', 'alias in group rewritten'); + is($conf->{ipset}->{set}->[0]->{cidr}, 'new', 'bare alias in ipset member rewritten'); + is($conf->{ipset}->{set}->[1]->{cidr}, 'dc/new', 'dc/ alias in ipset member rewritten'); + is($conf->{ipset}->{set}->[2]->{cidr}, '10.0.0.1', 'literal cidr member kept'); +}; + +subtest 'alias: cluster delete drops rules and ipset members' => sub { + my $conf = { + rules => [{ source => 'al' }, { source => 'keep' }], + ipset => { set => [{ cidr => 'dc/al' }, { cidr => '10.0.0.1' }] }, + }; + + rewrite($conf, $alias_spec, 'al', undef, 'cluster', 0); + + is(scalar($conf->{rules}->@*), 1, 'matching rule dropped'); + is($conf->{rules}->[0]->{source}, 'keep', 'unrelated rule kept'); + is(scalar($conf->{ipset}->{set}->@*), 1, 'matching member dropped'); + is($conf->{ipset}->{set}->[0]->{cidr}, '10.0.0.1', 'literal member kept'); +}; + +subtest 'alias: not confused with ipset' => sub { + my $conf = { rules => [{ source => '+al' }, { dest => 'al' }] }; + + rewrite($conf, $alias_spec, 'al', 'new', 'cluster', 0); + + is($conf->{rules}->[0]->{source}, '+al', 'ipset ref (+) not touched'); + is($conf->{rules}->[1]->{dest}, 'new', 'alias ref rewritten'); +}; + +subtest 'case-insensitive match, written back lowercase' => sub { + my $conf = { rules => [{ source => '+DC/FOO' }, { dest => '+Foo' }] }; + + rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 0); + + is($conf->{rules}->[0]->{source}, '+dc/baz', 'dc/ ref matched and lowercased'); + is($conf->{rules}->[1]->{dest}, '+baz', 'bare ref matched and lowercased'); +}; + +subtest 'no match reports not modified' => sub { + my $conf = { rules => [{ source => '+other' }, { dest => 'somealias' }] }; + + my $modified = rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 0); + + ok(!$modified, 'nothing matched -> not modified'); + is($conf->{rules}->[0]->{source}, '+other', 'unrelated refs not touched'); +}; + +done_testing(); -- 2.47.3