public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Alexandre Derumier <aderumier@odiso.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [pve-network 4/4] subnets/ipam : fix is_gateway
Date: Fri,  4 Jun 2021 13:25:00 +0200	[thread overview]
Message-ID: <20210604112500.2349926-5-aderumier@odiso.com> (raw)
In-Reply-To: <20210604112500.2349926-1-aderumier@odiso.com>

- add lost is_gateway in subnets subnet when creating subnet
- allow reuse ip as gateway in subnet create if it's already flagged gateway in the ipamdb
- add tests
---
 PVE/Network/SDN/Ipams/NetboxPlugin.pm         | 14 +++++++++++++-
 PVE/Network/SDN/Ipams/PVEPlugin.pm            |  4 ++--
 PVE/Network/SDN/Ipams/PhpIpamPlugin.pm        | 18 +++++++++++++++---
 PVE/Network/SDN/SubnetPlugin.pm               |  2 +-
 PVE/Network/SDN/Subnets.pm                    |  4 ++--
 test/ipams/netbox/expected.add_ip_notgateway  |  9 +++++++++
 test/ipams/phpipam/expected.add_ip_notgateway | 12 ++++++++++++
 test/run_test_ipams.pl                        | 18 +++++++++++++++++-
 test/run_test_subnets.pl                      |  8 ++++----
 9 files changed, 75 insertions(+), 14 deletions(-)
 create mode 100644 test/ipams/netbox/expected.add_ip_notgateway
 create mode 100644 test/ipams/phpipam/expected.add_ip_notgateway

diff --git a/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 5a03f39..f0e7168 100644
--- a/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -93,7 +93,11 @@ sub add_ip {
     };
 
     if ($@) {
-	die "error add subnet ip to ipam: ip already exist: $@" if !$noerr;
+        if($is_gateway) {
+           die "error add subnet ip to ipam: ip $ip already exist: $@" if !is_ip_gateway($url, $ip, $headers) && !$noerr;
+        } else {
+	    die "error add subnet ip to ipam: ip already exist: $@" if !$noerr;
+	}
     }
 }
 
@@ -208,6 +212,14 @@ sub get_ip_id {
     return $ip_id;
 }
 
+sub is_ip_gateway {
+    my ($url, $ip, $headers) = @_;
+    my $result = PVE::Network::SDN::api_request("GET", "$url/addresses/search/$ip", $headers);
+    my $data = @{$result->{data}}[0];
+    my $description = $data->{description};
+    my $is_gateway = 1 if $description eq 'gateway';
+    return $is_gateway;
+}
 
 1;
 
diff --git a/PVE/Network/SDN/Ipams/PVEPlugin.pm b/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 8fe5bbb..3e8ffc5 100644
--- a/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -95,9 +95,9 @@ sub add_ip {
 	my $dbsubnet = $dbzone->{subnets}->{$cidr};
 	die "subnet '$cidr' doesn't exist in IPAM DB\n" if !$dbsubnet;
 
-	die "IP '$ip' already exist\n" if defined($dbsubnet->{ips}->{$ip});
-
+	die "IP '$ip' already exist\n" if (!$is_gateway && defined($dbsubnet->{ips}->{$ip})) || ($is_gateway && defined($dbsubnet->{ips}->{$ip}) && !defined($dbsubnet->{ips}->{$ip}->{gateway}));
 	$dbsubnet->{ips}->{$ip} = {};
+	$dbsubnet->{ips}->{$ip} = {gateway => 1} if $is_gateway;
 
 	write_db($db);
     });
diff --git a/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index ed66ea9..ad5286b 100644
--- a/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -106,10 +106,10 @@ sub add_ip {
 
     my $params = { ip => $ip,
 		   subnetId => $internalid,
-		   is_gateway => $is_gateway,
 		   hostname => $hostname,
 		   description => $description,
 		  };
+    $params->{is_gateway} = 1 if $is_gateway;
     $params->{mac} = $mac if $mac;
 
     eval {
@@ -117,7 +117,11 @@ sub add_ip {
     };
 
     if ($@) {
-	die "error add subnet ip to ipam: ip $ip already exist: $@" if !$noerr;
+	if($is_gateway) {
+	    die "error add subnet ip to ipam: ip $ip already exist: $@" if !is_ip_gateway($url, $ip, $headers) && !$noerr;
+	} else {
+	    die "error add subnet ip to ipam: ip $ip already exist: $@" if !$noerr;
+	}
     }
 }
 
@@ -134,10 +138,10 @@ sub update_ip {
     die "can't find ip addresse in ipam" if !$ip_id;
 
     my $params = { 
-		   is_gateway => $is_gateway,
 		   hostname => $hostname,
 		   description => $description,
 		  };
+    $params->{is_gateway} = 1 if $is_gateway;
     $params->{mac} = $mac if $mac;
 
     eval {
@@ -242,6 +246,14 @@ sub get_ip_id {
     return $ip_id;
 }
 
+sub is_ip_gateway {
+    my ($url, $ip, $headers) = @_;
+    my $result = PVE::Network::SDN::api_request("GET", "$url/addresses/search/$ip", $headers);
+    my $data = @{$result->{data}}[0];
+    my $is_gateway = $data->{is_gateway};
+    return $is_gateway;
+}
+
 1;
 
 
diff --git a/PVE/Network/SDN/SubnetPlugin.pm b/PVE/Network/SDN/SubnetPlugin.pm
index b4c8954..15b370f 100644
--- a/PVE/Network/SDN/SubnetPlugin.pm
+++ b/PVE/Network/SDN/SubnetPlugin.pm
@@ -143,7 +143,7 @@ sub on_update_hook {
 	}
         if(!$old_gateway || $gateway && $gateway ne $old_gateway) {
 	    my $hostname = "$vnetid-gw";
-	    my $description = "$vnetid gw";
+	    my $description = "gateway";
 	    PVE::Network::SDN::Subnets::add_ip($zone, $subnetid, $subnet, $gateway, $hostname, $mac, $description, 1);
 	}
 
diff --git a/PVE/Network/SDN/Subnets.pm b/PVE/Network/SDN/Subnets.pm
index 46d9830..0231822 100644
--- a/PVE/Network/SDN/Subnets.pm
+++ b/PVE/Network/SDN/Subnets.pm
@@ -232,7 +232,7 @@ sub next_free_ip {
 }
 
 sub add_ip {
-    my ($zone, $subnetid, $subnet, $ip, $hostname, $mac, $description) = @_;
+    my ($zone, $subnetid, $subnet, $ip, $hostname, $mac, $description, $is_gateway) = @_;
 
     return if !$subnet || !$ip; 
 
@@ -259,7 +259,7 @@ sub add_ip {
 	my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
 
 	eval {
-	    $plugin->add_ip($plugin_config, $subnetid, $subnet, $ip, $hostname, $mac, $description);
+	    $plugin->add_ip($plugin_config, $subnetid, $subnet, $ip, $hostname, $mac, $description, $is_gateway);
 	};
 	die $@ if $@;
     }
diff --git a/test/ipams/netbox/expected.add_ip_notgateway b/test/ipams/netbox/expected.add_ip_notgateway
new file mode 100644
index 0000000..ae876f2
--- /dev/null
+++ b/test/ipams/netbox/expected.add_ip_notgateway
@@ -0,0 +1,9 @@
+bless( {
+                  '_content' => '{"address":"10.0.0.1/24","description":"mydescription mac:da:65:8f:18:9b:6f","dns_name":"myhostname"}',
+                  '_headers' => bless( {
+                                         'authorization' => 'token 0123456789abcdef0123456789abcdef01234567',
+                                         'content-type' => 'application/json; charset=UTF-8'
+                                       }, 'HTTP::Headers' ),
+                  '_method' => 'POST',
+                  '_uri' => bless( do{\(my $o = 'http://localhost:8000/api/ipam/ip-addresses/')}, 'URI::http' )
+                }, 'HTTP::Request' );
diff --git a/test/ipams/phpipam/expected.add_ip_notgateway b/test/ipams/phpipam/expected.add_ip_notgateway
new file mode 100644
index 0000000..7a91359
--- /dev/null
+++ b/test/ipams/phpipam/expected.add_ip_notgateway
@@ -0,0 +1,12 @@
+bless( {
+                  '_content' => '{"description":"mydescription","hostname":"myhostname","ip":"10.0.0.1","mac":"da:65:8f:18:9b:6f","subnetId":1}',
+                  '_headers' => bless( {
+                                         '::std_case' => {
+                                                           'token' => 'Token'
+                                                         },
+                                         'content-type' => 'application/json; charset=UTF-8',
+                                         'token' => 'JPHkPSLB4O_XL-GQz4qtEFmNpx-99Htw'
+                                       }, 'HTTP::Headers' ),
+                  '_method' => 'POST',
+                  '_uri' => bless( do{\(my $o = 'https://localhost/api/apiadmin/addresses/')}, 'URI::https' )
+                }, 'HTTP::Request' );
diff --git a/test/run_test_ipams.pl b/test/run_test_ipams.pl
index 6743eff..27bd441 100755
--- a/test/run_test_ipams.pl
+++ b/test/run_test_ipams.pl
@@ -99,6 +99,9 @@ foreach my $path (@plugins) {
 	},
 	get_ip_id => sub {
 	    return 1;
+	},
+	is_ip_gateway => sub {
+	    return 1;
 	}
     );
 
@@ -146,9 +149,22 @@ foreach my $path (@plugins) {
     $test = "update_ip";
     $expected = Dumper read_sdn_config("$path/expected.$test");
     $name = "$ipamid $test";
-
     $plugin->update_ip($plugin_config, $subnetid, $subnet, $ip, $hostname, $mac, $description, $is_gateway, 1);
 
+    if ($@) {
+	is ($@, $expected, $name);
+    } else {
+	fail($name);
+    }
+
+    ## add_ip_notgateway
+    $is_gateway = undef;
+    $test = "add_ip_notgateway";
+    $expected = Dumper read_sdn_config("$path/expected.$test");
+    $name = "$ipamid $test";
+
+    $plugin->add_ip($plugin_config, $subnetid, $subnet, $ip, $hostname, $mac, $description, $is_gateway, 1);
+
     if ($@) {
 	is ($@, $expected, $name);
     } else {
diff --git a/test/run_test_subnets.pl b/test/run_test_subnets.pl
index 9fca202..f6564e1 100755
--- a/test/run_test_subnets.pl
+++ b/test/run_test_subnets.pl
@@ -135,10 +135,10 @@ foreach my $path (@plugins) {
     $test = "add_ip $ip";
     $name = "$testid $test";
     $result = undef;
-    $expected = '{"zones":{"myzone":{"subnets":{"'.$subnet_cidr.'":{"ips":{"'.$ip.'":{}}}}}}}';
+    $expected = '{"zones":{"myzone":{"subnets":{"'.$subnet_cidr.'":{"ips":{"'.$ip.'":{"gateway":1}}}}}}}';
 
     eval {
-	PVE::Network::SDN::Subnets::add_ip($zone, $subnetid, $subnet, $ip, $hostname, $mac, $description);
+	PVE::Network::SDN::Subnets::add_ip($zone, $subnetid, $subnet, $ip, $hostname, $mac, $description, $is_gateway);
     };
 
     if ($@) {
@@ -170,7 +170,7 @@ foreach my $path (@plugins) {
     $test = "add_second_ip $ip2";
     $name = "$testid $test";
     $result = undef;
-    $expected = '{"zones":{"myzone":{"subnets":{"'.$subnet_cidr.'":{"ips":{"'.$ip.'":{},"'.$ip2.'":{}}}}}}}';
+    $expected = '{"zones":{"myzone":{"subnets":{"'.$subnet_cidr.'":{"ips":{"'.$ip.'":{"gateway":1},"'.$ip2.'":{}}}}}}}';
 
     eval {
 	PVE::Network::SDN::Subnets::add_ip($zone, $subnetid, $subnet, $ip2, $hostname, $mac, $description);
@@ -189,7 +189,7 @@ foreach my $path (@plugins) {
     $test = "find_next_freeip ($ipnextfree)";
     $name = "$testid $test";
     $result = undef;
-    $expected = '{"zones":{"myzone":{"subnets":{"'.$subnet_cidr.'":{"ips":{"'.$ip.'":{},"'.$ipnextfree.'":{},"'.$ip2.'":{}}}}}}}';
+    $expected = '{"zones":{"myzone":{"subnets":{"'.$subnet_cidr.'":{"ips":{"'.$ip.'":{"gateway":1},"'.$ipnextfree.'":{},"'.$ip2.'":{}}}}}}}';
 
     eval {
 	$ip3 = PVE::Network::SDN::Subnets::next_free_ip($zone, $subnetid, $subnet, $hostname, $mac, $description);
-- 
2.20.1




  parent reply	other threads:[~2021-06-04 11:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 11:24 [pve-devel] [pve-network 0/4] updates Alexandre Derumier
2021-06-04 11:24 ` [pve-devel] [pve-network 1/4] sdn: get_local_vnets : add ipam && vlanaware values Alexandre Derumier
2021-06-04 11:24 ` [pve-devel] [pve-network 2/4] add vnets test + ipam fixes Alexandre Derumier
2021-06-04 11:24 ` [pve-devel] [pve-network 3/4] vnets: subroutines: return if !$vnetid Alexandre Derumier
2021-06-04 11:25 ` Alexandre Derumier [this message]
2021-06-18 16:30 ` [pve-devel] applied-series: [pve-network 0/4] updates Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210604112500.2349926-5-aderumier@odiso.com \
    --to=aderumier@odiso.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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