Re: [pve-devel] [PATCH pve-network v4 01/21] sdn: fix value returned by pending_config

From: Stefan Hanreich <s.hanreich@proxmox.com>
To: Gabriel Goller <g.goller@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH pve-network v4 01/21] sdn: fix value returned by pending_config
Date: Fri, 4 Jul 2025 16:29:54 +0200	[thread overview]
Message-ID: <421e53c7-6fae-42dc-8662-ff37c65a11c1@proxmox.com> (raw)
In-Reply-To: <20250702145101.894299-37-g.goller@proxmox.com>

looks like some unrelated formatting changes are here? The previous
commit only contained one line of changes.

https://lore.proxmox.com/pve-devel/20250522161731.537011-35-s.hanreich@proxmox.com/

On 7/2/25 16:50, Gabriel Goller wrote:
> From: Stefan Hanreich <s.hanreich@proxmox.com>
> 
> For special types that were encoded by the encode_value function in
> SDN, we returned the encoded value in the API, rather than the actual
> value. Since we use the encoded value only for comparison, we need to
> return the original value instead of the encoded value.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  src/PVE/Network/SDN.pm | 120 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 89 insertions(+), 31 deletions(-)
> 
> diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
> index ef938c461d0a..391c6e26c7c7 100644
> --- a/src/PVE/Network/SDN.pm
> +++ b/src/PVE/Network/SDN.pm
> @@ -24,7 +24,9 @@ use PVE::Network::SDN::Dhcp;
>  my $running_cfg = "sdn/.running-config";
>  
>  my $parse_running_cfg = sub {
> -    my ($filename, $raw) = @_;
> +    my (
> +        $filename, $raw,
> +    ) = @_;
>  
>      my $cfg = {};
>  
> @@ -37,14 +39,18 @@ my $parse_running_cfg = sub {
>  };
>  
>  my $write_running_cfg = sub {
> -    my ($filename, $cfg) = @_;
> +    my (
> +        $filename, $cfg,
> +    ) = @_;
>  
>      my $json = to_json($cfg);
>  
>      return $json;
>  };
>  
> -PVE::Cluster::cfs_register_file($running_cfg, $parse_running_cfg, $write_running_cfg);
> +PVE::Cluster::cfs_register_file(
> +    $running_cfg, $parse_running_cfg, $write_running_cfg,
> +);
>  
>  # improve me : move status code inside plugins ?
>  
> @@ -74,8 +80,12 @@ sub ifquery_check {
>  
>  sub status {
>  
> -    my ($zone_status, $vnet_status) = PVE::Network::SDN::Zones::status();
> -    return ($zone_status, $vnet_status);
> +    my (
> +        $zone_status, $vnet_status,
> +    ) = PVE::Network::SDN::Zones::status();
> +    return (
> +        $zone_status, $vnet_status,
> +    );
>  }
>  
>  sub running_config {
> @@ -83,7 +93,9 @@ sub running_config {
>  }
>  
>  sub pending_config {
> -    my ($running_cfg, $cfg, $type) = @_;
> +    my (
> +        $running_cfg, $cfg, $type,
> +    ) = @_;
>  
>      my $pending = {};
>  
> @@ -100,8 +112,12 @@ sub pending_config {
>              } elsif (!defined($config_object->{$key})) {
>                  $pending->{$id}->{"pending"}->{$key} = 'deleted';
>                  $pending->{$id}->{state} = "changed";
> -            } elsif (PVE::Network::SDN::encode_value(undef, $key, $running_object->{$key}) ne
> -                PVE::Network::SDN::encode_value(undef, $key, $config_object->{$key})
> +            } elsif (
> +                PVE::Network::SDN::encode_value(
> +                    undef, $key, $running_object->{$key},
> +                ) ne PVE::Network::SDN::encode_value(
> +                    undef, $key, $config_object->{$key},
> +                )
>              ) {
>                  $pending->{$id}->{state} = "changed";
>              }
> @@ -115,18 +131,25 @@ sub pending_config {
>          my $config_object = $config_objects->{$id};
>  
>          foreach my $key (sort keys %{$config_object}) {
> -            my $config_value = PVE::Network::SDN::encode_value(undef, $key, $config_object->{$key});
> -            my $running_value =
> -                PVE::Network::SDN::encode_value(undef, $key, $running_object->{$key});
> +            my $config_value = PVE::Network::SDN::encode_value(
> +                undef, $key, $config_object->{$key},
> +            );
> +            my $running_value = PVE::Network::SDN::encode_value(
> +                undef, $key, $running_object->{$key},
> +            );
>              if ($key eq 'type' || $key eq 'vnet') {
>                  $pending->{$id}->{$key} = $config_value;
>              } else {
> -                $pending->{$id}->{"pending"}->{$key} = $config_value
> -                    if !defined($running_value) || ($config_value ne $running_value);
> +                $pending->{$id}->{"pending"}->{$key} = $config_object->{$key}
> +                    if !defined($running_value)
> +                    || ($config_value ne $running_value);
>              }
>              if (!keys %{$running_object}) {
>                  $pending->{$id}->{state} = "new";
> -            } elsif (!defined($running_value) && defined($config_value)) {
> +            } elsif (
> +                !defined($running_value)
> +                && defined($config_value)
> +            ) {
>                  $pending->{$id}->{state} = "changed";
>              }
>          }
> @@ -134,7 +157,9 @@ sub pending_config {
>              if $pending->{$id}->{state} && !defined($pending->{$id}->{"pending"});
>      }
>  
> -    return { ids => $pending };
> +    return {
> +        ids => $pending,
> +    };
>  
>  }
>  
> @@ -167,13 +192,19 @@ sub commit_config {
>          subnets => $subnets,
>      };
>  
> -    cfs_write_file($running_cfg, $cfg);
> +    cfs_write_file(
> +        $running_cfg, $cfg,
> +    );
>  }
>  
>  sub lock_sdn_config {
> -    my ($code, $errmsg) = @_;
> +    my (
> +        $code, $errmsg,
> +    ) = @_;
>  
> -    cfs_lock_file($running_cfg, undef, $code);
> +    cfs_lock_file(
> +        $running_cfg, undef, $code,
> +    );
>  
>      if (my $err = $@) {
>          $errmsg ? die "$errmsg: $err" : die $err;
> @@ -198,16 +229,22 @@ sub get_local_vnets {
>  
>      foreach my $vnetid (@vnetids) {
>  
> -        my $vnet = PVE::Network::SDN::Vnets::sdn_vnets_config($vnets_cfg, $vnetid);
> +        my $vnet = PVE::Network::SDN::Vnets::sdn_vnets_config(
> +            $vnets_cfg, $vnetid,
> +        );
>          my $zoneid = $vnet->{zone};
>          my $comments = $vnet->{alias};
>  
>          my $privs = ['SDN.Audit', 'SDN.Use'];
>  
>          next if !$zoneid;
> -        next if !$rpcenv->check_sdn_bridge($authuser, $zoneid, $vnetid, $privs, 1);
> +        next if !$rpcenv->check_sdn_bridge(
> +            $authuser, $zoneid, $vnetid, $privs, 1,
> +        );
>  
> -        my $zone_config = PVE::Network::SDN::Zones::sdn_zones_config($zones_cfg, $zoneid);
> +        my $zone_config = PVE::Network::SDN::Zones::sdn_zones_config(
> +            $zones_cfg, $zoneid,
> +        );
>  
>          next if defined($zone_config->{nodes}) && !$zone_config->{nodes}->{$nodename};
>          my $ipam = $zone_config->{ipam} ? 1 : 0;
> @@ -228,7 +265,9 @@ sub generate_zone_config {
>      my $raw_config = PVE::Network::SDN::Zones::generate_etc_network_config();
>      if ($raw_config) {
>          eval {
> -            my $net_cfg = PVE::INotify::read_file('interfaces', 1);
> +            my $net_cfg = PVE::INotify::read_file(
> +                'interfaces', 1,
> +            );
>              my $opts = $net_cfg->{data}->{options};
>              log_warn(
>                  "missing 'source /etc/network/interfaces.d/sdn' directive for SDN support!\n")
> @@ -255,13 +294,19 @@ sub generate_dhcp_config {
>  }
>  
>  sub encode_value {
> -    my ($type, $key, $value) = @_;
> +    my (
> +        $type, $key, $value,
> +    ) = @_;
>  
>      if ($key eq 'nodes' || $key eq 'exitnodes' || $key eq 'dhcp-range') {
>          if (ref($value) eq 'HASH') {
> -            return join(',', sort keys(%$value));
> +            return join(
> +                ',', sort keys(%$value),
> +            );
>          } elsif (ref($value) eq 'ARRAY') {
> -            return join(',', sort @$value);
> +            return join(
> +                ',', sort @$value,
> +            );
>          } else {
>              return $value;
>          }
> @@ -272,29 +317,42 @@ sub encode_value {
>  
>  #helpers
>  sub api_request {
> -    my ($method, $url, $headers, $data, $expected_fingerprint) = @_;
> +    my (
> +        $method, $url, $headers, $data, $expected_fingerprint,
> +    ) = @_;
>  
>      my $encoded_data = $data ? to_json($data) : undef;
>  
> -    my $req = HTTP::Request->new($method, $url, $headers, $encoded_data);
> +    my $req = HTTP::Request->new(
> +        $method, $url, $headers, $encoded_data,
> +    );
>  
> -    my $ua = LWP::UserAgent->new(protocols_allowed => ['http', 'https'], timeout => 30);
> +    my $ua = LWP::UserAgent->new(
> +        protocols_allowed => ['http', 'https'],
> +        timeout => 30,
> +    );
>      my $datacenter_cfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
>      if (my $proxy = $datacenter_cfg->{http_proxy}) {
> -        $ua->proxy(['http', 'https'], $proxy);
> +        $ua->proxy(
> +            ['http', 'https'], $proxy,
> +        );
>      } else {
>          $ua->env_proxy;
>      }
>  
>      if (defined($expected_fingerprint)) {
>          my $ssl_verify_callback = sub {
> -            my (undef, undef, undef, undef, $cert, $depth) = @_;
> +            my (
> +                undef, undef, undef, undef, $cert, $depth,
> +            ) = @_;
>  
>              # we don't care about intermediate or root certificates, always return as valid as the
>              # callback will be executed for all levels and all must be valid.
>              return 1 if $depth != 0;
>  
> -            my $fingerprint = Net::SSLeay::X509_get_fingerprint($cert, 'sha256');
> +            my $fingerprint = Net::SSLeay::X509_get_fingerprint(
> +                $cert, 'sha256',
> +            );
>  
>              return $fingerprint eq $expected_fingerprint ? 1 : 0;
>          };

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