SPAM: [PATCH pve-firewall v3 07/16] firewall: tests: add tests for object reference update logic

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

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 <a.bied-charreton@proxmox.com>
---
 test/Makefile                |   1 +
 test/referenceupdatetests.pl | 296 +++++++++++++++++++++++++++++++++++
 2 files changed, 297 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..3912aab
--- /dev/null
+++ b/test/referenceupdatetests.pl
@@ -0,0 +1,296 @@
+#!/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, $action = undef, $cluster_conf = {}) {
+    $action //= defined($new) ? 'rename' : 'disable';
+    $old = [$old] if ref($old) ne 'ARRAY';
+    return PVE::Firewall::Helpers::rewrite_refs_in_conf(
+        $conf, $spec, $old, $new, $env, $is_guest, $cluster_conf, $action,
+    );
+}
+
+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 disables matching rules' => sub {
+    my $conf = {
+        rules => [
+            { source => '+dc/foo', enable => 1 },
+            { source => '+foo', enable => 1 },
+            { source => '+other', enable => 1 },
+        ],
+        groups =>
+            { grp => [{ dest => '+dc/foo', enable => 1 }, { dest => '+keep', enable => 1 }] },
+    };
+
+    my $modified = rewrite($conf, $ipset_spec, 'foo', undef, 'cluster', 0);
+
+    ok($modified, 'reports modified');
+    is(scalar($conf->{rules}->@*), 3, 'no rule dropped');
+    is($conf->{rules}->[0]->{enable}, 0, 'dc/ ref rule disabled');
+    is($conf->{rules}->[1]->{enable}, 0, 'bare ref rule disabled');
+    is($conf->{rules}->[2]->{enable}, 1, 'unrelated rule untouched');
+    is($conf->{groups}->{grp}->[0]->{enable}, 0, 'group rule disabled');
+    is($conf->{groups}->{grp}->[1]->{enable}, 1, 'unrelated group rule untouched');
+};
+
+subtest 'ipset: cluster drop removes matching rules' => sub {
+    my $conf = {
+        rules => [
+            { source => '+dc/foo', enable => 1 },
+            { source => '+foo', enable => 1 },
+            { source => '+other', enable => 1 },
+        ],
+        groups =>
+            { grp => [{ dest => '+dc/foo', enable => 1 }, { dest => '+keep', enable => 1 }] },
+    };
+
+    my $modified = rewrite($conf, $ipset_spec, 'foo', undef, 'cluster', 0, 'drop');
+
+    ok($modified, 'reports modified');
+    is(scalar($conf->{rules}->@*), 1, 'matching rules dropped');
+    is($conf->{rules}->[0]->{source}, '+other', 'unrelated rule kept');
+    is($conf->{rules}->[0]->{enable}, 1, 'kept rule not disabled');
+    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: sdn scope, shadowing and multiple objects' => sub {
+    my $conf = {
+        rules => [
+            { source => '+sdn/vnet0-all', enable => 1 },
+            { dest => '+vnet0-all', enable => 1 },
+            { source => '+sdn/vnet0-dhcp', enable => 1 },
+            { dest => '+vnet0-dhcp', enable => 1 },
+            { source => '+dc/vnet0-all', enable => 1 },
+        ],
+        # shadows the SDN-generated IPSet of the same name
+        ipset => { 'vnet0-dhcp' => [] },
+    };
+
+    my $vnet_ipsets = ['vnet0-all', 'vnet0-dhcp'];
+    my $modified = rewrite($conf, $ipset_spec, $vnet_ipsets, undef, 'sdn', 0, 'disable', $conf);
+
+    ok($modified, 'reports modified');
+    is($conf->{rules}->[0]->{enable}, 0, 'sdn/ ref disabled');
+    is($conf->{rules}->[1]->{enable}, 0, 'bare ref disabled');
+    is($conf->{rules}->[2]->{enable}, 0, 'sdn/ ref disabled despite cluster IPSet');
+    is($conf->{rules}->[3]->{enable}, 1, 'bare ref kept, resolves to the cluster IPSet');
+    is($conf->{rules}->[4]->{enable}, 1, 'dc/ ref not touched in sdn env');
+
+    my $guest_conf = {
+        rules => [
+            { source => '+sdn/vnet0-all', enable => 1 },
+            { dest => '+vnet0-all', enable => 1 },
+        ],
+        ipset => { 'vnet0-all' => [] },
+    };
+
+    rewrite($guest_conf, $ipset_spec, $vnet_ipsets, undef, 'sdn', 1, 'disable', {});
+
+    is($guest_conf->{rules}->[0]->{enable}, 0, 'sdn/ ref disabled in guest conf');
+    is($guest_conf->{rules}->[1]->{enable}, 1, 'bare ref kept, resolves to the guest IPSet');
+};
+
+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 disables rules, drops ipset members' => sub {
+    my $conf = {
+        rules => [
+            { source => 'al', dest => '10.0.0.1', action => 'ACCEPT', enable => 1 },
+            { source => 'keep', enable => 1 },
+        ],
+        groups =>
+            { grp => [{ source => 'dc/al', enable => 1 }, { source => 'keep', enable => 1 }] },
+        ipset => {
+            set =>
+                [{ cidr => 'al' }, { cidr => 'dc/al', nomatch => 1 }, { cidr => '10.0.0.1' }],
+        },
+    };
+
+    my $modified = rewrite($conf, $alias_spec, 'al', undef, 'cluster', 0);
+
+    ok($modified, 'reports modified');
+
+    is(scalar($conf->{rules}->@*), 2, 'no rule dropped');
+    is($conf->{rules}->[0]->{enable}, 0, 'matching rule disabled');
+    is($conf->{rules}->[0]->{source}, 'al', 'disabled rule keeps its reference');
+    is($conf->{rules}->[0]->{dest}, '10.0.0.1', 'disabled rule keeps its other properties');
+    is($conf->{rules}->[1]->{enable}, 1, 'unrelated rule untouched');
+
+    is(scalar($conf->{groups}->{grp}->@*), 2, 'no group rule dropped');
+    is($conf->{groups}->{grp}->[0]->{enable}, 0, 'matching group rule disabled');
+    is($conf->{groups}->{grp}->[1]->{enable}, 1, 'unrelated group rule untouched');
+
+    is(scalar($conf->{ipset}->{set}->@*), 1, 'matching members dropped, not disabled');
+    is($conf->{ipset}->{set}->[0]->{cidr}, '10.0.0.1', 'literal member kept');
+    ok(
+        !(grep { exists($_->{enable}) } $conf->{ipset}->{set}->@*),
+        'no ipset member carries an enable flag',
+    );
+};
+
+subtest 'alias: cluster drop removes rules and ipset members' => sub {
+    my $conf = {
+        rules => [{ source => 'al', enable => 1 }, { source => 'keep', enable => 1 }],
+        ipset => { set => [{ cidr => 'al' }, { cidr => 'dc/al' }, { cidr => '10.0.0.1' }] },
+    };
+
+    rewrite($conf, $alias_spec, 'al', undef, 'cluster', 0, 'drop');
+
+    is(scalar($conf->{rules}->@*), 1, 'matching rule dropped');
+    is($conf->{rules}->[0]->{source}, 'keep', 'unrelated rule kept');
+    is($conf->{rules}->[0]->{enable}, 1, 'kept rule not disabled');
+    is(scalar($conf->{ipset}->{set}->@*), 1, 'matching members 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');
+};
+
+subtest 'update_refs: rejects invalid argument combinations' => sub {
+    my $conf = { rules => [] };
+
+    eval {
+        PVE::Firewall::Helpers::update_refs($conf, $ipset_spec, 'foo', 'baz', 'vm', 'bogus');
+    };
+    like($@, qr/invalid action/, 'unknown action rejected');
+
+    eval {
+        PVE::Firewall::Helpers::update_refs($conf, $ipset_spec, ['foo', 'bar'], 'baz', 'vm');
+    };
+    like($@, qr/cannot rename more than one object/, 'renaming several objects rejected');
+};
+
+done_testing();
-- 
2.47.3