[pve-devel] [PATCH cluster/network 0/6] Add global locking and configuration rollback to SDN configuration

[pve-devel] [PATCH cluster/network 0/6] Add global locking and configuration rollback to SDN configuration

[Stefan Hanreich]

## 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.

The existing lock_sdn_config function locked the running configuration file. For
backwards compatibility reasons, I left it this way. For Proxmox VE 9 we should
consider moving this to a domain-lock, as for instance done in the HA stack.


## Dependencies

* pve-network depends on pve-cluster

pve-cluster:

Stefan Hanreich (1):
  cfs: add 'sdn/.lock' file

 src/PVE/Cluster.pm  | 1 +
 src/pmxcfs/status.c | 1 +
 2 files changed, 2 insertions(+)


pve-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             | 153 +++++++++++++++++++++++-
 src/PVE/API2/Network/SDN/Controllers.pm |  18 ++-
 src/PVE/API2/Network/SDN/Dns.pm         |  18 ++-
 src/PVE/API2/Network/SDN/Ipams.pm       |  18 ++-
 src/PVE/API2/Network/SDN/Subnets.pm     |  19 ++-
 src/PVE/API2/Network/SDN/Vnets.pm       |  18 ++-
 src/PVE/API2/Network/SDN/Zones.pm       |  18 ++-
 src/PVE/Network/SDN.pm                  |  75 +++++++++++-
 8 files changed, 303 insertions(+), 34 deletions(-)


Summary over all repositories:
  10 files changed, 305 insertions(+), 34 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

[pve-devel] [PATCH pve-cluster 1/1] cfs: add 'sdn/.lock' file

[Stefan Hanreich]

This file stores a secret if the SDN configuration gets globally
locked.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Cluster.pm  | 1 +
 src/pmxcfs/status.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm
index e0e3ee9..e569d18 100644
--- a/src/PVE/Cluster.pm
+++ b/src/PVE/Cluster.pm
@@ -82,6 +82,7 @@ my $observed = {
     'sdn/mac-cache.json' => 1,
     'sdn/dns.cfg' => 1,
     'sdn/.running-config' => 1,
+    'sdn/.lock' => 1,
     'virtual-guest/cpu-models.conf' => 1,
     'virtual-guest/profiles.cfg' => 1,
     'mapping/pci.cfg' => 1,
diff --git a/src/pmxcfs/status.c b/src/pmxcfs/status.c
index ff5fcc4..dfa8e71 100644
--- a/src/pmxcfs/status.c
+++ b/src/pmxcfs/status.c
@@ -111,6 +111,7 @@ static memdb_change_t memdb_change_array[] = {
 	{ .path = "sdn/pve-ipam-state.json" },
 	{ .path = "sdn/dns.cfg" },
 	{ .path = "sdn/.running-config" },
+	{ .path = "sdn/.lock" },
 	{ .path = "virtual-guest/cpu-models.conf" },
 	{ .path = "virtual-guest/profiles.cfg" },
 	{ .path = "firewall/cluster.fw" },
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-network 1/5] sdn: add global lock for configuration

[Stefan Hanreich]

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.

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.

For now, we are still locking the running config, but we plan to
change over to using domain level locking with the next major release.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---

Notes:
    Do we have any better way of generating a random string? I didn't find a common function for this, so I repurposed code from qemu-server. Might make sense to extract this to pve-common?

 src/PVE/Network/SDN.pm | 46 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 4ac9720..3b3ea67 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -45,6 +45,12 @@ my $write_running_cfg = sub {
 
 PVE::Cluster::cfs_register_file($running_cfg, $parse_running_cfg, $write_running_cfg);
 
+my $LOCK_SECRET_FILE = "sdn/.lock";
+PVE::Cluster::cfs_register_file(
+    $LOCK_SECRET_FILE,
+    sub { my ($filename, $data) = @_; return $data; },
+    sub { my ($filename, $data) = @_; return $data; }
+);
 
 # improve me : move status code inside plugins ?
 
@@ -161,14 +167,50 @@ 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::Cluster::cfs_write_file($LOCK_SECRET_FILE, $secret);
+    return $secret;
+}
+
+sub delete_global_lock {
+    unlink "/etc/pve/$LOCK_SECRET_FILE" if -e "/etc/pve/$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 "/etc/pve/$LOCK_SECRET_FILE") {
+	    $lock_secret = cfs_read_file($LOCK_SECRET_FILE);
+	}
+
+	if (defined($lock_secret) && (!defined($lock_secret_user) || $lock_secret ne $lock_secret_user)) {
+	    die "invalid lock secret provided!";
+	}
+
+	return $code->();
+    };
+
+    # TODO: PVE 9+ change to domain lock
+    my $res = cfs_lock_file($running_cfg, undef, $lock_wrapper);
 
     if (my $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

[pve-devel] [PATCH pve-network 2/5] api: add lock-secret parameter to all api calls

[Stefan Hanreich]

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.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN/Controllers.pm | 18 +++++++++++++-----
 src/PVE/API2/Network/SDN/Dns.pm         | 18 +++++++++++++-----
 src/PVE/API2/Network/SDN/Ipams.pm       | 18 +++++++++++++-----
 src/PVE/API2/Network/SDN/Subnets.pm     | 19 ++++++++++++++-----
 src/PVE/API2/Network/SDN/Vnets.pm       | 18 +++++++++++++-----
 src/PVE/API2/Network/SDN/Zones.pm       | 18 +++++++++++++-----
 src/PVE/Network/SDN.pm                  |  6 ++++++
 7 files changed, 85 insertions(+), 30 deletions(-)

diff --git a/src/PVE/API2/Network/SDN/Controllers.pm b/src/PVE/API2/Network/SDN/Controllers.pm
index 38a685d..29f6ca4 100644
--- a/src/PVE/API2/Network/SDN/Controllers.pm
+++ b/src/PVE/API2/Network/SDN/Controllers.pm
@@ -164,13 +164,16 @@ __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);
@@ -194,7 +197,7 @@ __PACKAGE__->register_method ({
 
 		PVE::Network::SDN::Controllers::write_config($controller_cfg);
 
-	    }, "create sdn controller object failed");
+	    }, "create sdn controller object failed", $lock_secret);
 
 	return undef;
     }});
@@ -208,7 +211,9 @@ __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) = @_;
@@ -216,6 +221,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 {
@@ -244,7 +250,7 @@ __PACKAGE__->register_method ({
 	    PVE::Network::SDN::Controllers::write_config($controller_cfg);
 
 
-	    }, "update sdn controller object failed");
+	    }, "update sdn controller object failed", $lock_secret);
 
 	return undef;
     }});
@@ -264,6 +270,7 @@ __PACKAGE__->register_method ({
 	    controller => get_standard_option('pve-sdn-controller-id', {
                 completion => \&PVE::Network::SDN::Controllers::complete_sdn_controllers,
             }),
+	    'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
 	},
     },
     returns => { type => 'null' },
@@ -271,6 +278,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 {
@@ -288,7 +296,7 @@ __PACKAGE__->register_method ({
 		delete $cfg->{ids}->{$id};
 		PVE::Network::SDN::Controllers::write_config($cfg);
 
-	    }, "delete sdn controller object failed");
+	    }, "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 826d111..a60d0ca 100644
--- a/src/PVE/API2/Network/SDN/Dns.pm
+++ b/src/PVE/API2/Network/SDN/Dns.pm
@@ -120,13 +120,16 @@ __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);
@@ -152,7 +155,7 @@ __PACKAGE__->register_method ({
 
 		PVE::Network::SDN::Dns::write_config($dns_cfg);
 
-	    }, "create sdn dns object failed");
+	    }, "create sdn dns object failed", $lock_secret);
 
 	return undef;
     }});
@@ -166,7 +169,9 @@ __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) = @_;
@@ -174,6 +179,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 {
@@ -201,7 +207,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Network::SDN::Dns::write_config($dns_cfg);
 
-	    }, "update sdn dns object failed");
+	    }, "update sdn dns object failed", $lock_secret);
 
 	return undef;
     }});
@@ -221,6 +227,7 @@ __PACKAGE__->register_method ({
 	    dns => get_standard_option('pve-sdn-dns-id', {
                 completion => \&PVE::Network::SDN::Dns::complete_sdn_dns,
             }),
+	    'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
 	},
     },
     returns => { type => 'null' },
@@ -228,6 +235,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 {
@@ -241,7 +249,7 @@ __PACKAGE__->register_method ({
 		delete $cfg->{ids}->{$id};
 		PVE::Network::SDN::Dns::write_config($cfg);
 
-	    }, "delete sdn dns object failed");
+	    }, "delete sdn dns object failed", $lock_secret);
 
 	return undef;
     }});
diff --git a/src/PVE/API2/Network/SDN/Ipams.pm b/src/PVE/API2/Network/SDN/Ipams.pm
index 27ead02..33166c7 100644
--- a/src/PVE/API2/Network/SDN/Ipams.pm
+++ b/src/PVE/API2/Network/SDN/Ipams.pm
@@ -125,13 +125,16 @@ __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);
@@ -159,7 +162,7 @@ __PACKAGE__->register_method ({
 
 		PVE::Network::SDN::Ipams::write_config($ipam_cfg);
 
-	    }, "create sdn ipam object failed");
+	    }, "create sdn ipam object failed", $lock_secret);
 
 	return undef;
     }});
@@ -173,7 +176,9 @@ __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) = @_;
@@ -181,6 +186,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 {
@@ -208,7 +214,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Network::SDN::Ipams::write_config($ipam_cfg);
 
-	    }, "update sdn ipam object failed");
+	    }, "update sdn ipam object failed", $lock_secret);
 
 	return undef;
     }});
@@ -228,6 +234,7 @@ __PACKAGE__->register_method ({
 	    ipam => get_standard_option('pve-sdn-ipam-id', {
                 completion => \&PVE::Network::SDN::Ipams::complete_sdn_ipams,
             }),
+	    'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
 	},
     },
     returns => { type => 'null' },
@@ -235,6 +242,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 {
@@ -250,7 +258,7 @@ __PACKAGE__->register_method ({
 		delete $cfg->{ids}->{$id};
 		PVE::Network::SDN::Ipams::write_config($cfg);
 
-	    }, "delete sdn zone object failed");
+	    }, "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 7a4c331..cd14f30 100644
--- a/src/PVE/API2/Network/SDN/Subnets.pm
+++ b/src/PVE/API2/Network/SDN/Subnets.pm
@@ -183,13 +183,16 @@ __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' ];
@@ -225,7 +228,7 @@ __PACKAGE__->register_method ({
 
 		PVE::Network::SDN::Subnets::write_config($cfg);
 
-	    }, "create sdn subnet object failed");
+	    }, "create sdn subnet object failed", $lock_secret);
 
 	return undef;
     }});
@@ -240,7 +243,9 @@ __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) = @_;
@@ -248,6 +253,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};
 
@@ -283,7 +289,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Network::SDN::Subnets::write_config($cfg);
 
-	    }, "update sdn subnet object failed");
+	    }, "update sdn subnet object failed", $lock_secret);
 
 	return undef;
     }});
@@ -305,6 +311,7 @@ __PACKAGE__->register_method ({
 	    subnet => get_standard_option('pve-sdn-subnet-id', {
                 completion => \&PVE::Network::SDN::Subnets::complete_sdn_subnets,
             }),
+	    'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
 	},
     },
     returns => { type => 'null' },
@@ -313,6 +320,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);
 
@@ -336,7 +345,7 @@ __PACKAGE__->register_method ({
 
 		PVE::Network::SDN::Subnets::write_config($cfg);
 
-	    }, "delete sdn subnet object failed");
+	    }, "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 e48b048..bd37dd3 100644
--- a/src/PVE/API2/Network/SDN/Vnets.pm
+++ b/src/PVE/API2/Network/SDN/Vnets.pm
@@ -198,13 +198,16 @@ __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");
@@ -228,7 +231,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Network::SDN::Vnets::write_config($cfg);
 
-	}, "create sdn vnet object failed");
+	}, "create sdn vnet object failed", $lock_secret);
 
 	return undef;
     }});
@@ -243,7 +246,9 @@ __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) = @_;
@@ -251,6 +256,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);
@@ -292,7 +298,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Network::SDN::Vnets::write_config($cfg);
 
-	}, "update sdn vnet object failed");
+	}, "update sdn vnet object failed", $lock_secret);
 
 	return undef;
     }
@@ -314,6 +320,7 @@ __PACKAGE__->register_method ({
 	    vnet => get_standard_option('pve-sdn-vnet-id', {
 		completion => \&PVE::Network::SDN::Vnets::complete_sdn_vnets,
 	    }),
+	    'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
 	},
     },
     returns => { type => 'null' },
@@ -321,6 +328,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);
@@ -335,7 +343,7 @@ __PACKAGE__->register_method ({
 	    delete $cfg->{ids}->{$id};
 	    PVE::Network::SDN::Vnets::write_config($cfg);
 
-	}, "delete sdn vnet object failed");
+	}, "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 2c27983..6baee52 100644
--- a/src/PVE/API2/Network/SDN/Zones.pm
+++ b/src/PVE/API2/Network/SDN/Zones.pm
@@ -199,13 +199,16 @@ __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);
@@ -244,7 +247,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Network::SDN::Zones::write_config($zone_cfg);
 
-	}, "create sdn zone object failed");
+	}, "create sdn zone object failed", $lock_secret);
 
 	return;
     }});
@@ -258,7 +261,9 @@ __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) = @_;
@@ -266,6 +271,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) ];
@@ -320,7 +326,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Network::SDN::Zones::write_config($zone_cfg);
 
-	}, "update sdn zone object failed");
+	}, "update sdn zone object failed", $lock_secret);
 
 	return;
     }});
@@ -340,6 +346,7 @@ __PACKAGE__->register_method ({
 	    zone => get_standard_option('pve-sdn-zone-id', {
 		completion => \&PVE::Network::SDN::Zones::complete_sdn_zones,
 	    }),
+	    'lock-secret' => get_standard_option('pve-sdn-lock-secret'),
 	},
     },
     returns => { type => 'null' },
@@ -347,6 +354,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 {
 	    my $cfg = PVE::Network::SDN::Zones::config();
@@ -360,7 +368,7 @@ __PACKAGE__->register_method ({
 	    delete $cfg->{ids}->{$id};
 
 	    PVE::Network::SDN::Zones::write_config($cfg);
-	}, "delete sdn zone object failed");
+	}, "delete sdn zone object failed", $lock_secret);
 
 	return;
     }});
diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 3b3ea67..08c8700 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -52,6 +52,12 @@ PVE::Cluster::cfs_register_file(
     sub { my ($filename, $data) = @_; return $data; }
 );
 
+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 {
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-network 3/5] api: add lock secret parameter to apply endpoint

[Stefan Hanreich]

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.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN.pm | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index d216e48..b09a54b 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;
@@ -110,6 +110,15 @@ __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',
@@ -120,7 +129,17 @@ __PACKAGE__->register_method ({
         my $rpcenv = PVE::RPCEnvironment::get();
         my $authuser = $rpcenv->get_user();
 
-	PVE::Network::SDN::commit_config();
+	my $lock_secret = extract_param($param, 'lock-secret');
+	my $release_lock = extract_param($param, 'release-lock');
+
+	PVE::Network::SDN::lock_sdn_config(
+	    sub {
+		PVE::Network::SDN::commit_config();
+		PVE::Network::SDN::delete_global_lock() if $lock_secret && $release_lock;
+	    },
+	    "could not commit SDN config",
+	    $lock_secret
+	);
 
         my $code = sub {
             $rpcenv->{type} = 'priv'; # to start tasks in background
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-network 4/5] api: add lock and release endpoints for global configuration lock

[Stefan Hanreich]

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

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN.pm | 81 +++++++++++++++++++++++++++++++++++++
 src/PVE/Network/SDN.pm      | 23 +++++++++++
 2 files changed, 104 insertions(+)

diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index b09a54b..3377f76 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -99,6 +99,87 @@ my $create_reload_network_worker = sub {
     return $res->{pid};
 };
 
+__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,
diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 08c8700..0f6754a 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -173,6 +173,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

[pve-devel] [PATCH pve-network 5/5] api: add rollback endpoint

[Stefan Hanreich]

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.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN.pm | 49 +++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index 3377f76..0f1c547 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -180,6 +180,55 @@ __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});
+
+	    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,
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel