public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH network/manager 0/2] Refactor IPAM API methods
@ 2023-11-20 16:28 Stefan Hanreich
  2023-11-20 16:28 ` [pve-devel] [PATCH pve-network 1/2] api: refactor URL structure for Ipam Stefan Hanreich
  2023-11-20 16:28 ` [pve-devel] [PATCH pve-manager 2/2] sdn: Update IPAM API endpoints Stefan Hanreich
  0 siblings, 2 replies; 5+ messages in thread
From: Stefan Hanreich @ 2023-11-20 16:28 UTC (permalink / raw)
  To: pve-devel

Moving the API method for the IPAM index to /ipams/pve/status which seems
better suited as it returns the current state of the PVE IPAM. This could be
then extended further to return the current status of other IPAM plugins,
although it is probably preferrable to use the respective API / UI of the IPAM
for retrieving its current status rather than using the PVE API.

Additionally moved the IPAM Mapping create / update / delete endpoints to 
/vnets/{vnetid}/ips since they are part of the VNet and those methods
not only manage IPAM related state but also stuff like DNS plugin state.

Currently I did not add a links relation to the endpoint, since there is no
GET endpoint that would retrieve a single mapping from the API. I did not find
any other relations present in our current API. If we move to displaying the
state of the IPAM to the Zone panel, then we could move the Index as well as
the create / update / delete methods to the Zone endpoint and then it would be
possible to show a proper child relation.



pve-network:

Stefan Hanreich (1):
  api: refactor URL structure for Ipam

 src/PVE/API2/Network/SDN.pm                  |  6 --
 src/PVE/API2/Network/SDN/Ipams.pm            | 83 +++++++++++++++++
 src/PVE/API2/Network/SDN/{Ipam.pm => Ips.pm} | 97 ++------------------
 src/PVE/API2/Network/SDN/Makefile            |  2 +-
 src/PVE/API2/Network/SDN/Vnets.pm            |  6 ++
 5 files changed, 100 insertions(+), 94 deletions(-)
 rename src/PVE/API2/Network/SDN/{Ipam.pm => Ips.pm} (58%)


pve-manager:

Stefan Hanreich (1):
  sdn: Update IPAM API endpoints

 www/manager6/sdn/IpamEdit.js  |  4 +++-
 www/manager6/tree/DhcpTree.js | 15 +++++++++++----
 2 files changed, 14 insertions(+), 5 deletions(-)


Summary over all repositories:
  7 files changed, 114 insertions(+), 99 deletions(-)

-- 
murpp v0.4.0




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH pve-network 1/2] api: refactor URL structure for Ipam
  2023-11-20 16:28 [pve-devel] [PATCH network/manager 0/2] Refactor IPAM API methods Stefan Hanreich
@ 2023-11-20 16:28 ` Stefan Hanreich
  2023-11-20 16:43   ` [pve-devel] applied: " Thomas Lamprecht
  2023-11-20 16:28 ` [pve-devel] [PATCH pve-manager 2/2] sdn: Update IPAM API endpoints Stefan Hanreich
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Hanreich @ 2023-11-20 16:28 UTC (permalink / raw)
  To: pve-devel

The initial URL structure was less than optimal due to Ipam as well as
Ipams being endpoints in the API, which are too similar and might be
confusing to users.

Move the listing of PVE IPAM to /ipams/pve/status
Move the create / update / delete endpoints to /vnets/{vnetid}/ips

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN.pm                  |  6 --
 src/PVE/API2/Network/SDN/Ipams.pm            | 83 +++++++++++++++++
 src/PVE/API2/Network/SDN/{Ipam.pm => Ips.pm} | 97 ++------------------
 src/PVE/API2/Network/SDN/Makefile            |  2 +-
 src/PVE/API2/Network/SDN/Vnets.pm            |  6 ++
 5 files changed, 100 insertions(+), 94 deletions(-)
 rename src/PVE/API2/Network/SDN/{Ipam.pm => Ips.pm} (58%)

diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index 551afcf..d216e48 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -15,7 +15,6 @@ use PVE::Network::SDN;
 use PVE::API2::Network::SDN::Controllers;
 use PVE::API2::Network::SDN::Vnets;
 use PVE::API2::Network::SDN::Zones;
-use PVE::API2::Network::SDN::Ipam;
 use PVE::API2::Network::SDN::Ipams;
 use PVE::API2::Network::SDN::Dns;
 
@@ -36,11 +35,6 @@ __PACKAGE__->register_method ({
     path => 'controllers',
 });
 
-__PACKAGE__->register_method ({
-    subclass => "PVE::API2::Network::SDN::Ipam",
-    path => 'ipam',
-});
-
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::Network::SDN::Ipams",
     path => 'ipams',
diff --git a/src/PVE/API2/Network/SDN/Ipams.pm b/src/PVE/API2/Network/SDN/Ipams.pm
index 6410e8e..d6e0bc8 100644
--- a/src/PVE/API2/Network/SDN/Ipams.pm
+++ b/src/PVE/API2/Network/SDN/Ipams.pm
@@ -12,6 +12,9 @@ use PVE::Network::SDN::Ipams::Plugin;
 use PVE::Network::SDN::Ipams::PVEPlugin;
 use PVE::Network::SDN::Ipams::PhpIpamPlugin;
 use PVE::Network::SDN::Ipams::NetboxPlugin;
+use PVE::Network::SDN::Dhcp;
+use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Zones;
 
 use Storable qw(dclone);
 use PVE::JSONSchema qw(get_standard_option);
@@ -245,4 +248,84 @@ __PACKAGE__->register_method ({
 	return undef;
     }});
 
+__PACKAGE__->register_method ({
+    name => 'ipamindex',
+    path => '{ipam}/status',
+    method => 'GET',
+    description => 'List PVE IPAM Entries',
+    protected => 1,
+    permissions => {
+	description => "Only list entries where you have 'SDN.Audit' or 'SDN.Allocate' permissions on '/sdn/zones/<zone>/<vnet>'",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    ipam => get_standard_option('pve-sdn-ipam-id', {
+                completion => \&PVE::Network::SDN::Ipams::complete_sdn_ipams,
+            }),
+	},
+    },
+    returns => {
+	type => 'array',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $id = extract_param($param, 'ipam');
+	die "Currently only PVE IPAM is supported!" if $id ne 'pve';
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];
+
+	my $ipam_plugin = PVE::Network::SDN::Ipams::Plugin->lookup('pve');
+	my $ipam_db = $ipam_plugin->read_db();
+
+	my $result = [];
+
+	for my $zone_id (keys %{$ipam_db->{zones}}) {
+	    my $zone_config = PVE::Network::SDN::Zones::get_zone($zone_id, 1);
+            next if !$zone_config || $zone_config->{ipam} ne 'pve' || !$zone_config->{dhcp};
+
+	    my $zone = $ipam_db->{zones}->{$zone_id};
+
+	    my $vnets = PVE::Network::SDN::Zones::get_vnets($zone_id, 1);
+
+	    for my $subnet_cidr (keys %{$zone->{subnets}}) {
+		my $subnet = $zone->{subnets}->{$subnet_cidr};
+		my $ip = new NetAddr::IP($subnet_cidr) or die 'Found invalid CIDR in IPAM';
+
+		my $vnet = undef;
+		for my $vnet_id (keys %$vnets) {
+		    eval {
+			my ($zone, $subnetid, $subnet_cfg, $ip) = PVE::Network::SDN::Vnets::get_subnet_from_vnet_ip(
+			    $vnet_id,
+			    $ip->addr,
+			);
+
+			$vnet = $subnet_cfg->{vnet};
+		    };
+
+		    last if $vnet;
+		}
+
+		next if !$vnet || !$rpcenv->check_any($authuser, "/sdn/zones/$zone_id/$vnet", $privs, 1);
+
+		for my $ip (keys %{$subnet->{ips}}) {
+		    my $entry = $subnet->{ips}->{$ip};
+		    $entry->{zone} = $zone_id;
+		    $entry->{subnet} = $subnet_cidr;
+		    $entry->{ip} = $ip;
+		    $entry->{vnet} = $vnet;
+
+		    push @$result, $entry;
+		}
+	    }
+	}
+
+	return $result;
+    },
+});
+
 1;
diff --git a/src/PVE/API2/Network/SDN/Ipam.pm b/src/PVE/API2/Network/SDN/Ips.pm
similarity index 58%
rename from src/PVE/API2/Network/SDN/Ipam.pm
rename to src/PVE/API2/Network/SDN/Ips.pm
index e71ca7d..31dec04 100644
--- a/src/PVE/API2/Network/SDN/Ipam.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -1,100 +1,23 @@
-package PVE::API2::Network::SDN::Ipam;
+package PVE::API2::Network::SDN::Ips;
 
 use strict;
 use warnings;
 
 use PVE::Tools qw(extract_param);
-use PVE::Cluster qw(cfs_read_file cfs_write_file);
 
-use PVE::Network::SDN;
-use PVE::Network::SDN::Dhcp;
 use PVE::Network::SDN::Vnets;
-use PVE::Network::SDN::Ipams::Plugin;
+use PVE::Network::SDN::Dhcp;
 
 use PVE::JSONSchema qw(get_standard_option);
-use PVE::RPCEnvironment;
-
 use PVE::RESTHandler;
 
 use base qw(PVE::RESTHandler);
 
 __PACKAGE__->register_method ({
-    name => 'ipamindex',
+    name => 'ipdelete',
     path => '',
-    method => 'GET',
-    description => 'List PVE IPAM Entries',
-    protected => 1,
-    permissions => {
-	description => "Only list entries where you have 'SDN.Audit' or 'SDN.Allocate' permissions on '/sdn/zones/<zone>/<vnet>'",
-	user => 'all',
-    },
-    parameters => {
-	additionalProperties => 0,
-    },
-    returns => {
-	type => 'array',
-    },
-    code => sub {
-	my ($param) = @_;
-
-	my $rpcenv = PVE::RPCEnvironment::get();
-	my $authuser = $rpcenv->get_user();
-	my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];
-
-	my $ipam_plugin = PVE::Network::SDN::Ipams::Plugin->lookup('pve');
-	my $ipam_db = $ipam_plugin->read_db();
-
-	my $result = [];
-
-	for my $zone_id (keys %{$ipam_db->{zones}}) {
-	    my $zone_config = PVE::Network::SDN::Zones::get_zone($zone_id, 1);
-            next if !$zone_config || $zone_config->{ipam} ne 'pve' || !$zone_config->{dhcp};
-
-	    my $zone = $ipam_db->{zones}->{$zone_id};
-
-	    my $vnets = PVE::Network::SDN::Zones::get_vnets($zone_id, 1);
-
-	    for my $subnet_cidr (keys %{$zone->{subnets}}) {
-		my $subnet = $zone->{subnets}->{$subnet_cidr};
-		my $ip = new NetAddr::IP($subnet_cidr) or die 'Found invalid CIDR in IPAM';
-
-		my $vnet = undef;
-		for my $vnet_id (keys %$vnets) {
-		    eval {
-			my ($zone, $subnetid, $subnet_cfg, $ip) = PVE::Network::SDN::Vnets::get_subnet_from_vnet_ip(
-			    $vnet_id,
-			    $ip->addr,
-			);
-
-			$vnet = $subnet_cfg->{vnet};
-		    };
-
-		    last if $vnet;
-		}
-
-		next if !$vnet || !$rpcenv->check_any($authuser, "/sdn/zones/$zone_id/$vnet", $privs, 1);
-
-		for my $ip (keys %{$subnet->{ips}}) {
-		    my $entry = $subnet->{ips}->{$ip};
-		    $entry->{zone} = $zone_id;
-		    $entry->{subnet} = $subnet_cidr;
-		    $entry->{ip} = $ip;
-		    $entry->{vnet} = $vnet;
-
-		    push @$result, $entry;
-		}
-	    }
-	}
-
-	return $result;
-    },
-});
-
-__PACKAGE__->register_method ({
-    name => 'dhcpdelete',
-    path => '{zone}/{vnet}/{mac}',
     method => 'DELETE',
-    description => 'Delete DHCP Mappings in a VNet for a MAC address',
+    description => 'Delete IP Mappings in a VNet',
     protected => 1,
     permissions => {
 	check => ['perm', '/sdn/zones/{zone}/{vnet}', [ 'SDN.Allocate' ]],
@@ -129,10 +52,10 @@ __PACKAGE__->register_method ({
 });
 
 __PACKAGE__->register_method ({
-    name => 'dhcpcreate',
-    path => '{zone}/{vnet}/{mac}',
+    name => 'ipcreate',
+    path => '',
     method => 'POST',
-    description => 'Create DHCP Mapping',
+    description => 'Create IP Mapping in a VNet',
     protected => 1,
     permissions => {
 	check => ['perm', '/sdn/zones/{zone}/{vnet}', [ 'SDN.Allocate' ]],
@@ -165,10 +88,10 @@ __PACKAGE__->register_method ({
     },
 });
 __PACKAGE__->register_method ({
-    name => 'dhcpupdate',
-    path => '{zone}/{vnet}/{mac}',
+    name => 'ipupdate',
+    path => '',
     method => 'PUT',
-    description => 'Update DHCP Mapping',
+    description => 'Update IP Mapping in a VNet',
     protected => 1,
     permissions => {
 	check => ['perm', '/sdn/zones/{zone}/{vnet}', [ 'SDN.Allocate' ]],
diff --git a/src/PVE/API2/Network/SDN/Makefile b/src/PVE/API2/Network/SDN/Makefile
index 2480c09..abd1bfa 100644
--- a/src/PVE/API2/Network/SDN/Makefile
+++ b/src/PVE/API2/Network/SDN/Makefile
@@ -1,4 +1,4 @@
-SOURCES=Vnets.pm Zones.pm Controllers.pm Subnets.pm Ipams.pm Ipam.pm Dns.pm
+SOURCES=Vnets.pm Zones.pm Controllers.pm Subnets.pm Ipams.pm Dns.pm Ips.pm
 
 
 PERL5DIR=${DESTDIR}/usr/share/perl5
diff --git a/src/PVE/API2/Network/SDN/Vnets.pm b/src/PVE/API2/Network/SDN/Vnets.pm
index 864dc4a..a32df8c 100644
--- a/src/PVE/API2/Network/SDN/Vnets.pm
+++ b/src/PVE/API2/Network/SDN/Vnets.pm
@@ -13,6 +13,7 @@ use PVE::Network::SDN::Vnets;
 use PVE::Network::SDN::VnetPlugin;
 use PVE::Network::SDN::Subnets;
 use PVE::API2::Network::SDN::Subnets;
+use PVE::API2::Network::SDN::Ips;
 
 use Storable qw(dclone);
 use PVE::JSONSchema qw(get_standard_option);
@@ -28,6 +29,11 @@ __PACKAGE__->register_method ({
     path => '{vnet}/subnets',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Network::SDN::Ips",
+    path => '{vnet}/ips',
+});
+
 my $api_sdn_vnets_config = sub {
     my ($cfg, $id) = @_;
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH pve-manager 2/2] sdn: Update IPAM API endpoints
  2023-11-20 16:28 [pve-devel] [PATCH network/manager 0/2] Refactor IPAM API methods Stefan Hanreich
  2023-11-20 16:28 ` [pve-devel] [PATCH pve-network 1/2] api: refactor URL structure for Ipam Stefan Hanreich
@ 2023-11-20 16:28 ` Stefan Hanreich
  2023-11-21 21:14   ` [pve-devel] applied: " Thomas Lamprecht
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Hanreich @ 2023-11-20 16:28 UTC (permalink / raw)
  To: pve-devel

The IPAM-related API endpoints were moved, reflect those changes in
the UI as well.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/sdn/IpamEdit.js  |  4 +++-
 www/manager6/tree/DhcpTree.js | 15 +++++++++++----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/www/manager6/sdn/IpamEdit.js b/www/manager6/sdn/IpamEdit.js
index 18e22c592..73e5d2e1a 100644
--- a/www/manager6/sdn/IpamEdit.js
+++ b/www/manager6/sdn/IpamEdit.js
@@ -52,8 +52,10 @@ Ext.define('PVE.sdn.IpamEdit', {
     isCreate: false,
     mapping: {},
 
+    url: '/cluster/sdn/vnets',
+
     submitUrl: function(url, values) {
-	return `${url}/${values.zone}/${values.vnet}/${values.mac}`;
+	return `${url}/${values.vnet}/ips`;
     },
 
     initComponent: function() {
diff --git a/www/manager6/tree/DhcpTree.js b/www/manager6/tree/DhcpTree.js
index ca279c29a..b7baba606 100644
--- a/www/manager6/tree/DhcpTree.js
+++ b/www/manager6/tree/DhcpTree.js
@@ -17,7 +17,7 @@ Ext.define('PVE.sdn.DhcpTree', {
 	    let me = this;
 
 	    Proxmox.Utils.API2Request({
-		url: `/cluster/sdn/ipam`,
+		url: `/cluster/sdn/ipams/pve/status`,
 		method: 'GET',
 		success: function(response, opts) {
 		    let root = {
@@ -105,8 +105,17 @@ Ext.define('PVE.sdn.DhcpTree', {
 		        return;
 		    }
 
+		    let params = {
+			zone: data.zone,
+			mac: data.mac,
+		    };
+
+		    let encodedParams = Ext.Object.toQueryString(params);
+
+		    let url = `/cluster/sdn/vnets/${data.vnet}/ips?${encodedParams}`;
+
 		    Proxmox.Utils.API2Request({
-			url: `/cluster/sdn/ipam/${data.zone}/${data.vnet}/${data.mac}`,
+			url,
 			method: 'DELETE',
 			waitMsgTarget: view,
 			failure: function(response, opts) {
@@ -149,7 +158,6 @@ Ext.define('PVE.sdn.DhcpTree', {
 	    Ext.create('PVE.sdn.IpamEdit', {
 		autoShow: true,
 		mapping: data,
-		url: `/cluster/sdn/ipam`,
 		extraRequestParams: {
 		    vmid: data.vmid,
 		    mac: data.mac,
@@ -217,7 +225,6 @@ Ext.define('PVE.sdn.DhcpTree', {
 			Ext.create('PVE.sdn.IpamEdit', {
 			    autoShow: true,
 			    mapping: {},
-			    url: `/cluster/sdn/ipam`,
 			    isCreate: true,
 			    extraRequestParams: {
 				vnet: data.name,
-- 
2.39.2




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] applied: [PATCH pve-network 1/2] api: refactor URL structure for Ipam
  2023-11-20 16:28 ` [pve-devel] [PATCH pve-network 1/2] api: refactor URL structure for Ipam Stefan Hanreich
@ 2023-11-20 16:43   ` Thomas Lamprecht
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2023-11-20 16:43 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Am 20/11/2023 um 17:28 schrieb Stefan Hanreich:
> The initial URL structure was less than optimal due to Ipam as well as
> Ipams being endpoints in the API, which are too similar and might be
> confusing to users.
> 
> Move the listing of PVE IPAM to /ipams/pve/status
> Move the create / update / delete endpoints to /vnets/{vnetid}/ips
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  src/PVE/API2/Network/SDN.pm                  |  6 --
>  src/PVE/API2/Network/SDN/Ipams.pm            | 83 +++++++++++++++++
>  src/PVE/API2/Network/SDN/{Ipam.pm => Ips.pm} | 97 ++------------------
>  src/PVE/API2/Network/SDN/Makefile            |  2 +-
>  src/PVE/API2/Network/SDN/Vnets.pm            |  6 ++
>  5 files changed, 100 insertions(+), 94 deletions(-)
>  rename src/PVE/API2/Network/SDN/{Ipam.pm => Ips.pm} (58%)
> 
>

applied, this one, thanks!




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] applied: [PATCH pve-manager 2/2] sdn: Update IPAM API endpoints
  2023-11-20 16:28 ` [pve-devel] [PATCH pve-manager 2/2] sdn: Update IPAM API endpoints Stefan Hanreich
@ 2023-11-21 21:14   ` Thomas Lamprecht
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2023-11-21 21:14 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Am 20/11/2023 um 17:28 schrieb Stefan Hanreich:
> The IPAM-related API endpoints were moved, reflect those changes in
> the UI as well.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  www/manager6/sdn/IpamEdit.js  |  4 +++-
>  www/manager6/tree/DhcpTree.js | 15 +++++++++++----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
>

applied, thanks!




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-11-21 21:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-20 16:28 [pve-devel] [PATCH network/manager 0/2] Refactor IPAM API methods Stefan Hanreich
2023-11-20 16:28 ` [pve-devel] [PATCH pve-network 1/2] api: refactor URL structure for Ipam Stefan Hanreich
2023-11-20 16:43   ` [pve-devel] applied: " Thomas Lamprecht
2023-11-20 16:28 ` [pve-devel] [PATCH pve-manager 2/2] sdn: Update IPAM API endpoints Stefan Hanreich
2023-11-21 21:14   ` [pve-devel] applied: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal