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

[Stefan Hanreich <s.hanreich@proxmox.com>]

Thanks for tackling this!

Tested the following things:
* Removing Zone / VNet / Fabric / Controller / Route Map / Prefix List
* Moving a VNet to another zone
* collision detection

Took a look at the code as well, some nits but no real blockers from my POV.

Consider this:

Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>

On 7/7/26 2:04 PM, David Riley wrote:
> Thanks @Daniel K. for the thorough review.
> 
> Difference from v4:
> * Instead of passing arrays of path segments [<zone>, <vnet>], paths
>   are now passed as strings <zone>/<vnet> to match the actual ACL
>   paths as there was no real benefit of using arrays.
> * Moved helper functions into seperate commits and updated the commit
>   messages accordingly.
> * Fix: style nits (post if, for loop dereferencing, ...)
> * Fix: tests nits (grouped mocking, removed main subroutine)
> 
> Difference from v3:
> * Refactored tests
> * Perl style guide refactor (for instead of foreach)
> * Update: route_map_suffix regex used for detecting digits to match
>   style guide recommendation
> 
> 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 
> 
> === Original Cover Letter ===
> 
> 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.
> 
> 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 (6):
>   permission: add acl tree node parent lookup without node creation
>   fix #7520: sdn: add pruning SDN resources ACLs
>   test: add sdn acl pruning tests
>   permission: add helper to check for active permissions
>   fix #7520: sdn: add VNet ACL migration
>   test: add sdn acl migration tests
> 
>  src/PVE/AccessControl.pm           | 155 ++++++++++++++++++++++++++
>  src/test/Makefile                  |   3 +
>  src/test/sdn_acl_migration_test.pl | 171 +++++++++++++++++++++++++++++
>  src/test/sdn_acl_pruning_test.pl   | 134 ++++++++++++++++++++++
>  4 files changed, 463 insertions(+)
>  create mode 100644 src/test/sdn_acl_migration_test.pl
>  create mode 100644 src/test/sdn_acl_pruning_test.pl
> 
> 
> pve-network:
> 
> David Riley (1):
>   fix #7520: config: prune orphaned and relocate moved VNet ACLs
> 
>  src/PVE/Network/SDN.pm | 115 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 115 insertions(+)
> 
> 
> Summary over all repositories:
>   5 files changed, 578 insertions(+), 0 deletions(-)
>