all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal