From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Date: Mon, 18 Nov 2024 15:34:37 +0000 (+0100)
Subject: ipam: move state file of PVE plugin over to common sdn directory
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=0f48bc6561f2fd901f2665387b4954c8105614e0;p=mirror%2Fpve-network.git

ipam: move state file of PVE plugin over to common sdn directory

This does not contain data that needs to be protected to avoid
hijacking (external) systems, like our credentials for certain storage
types or encryption keys, so placing it in the strictly root-only
'priv/' folder was always a bit overkill.

Now we want to make the firewall more SDN aware and thus need also to
parse the SDN config there. This means having to also read the IPAM
statefile here, and as of now we would need to move over quite a few
API endpoints to be proxied to the privileged pvedaemon running as
root, as otherwise they would fail to read the full SDN config & state
required.

That is not a big problem, but it's also not really nice, we got the
privilege separation for a reason after all. Thus, move the backing
file for the PVE IPAM plugin state over to the general /etc/pve/sdn
path, where www-data (and thus pveproxy) can read it, but still not
write it. Fallback to the old location for backward compatibility.
This way the file will be automatically written to the new place on
the first change. This is not fool-proof, but there's only so much we
can do here to support a sane upgrade path, so fall back to a base
requirement of all cluster nodes using the same package versions.

FWIW, Stefan Hanreich tested a very similar diff I sent to him
off-list, but it was not close enough to add a T-b now.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index f274b02..efc1841 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -14,12 +14,36 @@ use Digest::SHA;
 use base('PVE::Network::SDN::Ipams::Plugin');
 
 
-my $ipamdb_file = "priv/ipam.db";
+my $ipamdb_file = "sdn/pve-ipam-state.json";
+my $ipamdb_file_legacy = "priv/ipam.db";
 
 PVE::Cluster::cfs_register_file(
     $ipamdb_file,
+    sub {
+	my ($filename, $data) = @_;
+	if (defined($data)) {
+	    return PVE::Network::SDN::Ipams::PVEPlugin->parse_config($filename, $data);
+	} else {
+	    # TODO: remove legacy state file handling with PVE 9+ after ensuring all call sites got
+	    # switched over.
+	    return cfs_read_file($ipamdb_file_legacy);
+	}
+    },
+    sub {
+	my ($filename, $data) = @_;
+	# TODO: remove below with PVE 9+, add a pve8to9 check to allow doing so.
+	if (-e $ipamdb_file_legacy && $ipamdb_file) {
+	    # only clean-up if we succeeded to write the new path at least once
+	    unlink $ipamdb_file_legacy or $!{ENOENT} or warn "failed to unlink legacy IPAM DB - $!\n";
+	}
+	return PVE::Network::SDN::Ipams::PVEPlugin->write_config($filename, $data);
+    },
+);
+
+PVE::Cluster::cfs_register_file(
+    $ipamdb_file_legacy,
     sub { PVE::Network::SDN::Ipams::PVEPlugin->parse_config(@_); },
-    sub { PVE::Network::SDN::Ipams::PVEPlugin->write_config(@_); },
+    undef, # no writer for legacy file, all must go to the new file.
 );
 
 sub type {