* [PATCH pve-access-control v3 1/5] fix: #7520: sdn: prune orphaned ACLs on resource deletion
2026-06-03 14:55 [PATCH access-control/network v3 0/5] fix #7520: sdn: prune orphaned ACLs and handle VNet migrations David Riley
@ 2026-06-03 14:55 ` David Riley
2026-06-03 14:55 ` [PATCH pve-access-control v3 2/5] fix #7520: test: add unit tests for sdn acl pruning logic David Riley
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: David Riley @ 2026-06-03 14:55 UTC (permalink / raw)
To: pve-devel; +Cc: David Riley
Add a helper routine to prune ACL entries under the '/sdn/' path when
the corresponding resources are deleted. Compare the running
configuration against the newly compiled state during config commit.
This prevents dangling permission states and ensures configuration
consistency across manual API/UI applies as well as automatic reloads
during system boot.
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7520
Signed-off-by: David Riley <d.riley@proxmox.com>
---
src/PVE/AccessControl.pm | 46 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 0d632b3..7581394 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1974,6 +1974,52 @@ sub remove_vm_from_pool {
lock_user_config($delVMfromPoolFn, "pool cleanup for VM $vmid failed");
}
+sub remove_sdn_resource_access {
+ my ($paths) = @_; # [ ['zones', '<zone>'], ['zones', '<zone>', '<vnet>'] ]
+
+ my $delete_resource_fn = sub {
+ my $usercfg = cfs_read_file("user.cfg");
+ my $modified = 0;
+
+ foreach my $segments (@$paths) {
+ my @full_path = ('sdn', @$segments);
+ my $current = $usercfg->{acl_root};
+ my ($parent, $last_key) = lookup_acl_node($current, \@full_path);
+
+ if ($parent && $last_key) {
+ delete $parent->{children}->{$last_key};
+ $modified = 1;
+ }
+ }
+
+ if ($modified) {
+ cfs_write_file("user.cfg", $usercfg);
+ }
+ };
+
+ lock_user_config($delete_resource_fn, "SDN ACL cleanup failed");
+}
+
+sub lookup_acl_node {
+ my ($root, $path) = @_;
+
+ my $current = $root;
+ my $parent = undef;
+ my $last_key = undef;
+
+ foreach my $step (@$path) {
+ if ($current->{children} && $current->{children}->{$step}) {
+ $parent = $current;
+ $last_key = $step;
+ $current = $current->{children}->{$step};
+ } else {
+ return;
+ }
+ }
+
+ return ($parent, $last_key, $current);
+}
+
my $USER_CONTROLLED_TFA_TYPES = {
u2f => 1,
oath => 1,
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH pve-access-control v3 2/5] fix #7520: test: add unit tests for sdn acl pruning logic
2026-06-03 14:55 [PATCH access-control/network v3 0/5] fix #7520: sdn: prune orphaned ACLs and handle VNet migrations David Riley
2026-06-03 14:55 ` [PATCH pve-access-control v3 1/5] fix: #7520: sdn: prune orphaned ACLs on resource deletion David Riley
@ 2026-06-03 14:55 ` David Riley
2026-06-03 14:55 ` [PATCH pve-access-control v3 3/5] fix: #7520: sdn: add VNet ACL migration David Riley
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: David Riley @ 2026-06-03 14:55 UTC (permalink / raw)
To: pve-devel; +Cc: David Riley
Add unit tests to validate the behavior of the SDN resource access
pruning logic.
The tests cover all designated SDN resource paths including zones,
vnets (with/without tags), controllers, prefix-lists, route-maps, and
fabrics, ensuring explicit permissions are dropped when resources are
deleted.
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7520
Signed-off-by: David Riley <d.riley@proxmox.com>
---
src/test/Makefile | 1 +
src/test/sdn_acl_pruning.cfg | 22 ++++++
src/test/sdn_acl_pruning_test.pl | 116 +++++++++++++++++++++++++++++++
3 files changed, 139 insertions(+)
create mode 100644 src/test/sdn_acl_pruning.cfg
create mode 100644 src/test/sdn_acl_pruning_test.pl
diff --git a/src/test/Makefile b/src/test/Makefile
index 53f2da7..2f44186 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -14,3 +14,4 @@ check:
perl -I.. perm-test8.pl
perl -I.. realm_sync_test.pl
perl -I.. api-tests.pl
+ perl -I.. sdn_acl_pruning_test.pl
diff --git a/src/test/sdn_acl_pruning.cfg b/src/test/sdn_acl_pruning.cfg
new file mode 100644
index 0000000..f8b47a6
--- /dev/null
+++ b/src/test/sdn_acl_pruning.cfg
@@ -0,0 +1,22 @@
+user:user1@pve:1:0::::::
+user:user2@pve:1:0::::::
+user:user3@pve:1:0::::::
+
+group:admins:user3@pve:::
+role:SDNAdmin:SDN.Allocate,SDN.Audit
+role:SDNAudit:SDN.Audit
+
+acl:1:/sdn/zones/zone1:@admins:SDNAdmin:
+acl:1:/sdn/zones/zone2:@admins:SDNAdmin:
+
+acl:1:/sdn/zones/zone1/vnet1:user1@pve:SDNAdmin:
+acl:1:/sdn/zones/zone1/vnet1/10:user1@pve:SDNAdmin:
+acl:1:/sdn/zones/zone1/vnet2:user1@pve:SDNAdmin:
+acl:1:/sdn/zones/zone1/vnet2/100:user1@pve:SDNAudit:
+
+acl:1:/sdn/zones/zone2:user1@pve:SDNAdmin:
+
+acl:1:/sdn/controllers/control:user2@pve:SDNAdmin:
+acl:1:/sdn/prefix-lists/list:user2@pve:SDNAdmin:
+acl:1:/sdn/route-maps/map:user2@pve:SDNAdmin:
+acl:1:/sdn/fabrics/fabric:user2@pve:SDNAdmin:
diff --git a/src/test/sdn_acl_pruning_test.pl b/src/test/sdn_acl_pruning_test.pl
new file mode 100644
index 0000000..60f6c9b
--- /dev/null
+++ b/src/test/sdn_acl_pruning_test.pl
@@ -0,0 +1,116 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use lib qw(..);
+
+use PVE::Tools;
+use Test::More;
+use Test::MockModule;
+
+use PVE::AccessControl;
+use PVE::RPCEnvironment;
+
+my $cluster_module = Test::MockModule->new('PVE::Cluster');
+$cluster_module->noop('cfs_update');
+
+my $cfgfn = "sdn_acl_pruning.cfg";
+my $raw_cfg = PVE::Tools::file_get_contents($cfgfn);
+my $mocked_user_cfg_tree = PVE::AccessControl::parse_user_config("user.cfg", $raw_cfg);
+
+my $access_control_module = Test::MockModule->new('PVE::AccessControl');
+
+# Mocking
+$access_control_module->mock(
+ 'cfs_read_file',
+ sub {
+ my ($filename) = @_;
+ return $mocked_user_cfg_tree if $filename eq 'user.cfg';
+ die "mock cfs_read_file: unexpected file $filename";
+ },
+);
+
+$access_control_module->mock(
+ 'cfs_write_file',
+ sub {
+ my ($filename, $cfg) = @_;
+ $mocked_user_cfg_tree = $cfg if $filename eq 'user.cfg';
+ },
+);
+
+$access_control_module->mock(
+ 'lock_user_config',
+ sub {
+ my ($code, $errmsg) = @_;
+ eval { $code->() };
+ if (my $err = $@) {
+ die "$errmsg: $err\n";
+ }
+ },
+);
+
+my $rpcenv = PVE::RPCEnvironment->init('cli');
+$rpcenv->{user_cfg} = $mocked_user_cfg_tree;
+
+sub check_roles {
+ my ($user, $path, $expected_result) = @_;
+
+ my $roles = PVE::AccessControl::roles($rpcenv->{user_cfg}, $user, $path);
+ my $res = join(',', sort keys %$roles);
+
+ is($res, $expected_result, "Roles for $user on $path should be '$expected_result'");
+}
+
+# Tests
+# Check ACLs state before pruning
+# User3 (Group)
+check_roles('user3@pve', '/sdn/zones/zone1', 'SDNAdmin');
+check_roles('user3@pve', '/sdn/zones/zone2', 'SDNAdmin');
+
+# vnets
+check_roles('user1@pve', '/sdn/zones/zone1/vnet1', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet1/10', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2/100', 'SDNAudit');
+
+# controllers, prefix-lists, route-maps, fabrics
+check_roles('user2@pve', '/sdn/controllers/control', 'SDNAdmin');
+check_roles('user2@pve', '/sdn/prefix-lists/list', 'SDNAdmin');
+check_roles('user2@pve', '/sdn/route-maps/map', 'SDNAdmin');
+check_roles('user2@pve', '/sdn/fabrics/fabric', 'SDNAdmin');
+
+# Prune
+my $paths_to_delete = [
+ ['zones', 'zone2'],
+
+ ['zones', 'zone1', 'vnet1'],
+ ['zones', 'zone1', 'vnet2', '100'],
+
+ ['controllers', 'control'],
+ ['prefix-lists', 'list'],
+ ['route-maps', 'map'],
+ ['fabrics', 'fabric'],
+];
+PVE::AccessControl::remove_sdn_resource_access($paths_to_delete);
+
+$rpcenv->{user_cfg} = $mocked_user_cfg_tree;
+
+# Check ACLs after pruning
+check_roles('user3@pve', '/sdn/zones/zone1', 'SDNAdmin');
+check_roles('user3@pve', '/sdn/zones/zone2', '');
+
+check_roles('user1@pve', '/sdn/zones/zone1/vnet1', '');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet1/10', '');
+
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2/100', 'SDNAdmin');
+
+check_roles('user2@pve', '/sdn/controllers/control', '');
+check_roles('user2@pve', '/sdn/prefix-lists/list', '');
+check_roles('user2@pve', '/sdn/route-maps/map', '');
+check_roles('user2@pve', '/sdn/fabrics/fabric', '');
+
+done_testing();
+
+exit(0);
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH pve-access-control v3 3/5] fix: #7520: sdn: add VNet ACL migration
2026-06-03 14:55 [PATCH access-control/network v3 0/5] fix #7520: sdn: prune orphaned ACLs and handle VNet migrations David Riley
2026-06-03 14:55 ` [PATCH pve-access-control v3 1/5] fix: #7520: sdn: prune orphaned ACLs on resource deletion David Riley
2026-06-03 14:55 ` [PATCH pve-access-control v3 2/5] fix #7520: test: add unit tests for sdn acl pruning logic David Riley
@ 2026-06-03 14:55 ` David Riley
2026-06-03 14:55 ` [PATCH pve-access-control v3 4/5] fix #7520: test: add unit tests for sdn acl migration logic David Riley
2026-06-03 14:55 ` [PATCH pve-network v3 5/5] fix #7520: config: prune orphaned ACLs and relocate moved VNets David Riley
4 siblings, 0 replies; 6+ messages in thread
From: David Riley @ 2026-06-03 14:55 UTC (permalink / raw)
To: pve-devel; +Cc: David Riley
Moving VNets between zones can lead to stale or orphaned ACL entries.
Introduce a migration routine to safely relocate ACLs during VNet
moves to maintain configuration integrity.
* Conflict Validation: Abort the migration and the SDN apply
operation if the destination path has existing permissions. This
prevents accidental privilege escalation or overwrites.
* Relocation: Move ACLs to the new zone path.
* Error Handling: Report both the source and destination paths on
failure to guide manual resolution.
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7520
Signed-off-by: David Riley <d.riley@proxmox.com>
---
src/PVE/AccessControl.pm | 86 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 7581394..b7bb3eb 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -2020,6 +2020,92 @@ sub lookup_acl_node {
return ($parent, $last_key, $current);
}
+sub migrate_sdn_resource_access {
+ my ($migrations) = @_; # [ { src_path => [...], dest_path => [...] } ]
+ return if !$migrations || !@$migrations;
+
+ my $migrate_fn = sub {
+ my $usercfg = cfs_read_file("user.cfg");
+ my $modified = 0;
+
+ my @prepared_moves;
+ foreach my $migration (@$migrations) {
+ my $src_path = ['sdn', @{ $migration->{src_path} }];
+ my $dest_path = ['sdn', @{ $migration->{dest_path} }];
+
+ my ($src_parent, $src_last_key, $acl_node) =
+ lookup_acl_node($usercfg->{acl_root}, $src_path);
+
+ if ($src_parent && $src_last_key && $acl_node) {
+ # probe destination for conflicts
+ my (undef, undef, $existing_dest_node) =
+ lookup_acl_node($usercfg->{acl_root}, $dest_path);
+
+ if (acl_tree_has_permissions($existing_dest_node)) {
+ my $conflict_path = '/' . join('/', @$dest_path);
+ my $source_path = '/' . join('/', @$src_path);
+ die
+ "Destination '$conflict_path' already has permissions configured (Source:"
+ . " '$source_path'). Please remove the target ACLs manually before "
+ . "retrying.\n";
+ }
+ # stage move
+ push @prepared_moves,
+ {
+ src_parent => $src_parent,
+ src_last_key => $src_last_key,
+ dest_path => $dest_path,
+ node => $acl_node,
+ };
+ }
+ }
+
+ foreach my $move (@prepared_moves) {
+ delete $move->{src_parent}->{children}->{ $move->{src_last_key} };
+
+ my $current = $usercfg->{acl_root};
+ my @dest_segments = @{ $move->{dest_path} };
+ my $dest_last_key = pop @dest_segments;
+
+ foreach my $segment (@dest_segments) {
+ $current = ($current->{children}->{$segment} //= {});
+ }
+
+ $current->{children} //= {};
+ $current->{children}->{$dest_last_key} = $move->{node};
+
+ $modified = 1;
+ }
+
+ if ($modified) {
+ cfs_write_file("user.cfg", $usercfg);
+ }
+ };
+
+ lock_user_config($migrate_fn, "ACL migration failed");
+}
+
+# Recursively checks a node and all its children for active permissions
+sub acl_tree_has_permissions {
+ my ($node) = @_;
+
+ return 0 unless ref($node) eq 'HASH';
+
+ foreach my $key (keys %$node) {
+ return 1 if $key ne 'children';
+ }
+
+ if (ref($node->{children}) eq 'HASH') {
+ foreach my $child_key (keys %{ $node->{children} }) {
+ if (acl_tree_has_permissions($node->{children}->{$child_key})) {
+ return 1;
+ }
+ }
+ }
+
+ return 0;
+}
+
my $USER_CONTROLLED_TFA_TYPES = {
u2f => 1,
oath => 1,
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH pve-access-control v3 4/5] fix #7520: test: add unit tests for sdn acl migration logic
2026-06-03 14:55 [PATCH access-control/network v3 0/5] fix #7520: sdn: prune orphaned ACLs and handle VNet migrations David Riley
` (2 preceding siblings ...)
2026-06-03 14:55 ` [PATCH pve-access-control v3 3/5] fix: #7520: sdn: add VNet ACL migration David Riley
@ 2026-06-03 14:55 ` David Riley
2026-06-03 14:55 ` [PATCH pve-network v3 5/5] fix #7520: config: prune orphaned ACLs and relocate moved VNets David Riley
4 siblings, 0 replies; 6+ messages in thread
From: David Riley @ 2026-06-03 14:55 UTC (permalink / raw)
To: pve-devel; +Cc: David Riley
Add unit tests to validate the behavior of the SDN VNet access
migration logic.
The tests cover multiple migration edge cases:
* VNet migration between zones
* Subtree retention, moving multiple VLAN tags under a VNet
* Unconfigured resources, ensuring empty source paths do not create
empty target nodes
* Conflict prevention, aborting on destination collisions
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7520
Signed-off-by: David Riley <d.riley@proxmox.com>
---
src/test/Makefile | 2 +
src/test/sdn_acl_migration.cfg | 13 ++++
src/test/sdn_acl_migration_test.pl | 118 +++++++++++++++++++++++++++++
3 files changed, 133 insertions(+)
create mode 100644 src/test/sdn_acl_migration.cfg
create mode 100644 src/test/sdn_acl_migration_test.pl
diff --git a/src/test/Makefile b/src/test/Makefile
index 2f44186..7d4267f 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -15,3 +15,5 @@ check:
perl -I.. realm_sync_test.pl
perl -I.. api-tests.pl
perl -I.. sdn_acl_pruning_test.pl
+ perl -I.. sdn_acl_migration_test.pl
+
diff --git a/src/test/sdn_acl_migration.cfg b/src/test/sdn_acl_migration.cfg
new file mode 100644
index 0000000..58a1b17
--- /dev/null
+++ b/src/test/sdn_acl_migration.cfg
@@ -0,0 +1,13 @@
+user:user1@pve:1:0::::::
+role:SDNAdmin:SDN.Allocate,SDN.Audit
+
+acl:1:/sdn/zones/zone1/vnet1:user1@pve:SDNAdmin:
+acl:1:/sdn/zones/zone1/vnet2:user1@pve:SDNAdmin:
+acl:1:/sdn/zones/zone1/vnet2/100:user1@pve:SDNAdmin:
+acl:1:/sdn/zones/zone1/vnet2/200:user1@pve:SDNAdmin:
+
+acl:1:/sdn/zones/zone1/vnet3:user1@pve:SDNAdmin:
+acl:1:/sdn/zones/zone2/vnet3:user1@pve:SDNAdmin:
+
+acl:1:/sdn/zones/zone1/vnet5:user1@pve:SDNAdmin:
+acl:1:/sdn/zones/zone2/vnet5/300:user1@pve:SDNAdmin:
diff --git a/src/test/sdn_acl_migration_test.pl b/src/test/sdn_acl_migration_test.pl
new file mode 100644
index 0000000..043c012
--- /dev/null
+++ b/src/test/sdn_acl_migration_test.pl
@@ -0,0 +1,118 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use lib qw(..);
+
+use PVE::Tools;
+use Test::More;
+use Test::MockModule;
+
+use PVE::AccessControl;
+use PVE::RPCEnvironment;
+
+my $cluster_module = Test::MockModule->new('PVE::Cluster');
+$cluster_module->noop('cfs_update');
+
+my $cfgfn = "sdn_acl_migration.cfg";
+my $raw_cfg = PVE::Tools::file_get_contents($cfgfn);
+my $mocked_user_cfg_tree = PVE::AccessControl::parse_user_config("user.cfg", $raw_cfg);
+
+my $access_control_module = Test::MockModule->new('PVE::AccessControl');
+
+# Mocking
+$access_control_module->mock(
+ 'cfs_read_file',
+ sub {
+ my ($filename) = @_;
+ return $mocked_user_cfg_tree if $filename eq 'user.cfg';
+ die "mock cfs_read_file: unexpected file $filename";
+ },
+);
+
+$access_control_module->mock(
+ 'cfs_write_file',
+ sub {
+ my ($filename, $cfg) = @_;
+ $mocked_user_cfg_tree = $cfg if $filename eq 'user.cfg';
+ },
+);
+
+$access_control_module->mock(
+ 'lock_user_config',
+ sub {
+ my ($code, $errmsg) = @_;
+ eval { $code->() };
+ if (my $err = $@) {
+ die "$errmsg: $err\n";
+ }
+ },
+);
+
+my $rpcenv = PVE::RPCEnvironment->init('cli');
+$rpcenv->{user_cfg} = $mocked_user_cfg_tree;
+
+sub check_roles {
+ my ($user, $path, $expected_result) = @_;
+
+ my $roles = PVE::AccessControl::roles($rpcenv->{user_cfg}, $user, $path);
+ my $res = join(',', sort keys %$roles);
+
+ is($res, $expected_result, "Roles for $user on $path should be '$expected_result'");
+}
+
+# Tests
+# Check ACLs pre migration
+check_roles('user1@pve', '/sdn/zones/zone1/vnet1', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2/100', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2/200', 'SDNAdmin');
+
+# Migration
+my $migrations = [
+ { src_path => ['zones', 'zone1', 'vnet1'], dest_path => ['zones', 'zone2', 'vnet1'] },
+ { src_path => ['zones', 'zone1', 'vnet2'], dest_path => ['zones', 'zone2', 'vnet2'] },
+ { src_path => ['zones', 'zone1', 'vnet4'], dest_path => ['zones', 'zone2', 'vnet4'] }, # Ghost move (no ACLs exist)
+];
+PVE::AccessControl::migrate_sdn_resource_access($migrations);
+
+$rpcenv->{user_cfg} = $mocked_user_cfg_tree;
+
+# Check ACLs after migration
+# source paths
+check_roles('user1@pve', '/sdn/zones/zone1/vnet1', '');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2', '');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2/100', '');
+check_roles('user1@pve', '/sdn/zones/zone1/vnet2/200', '');
+
+# destionation paths
+check_roles('user1@pve', '/sdn/zones/zone2/vnet1', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone2/vnet2', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone2/vnet2/100', 'SDNAdmin');
+check_roles('user1@pve', '/sdn/zones/zone2/vnet2/200', 'SDNAdmin');
+
+# fictional move
+check_roles('user1@pve', '/sdn/zones/zone1/vnet4', '');
+check_roles('user1@pve', '/sdn/zones/zone2/vnet4', '');
+
+# conflict without vlan tag
+eval {
+ PVE::AccessControl::migrate_sdn_resource_access([
+ { src_path => ['zones', 'zone1', 'vnet3'], dest_path => ['zones', 'zone2', 'vnet3'] }
+ ]);
+};
+ok($@, "Migration must abort if direct target destination node already exists");
+like($@, qr/already has permissions configured/, "Error message matches custom conflict string");
+
+# conflict with vlan tag
+eval {
+ PVE::AccessControl::migrate_sdn_resource_access([
+ { src_path => ['zones', 'zone1', 'vnet5'], dest_path => ['zones', 'zone2', 'vnet5'] }
+ ]);
+};
+ok($@, "Migration must abort if a child of the target destination node already exists");
+like($@, qr/already has permissions configured/, "Error message matches custom conflict string");
+
+done_testing();
+
+exit(0);
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH pve-network v3 5/5] fix #7520: config: prune orphaned ACLs and relocate moved VNets
2026-06-03 14:55 [PATCH access-control/network v3 0/5] fix #7520: sdn: prune orphaned ACLs and handle VNet migrations David Riley
` (3 preceding siblings ...)
2026-06-03 14:55 ` [PATCH pve-access-control v3 4/5] fix #7520: test: add unit tests for sdn acl migration logic David Riley
@ 2026-06-03 14:55 ` David Riley
4 siblings, 0 replies; 6+ messages in thread
From: David Riley @ 2026-06-03 14:55 UTC (permalink / raw)
To: pve-devel; +Cc: David Riley
Compare the running configuration with the newly compiled state during
config commit to identify and prune orphaned SDN ACL entries. This
ensures state consistency for manual applies via the UI/API as well as
during automatic configuration reloads on system boot.
Track configuration changes across:
* Zones
* VNets
* Fabrics
* Controllers
* Route maps
* Prefix lists
Trigger a validation check when a VNet is moved between zones to
mitigate potential ACL path conflicts. Abort the apply operation if a
conflict is detected to prevent configuration overwrites. If the path
is clear, relocate the ACLs to the updated zone path.
Suggested-by: Stefan Hanreich <s.hanreich@proxmox.com>
Signed-off-by: David Riley <d.riley@proxmox.com>
---
src/PVE/Network/SDN.pm | 123 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 123 insertions(+)
diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 33a3cf3..6a49621 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -12,6 +12,7 @@ use UUID;
use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
use PVE::INotify;
+use PVE::AccessControl;
use PVE::RESTEnvironment qw(log_warn);
use PVE::RPCEnvironment;
use PVE::Tools qw(file_get_contents file_set_contents extract_param dir_glob_regex run_command);
@@ -238,11 +239,133 @@ sub compile_running_cfg {
}
sub commit_config {
+ my $old_cfg = cfs_read_file($RUNNING_CFG_FILENAME);
my $cfg = compile_running_cfg();
+ cleanup($old_cfg, $cfg);
+
cfs_write_file($RUNNING_CFG_FILENAME, $cfg);
}
+sub cleanup {
+ my ($old_cfg, $cfg) = @_;
+ return if !$old_cfg || !$cfg;
+
+ my $route_map_paths = diff_route_maps($old_cfg, $cfg);
+ my $generic_paths = diff_generic_resources($old_cfg, $cfg);
+ my ($vnet_delete_paths, $vnet_move_paths) = diff_vnets($old_cfg, $cfg);
+
+ if (@$vnet_move_paths) {
+ PVE::AccessControl::migrate_sdn_resource_access($vnet_move_paths);
+ }
+
+ my @paths_to_delete = (@$vnet_delete_paths, @$route_map_paths, @$generic_paths);
+ if (@paths_to_delete) {
+ PVE::AccessControl::remove_sdn_resource_access(\@paths_to_delete);
+ }
+
+}
+
+sub diff_generic_resources {
+ my ($old_cfg, $cfg) = @_;
+
+ my @paths_to_delete;
+
+ my @types = qw(zones controllers fabrics prefix-lists);
+ foreach my $type (@types) {
+ next if !$old_cfg->{$type} || !$old_cfg->{$type}->{ids};
+
+ my $old_ids = $old_cfg->{$type}->{ids};
+ my $new_ids = {};
+ if (my $type_block = $cfg->{$type}) {
+ $new_ids = $type_block->{ids} // {};
+ }
+
+ foreach my $id (keys %$old_ids) {
+ next if $new_ids->{$id};
+ push @paths_to_delete, [$type, $id];
+ }
+ }
+
+ return \@paths_to_delete;
+}
+
+sub diff_route_maps {
+ my ($old_cfg, $cfg) = @_;
+
+ my @paths_to_delete;
+
+ if ($old_cfg->{'route-maps'} && $old_cfg->{'route-maps'}->{ids}) {
+ my $old_route_maps = $old_cfg->{'route-maps'}->{ids};
+ my $route_map_suffix = qr/_\d+$/;
+
+ my %active_route_maps;
+ if ($cfg->{'route-maps'} && $cfg->{'route-maps'}->{ids}) {
+ foreach my $id (keys %{ $cfg->{'route-maps'}->{ids} }) {
+ (my $base_name = $id) =~ s/$route_map_suffix//;
+ $active_route_maps{$base_name} = 1;
+ }
+ }
+
+ my %queued_route_maps;
+ foreach my $id (keys %$old_route_maps) {
+ next if $cfg->{'route-maps'}->{ids}->{$id};
+
+ (my $base_name = $id) =~ s/$route_map_suffix//;
+ next if $active_route_maps{$base_name};
+ next if $queued_route_maps{$base_name};
+
+ push @paths_to_delete, ['route-maps', $base_name];
+ $queued_route_maps{$base_name} = 1;
+ }
+ }
+
+ return \@paths_to_delete;
+}
+
+sub diff_vnets {
+ my ($old_cfg, $cfg) = @_;
+
+ my @paths_to_delete;
+ my @paths_to_move;
+
+ if ($old_cfg->{vnets} && $old_cfg->{vnets}->{ids}) {
+ my $old_vnets = $old_cfg->{vnets}->{ids};
+ my $new_vnets = {};
+
+ if (my $vnet_cfg = $cfg->{vnets}) {
+ $new_vnets = $vnet_cfg->{ids} // {};
+ }
+
+ foreach my $vnetid (keys %$old_vnets) {
+ my $old_zone = $old_vnets->{$vnetid}->{zone};
+ my $new_zone;
+ $new_zone = $new_vnets->{$vnetid}->{zone} if $new_vnets->{$vnetid};
+
+ if (!$new_vnets->{$vnetid}) {
+ if ($old_zone) {
+ push @paths_to_delete, ['zones', $old_zone, $vnetid];
+ } else {
+ log_warn(
+ "SDN Cleanup: Could not find zone for deleted VNet $vnetid, skipping ACL"
+ . " cleanup");
+ }
+ next;
+ }
+
+ if ($old_zone && $new_zone && $old_zone ne $new_zone) {
+ push @paths_to_move,
+ {
+ src_path => ['zones', $old_zone, $vnetid],
+ dest_path => ['zones', $new_zone, $vnetid],
+ };
+ }
+ }
+ }
+
+ return (\@paths_to_delete, \@paths_to_move);
+}
+
sub has_pending_changes {
my $running_cfg = PVE::Network::SDN::running_config();
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread