[PATCH access-control/network v3 0/5] fix #7520: sdn: prune orphaned ACLs and handle VNet migrations

[PATCH access-control/network v3 0/5] fix #7520: sdn: prune orphaned ACLs and handle VNet migrations

[David Riley]

Implement a pruning mechanism to clean up orphaned SDN ACL entries by
comparing the running configuration with the newly compiled state
during configuration commit.

This ensures state consistency for manual applies via the UI/API
as well as during the automatic configuration reload on system boot.
The pruning covers: 
* Zones
* VNets
* Fabrics
* Controllers
* Route maps
* Prefix lists 

IPAMs and DNS are excluded as they are not staged.

Difference from v2:
* Relocate VNet ACLs to the new zone path when a VNet is moved,
  including a validation check to abort on path conflicts.
* Refactor diff generation in preparation for an upcoming patch 
  series, resolving #7294 [0]. The upcoming series will hook into the 
  VNet diff to clean up pool members.
* Add unit testing for pruning and migration mechanism 

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7520

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=7294


pve-access-control:

David Riley (4):
  fix: #7520: sdn: prune orphaned ACLs on resource deletion
  fix #7520: test: add unit tests for sdn acl pruning logic
  fix: #7520: sdn: add VNet ACL migration
  fix #7520: test: add unit tests for sdn acl migration logic

 src/PVE/AccessControl.pm           | 132 +++++++++++++++++++++++++++++
 src/test/Makefile                  |   3 +
 src/test/sdn_acl_migration.cfg     |  13 +++
 src/test/sdn_acl_migration_test.pl | 118 ++++++++++++++++++++++++++
 src/test/sdn_acl_pruning.cfg       |  22 +++++
 src/test/sdn_acl_pruning_test.pl   | 116 +++++++++++++++++++++++++
 6 files changed, 404 insertions(+)
 create mode 100644 src/test/sdn_acl_migration.cfg
 create mode 100644 src/test/sdn_acl_migration_test.pl
 create mode 100644 src/test/sdn_acl_pruning.cfg
 create mode 100644 src/test/sdn_acl_pruning_test.pl


pve-network:

David Riley (1):
  fix #7520: config: prune orphaned ACLs and relocate moved VNets

 src/PVE/Network/SDN.pm | 123 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)


Summary over all repositories:
  7 files changed, 527 insertions(+), 0 deletions(-)

-- 
Generated by murpp 0.11.0

[PATCH pve-access-control v3 1/5] fix: #7520: sdn: prune orphaned ACLs on resource deletion

[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

[PATCH pve-access-control v3 2/5] fix #7520: test: add unit tests for sdn acl pruning logic

[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

[PATCH pve-access-control v3 3/5] fix: #7520: sdn: add VNet ACL migration

[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

[PATCH pve-access-control v3 4/5] fix #7520: test: add unit tests for sdn acl migration logic

[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

[PATCH pve-network v3 5/5] fix #7520: config: prune orphaned ACLs and relocate moved VNets

[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