From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 9AAD71FF195
for <inbox@lore.proxmox.com>; Fri, 7 Mar 2025 18:44:29 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 6FF7D30BA8;
Fri, 7 Mar 2025 18:43:59 +0100 (CET)
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Fri, 7 Mar 2025 18:43:52 +0100
Message-Id: <20250307174352.337597-8-s.hanreich@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250307174352.337597-1-s.hanreich@proxmox.com>
References: <20250307174352.337597-1-s.hanreich@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.223 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery
methods
RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_NONE 0.001 SPF: sender does not publish an SPF Record
Subject: [pve-devel] [PATCH pve-network 8/8] partial fix #5496: subnet:
ipam: add update_subnet hook
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Because of how the Netbox IPAM plugin works (utilizing IP ranges to
represent DHCP ranges), we need a hook in the IPAM plugin that runs on
updates to the subnet because DHCP ranges can be edited. The update
hook in Netbox checks which DHCP ranges got added and which got
deleted and then performs the respective changes in the Netbox IPAM.
This operates under the assumption that DHCP ranges do not overlap
(which is not supported by Netbox anyway).
Only Netbox needs to do work on update, so we can leave this as noop
in phpIPAM and the PVE IPAM, because they have no notion of IP ranges
or similar entities. phpIPAM doesn't support DHCP ranges at all and
PVE IPAM simply uses DHCP ranges as a constraint when allocating an
IP.
I decided on this approach over just creating IP ranges on demand when
assigning IPs, because this keeps Netbox clean and in sync with the
PVE state. It doesn't leave remnants of IP ranges in the Netbox
database, which can lead to errors when trying to create IP ranges
that overlap with IP ranges that already existed in an SDN subnet.
This method tries to check for any possible errors before editing the
entities. There is still a small window where external changes can
occur that lead to errors. We are touching multiple entities here, so
in case of errors users have to fix their Netbox instance manually.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 54 ++++++++++++++++++++++
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 5 ++
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 5 ++
src/PVE/Network/SDN/Ipams/Plugin.pm | 6 +++
src/PVE/Network/SDN/SubnetPlugin.pm | 6 ++-
src/PVE/Network/SDN/Subnets.pm | 12 +++++
6 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index b696dd4..4984e5a 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -96,6 +96,60 @@ sub add_subnet {
}
}
+sub update_subnet {
+ my ($class, $plugin_config, $subnetid, $subnet, $old_subnet, $noerr) = @_;
+
+ # old subnet in SubnetPlugin hook has already parsed dhcp-ranges
+ # new subnet doesn't
+ my $old_dhcp_ranges = $old_subnet->{'dhcp-range'};
+ my $new_dhcp_ranges = PVE::Network::SDN::Subnets::get_dhcp_ranges($subnet);
+
+ my $hash_range = sub {
+ my ($dhcp_range) = @_;
+ "$dhcp_range->{'start-address'} - $dhcp_range->{'end-address'}"
+ };
+
+ my $old_lookup = {};
+ for my $dhcp_range (@$old_dhcp_ranges) {
+ my $hash = $hash_range->($dhcp_range);
+ $old_lookup->{$hash} = undef;
+ }
+
+ my $new_lookup = {};
+ for my $dhcp_range (@$new_dhcp_ranges) {
+ my $hash = $hash_range->($dhcp_range);
+ $new_lookup->{$hash} = undef;
+ }
+
+ my $to_delete_ids = ();
+
+ # delete first so we don't get errors with overlapping ranges
+ for my $dhcp_range (@$old_dhcp_ranges) {
+ my $hash = $hash_range->($dhcp_range);
+
+ if (exists($new_lookup->{$hash})) {
+ next;
+ }
+
+ my $internalid = get_iprange_id($plugin_config, $dhcp_range, $noerr);
+
+ # definedness check, because ID could be 0
+ if (!defined($internalid)) {
+ warn "could not find id for ip range $dhcp_range->{'start-address'}:$dhcp_range->{'end-address'}";
+ next;
+ }
+
+ del_dhcp_range($plugin_config, $internalid, $noerr);
+ }
+
+ for my $dhcp_range (@$new_dhcp_ranges) {
+ my $hash = $hash_range->($dhcp_range);
+
+ add_dhcp_range($plugin_config, $dhcp_range, $noerr)
+ if !exists($old_lookup->{$hash});
+ }
+}
+
sub del_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 742f1b1..59ad4ea 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -82,6 +82,11 @@ sub add_subnet {
die "$@" if $@;
}
+sub update_subnet {
+ my ($class, $plugin_config, $subnetid, $subnet, $old_subnet, $noerr) = @_;
+ # we don't need to do anything on update
+}
+
sub only_gateway_remains {
my ($ips) = @_;
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index df5048d..8ee430a 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -67,6 +67,11 @@ sub add_subnet {
}
}
+sub update_subnet {
+ my ($class, $plugin_config, $subnetid, $subnet, $old_subnet, $noerr) = @_;
+ # we don't need to do anything on update
+}
+
sub del_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index ab4cae8..6190c24 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -75,6 +75,12 @@ sub add_subnet {
die "please implement inside plugin";
}
+sub update_subnet {
+ my ($class, $plugin_config, $subnetid, $subnet, $old_subnet, $noerr) = @_;
+
+ die "please implement inside plugin";
+}
+
sub del_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
diff --git a/src/PVE/Network/SDN/SubnetPlugin.pm b/src/PVE/Network/SDN/SubnetPlugin.pm
index b911d69..8a79eae 100644
--- a/src/PVE/Network/SDN/SubnetPlugin.pm
+++ b/src/PVE/Network/SDN/SubnetPlugin.pm
@@ -201,7 +201,11 @@ sub on_update_hook {
validate_dhcp_ranges($subnet);
if ($ipam) {
- PVE::Network::SDN::Subnets::add_subnet($zone, $subnetid, $subnet);
+ if ($old_subnet) {
+ PVE::Network::SDN::Subnets::update_subnet($zone, $subnetid, $subnet, $old_subnet);
+ } else {
+ PVE::Network::SDN::Subnets::add_subnet($zone, $subnetid, $subnet);
+ }
#don't register gateway for pointopoint
return if $pointopoint;
diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index e2c8c9c..18847c2 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -194,6 +194,18 @@ sub add_subnet {
$plugin->add_subnet($plugin_config, $subnetid, $subnet);
}
+sub update_subnet {
+ my ($zone, $subnetid, $subnet, $old_subnet) = @_;
+
+ my $ipam = $zone->{ipam};
+ return if !$ipam;
+
+ my $ipam_cfg = PVE::Network::SDN::Ipams::config();
+ my $plugin_config = $ipam_cfg->{ids}->{$ipam};
+ my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
+ $plugin->update_subnet($plugin_config, $subnetid, $subnet, $old_subnet);
+}
+
sub del_subnet {
my ($zone, $subnetid, $subnet) = @_;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel