[pve-devel] applied-series: [PATCH network 0/3] smaller fixes and clean-ups

[pve-devel] applied-series: [PATCH network 0/3] smaller fixes and clean-ups

[Thomas Lamprecht]

To bugs got noticed by Fiona, namely missing use statements – which was
not relevant in practice, but still good to have them correct as it
easily can cause "spooky actions at a distance" when changing things
elsewhere. The other one was a conditional use-statement, which is
always a rather nasty bug that caused that the previously encoded data
from the last call to this method was kept if no new one was passed.
The third one is a clean-up of duplicated code in the PowerDNS
integration plugin.

All three patches already applied to git master branch.

Thomas Lamprecht (3):
  fix missing use statements in core SDN module
  api request helper: fix conditional declaration
  ipam dns: powerdns integration: factor out common API request code

 src/PVE/Network/SDN.pm                    | 14 +++--
 src/PVE/Network/SDN/Dns/PowerdnsPlugin.pm | 77 ++++++-----------------
 2 files changed, 28 insertions(+), 63 deletions(-)

-- 
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 network 1/3] fix missing use statements in core SDN module

[Thomas Lamprecht]

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

and group and sort correctly while at it.

 src/PVE/Network/SDN.pm | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 8cd9ad8..cc5febe 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -3,11 +3,17 @@ package PVE::Network::SDN;
 use strict;
 use warnings;
 
+use HTTP::Request;
 use IO::Socket::SSL; # important for SSL_verify_callback
-use JSON;
+use JSON qw(decode_json from_json to_json);
+use LWP::UserAgent;
 use Net::SSLeay;
 
+use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
 use PVE::INotify;
+use PVE::RESTEnvironment qw(log_warn);
+use PVE::RPCEnvironment;
+use PVE::Tools qw(extract_param dir_glob_regex run_command);
 
 use PVE::Network::SDN::Vnets;
 use PVE::Network::SDN::Zones;
@@ -15,10 +21,6 @@ use PVE::Network::SDN::Controllers;
 use PVE::Network::SDN::Subnets;
 use PVE::Network::SDN::Dhcp;
 
-use PVE::Tools qw(extract_param dir_glob_regex run_command);
-use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
-use PVE::RESTEnvironment qw(log_warn);
-
 my $running_cfg = "sdn/.running-config";
 
 my $parse_running_cfg = sub {
-- 
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 network 2/3] api request helper: fix conditional declaration

[Thomas Lamprecht]

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/Network/SDN.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index cc5febe..68f9e0f 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -262,7 +262,7 @@ sub encode_value {
 sub api_request {
     my ($method, $url, $headers, $data, $expected_fingerprint) = @_;
 
-    my $encoded_data = to_json($data) if $data;
+    my $encoded_data = $data ? to_json($data) : undef;
 
     my $req = HTTP::Request->new($method,$url, $headers, $encoded_data);
 
-- 
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 network 3/3] ipam dns: powerdns integration: factor out common API request code

[Thomas Lamprecht]

No point in having the same generic code to prepare variables for an
API request to PowerDNS 7 times basically duplicated.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

There might be other such bloat lurking, just stumbled into this by
chance..

 src/PVE/Network/SDN/Dns/PowerdnsPlugin.pm | 77 ++++++-----------------
 1 file changed, 20 insertions(+), 57 deletions(-)

diff --git a/src/PVE/Network/SDN/Dns/PowerdnsPlugin.pm b/src/PVE/Network/SDN/Dns/PowerdnsPlugin.pm
index 74735e7..15afb4a 100644
--- a/src/PVE/Network/SDN/Dns/PowerdnsPlugin.pm
+++ b/src/PVE/Network/SDN/Dns/PowerdnsPlugin.pm
@@ -44,17 +44,24 @@ sub options {
     };
 }
 
+my sub powerdns_api_request {
+    my ($config, $method, $path, $params) = @_;
+
+    return PVE::Network::SDN::api_request(
+	$method,
+	"$config->{url}${path}",
+	['Content-Type' => 'application/json; charset=UTF-8', 'X-API-Key' => $config->{key}],
+	$params,
+	$config->{fingerprint},
+    );
+}
+
 # Plugin implementation
 
 sub add_a_record {
     my ($class, $plugin_config, $zone, $hostname, $ip, $noerr) = @_;
 
-    my $url = $plugin_config->{url};
-    my $key = $plugin_config->{key};
     my $ttl = $plugin_config->{ttl} ? $plugin_config->{ttl} : 14400;
-    my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'X-API-Key' => $key];
-    my $fingerprint = $plugin_config->{fingerprint};
-
     my $type = Net::IP::ip_is_ipv6($ip) ? "AAAA" : "A";
     my $fqdn = $hostname.".".$zone.".";
 
@@ -87,21 +94,15 @@ sub add_a_record {
 	}],
     };
 
-    eval {
-	PVE::Network::SDN::api_request("PATCH", "$url/zones/$zone", $headers, $params, $fingerprint)
-    };
+    eval { powerdns_api_request($plugin_config, 'PATCH', "/zones/$zone", $params) };
     die "error add $fqdn to zone $zone: $@" if $@ && !$noerr;
 }
 
 sub add_ptr_record {
     my ($class, $plugin_config, $zone, $hostname, $ip, $noerr) = @_;
 
-    my $url = $plugin_config->{url};
-    my $key = $plugin_config->{key};
     my $ttl = $plugin_config->{ttl} ? $plugin_config->{ttl} : 14400;
-    my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'X-API-Key' => $key];
     $hostname .= ".";
-    my $fingerprint = $plugin_config->{fingerprint};
 
     my $reverseip = Net::IP->new($ip)->reverse_ip();
 
@@ -124,21 +125,15 @@ sub add_ptr_record {
 	}],
     };
 
-    eval {
-	PVE::Network::SDN::api_request("PATCH", "$url/zones/$zone", $headers, $params, $fingerprint)
-    };
+    eval { powerdns_api_request($plugin_config, 'PATCH', "/zones/$zone", $params) };
     die "error add $reverseip to zone $zone: $@" if $@ && !$noerr;
 }
 
 sub del_a_record {
     my ($class, $plugin_config, $zone, $hostname, $ip, $noerr) = @_;
 
-    my $url = $plugin_config->{url};
-    my $key = $plugin_config->{key};
-    my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'X-API-Key' => $key];
     my $fqdn = $hostname.".".$zone.".";
     my $type = Net::IP::ip_is_ipv6($ip) ? "AAAA" : "A";
-    my $fingerprint = $plugin_config->{fingerprint};
 
     my $zonecontent = get_zone_content($plugin_config, $zone);
     my $existing_rrset = get_zone_rrset($zonecontent, $fqdn);
@@ -165,20 +160,13 @@ sub del_a_record {
 
     my $params = { rrsets => [ $rrset ] };
 
-    eval {
-	PVE::Network::SDN::api_request("PATCH", "$url/zones/$zone", $headers, $params, $fingerprint)
-    };
+    eval { powerdns_api_request($plugin_config, 'PATCH', "/zones/$zone", $params) };
     die "error delete $fqdn from zone $zone: $@" if $@ && !$noerr;
 }
 
 sub del_ptr_record {
     my ($class, $plugin_config, $zone, $ip, $noerr) = @_;
 
-    my $url = $plugin_config->{url};
-    my $key = $plugin_config->{key};
-    my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'X-API-Key' => $key];
-    my $fingerprint = $plugin_config->{fingerprint};
-
     my $reverseip = Net::IP->new($ip)->reverse_ip();
 
     my $type = "PTR";
@@ -192,26 +180,15 @@ sub del_ptr_record {
 	}],
     };
 
-    eval {
-	PVE::Network::SDN::api_request("PATCH", "$url/zones/$zone", $headers, $params, $fingerprint)
-    };
+    eval { powerdns_api_request($plugin_config, 'PATCH', "/zones/$zone", $params) };
     die "error delete $reverseip from zone $zone: $@" if $@ && !$noerr;
 }
 
 sub verify_zone {
     my ($class, $plugin_config, $zone, $noerr) = @_;
 
-    #verify that api is working
-
-    my $url = $plugin_config->{url};
-    my $key = $plugin_config->{key};
-    my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'X-API-Key' => $key];
-    my $fingerprint = $plugin_config->{fingerprint};
-
-    eval {
-	PVE::Network::SDN::api_request(
-	    "GET", "$url/zones/$zone?rrsets=false", $headers, undef, $fingerprint)
-    };
+    # verify that zone exists
+    eval { powerdns_api_request($plugin_config, 'GET', "/zones/$zone?rrsets=false") };
     die "can't read zone $zone: $@" if $@ && !$noerr;
 }
 
@@ -269,13 +246,7 @@ sub on_update_hook {
     my ($class, $plugin_config) = @_;
 
     # verify that api is working
-
-    my $url = $plugin_config->{url};
-    my $key = $plugin_config->{key};
-    my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'X-API-Key' => $key];
-    my $fingerprint = $plugin_config->{fingerprint};
-
-    eval { PVE::Network::SDN::api_request("GET", "$url", $headers, undef, $fingerprint) };
+    eval { powerdns_api_request($plugin_config, 'GET', '') };
     die "dns api error: $@" if $@;
 }
 
@@ -284,15 +255,7 @@ sub get_zone_content {
     my ($plugin_config, $zone) = @_;
 
     # verify that api is working
-
-    my $url = $plugin_config->{url};
-    my $key = $plugin_config->{key};
-    my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'X-API-Key' => $key];
-    my $fingerprint = $plugin_config->{fingerprint};
-
-    my $result = eval {
-	PVE::Network::SDN::api_request("GET", "$url/zones/$zone", $headers, undef, $fingerprint)
-    };
+    my $result = eval { powerdns_api_request($plugin_config, 'GET', "/zones/$zone") };
     die "can't read zone $zone: $@" if $@;
 
     return $result;
-- 
2.39.5



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