all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: David Riley <d.riley@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-network v4 5/5] fix #7520: config: prune orphaned ACLs and relocate moved VNets
Date: Fri, 26 Jun 2026 12:52:58 +0200	[thread overview]
Message-ID: <20260626105258.56914-6-d.riley@proxmox.com> (raw)
In-Reply-To: <20260626105258.56914-1-d.riley@proxmox.com>

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 | 141 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 33a3cf3..cd563d2 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -10,6 +10,7 @@ use LWP::UserAgent;
 use Net::SSLeay;
 use UUID;
 
+use PVE::AccessControl;
 use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
 use PVE::INotify;
 use PVE::RESTEnvironment qw(log_warn);
@@ -238,11 +239,151 @@ 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) = @_;
+    if (!defined($old_cfg) || !defined($cfg)) {
+        return;
+    }
+
+    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);
+    for my $type (@types) {
+        if (!defined($old_cfg->{$type}) || !defined($old_cfg->{$type}->{ids})) {
+            next;
+        }
+
+        my $old_ids = $old_cfg->{$type}->{ids};
+        my $new_ids = {};
+
+        if (defined($cfg->{$type}) && defined($cfg->{$type}->{ids})) {
+            $new_ids = $cfg->{$type}->{ids};
+        }
+
+        for my $id (keys %$old_ids) {
+            if (defined($new_ids->{$id})) {
+                next;
+            }
+
+            push(@paths_to_delete, [$type, $id]);
+        }
+    }
+
+    return \@paths_to_delete;
+}
+
+sub diff_route_maps {
+    my ($old_cfg, $cfg) = @_;
+
+    my @paths_to_delete;
+
+    if (defined($old_cfg->{'route-maps'}) && defined($old_cfg->{'route-maps'}->{ids})) {
+        my $old_route_maps = $old_cfg->{'route-maps'}->{ids};
+        my $route_map_suffix = qr/_[0-9]+$/;
+
+        my %active_route_maps;
+        if (defined($cfg->{'route-maps'}) && defined($cfg->{'route-maps'}->{ids})) {
+            for 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;
+        for my $id (keys %$old_route_maps) {
+            if (defined($cfg->{'route-maps'}->{ids}->{$id})) {
+                next;
+            }
+
+            (my $base_name = $id) =~ s/$route_map_suffix//;
+
+            if ($active_route_maps{$base_name}) {
+                next;
+            }
+
+            if ($queued_route_maps{$base_name}) {
+                next;
+            }
+
+            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 (defined($old_cfg->{vnets}) && defined($old_cfg->{vnets}->{ids})) {
+        my $old_vnets = $old_cfg->{vnets}->{ids};
+        my $new_vnets = {};
+
+        if (defined($cfg->{vnets}) && defined($cfg->{vnets}->{ids})) {
+            $new_vnets = $cfg->{vnets}->{ids};
+        }
+
+        for my $vnetid (keys %$old_vnets) {
+            my $old_zone = $old_vnets->{$vnetid}->{zone};
+
+            if (!defined($new_vnets->{$vnetid})) {
+                if (defined($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;
+            }
+
+            my $new_zone = $new_vnets->{$vnetid}->{zone};
+
+            if (defined($old_zone) && defined($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





      parent reply	other threads:[~2026-06-26 10:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26 10:52 [PATCH access-control/network v4 0/5] fix #7520: sdn: prune orphaned ACLs and handle VNet migrations David Riley
2026-06-26 10:52 ` [PATCH pve-access-control v4 1/5] fix: #7520: sdn: prune orphaned ACLs on resource deletion David Riley
2026-06-26 10:52 ` [PATCH pve-access-control v4 2/5] fix #7520: test: add unit tests for sdn acl pruning logic David Riley
2026-06-26 10:52 ` [PATCH pve-access-control v4 3/5] fix: #7520: sdn: add VNet ACL migration David Riley
2026-06-26 10:52 ` [PATCH pve-access-control v4 4/5] fix #7520: test: add unit tests for sdn acl migration logic David Riley
2026-06-26 10:52 ` David Riley [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260626105258.56914-6-d.riley@proxmox.com \
    --to=d.riley@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal