* [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration
@ 2025-07-24 14:17 Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration Gabriel Goller
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-07-24 14:17 UTC (permalink / raw)
To: pve-devel
## Introduction
This patch series lays the groundwork for the Proxmox Datacenter Manager
SDN/EVPN integration on the Proxmox VE side by introducing global locking for
the SDN configuration. It is intended to be used by the PDM implementation to
prevent concurrent changes to the SDN configuration, while the datacenter
manager is making configuration changes.
## How it works
This patch series adds three new API calls:
* POST /cluster/sdn/lock
* DELETE /cluster/sdn/lock
* POST /cluster/sdn/rollback
The SDN configuration can be locked by invoking the lock endpoint, which returns
a lock-secret when the configuration has been locked successfully. This
lock-secret needs to be used for subsequent API calls that perform configuration
changes. For this purpose, a new parameter has been added to all SDN API
endpoints that perform configuration changes. If the lock is currently set, then
API callers have to provide the lock-secret in order for the API endpoints to
work. If there is no global lock set, then the endpoints work the same as
before.
The lock-secret is stored in a new file in the pmxcfs: `/etc/pve/sdn/.lock`.
The lock can be released automatically on applying, where I added a flag that
governs whether the global lock should automatically be released on applying the
configuration. Otherwise the lock can always be removed by the release endpoint,
which has a force flag for forcibly releasing the lock without providing the
secret.
In order to provide an escape hatch in the case of errors on the PDM side, I
added the functionality of rolling back to the current running configuration,
which has not been possible before. This endpoint throws away all pending
changes. This saves us from introducing a third layer of configuration files,
while also adding a new feature to the existing SDN stack, where one had to
tediously revert all changes one-by-one if one wanted to rollback to the running
configuration. We could consider doing this automatically in the future from
PDM, or at least expose it as opt-in behavior in the PDM settings.
For now, in case of failures, users have to manually unlock the SDN
configuration and then rollback using the following API endpoints:
pvesh delete /cluster/sdn/lock --force 1
pvesh create /cluster/sdn/rollback
If we want to introduce automatic rollback, implementing it this way saves us
from having to manually revert every single change we make. We lock the SDN
configuration only if there are no pending changes (the lock endpoint includes a
flag that governs this behavior), then proceed to make our changes. If we run
into any error we can be sure that only the changes we made to the SDN
configuration are pending, so this enables us to safely roll back the
configuration changes we made and unlock the SDN configuration.
## Changelog:
v1, thanks @Stefan and @Thomas:
* rebase to trixie
* remove lock file from pmxcfs (just use file_get_contents, file_set_contents)
* change to a domain-lock like in the ha-stack
* implement rollback and locking for the fabrics
network:
Stefan Hanreich (5):
sdn: add global lock for configuration
api: add lock-secret parameter to all api calls
api: add lock secret parameter to apply endpoint
api: add lock and release endpoints for global configuration lock
api: add rollback endpoint
src/PVE/API2/Network/SDN.pm | 169 +++++++++++++++++-
src/PVE/API2/Network/SDN/Controllers.pm | 21 ++-
src/PVE/API2/Network/SDN/Dns.pm | 21 ++-
src/PVE/API2/Network/SDN/Fabrics/Fabric.pm | 8 +
.../API2/Network/SDN/Fabrics/FabricNode.pm | 9 +
src/PVE/API2/Network/SDN/Ipams.pm | 21 ++-
src/PVE/API2/Network/SDN/Subnets.pm | 24 ++-
src/PVE/API2/Network/SDN/Vnets.pm | 21 ++-
src/PVE/API2/Network/SDN/Zones.pm | 21 ++-
src/PVE/Network/SDN.pm | 85 ++++++++-
src/PVE/Network/SDN/Fabrics.pm | 2 +
11 files changed, 380 insertions(+), 22 deletions(-)
Summary over all repositories:
11 files changed, 380 insertions(+), 22 deletions(-)
--
Generated by git-murpp 0.8.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration
2025-07-24 14:17 [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
@ 2025-07-24 14:17 ` Gabriel Goller
2025-07-29 7:27 ` Thomas Lamprecht
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 2/5] api: add lock-secret parameter to all api calls Gabriel Goller
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Gabriel Goller @ 2025-07-24 14:17 UTC (permalink / raw)
To: pve-devel
From: Stefan Hanreich <s.hanreich@proxmox.com>
Add a new cluster-wide lock for SDN that prevents any changes to the
configuration if the generated lock-secret is not provided. It works
by generating and storing a secret in sdn/.lock which gets checked by
lock_sdn_config on every invocation. If the lock file exists, then the
lock secret has to be supplied in order to make changes to the SDN
configuration.
Lock using the domain lock (`PVE::Cluster::cfs_lock_domain`) and "sdn"
string.
This is mainly a preparation for PDM, where PDM can take the lock and
prevent concurrent modifications to the SDN configuration from other
sources, even across multiple API calls.
Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/Network/SDN.pm | 53 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 49 insertions(+), 4 deletions(-)
diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 0e7d1dfd239c..efee21543387 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -13,7 +13,7 @@ use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
use PVE::INotify;
use PVE::RESTEnvironment qw(log_warn);
use PVE::RPCEnvironment;
-use PVE::Tools qw(extract_param dir_glob_regex run_command);
+use PVE::Tools qw(file_get_contents file_set_contents extract_param dir_glob_regex run_command);
use PVE::Network::SDN::Vnets;
use PVE::Network::SDN::Zones;
@@ -48,6 +48,8 @@ my $write_running_cfg = sub {
PVE::Cluster::cfs_register_file($running_cfg, $parse_running_cfg, $write_running_cfg);
+my $LOCK_SECRET_FILE = "/etc/pve/sdn/.lock";
+
# improve me : move status code inside plugins ?
sub ifquery_check {
@@ -197,14 +199,57 @@ sub commit_config {
cfs_write_file($running_cfg, $cfg);
}
+sub generate_lock_secret {
+ my $min = ord('!'); # first printable ascii
+
+ my $rand_bytes = Crypt::OpenSSL::Random::random_bytes(32);
+ die "failed to generate lock secret!\n" if !$rand_bytes;
+
+ my $str = join('', map { chr((ord($_) & 0x3F) + $min) } split('', $rand_bytes));
+ return $str;
+}
+
+sub create_global_lock {
+ my $secret = generate_lock_secret();
+ PVE::Tools::file_set_contents($LOCK_SECRET_FILE, $secret);
+ return $secret;
+}
+
+sub delete_global_lock {
+ unlink $LOCK_SECRET_FILE if -e $LOCK_SECRET_FILE;
+}
+
sub lock_sdn_config {
- my ($code, $errmsg) = @_;
+ my ($code, $errmsg, $lock_secret_user) = @_;
- cfs_lock_file($running_cfg, undef, $code);
+ my $lock_wrapper = sub {
+ my $lock_secret = undef;
+ if (-e $LOCK_SECRET_FILE) {
+ $lock_secret = PVE::Tools::file_get_contents($LOCK_SECRET_FILE);
+ }
+
+ if (
+ defined($lock_secret)
+ && (!defined($lock_secret_user) || $lock_secret ne $lock_secret_user)
+ ) {
+ die "invalid lock secret provided!";
+ }
+
+ return $code->();
+ };
- if (my $err = $@) {
+ return lock_sdn_domain($lock_wrapper, $errmsg);
+}
+
+sub lock_sdn_domain {
+ my ($code, $errmsg) = @_;
+
+ my $res = PVE::Cluster::cfs_lock_domain("sdn", undef, $code);
+ my $err = $@;
+ if ($err) {
$errmsg ? die "$errmsg: $err" : die $err;
}
+ return $res;
}
sub get_local_vnets {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pve-devel] [PATCH network v2 2/5] api: add lock-secret parameter to all api calls
2025-07-24 14:17 [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration Gabriel Goller
@ 2025-07-24 14:17 ` Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 3/5] api: add lock secret parameter to apply endpoint Gabriel Goller
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-07-24 14:17 UTC (permalink / raw)
To: pve-devel
From: Stefan Hanreich <s.hanreich@proxmox.com>
The parameter is optional, so all existing create/update/delete
invocations can work as before, only failing if the global lock is
currently set. This ensures backwards-compatibility with the existing
calls to the API in the frontend. If the lock is set, users will get
an error message when trying to modify the configuration from the web
UI.
Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/API2/Network/SDN/Controllers.pm | 21 ++++++++++++++--
src/PVE/API2/Network/SDN/Dns.pm | 21 ++++++++++++++--
src/PVE/API2/Network/SDN/Fabrics/Fabric.pm | 8 +++++++
.../API2/Network/SDN/Fabrics/FabricNode.pm | 9 +++++++
src/PVE/API2/Network/SDN/Ipams.pm | 21 ++++++++++++++--
src/PVE/API2/Network/SDN/Subnets.pm | 24 +++++++++++++++----
src/PVE/API2/Network/SDN/Vnets.pm | 21 ++++++++++++++--
src/PVE/API2/Network/SDN/Zones.pm | 21 ++++++++++++++--
src/PVE/Network/SDN.pm | 9 +++++++
src/PVE/Network/SDN/Fabrics.pm | 2 ++
10 files changed, 143 insertions(+), 14 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Controllers.pm b/src/PVE/API2/Network/SDN/Controllers.pm
index e6eb4cb39248..675e79661bd8 100644
--- a/src/PVE/API2/Network/SDN/Controllers.pm
+++ b/src/PVE/API2/Network/SDN/Controllers.pm
@@ -168,13 +168,19 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/controllers', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::Controllers::Plugin->createSchema(),
+ parameters => PVE::Network::SDN::Controllers::Plugin->createSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
my $type = extract_param($param, 'type');
my $id = extract_param($param, 'controller');
+ my $lock_secret = extract_param($param, 'lock-secret');
my $plugin = PVE::Network::SDN::Controllers::Plugin->lookup($type);
my $opts = $plugin->check_config($id, $param, 1, 1);
@@ -204,6 +210,7 @@ __PACKAGE__->register_method({
},
"create sdn controller object failed",
+ $lock_secret,
);
return undef;
@@ -219,7 +226,12 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/controllers', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::Controllers::Plugin->updateSchema(),
+ parameters => PVE::Network::SDN::Controllers::Plugin->updateSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
@@ -227,6 +239,7 @@ __PACKAGE__->register_method({
my $id = extract_param($param, 'controller');
my $digest = extract_param($param, 'digest');
my $delete = extract_param($param, 'delete');
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Network::SDN::lock_sdn_config(
sub {
@@ -257,6 +270,7 @@ __PACKAGE__->register_method({
},
"update sdn controller object failed",
+ $lock_secret,
);
return undef;
@@ -281,6 +295,7 @@ __PACKAGE__->register_method({
completion => \&PVE::Network::SDN::Controllers::complete_sdn_controllers,
},
),
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
},
},
returns => { type => 'null' },
@@ -288,6 +303,7 @@ __PACKAGE__->register_method({
my ($param) = @_;
my $id = extract_param($param, 'controller');
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Network::SDN::lock_sdn_config(
sub {
@@ -307,6 +323,7 @@ __PACKAGE__->register_method({
},
"delete sdn controller object failed",
+ $lock_secret,
);
return undef;
diff --git a/src/PVE/API2/Network/SDN/Dns.pm b/src/PVE/API2/Network/SDN/Dns.pm
index c82e3544252f..941af8b393d3 100644
--- a/src/PVE/API2/Network/SDN/Dns.pm
+++ b/src/PVE/API2/Network/SDN/Dns.pm
@@ -123,13 +123,19 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/dns', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::Dns::Plugin->createSchema(),
+ parameters => PVE::Network::SDN::Dns::Plugin->createSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
my $type = extract_param($param, 'type');
my $id = extract_param($param, 'dns');
+ my $lock_secret = extract_param($param, 'lock-secret');
my $plugin = PVE::Network::SDN::Dns::Plugin->lookup($type);
my $opts = $plugin->check_config($id, $param, 1, 1);
@@ -157,6 +163,7 @@ __PACKAGE__->register_method({
},
"create sdn dns object failed",
+ $lock_secret,
);
return undef;
@@ -172,7 +179,12 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/dns', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::Dns::Plugin->updateSchema(),
+ parameters => PVE::Network::SDN::Dns::Plugin->updateSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
@@ -180,6 +192,7 @@ __PACKAGE__->register_method({
my $id = extract_param($param, 'dns');
my $digest = extract_param($param, 'digest');
my $delete = extract_param($param, 'delete');
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Network::SDN::lock_sdn_config(
sub {
@@ -209,6 +222,7 @@ __PACKAGE__->register_method({
},
"update sdn dns object failed",
+ $lock_secret,
);
return undef;
@@ -233,6 +247,7 @@ __PACKAGE__->register_method({
completion => \&PVE::Network::SDN::Dns::complete_sdn_dns,
},
),
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
},
},
returns => { type => 'null' },
@@ -240,6 +255,7 @@ __PACKAGE__->register_method({
my ($param) = @_;
my $id = extract_param($param, 'dns');
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Network::SDN::lock_sdn_config(
sub {
@@ -255,6 +271,7 @@ __PACKAGE__->register_method({
},
"delete sdn dns object failed",
+ $lock_secret,
);
return undef;
diff --git a/src/PVE/API2/Network/SDN/Fabrics/Fabric.pm b/src/PVE/API2/Network/SDN/Fabrics/Fabric.pm
index 8c47b1bc5f00..9cfe88cfea21 100644
--- a/src/PVE/API2/Network/SDN/Fabrics/Fabric.pm
+++ b/src/PVE/API2/Network/SDN/Fabrics/Fabric.pm
@@ -138,6 +138,8 @@ __PACKAGE__->register_method({
code => sub {
my ($param) = @_;
+ my $lock_secret = extract_param($param, 'lock-secret');
+
PVE::Network::SDN::lock_sdn_config(
sub {
my $config = PVE::Network::SDN::Fabrics::config();
@@ -149,6 +151,7 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Fabrics::write_config($config);
},
"adding fabric failed",
+ $lock_secret,
);
},
});
@@ -170,6 +173,7 @@ __PACKAGE__->register_method({
},
code => sub {
my ($param) = @_;
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Network::SDN::lock_sdn_config(
sub {
@@ -184,6 +188,7 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Fabrics::write_config($config);
},
"updating fabric failed",
+ $lock_secret,
);
},
});
@@ -208,6 +213,8 @@ __PACKAGE__->register_method({
code => sub {
my ($param) = @_;
+ my $lock_secret = extract_param($param, 'lock-secret');
+
PVE::Network::SDN::lock_sdn_config(
sub {
my $id = extract_param($param, 'id');
@@ -253,6 +260,7 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Fabrics::write_config($config);
},
"deleting fabric failed",
+ $lock_secret,
);
},
});
diff --git a/src/PVE/API2/Network/SDN/Fabrics/FabricNode.pm b/src/PVE/API2/Network/SDN/Fabrics/FabricNode.pm
index b28884434b37..ad352e6a4a2e 100644
--- a/src/PVE/API2/Network/SDN/Fabrics/FabricNode.pm
+++ b/src/PVE/API2/Network/SDN/Fabrics/FabricNode.pm
@@ -153,6 +153,8 @@ __PACKAGE__->register_method({
code => sub {
my ($param) = @_;
+ my $lock_secret = extract_param($param, 'lock-secret');
+
PVE::Network::SDN::lock_sdn_config(
sub {
my $config = PVE::Network::SDN::Fabrics::config();
@@ -164,6 +166,7 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Fabrics::write_config($config);
},
"adding node failed",
+ $lock_secret,
);
},
});
@@ -190,6 +193,8 @@ __PACKAGE__->register_method({
code => sub {
my ($param) = @_;
+ my $lock_secret = extract_param($param, 'lock-secret');
+
PVE::Network::SDN::lock_sdn_config(
sub {
my $fabric_id = extract_param($param, 'fabric_id');
@@ -204,6 +209,7 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Fabrics::write_config($config);
},
"updating node failed",
+ $lock_secret,
);
},
});
@@ -233,6 +239,8 @@ __PACKAGE__->register_method({
code => sub {
my ($param) = @_;
+ my $lock_secret = extract_param($param, 'lock-secret');
+
PVE::Network::SDN::lock_sdn_config(
sub {
my $fabric_id = extract_param($param, 'fabric_id');
@@ -247,6 +255,7 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Fabrics::write_config($config);
},
"deleting node failed",
+ $lock_secret,
);
},
});
diff --git a/src/PVE/API2/Network/SDN/Ipams.pm b/src/PVE/API2/Network/SDN/Ipams.pm
index e30d28ffec56..283b33f67d69 100644
--- a/src/PVE/API2/Network/SDN/Ipams.pm
+++ b/src/PVE/API2/Network/SDN/Ipams.pm
@@ -128,13 +128,19 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/ipams', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::Ipams::Plugin->createSchema(),
+ parameters => PVE::Network::SDN::Ipams::Plugin->createSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
my $type = extract_param($param, 'type');
my $id = extract_param($param, 'ipam');
+ my $lock_secret = extract_param($param, 'lock-secret');
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($type);
my $opts = $plugin->check_config($id, $param, 1, 1);
@@ -164,6 +170,7 @@ __PACKAGE__->register_method({
},
"create sdn ipam object failed",
+ $lock_secret,
);
return undef;
@@ -179,7 +186,12 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/ipams', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::Ipams::Plugin->updateSchema(),
+ parameters => PVE::Network::SDN::Ipams::Plugin->updateSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
@@ -187,6 +199,7 @@ __PACKAGE__->register_method({
my $id = extract_param($param, 'ipam');
my $digest = extract_param($param, 'digest');
my $delete = extract_param($param, 'delete');
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Network::SDN::lock_sdn_config(
sub {
@@ -216,6 +229,7 @@ __PACKAGE__->register_method({
},
"update sdn ipam object failed",
+ $lock_secret,
);
return undef;
@@ -240,6 +254,7 @@ __PACKAGE__->register_method({
completion => \&PVE::Network::SDN::Ipams::complete_sdn_ipams,
},
),
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
},
},
returns => { type => 'null' },
@@ -247,6 +262,7 @@ __PACKAGE__->register_method({
my ($param) = @_;
my $id = extract_param($param, 'ipam');
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Network::SDN::lock_sdn_config(
sub {
@@ -264,6 +280,7 @@ __PACKAGE__->register_method({
},
"delete sdn zone object failed",
+ $lock_secret,
);
return undef;
diff --git a/src/PVE/API2/Network/SDN/Subnets.pm b/src/PVE/API2/Network/SDN/Subnets.pm
index c9f5452b1883..c26a38c17c7f 100644
--- a/src/PVE/API2/Network/SDN/Subnets.pm
+++ b/src/PVE/API2/Network/SDN/Subnets.pm
@@ -190,13 +190,19 @@ __PACKAGE__->register_method({
description => "Require 'SDN.Allocate' permission on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
},
- parameters => PVE::Network::SDN::SubnetPlugin->createSchema(),
+ parameters => PVE::Network::SDN::SubnetPlugin->createSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
my $type = extract_param($param, 'type');
my $cidr = extract_param($param, 'subnet');
+ my $lock_secret = extract_param($param, 'lock-secret');
my $vnet = $param->{vnet};
my $privs = ['SDN.Allocate'];
@@ -234,6 +240,7 @@ __PACKAGE__->register_method({
},
"create sdn subnet object failed",
+ $lock_secret,
);
return undef;
@@ -250,7 +257,12 @@ __PACKAGE__->register_method({
description => "Require 'SDN.Allocate' permission on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
},
- parameters => PVE::Network::SDN::SubnetPlugin->updateSchema(),
+ parameters => PVE::Network::SDN::SubnetPlugin->updateSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
@@ -258,6 +270,7 @@ __PACKAGE__->register_method({
my $id = extract_param($param, 'subnet');
my $digest = extract_param($param, 'digest');
my $delete = extract_param($param, 'delete');
+ my $lock_secret = extract_param($param, 'lock-secret');
my $vnet = $param->{vnet};
@@ -295,6 +308,7 @@ __PACKAGE__->register_method({
},
"update sdn subnet object failed",
+ $lock_secret,
);
return undef;
@@ -321,6 +335,7 @@ __PACKAGE__->register_method({
completion => \&PVE::Network::SDN::Subnets::complete_sdn_subnets,
},
),
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
},
},
returns => { type => 'null' },
@@ -329,6 +344,8 @@ __PACKAGE__->register_method({
my $id = extract_param($param, 'subnet');
my $vnet = extract_param($param, 'vnet');
+ my $lock_secret = extract_param($param, 'lock-secret');
+
my $privs = ['SDN.Allocate'];
&$check_vnet_access($vnet, $privs);
@@ -350,10 +367,9 @@ __PACKAGE__->register_method({
delete $cfg->{ids}->{$id};
- PVE::Network::SDN::Subnets::write_config($cfg);
-
},
"delete sdn subnet object failed",
+ $lock_secret,
);
return undef;
diff --git a/src/PVE/API2/Network/SDN/Vnets.pm b/src/PVE/API2/Network/SDN/Vnets.pm
index 560828371beb..dd4cf43f35a3 100644
--- a/src/PVE/API2/Network/SDN/Vnets.pm
+++ b/src/PVE/API2/Network/SDN/Vnets.pm
@@ -205,13 +205,19 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/zones/{zone}', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::VnetPlugin->createSchema(),
+ parameters => PVE::Network::SDN::VnetPlugin->createSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
my $type = extract_param($param, 'type');
my $id = extract_param($param, 'vnet');
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Cluster::check_cfs_quorum();
mkdir("/etc/pve/sdn");
@@ -238,6 +244,7 @@ __PACKAGE__->register_method({
},
"create sdn vnet object failed",
+ $lock_secret,
);
return undef;
@@ -254,7 +261,12 @@ __PACKAGE__->register_method({
description => "Require 'SDN.Allocate' permission on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
},
- parameters => PVE::Network::SDN::VnetPlugin->updateSchema(),
+ parameters => PVE::Network::SDN::VnetPlugin->updateSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
@@ -262,6 +274,7 @@ __PACKAGE__->register_method({
my $id = extract_param($param, 'vnet');
my $digest = extract_param($param, 'digest');
my $delete = extract_param($param, 'delete');
+ my $lock_secret = extract_param($param, 'lock-secret');
my $privs = ['SDN.Allocate'];
&$check_vnet_access($id, $privs);
@@ -307,6 +320,7 @@ __PACKAGE__->register_method({
},
"update sdn vnet object failed",
+ $lock_secret,
);
return undef;
@@ -332,6 +346,7 @@ __PACKAGE__->register_method({
completion => \&PVE::Network::SDN::Vnets::complete_sdn_vnets,
},
),
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
},
},
returns => { type => 'null' },
@@ -339,6 +354,7 @@ __PACKAGE__->register_method({
my ($param) = @_;
my $id = extract_param($param, 'vnet');
+ my $lock_secret = extract_param($param, 'lock-secret');
my $privs = ['SDN.Allocate'];
&$check_vnet_access($id, $privs);
@@ -356,6 +372,7 @@ __PACKAGE__->register_method({
},
"delete sdn vnet object failed",
+ $lock_secret,
);
return undef;
diff --git a/src/PVE/API2/Network/SDN/Zones.pm b/src/PVE/API2/Network/SDN/Zones.pm
index e53e6e7d430d..a7ee85bb5ba7 100644
--- a/src/PVE/API2/Network/SDN/Zones.pm
+++ b/src/PVE/API2/Network/SDN/Zones.pm
@@ -207,13 +207,19 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/zones', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::Zones::Plugin->createSchema(),
+ parameters => PVE::Network::SDN::Zones::Plugin->createSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
my $type = extract_param($param, 'type');
my $id = extract_param($param, 'zone');
+ my $lock_secret = extract_param($param, 'lock-secret');
my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($type);
my $opts = $plugin->check_config($id, $param, 1, 1);
@@ -256,6 +262,7 @@ __PACKAGE__->register_method({
},
"create sdn zone object failed",
+ $lock_secret,
);
return;
@@ -271,7 +278,12 @@ __PACKAGE__->register_method({
permissions => {
check => ['perm', '/sdn/zones/{zone}', ['SDN.Allocate']],
},
- parameters => PVE::Network::SDN::Zones::Plugin->updateSchema(),
+ parameters => PVE::Network::SDN::Zones::Plugin->updateSchema(
+ undef,
+ {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ },
+ ),
returns => { type => 'null' },
code => sub {
my ($param) = @_;
@@ -279,6 +291,7 @@ __PACKAGE__->register_method({
my $id = extract_param($param, 'zone');
my $digest = extract_param($param, 'digest');
my $delete = extract_param($param, 'delete');
+ my $lock_secret = extract_param($param, 'lock-secret');
if ($delete) {
$delete = [PVE::Tools::split_list($delete)];
@@ -344,6 +357,7 @@ __PACKAGE__->register_method({
},
"update sdn zone object failed",
+ $lock_secret,
);
return;
@@ -368,6 +382,7 @@ __PACKAGE__->register_method({
completion => \&PVE::Network::SDN::Zones::complete_sdn_zones,
},
),
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
},
},
returns => { type => 'null' },
@@ -375,6 +390,7 @@ __PACKAGE__->register_method({
my ($param) = @_;
my $id = extract_param($param, 'zone');
+ my $lock_secret = extract_param($param, 'lock-secret');
PVE::Network::SDN::lock_sdn_config(
sub {
@@ -391,6 +407,7 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Zones::write_config($cfg);
},
"delete sdn zone object failed",
+ $lock_secret,
);
return;
diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index efee21543387..adcae175aa2d 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -50,6 +50,15 @@ PVE::Cluster::cfs_register_file($running_cfg, $parse_running_cfg, $write_running
my $LOCK_SECRET_FILE = "/etc/pve/sdn/.lock";
+PVE::JSONSchema::register_standard_option(
+ 'pve-sdn-lock-secret',
+ {
+ type => 'string',
+ description => "the secret for unlocking the global SDN configuration",
+ optional => 1,
+ },
+);
+
# improve me : move status code inside plugins ?
sub ifquery_check {
diff --git a/src/PVE/Network/SDN/Fabrics.pm b/src/PVE/Network/SDN/Fabrics.pm
index 796d14978cfe..fcc9679a61f1 100644
--- a/src/PVE/Network/SDN/Fabrics.pm
+++ b/src/PVE/Network/SDN/Fabrics.pm
@@ -128,6 +128,7 @@ sub node_properties {
node_id => get_standard_option('pve-sdn-fabric-node-id'),
protocol => get_standard_option('pve-sdn-fabric-protocol'),
digest => get_standard_option('pve-config-digest'),
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
ip => {
type => 'string',
format => 'ipv4',
@@ -227,6 +228,7 @@ sub fabric_properties {
id => get_standard_option('pve-sdn-fabric-id'),
protocol => get_standard_option('pve-sdn-fabric-protocol'),
digest => get_standard_option('pve-config-digest'),
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
ip_prefix => {
type => 'string',
format => 'CIDR',
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pve-devel] [PATCH network v2 3/5] api: add lock secret parameter to apply endpoint
2025-07-24 14:17 [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 2/5] api: add lock-secret parameter to all api calls Gabriel Goller
@ 2025-07-24 14:17 ` Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 4/5] api: add lock and release endpoints for global configuration lock Gabriel Goller
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-07-24 14:17 UTC (permalink / raw)
To: pve-devel
From: Stefan Hanreich <s.hanreich@proxmox.com>
Committing the configuration now requires a lock on the SDN
configuration, which was not required before. This is to prevent
concurrent callers from applying the SDN configuration, while the lock
is held. If there is no lock set, then this function behaves the same
as before.
Also add the functionality to automatically release the lock after
applying the configuration, for convenience reasons.
Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/API2/Network/SDN.pm | 32 ++++++++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index 6645f28b5de1..dc271ec4fbf9 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -9,7 +9,7 @@ use PVE::JSONSchema qw(get_standard_option);
use PVE::RESTHandler;
use PVE::RPCEnvironment;
use PVE::SafeSyslog;
-use PVE::Tools qw(run_command);
+use PVE::Tools qw(run_command extract_param);
use PVE::Network::SDN;
use PVE::API2::Network::SDN::Controllers;
@@ -126,6 +126,16 @@ __PACKAGE__->register_method({
},
parameters => {
additionalProperties => 0,
+ properties => {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ 'release-lock' => {
+ type => 'boolean',
+ optional => 1,
+ default => 1,
+ description =>
+ 'When lock-secret has been provided and configuration successfully commited, release the lock automatically afterwards',
+ },
+ },
},
returns => {
type => 'string',
@@ -136,10 +146,24 @@ __PACKAGE__->register_method({
my $rpcenv = PVE::RPCEnvironment::get();
my $authuser = $rpcenv->get_user();
- my $previous_config_has_frr = PVE::Network::SDN::running_config_has_frr();
- PVE::Network::SDN::commit_config();
+ my $lock_secret = extract_param($param, 'lock-secret');
+ my $release_lock = extract_param($param, 'release-lock');
+
+ my $previous_config_has_frr;
+ my $new_config_has_frr;
+
+ PVE::Network::SDN::lock_sdn_config(
+ sub {
+ $previous_config_has_frr = PVE::Network::SDN::running_config_has_frr();
+ PVE::Network::SDN::commit_config();
+ $new_config_has_frr = PVE::Network::SDN::running_config_has_frr();
+
+ PVE::Network::SDN::delete_global_lock() if $lock_secret && $release_lock;
+ },
+ "could not commit SDN config",
+ $lock_secret,
+ );
- my $new_config_has_frr = PVE::Network::SDN::running_config_has_frr();
my $skip_frr = !($previous_config_has_frr || $new_config_has_frr);
my $code = sub {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pve-devel] [PATCH network v2 4/5] api: add lock and release endpoints for global configuration lock
2025-07-24 14:17 [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
` (2 preceding siblings ...)
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 3/5] api: add lock secret parameter to apply endpoint Gabriel Goller
@ 2025-07-24 14:17 ` Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 5/5] api: add rollback endpoint Gabriel Goller
2025-07-29 9:30 ` [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-07-24 14:17 UTC (permalink / raw)
To: pve-devel
From: Stefan Hanreich <s.hanreich@proxmox.com>
This endpoint exposes the newly introduced global lock functionality
via the API. It adds endpoints for acquiring and releasing the lock.
Acquiring the lock is as simple as:
pvesh create /cluster/sdn/lock
The flag 'allow-pending' governs whether the lock should be acquired
if there are pending configuration changes.
The release endpoint can also be used to forcibly release the lock via
CLI without requiring the knowledge of the lock-secret:
pvesh delete /cluster/sdn/lock --force 1
Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/API2/Network/SDN.pm | 82 +++++++++++++++++++++++++++++++++++++
src/PVE/Network/SDN.pm | 23 +++++++++++
2 files changed, 105 insertions(+)
diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index dc271ec4fbf9..692dec8718f7 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -116,6 +116,88 @@ my $create_reload_network_worker = sub {
};
__PACKAGE__->register_method({
+ name => 'lock',
+ protected => 1,
+ path => 'lock',
+ method => 'POST',
+ description => "Acquire global lock for SDN configuration",
+ permissions => {
+ check => ['perm', '/sdn', ['SDN.Allocate']],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ 'allow-pending' => {
+ type => 'boolean',
+ optional => 1,
+ default => 0,
+ description =>
+ 'if true, allow acquiring lock even though there are pending changes',
+ },
+ },
+ },
+ returns => {
+ type => 'string',
+ },
+ code => sub {
+ my ($param) = @_;
+
+ return PVE::Network::SDN::lock_sdn_config(
+ sub {
+ die "configuration has pending changes"
+ if !$param->{'allow-pending'} && PVE::Network::SDN::has_pending_changes();
+
+ return PVE::Network::SDN::create_global_lock();
+ },
+ "could not acquire lock for SDN config",
+ );
+ },
+});
+
+__PACKAGE__->register_method({
+ name => 'release_lock',
+ protected => 1,
+ path => 'lock',
+ method => 'DELETE',
+ description => "Release global lock for SDN configuration",
+ permissions => {
+ check => ['perm', '/sdn', ['SDN.Allocate']],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ 'force' => {
+ type => 'boolean',
+ optional => 1,
+ default => 0,
+ description => 'if true, allow releasing lock without providing the secret',
+ },
+ },
+ },
+ returns => {
+ type => 'null',
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $code = sub {
+ PVE::Network::SDN::delete_global_lock();
+ };
+
+ if ($param->{force}) {
+ $code->();
+ } else {
+ PVE::Network::SDN::lock_sdn_config(
+ $code,
+ "could not release lock",
+ $param->{'lock-secret'},
+ );
+ }
+ },
+});
+
+__PACKAGE__->register_method ({
name => 'reload',
protected => 1,
path => '',
diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index adcae175aa2d..59185bfd5a16 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -208,6 +208,29 @@ sub commit_config {
cfs_write_file($running_cfg, $cfg);
}
+sub has_pending_changes {
+ my $running_cfg = PVE::Network::SDN::running_config();
+
+ # only use configuration files which get written by commit_config here
+ my $config_files = {
+ zones => PVE::Network::SDN::Zones::config(),
+ vnets => PVE::Network::SDN::Vnets::config(),
+ subnets => PVE::Network::SDN::Subnets::config(),
+ controllers => PVE::Network::SDN::Controllers::config(),
+ };
+
+ for my $config_file (keys %$config_files) {
+ my $config = $config_files->{$config_file};
+ my $pending_config = PVE::Network::SDN::pending_config($running_cfg, $config, $config_file);
+
+ for my $id (keys %{ $pending_config->{ids} }) {
+ return 1 if $pending_config->{ids}->{$id}->{pending};
+ }
+ }
+
+ return 0;
+}
+
sub generate_lock_secret {
my $min = ord('!'); # first printable ascii
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pve-devel] [PATCH network v2 5/5] api: add rollback endpoint
2025-07-24 14:17 [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
` (3 preceding siblings ...)
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 4/5] api: add lock and release endpoints for global configuration lock Gabriel Goller
@ 2025-07-24 14:17 ` Gabriel Goller
2025-07-29 9:30 ` [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-07-24 14:17 UTC (permalink / raw)
To: pve-devel
From: Stefan Hanreich <s.hanreich@proxmox.com>
This adds the functionality of rolling back the pending configuration
to the currently running configuration, resetting all changes made
since last applying the SDN configuration. This is mainly thought as
an escape hatch for failed PDM transactions.
You can invoke the endpoint via CLI:
pvesh create /cluster/sdn/rollback [--lock-secret X [--release-lock]]
If a lock is currently held on the configuration and you want to
forcibly rollback, you need to release the lock first via the
lock_release API endpoint.
Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/API2/Network/SDN.pm | 57 ++++++++++++++++++++++++++++++++++++-
1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index 692dec8718f7..16f92a4ee758 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -197,7 +197,62 @@ __PACKAGE__->register_method({
},
});
-__PACKAGE__->register_method ({
+__PACKAGE__->register_method({
+ name => 'rollback',
+ protected => 1,
+ path => 'rollback',
+ method => 'POST',
+ description => "Rollback pending changes to SDN configuration",
+ permissions => {
+ check => ['perm', '/sdn', ['SDN.Allocate']],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ 'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
+ 'release-lock' => {
+ type => 'boolean',
+ optional => 1,
+ default => 1,
+ description =>
+ 'When lock-secret has been provided and configuration successfully rollbacked, release the lock automatically afterwards',
+ },
+ },
+ },
+ returns => {
+ type => 'null',
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $lock_secret = extract_param($param, 'lock-secret');
+ my $release_lock = extract_param($param, 'release-lock');
+
+ my $rollback = sub {
+ my $running_config = PVE::Network::SDN::running_config();
+
+ PVE::Network::SDN::Zones::write_config($running_config->{zones});
+ PVE::Network::SDN::Vnets::write_config($running_config->{vnets});
+ PVE::Network::SDN::Subnets::write_config($running_config->{subnets});
+ PVE::Network::SDN::Controllers::write_config($running_config->{controllers});
+
+ # if the config hasn't yet been applied after the introduction of
+ # fabrics then the key does not exist in the running config so we
+ # default to an empty hash
+ my $fabrics_config = $running_config->{fabrics}->{ids} // {};
+ my $parsed_fabrics_config = PVE::RS::SDN::Fabrics->running_config($fabrics_config);
+ PVE::Network::SDN::Fabrics::write_config($parsed_fabrics_config);
+
+ PVE::Network::SDN::delete_global_lock() if $lock_secret && $release_lock;
+ };
+
+ PVE::Network::SDN::lock_sdn_config(
+ $rollback, "could not rollback SDN configuration", $lock_secret,
+ );
+ },
+});
+
+__PACKAGE__->register_method({
name => 'reload',
protected => 1,
path => '',
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration Gabriel Goller
@ 2025-07-29 7:27 ` Thomas Lamprecht
2025-07-29 7:59 ` Stefan Hanreich
0 siblings, 1 reply; 10+ messages in thread
From: Thomas Lamprecht @ 2025-07-29 7:27 UTC (permalink / raw)
To: Proxmox VE development discussion, Gabriel Goller
Am 24.07.25 um 16:18 schrieb Gabriel Goller:
> From: Stefan Hanreich <s.hanreich@proxmox.com>
>
> Add a new cluster-wide lock for SDN that prevents any changes to the
> configuration if the generated lock-secret is not provided. It works
> by generating and storing a secret in sdn/.lock which gets checked by
> lock_sdn_config on every invocation. If the lock file exists, then the
> lock secret has to be supplied in order to make changes to the SDN
> configuration.
>
> Lock using the domain lock (`PVE::Cluster::cfs_lock_domain`) and "sdn"
> string.
>
> This is mainly a preparation for PDM, where PDM can take the lock and
> prevent concurrent modifications to the SDN configuration from other
> sources, even across multiple API calls.
>
> Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> src/PVE/Network/SDN.pm | 53 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
> index 0e7d1dfd239c..efee21543387 100644
> --- a/src/PVE/Network/SDN.pm
> +++ b/src/PVE/Network/SDN.pm
> @@ -13,7 +13,7 @@ use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
> use PVE::INotify;
> use PVE::RESTEnvironment qw(log_warn);
> use PVE::RPCEnvironment;
> -use PVE::Tools qw(extract_param dir_glob_regex run_command);
> +use PVE::Tools qw(file_get_contents file_set_contents extract_param dir_glob_regex run_command);
>
> use PVE::Network::SDN::Vnets;
> use PVE::Network::SDN::Zones;
> @@ -48,6 +48,8 @@ my $write_running_cfg = sub {
>
> PVE::Cluster::cfs_register_file($running_cfg, $parse_running_cfg, $write_running_cfg);
>
> +my $LOCK_SECRET_FILE = "/etc/pve/sdn/.lock";
> +
> # improve me : move status code inside plugins ?
>
> sub ifquery_check {
> @@ -197,14 +199,57 @@ sub commit_config {
> cfs_write_file($running_cfg, $cfg);
> }
>
> +sub generate_lock_secret {
nit: might be better to avoid the "secret" terminology here? As this is not really
a secret but rather something like a token, handle or maybe even cookie.
I.e., this hasn't to stay secret, as it does not provide access on it's own, it's
just for ensuring orderly locking by identifying the locker.
I'm mostly mentioning this as such method and variable names tend to leak into
docs and other communications, and especially secrets are a bit delicate topic,
for me that's the biggest reason why it would be better to avoid the term here.
Could be fixed up though, if you agree with changing this and have an opinion
on what variant (handle, token, cookie, ...?) would be best.
> + my $min = ord('!'); # first printable ascii
> +
> + my $rand_bytes = Crypt::OpenSSL::Random::random_bytes(32);
> + die "failed to generate lock secret!\n" if !$rand_bytes;
> +
> + my $str = join('', map { chr((ord($_) & 0x3F) + $min) } split('', $rand_bytes));
hmm, might have overlooked when checking the v1, but would it be a better option
to decode the $rand_bytes as base64? That keeps the full entropy and ensures we
got an easy to handle character-set.
Another option might be to use a UUIDv7 [0], as that version includes the
milliseconds since the unix expoch in the first 48 bits, that would also give
some hints for when the handle was created, that info could be even used for
expiring a lock handle.
[0]: https://www.rfc-editor.org/rfc/rfc9562.html#name-uuid-version-7
As the users of this should not really expect any specific format, we could still
change that after applying though, so just tell me what you think/prefer.
> + return $str;
> +}
> +
> +sub create_global_lock {
> + my $secret = generate_lock_secret();
> + PVE::Tools::file_set_contents($LOCK_SECRET_FILE, $secret);
> + return $secret;
> +}
> +
> +sub delete_global_lock {
> + unlink $LOCK_SECRET_FILE if -e $LOCK_SECRET_FILE;
> +}
> +
> sub lock_sdn_config {
> - my ($code, $errmsg) = @_;
> + my ($code, $errmsg, $lock_secret_user) = @_;
>
> - cfs_lock_file($running_cfg, undef, $code);
> + my $lock_wrapper = sub {
> + my $lock_secret = undef;
> + if (-e $LOCK_SECRET_FILE) {
> + $lock_secret = PVE::Tools::file_get_contents($LOCK_SECRET_FILE);
> + }
> +
> + if (
> + defined($lock_secret)
> + && (!defined($lock_secret_user) || $lock_secret ne $lock_secret_user)
> + ) {
> + die "invalid lock secret provided!";
> + }
> +
> + return $code->();
> + };
>
> - if (my $err = $@) {
> + return lock_sdn_domain($lock_wrapper, $errmsg);
> +}
> +
> +sub lock_sdn_domain {
> + my ($code, $errmsg) = @_;
> +
> + my $res = PVE::Cluster::cfs_lock_domain("sdn", undef, $code);
> + my $err = $@;
> + if ($err) {
> $errmsg ? die "$errmsg: $err" : die $err;
> }
> + return $res;
> }
>
> sub get_local_vnets {
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration
2025-07-29 7:27 ` Thomas Lamprecht
@ 2025-07-29 7:59 ` Stefan Hanreich
2025-07-29 8:21 ` Thomas Lamprecht
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Hanreich @ 2025-07-29 7:59 UTC (permalink / raw)
To: Proxmox VE development discussion, Thomas Lamprecht, Gabriel Goller
On 7/29/25 9:28 AM, Thomas Lamprecht wrote:
[snip]
>> +my $LOCK_SECRET_FILE = "/etc/pve/sdn/.lock";
>> +
>> # improve me : move status code inside plugins ?
>>
>> sub ifquery_check {
>> @@ -197,14 +199,57 @@ sub commit_config {
>> cfs_write_file($running_cfg, $cfg);
>> }
>>
>> +sub generate_lock_secret {
>
> nit: might be better to avoid the "secret" terminology here? As this is not really
> a secret but rather something like a token, handle or maybe even cookie.
>
> I.e., this hasn't to stay secret, as it does not provide access on it's own, it's
> just for ensuring orderly locking by identifying the locker.
>
> I'm mostly mentioning this as such method and variable names tend to leak into
> docs and other communications, and especially secrets are a bit delicate topic,
> for me that's the biggest reason why it would be better to avoid the term here.
>
> Could be fixed up though, if you agree with changing this and have an opinion
> on what variant (handle, token, cookie, ...?) would be best.
Makes sense, I'm gravitating towards token then - although handle would
be fine by me as well. Cookie has the same issues with pre-existing
sentiment / connotations imo?
>> + my $min = ord('!'); # first printable ascii
>> +
>> + my $rand_bytes = Crypt::OpenSSL::Random::random_bytes(32);
>> + die "failed to generate lock secret!\n" if !$rand_bytes;
>> +
>> + my $str = join('', map { chr((ord($_) & 0x3F) + $min) } split('', $rand_bytes));
>
> hmm, might have overlooked when checking the v1, but would it be a better option
> to decode the $rand_bytes as base64? That keeps the full entropy and ensures we
> got an easy to handle character-set.
>
> Another option might be to use a UUIDv7 [0], as that version includes the
> milliseconds since the unix expoch in the first 48 bits, that would also give
> some hints for when the handle was created, that info could be even used for
> expiring a lock handle.
>
> [0]: https://www.rfc-editor.org/rfc/rfc9562.html#name-uuid-version-7
>
> As the users of this should not really expect any specific format, we could still
> change that after applying though, so just tell me what you think/prefer.
Gabriel mentioned something similar about the used characters, because
the current character set is also inconvenient for running CLI commands.
UUIDv7 sounds sensible for this use-case and since we already use the
UUID module in our stack we could just opt for that?
[snip]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration
2025-07-29 7:59 ` Stefan Hanreich
@ 2025-07-29 8:21 ` Thomas Lamprecht
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Lamprecht @ 2025-07-29 8:21 UTC (permalink / raw)
To: Stefan Hanreich, Proxmox VE development discussion, Gabriel Goller
Am 29.07.25 um 09:59 schrieb Stefan Hanreich:
> On 7/29/25 9:28 AM, Thomas Lamprecht wrote:
>> nit: might be better to avoid the "secret" terminology here? As this is not really
>> a secret but rather something like a token, handle or maybe even cookie.
...
>> Could be fixed up though, if you agree with changing this and have an opinion
>> on what variant (handle, token, cookie, ...?) would be best.
>
> Makes sense, I'm gravitating towards token then - although handle would
> be fine by me as well. Cookie has the same issues with pre-existing
> sentiment / connotations imo?
While lock handle is common term, those handles more often refer to file
handles, so token might be indeed the least problematic / overloaded
term here.
> Gabriel mentioned something similar about the used characters, because
> the current character set is also inconvenient for running CLI commands.
> UUIDv7 sounds sensible for this use-case and since we already use the
> UUID module in our stack we could just opt for that?
ack.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration
2025-07-24 14:17 [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
` (4 preceding siblings ...)
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 5/5] api: add rollback endpoint Gabriel Goller
@ 2025-07-29 9:30 ` Gabriel Goller
5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-07-29 9:30 UTC (permalink / raw)
To: pve-devel
Superseded-by: https://lore.proxmox.com/pve-devel/20250729092933.90118-1-g.goller@proxmox.com/
Thanks Thomas and Stefan!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-07-29 9:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-24 14:17 [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration Gabriel Goller
2025-07-29 7:27 ` Thomas Lamprecht
2025-07-29 7:59 ` Stefan Hanreich
2025-07-29 8:21 ` Thomas Lamprecht
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 2/5] api: add lock-secret parameter to all api calls Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 3/5] api: add lock secret parameter to apply endpoint Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 4/5] api: add lock and release endpoints for global configuration lock Gabriel Goller
2025-07-24 14:17 ` [pve-devel] [PATCH network v2 5/5] api: add rollback endpoint Gabriel Goller
2025-07-29 9:30 ` [pve-devel] [PATCH network v2 0/5] Add global locking and configuration rollback to SDN configuration Gabriel Goller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox