[pve-devel] applied-series: [PATCH network 0/7] PUT API endpoint changes

[pve-devel] applied-series: [PATCH network 0/7] PUT API endpoint changes

[Wolfgang Bumiller]

This is a breaking API change for zones, vnets and subnets!

These previously expected the *complete* config, which is not how our
usual SectionConfigs work.

Further, they advertised a 'delete' API parameter which was simply
passed through as a config property which of course failed validation.

Instead, they now merge the provided changes the way we normally do, and
handle the 'delete' parameter.

This affects:
- /cluster/sdn/zones/<id>
- /cluster/sdn/vnets/<id>
- /cluster/sdn/vnets/<id>/subnets/<subnet>

Additionally, these already took partial updates, but ignored the
'delete' parameter, which is fixed in this series:
- /cluster/sdn/ipams/<id>
- /cluster/sdn/dns/<id>
- /cluster/sdn/controllers/<id>

Wolfgang Bumiller (7):
  Revert "sdn: require ipam in simple plugin for dhcp"
  api: take partial configs for PUT /cluster/sdn/zones/<id>
  api: take partial configs for PUT /cluster/sdn/vnets/<id>
  api: take partial configs for PUT /cluster/sdn/vnets/<n>/subnets/<i>
  api: handle delete parameter when updating ipams
  api: handle delete parameter when updating dns entries
  api: handle delete parameter when updating controllers

 src/PVE/API2/Network/SDN/Controllers.pm   |  7 +++++++
 src/PVE/API2/Network/SDN/Dns.pm           |  7 +++++++
 src/PVE/API2/Network/SDN/Ipams.pm         |  7 +++++++
 src/PVE/API2/Network/SDN/Subnets.pm       | 12 ++++++++++--
 src/PVE/API2/Network/SDN/Vnets.pm         | 24 +++++++++++++++++++----
 src/PVE/API2/Network/SDN/Zones.pm         | 18 ++++++++++++++---
 src/PVE/Network/SDN/Zones/SimplePlugin.pm |  1 -
 7 files changed, 66 insertions(+), 10 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH network 1/7] Revert "sdn: require ipam in simple plugin for dhcp"

[Wolfgang Bumiller]

This reverts commit 53ab1495621f46c8af4dc560905f7e501bee75a7.

This also affects the updateSchema which is not intentional, since the
update API calls are supposed to take changes, not full replacements.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/Network/SDN/Zones/SimplePlugin.pm | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Zones/SimplePlugin.pm b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
index 7cb65c2..c996bf3 100644
--- a/src/PVE/Network/SDN/Zones/SimplePlugin.pm
+++ b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
@@ -32,7 +32,6 @@ sub properties {
 	    description => 'Type of the DHCP backend for this zone',
 	    type => 'string',
 	    enum => PVE::Network::SDN::Dhcp::Plugin->lookup_types(),
-	    requires => 'ipam',
 	},
     };
 }
-- 
2.39.2

[pve-devel] [PATCH network 2/7] api: take partial configs for PUT /cluster/sdn/zones/<id>

[Wolfgang Bumiller]

Zones previously expected a complete config, but the API schema
also contains a 'delete' parameter via the SectionConfig's
updateSchema() helper. This was not handled, and instead failed to
validate as part of the config.

The same is true for vnets and subnets, while ipams, dns and
controller entries followed our usual update procedures (but also
ignored the 'delete' parameter).

Since all of our SectionConfig based API endpoints are supposed to
take changes, rather than complete configs, this changes these
endpoints to not replace the full configuration anymore.

This is a major break for automation tools (the web UI already passed
the full config each time).

Cc: Alexandre Derumier <aderumier@odiso.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/API2/Network/SDN/Zones.pm | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/PVE/API2/Network/SDN/Zones.pm b/src/PVE/API2/Network/SDN/Zones.pm
index 1c3356e..b09c9ad 100644
--- a/src/PVE/API2/Network/SDN/Zones.pm
+++ b/src/PVE/API2/Network/SDN/Zones.pm
@@ -261,6 +261,11 @@ __PACKAGE__->register_method ({
 
 	my $id = extract_param($param, 'zone');
 	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
+
+	if ($delete) {
+	    $delete = [ PVE::Tools::split_list($delete) ];
+	}
 
 	PVE::Network::SDN::lock_sdn_config(sub {
 	    my $zone_cfg = PVE::Network::SDN::Zones::config();
@@ -274,8 +279,17 @@ __PACKAGE__->register_method ({
 	    my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($scfg->{type});
 	    my $opts = $plugin->check_config($id, $param, 0, 1);
 
-	    if ($opts->{ipam} && !$scfg->{ipam} || $opts->{ipam} ne $scfg->{ipam}) {
+	    my $old_ipam = $scfg->{ipam};
+
+	    if ($delete) {
+		my $options = $plugin->private()->{options}->{$scfg->{type}};
+		PVE::SectionConfig::delete_from_config($scfg, $options, $opts, $delete);
+	    }
 
+	    $scfg->{$_} = $opts->{$_} for keys $opts->%*;
+
+	    my $new_ipam = $scfg->{ipam};
+	    if (!$new_ipam != !$old_ipam || (($new_ipam//'') ne ($old_ipam//''))) {
 		# don't allow ipam change if subnet are defined for now, need to implement resync ipam content
 		my $subnets_cfg = PVE::Network::SDN::Subnets::config();
 		for my $subnetid (sort keys %{$subnets_cfg->{ids}}) {
@@ -285,8 +299,6 @@ __PACKAGE__->register_method ({
 		}
 	    }
 
-	    $zone_cfg->{ids}->{$id} = $opts;
-
 	    my $dnsserver = $opts->{dns};
 	    raise_param_exc({ dns => "$dnsserver don't exist"}) if $dnsserver && !$dns_cfg->{ids}->{$dnsserver};
 
-- 
2.39.2

[pve-devel] [PATCH network 3/7] api: take partial configs for PUT /cluster/sdn/vnets/<id>

[Wolfgang Bumiller]

Handle 'delete' parameter and partial updates.
See previous commit for explanation.

Cc: Alexandre Derumier <aderumier@odiso.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/API2/Network/SDN/Vnets.pm | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/PVE/API2/Network/SDN/Vnets.pm b/src/PVE/API2/Network/SDN/Vnets.pm
index a32df8c..57de295 100644
--- a/src/PVE/API2/Network/SDN/Vnets.pm
+++ b/src/PVE/API2/Network/SDN/Vnets.pm
@@ -244,21 +244,37 @@ __PACKAGE__->register_method ({
 
 	my $id = extract_param($param, 'vnet');
 	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
 
 	my $privs = [ 'SDN.Allocate' ];
 	&$check_vnet_access($id, $privs);
 
+	if ($delete) {
+	    $delete = [ PVE::Tools::split_list($delete) ];
+	}
+
 	PVE::Network::SDN::lock_sdn_config(sub {
 	    my $cfg = PVE::Network::SDN::Vnets::config();
 
 	    PVE::SectionConfig::assert_if_modified($cfg, $digest);
 
 	    my $opts = PVE::Network::SDN::VnetPlugin->check_config($id, $param, 0, 1);
-	    raise_param_exc({ zone => "missing zone"}) if !$opts->{zone};
-	    my $subnets = PVE::Network::SDN::Vnets::get_subnets($id);
-	    raise_param_exc({ zone => "can't change zone if subnets exists"}) if($subnets && $opts->{zone} ne $cfg->{ids}->{$id}->{zone});
 
-	    $cfg->{ids}->{$id} = $opts;
+	    my $data = $cfg->{ids}->{$id};
+	    my $old_zone = $data->{zone};
+
+	    if ($delete) {
+		my $options = PVE::Network::SDN::VnetPlugin->private()->{options}->{$data->{type}};
+		PVE::SectionConfig::delete_from_config($data, $options, $opts, $delete);
+	    }
+
+	    $data->{$_} = $opts->{$_} for keys $opts->%*;
+
+	    my $new_zone = $data->{zone};
+	    raise_param_exc({ zone => "cannot delete zone"}) if !$new_zone;
+	    my $subnets = PVE::Network::SDN::Vnets::get_subnets($id);
+	    raise_param_exc({ zone => "can't change zone if subnets exist"})
+		if $subnets && $old_zone ne $new_zone;
 
 	    my $zone_cfg = PVE::Network::SDN::Zones::config();
 	    my $zoneid = $cfg->{ids}->{$id}->{zone};
-- 
2.39.2

[pve-devel] [PATCH network 4/7] api: take partial configs for PUT /cluster/sdn/vnets/<n>/subnets/<i>

[Wolfgang Bumiller]

Handle 'delete' parameter and partial updates.
See 2 commits earlier for explanation.

Cc: Alexandre Derumier <aderumier@odiso.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/API2/Network/SDN/Subnets.pm | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/Network/SDN/Subnets.pm b/src/PVE/API2/Network/SDN/Subnets.pm
index c263cd5..7a4c331 100644
--- a/src/PVE/API2/Network/SDN/Subnets.pm
+++ b/src/PVE/API2/Network/SDN/Subnets.pm
@@ -247,6 +247,8 @@ __PACKAGE__->register_method ({
 
 	my $id = extract_param($param, 'subnet');
 	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
+
 	my $vnet = $param->{vnet};
 
 	my $privs = [ 'SDN.Allocate' ];
@@ -266,9 +268,15 @@ __PACKAGE__->register_method ({
 	    PVE::SectionConfig::assert_if_modified($cfg, $digest);
 
 	    my $opts = PVE::Network::SDN::SubnetPlugin->check_config($id, $param, 0, 1);
-	    $cfg->{ids}->{$id} = $opts;
 
-	    raise_param_exc({ ipam => "you can't change ipam"}) if $opts->{ipam} && $scfg->{ipam} && $opts->{ipam} ne $scfg->{ipam};
+	    my $data = $cfg->{ids}->{$id};
+	    if ($delete) {
+		$delete = [ PVE::Tools::split_list($delete) ];
+		my $options =
+		    PVE::Network::SDN::SubnetPlugin->private()->{options}->{$data->{type}};
+		PVE::SectionConfig::delete_from_config($data, $options, $opts, $delete);
+	    }
+	    $data->{$_} = $opts->{$_} for keys $opts->%*;
 
 	    my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($cfg, $id);
 	    PVE::Network::SDN::SubnetPlugin->on_update_hook($zone, $id, $subnet, $scfg);
-- 
2.39.2

[pve-devel] [PATCH network 5/7] api: handle delete parameter when updating ipams

[Wolfgang Bumiller]

this is for completeness, currently no plugin has optional
properties...

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/API2/Network/SDN/Ipams.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/PVE/API2/Network/SDN/Ipams.pm b/src/PVE/API2/Network/SDN/Ipams.pm
index d6e0bc8..27ead02 100644
--- a/src/PVE/API2/Network/SDN/Ipams.pm
+++ b/src/PVE/API2/Network/SDN/Ipams.pm
@@ -180,6 +180,7 @@ __PACKAGE__->register_method ({
 
 	my $id = extract_param($param, 'ipam');
 	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
 
         PVE::Network::SDN::lock_sdn_config(
 	 sub {
@@ -193,6 +194,12 @@ __PACKAGE__->register_method ({
 	    my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($scfg->{type});
 	    my $opts = $plugin->check_config($id, $param, 0, 1);
 
+	    if ($delete) {
+		$delete = [ PVE::Tools::split_list($delete) ];
+		my $options = $plugin->private()->{options}->{$scfg->{type}};
+		PVE::SectionConfig::delete_from_config($scfg, $options, $opts, $delete);
+	    }
+
 	    foreach my $k (%$opts) {
 		$scfg->{$k} = $opts->{$k};
 	    }
-- 
2.39.2

[pve-devel] [PATCH network 6/7] api: handle delete parameter when updating dns entries

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/API2/Network/SDN/Dns.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/PVE/API2/Network/SDN/Dns.pm b/src/PVE/API2/Network/SDN/Dns.pm
index 3d08552..826d111 100644
--- a/src/PVE/API2/Network/SDN/Dns.pm
+++ b/src/PVE/API2/Network/SDN/Dns.pm
@@ -173,6 +173,7 @@ __PACKAGE__->register_method ({
 
 	my $id = extract_param($param, 'dns');
 	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
 
         PVE::Network::SDN::lock_sdn_config(
 	 sub {
@@ -186,6 +187,12 @@ __PACKAGE__->register_method ({
 	    my $plugin = PVE::Network::SDN::Dns::Plugin->lookup($scfg->{type});
 	    my $opts = $plugin->check_config($id, $param, 0, 1);
 
+	    if ($delete) {
+		$delete = [ PVE::Tools::split_list($delete) ];
+		my $options = $plugin->private()->{options}->{$scfg->{type}};
+		PVE::SectionConfig::delete_from_config($scfg, $options, $opts, $delete);
+	    }
+
 	    foreach my $k (%$opts) {
 		$scfg->{$k} = $opts->{$k};
 	    }
-- 
2.39.2

[pve-devel] [PATCH network 7/7] api: handle delete parameter when updating controllers

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/API2/Network/SDN/Controllers.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/PVE/API2/Network/SDN/Controllers.pm b/src/PVE/API2/Network/SDN/Controllers.pm
index d8f18ab..0540a65 100644
--- a/src/PVE/API2/Network/SDN/Controllers.pm
+++ b/src/PVE/API2/Network/SDN/Controllers.pm
@@ -215,6 +215,7 @@ __PACKAGE__->register_method ({
 
 	my $id = extract_param($param, 'controller');
 	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
 
         PVE::Network::SDN::lock_sdn_config(
 	 sub {
@@ -228,6 +229,12 @@ __PACKAGE__->register_method ({
 	    my $plugin = PVE::Network::SDN::Controllers::Plugin->lookup($scfg->{type});
 	    my $opts = $plugin->check_config($id, $param, 0, 1);
 
+	    if ($delete) {
+		$delete = [ PVE::Tools::split_list($delete) ];
+		my $options = $plugin->private()->{options}->{$scfg->{type}};
+		PVE::SectionConfig::delete_from_config($scfg, $options, $opts, $delete);
+	    }
+
 	    foreach my $k (%$opts) {
 		$scfg->{$k} = $opts->{$k};
 	    }
-- 
2.39.2

Re: [pve-devel] applied-series: [PATCH network 0/7] PUT API endpoint changes

[Stefan Hanreich]

On 11/22/23 12:53, Wolfgang Bumiller wrote:
> This is a breaking API change for zones, vnets and subnets!
> 
> These previously expected the *complete* config, which is not how our
> usual SectionConfigs work.
> 
> Further, they advertised a 'delete' API parameter which was simply
> passed through as a config property which of course failed validation.
> 
> Instead, they now merge the provided changes the way we normally do, and
> handle the 'delete' parameter.

Will have to check now whether this affects the UI in any way. I fear it
does. Will try to provide respective pve-manager patches ASAP.