Re: [pve-devel] [PATCH pve-network 11/17] api: add fabrics subfolder

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On March 28, 2025 6:13 pm, Gabriel Goller wrote:
> From: Stefan Hanreich <s.hanreich@proxmox.com>
> 
> Add a new subfolder that contains the API methods for the sdn
> fabrics. We also add a method for listing all fabrics of all types as
> a GET endpoint, with the respective schemas. It supports the same
> filtering options as the other SDN GET endpoints (pending / running).
> 
> We also need to add a special case in encode_value for the interface
> key of nodes, since they require special handling when encoding
> because they are arrays.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  src/PVE/API2/Network/SDN.pm         |   7 +
>  src/PVE/API2/Network/SDN/Fabrics.pm | 294 ++++++++++++++++++++++++++++
>  src/PVE/API2/Network/SDN/Makefile   |   2 +-
>  src/PVE/Network/SDN.pm              |   2 +-
>  4 files changed, 303 insertions(+), 2 deletions(-)
>  create mode 100644 src/PVE/API2/Network/SDN/Fabrics.pm
> 
> diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
> index d216e4878b61..ccbf0777e3d4 100644
> --- a/src/PVE/API2/Network/SDN.pm
> +++ b/src/PVE/API2/Network/SDN.pm
> @@ -17,6 +17,7 @@ use PVE::API2::Network::SDN::Vnets;
>  use PVE::API2::Network::SDN::Zones;
>  use PVE::API2::Network::SDN::Ipams;
>  use PVE::API2::Network::SDN::Dns;
> +use PVE::API2::Network::SDN::Fabrics;
>  
>  use base qw(PVE::RESTHandler);
>  
> @@ -45,6 +46,11 @@ __PACKAGE__->register_method ({
>      path => 'dns',
>  });
>  
> +__PACKAGE__->register_method ({
> +    subclass => "PVE::API2::Network::SDN::Fabrics",
> +    path => 'fabrics',
> +});
> +
>  __PACKAGE__->register_method({
>      name => 'index',
>      path => '',
> @@ -76,6 +82,7 @@ __PACKAGE__->register_method({
>  	    { id => 'controllers' },
>  	    { id => 'ipams' },
>  	    { id => 'dns' },
> +	    { id => 'fabrics' },
>  	];
>  
>  	return $res;
> diff --git a/src/PVE/API2/Network/SDN/Fabrics.pm b/src/PVE/API2/Network/SDN/Fabrics.pm
> new file mode 100644
> index 000000000000..c9064b0ea05b
> --- /dev/null
> +++ b/src/PVE/API2/Network/SDN/Fabrics.pm
> @@ -0,0 +1,294 @@
> +package PVE::API2::Network::SDN::Fabrics;
> +
> +use strict;
> +use warnings;
> +
> +use Storable qw(dclone);
> +
> +use PVE::RPCEnvironment;
> +use PVE::Tools qw(extract_param);
> +
> +use PVE::API2::Network::SDN::Fabrics::OpenFabric;
> +use PVE::API2::Network::SDN::Fabrics::Ospf;
> +
> +use PVE::Network::SDN::Fabrics;
> +
> +use PVE::RESTHandler;
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> +    subclass => "PVE::API2::Network::SDN::Fabrics::OpenFabric",
> +    path => 'openfabric',
> +});
> +__PACKAGE__->register_method ({
> +    subclass => "PVE::API2::Network::SDN::Fabrics::Ospf",
> +    path => 'ospf',
> +});
> +
> +my $openfabric_interface_fmt = {
> +    name => {
> +	type => 'string',
> +	description => 'Name of the interface',

string without format

> +    },
> +    ip => {
> +	type => 'string',
> +	description => 'The IPv4 address of the interface',

string without format

> +	optional => 1,
> +    },
> +    ipv6 => {
> +	type => 'string',
> +	description => 'The IPv6 address of the interface',

string without format

> +	optional => 1,
> +    },
> +    passive => {
> +	type => 'boolean',
> +	description => 'The passive property of the interface',
> +	optional => 1,
> +    },
> +    hello_interval => {
> +	type => 'number',
> +	description => 'The hello_interval property of the interface',
> +	optional => 1,
> +    },
> +    csnp_interval => {
> +	type => 'number',
> +	description => 'The csnp_interval property of the interface',
> +	optional => 1,
> +    },
> +    hello_multiplier => {
> +	type => 'number',
> +	description => 'The hello_multiplier property of the interface',
> +	optional => 1,
> +    },

not sure whether these have min/max values?

> +};
> +
> +PVE::JSONSchema::register_format('pve-sdn-openfabric-interface', $openfabric_interface_fmt);
> +
> +my $ospf_interface_fmt = {
> +    name => {
> +	type => 'string',
> +	description => 'Name of the interface',

string without format

> +    },
> +    passive => {
> +	type => 'boolean',
> +	description => 'The passive property of the interface',
> +	optional => 1,
> +    },
> +    ip => {
> +	type => 'string',
> +	description => 'The IPv4 address of the interface',

string without format

> +	optional => 1,
> +    },
> +    unnumbered => {
> +	type => 'boolean',
> +	description => 'If the interface is unnumbered',
> +	optional => 1,
> +    },
> +};
> +
> +PVE::JSONSchema::register_format('pve-sdn-ospf-interface', $ospf_interface_fmt);
> +
> +__PACKAGE__->register_method({
> +    name => 'index',
> +    path => '',
> +    method => 'GET',
> +    description => 'Index of SDN Fabrics',
> +    permissions => {
> +	description => "Only list entries where you have 'SDN.Audit' or 'SDN.Allocate' permissions on '/sdn/fabrics/<protocol>/<fabric>'",
> +	user => 'all',
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    running => {
> +		type => 'boolean',
> +		optional => 1,
> +		description => "Display running config.",
> +	    },
> +	    pending => {
> +		type => 'boolean',
> +		optional => 1,
> +		description => "Display pending config.",
> +	    },
> +	},
> +    },
> +    returns => {
> +	type => 'object',
> +	properties => {
> +	    openfabric => {
> +		type => 'array',
> +		items => {
> +		    type => 'object',
> +		    properties => {
> +			name => {
> +			    type => 'string',
> +			    description => 'Id of the fabric'
> +			},
> +			'type' => {
> +			    type => 'string',
> +			    description => 'What type of config is this',
> +			},
> +			'config' => {
> +			    type => 'object',
> +			    'type-property' => 'type',
> +			    oneOf => [
> +				{
> +				    'instance-types' => ['node'],
> +				    type => 'object',
> +				    description => 'Node config',
> +				    properties => {
> +					node => {
> +					    type => 'object',
> +					    properties => {
> +						net => {
> +						    type => 'string',
> +						    description => 'The NET (Network Entity Title) of this node',
> +						},
> +						loopback_prefix => {
> +						    type => 'string',
> +						    description => 'The IP prefix for Loopback IPs',
> +						},
> +						interface => {
> +						    type => 'array',
> +						    description => 'The OpenFabric interfaces on this node',
> +						    items => {
> +							type => 'string',
> +							description => 'OpenFabric interface',
> +							format => 'pve-sdn-openfabric-interface'
> +						    },
> +						},
> +					    },
> +					},
> +				    },
> +				},
> +				{
> +				    'instance-types' => ['fabric'],
> +				    type => 'object',
> +				    description => 'Fabric config',
> +				    properties => {
> +					fabric => {
> +					    type => 'object',
> +					    properties => {
> +						loopback_prefix => {
> +						    type => 'string',
> +						    description => 'The IP prefix for Loopback IPs',
> +						},
> +						hello_interval => {
> +						    type => 'integer',
> +						    optional => 1,
> +						    description => 'The global hello_interval parameter in seconds that will be set on every interface',
> +						},
> +					    },
> +					},
> +				    },
> +				}
> +			    ],
> +			},
> +		    },
> +		},
> +	    },
> +	    ospf => {
> +		type => 'array',
> +		items => {
> +		    type => 'object',
> +		    properties => {
> +			name => {
> +			    type => 'string',
> +			    description => 'Id of the fabric'
> +			},
> +			config => {
> +			    type => 'object',
> +			    'type-property' => 'type',
> +			    oneOf => [
> +				{
> +				    'instance-types' => [ 'node' ],
> +				    type => 'object',
> +				    description => 'Node config',
> +				    properties => {
> +					node => {
> +					    type => 'object',
> +					    properties => {
> +						router_id => {
> +						    type => 'string',
> +						    description => 'The Router ID of this node',
> +						},
> +						interface => {
> +						    type => 'array',
> +						    description => 'The OSPF interfaces on this node',
> +						    items => {
> +							type => 'string',
> +							description => 'OSPF interface',
> +							format => 'pve-sdn-ospf-interface',
> +						    },
> +						},
> +					    },
> +					},
> +				    },
> +				},
> +				{
> +				    'instance-types' => [ 'fabric' ],
> +				    type => 'object',
> +				    description => 'Fabric config',
> +				    properties => {
> +					fabric => {
> +					    type => 'object',
> +					},
> +				    },
> +				}
> +			    ]
> +			},
> +		    },
> +		},
> +	    },
> +	},
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +
> +	my $running = extract_param($param, 'running');
> +	my $pending = extract_param($param, 'pending');
> +
> +	my $fabric_config = PVE::Network::SDN::Fabrics::config();
> +	my $running_config = PVE::Network::SDN::running_config();
> +	my $config;
> +
> +	my $authuser = $rpcenv->get_user();
> +	my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];

I wonder whether it would make sense to check whether there are any
privs below the /sdn/fabrics root here, and move the config loading
below that check, to avoid leaking things via error messages if
something is misconfigured?

also, doesn't this return quite a lot of information for an "index"
call that just requires SDN.Audit? it might make sense to filter the
information below based on whether we have Audit or Allocate privs?

> +
> +	my $res = {};
> +	foreach my $protocol (keys %$fabric_config) {
> +	    $res->{$protocol} = [];
> +
> +	    if ($pending) {
> +		# pending_config expects the section config to be under the ids
> +		# key, but get_inner() returns it without that key
> +		my $section_config = {
> +		    ids => $fabric_config->{$protocol}->get_inner(),
> +		};
> +
> +		$config = PVE::Network::SDN::pending_config(
> +		    $running_config,
> +		    $section_config,
> +		    $protocol
> +		);
> +
> +		$config = $config->{ids};
> +	    } elsif ($running) {
> +		$config = $running_config->{$protocol}->{ids};
> +	    } else {
> +		$config = $fabric_config->{$protocol}->get_inner();
> +	    }
> +
> +	    foreach my $id (sort keys %$config) {
> +		my $entry = $config->{$id};
> +		next if !$rpcenv->check_any($authuser, "/sdn/fabrics/$protocol/$entry->{name}", $privs, 1);

this is a new ACL path, but it's not possible to configure it because
there is no pve-access-control patch allowing it - did you test the
permissions part? ;)

> +
> +		push @{$res->{$protocol}}, dclone($entry);
> +	    }
> +	}
> +	return $res;
> +    },
> +});
> +
> +1;
> diff --git a/src/PVE/API2/Network/SDN/Makefile b/src/PVE/API2/Network/SDN/Makefile
> index abd1bfae020e..4dbb6c92fd82 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 Dns.pm Ips.pm
> +SOURCES=Vnets.pm Zones.pm Controllers.pm Subnets.pm Ipams.pm Dns.pm Ips.pm Fabrics.pm
>  
>  
>  PERL5DIR=${DESTDIR}/usr/share/perl5
> diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
> index 24879dc0e76a..b35767b667b4 100644
> --- a/src/PVE/Network/SDN.pm
> +++ b/src/PVE/Network/SDN.pm
> @@ -344,7 +344,7 @@ sub generate_dhcp_config {
>  sub encode_value {
>      my ($type, $key, $value) = @_;
>  
> -    if ($key eq 'nodes' || $key eq 'exitnodes' || $key eq 'dhcp-range') {
> +    if ($key eq 'nodes' || $key eq 'exitnodes' || $key eq 'dhcp-range' || $key eq 'interface') {

I hope this doesn't ever bite us, 'interface' (and 'nodes' for that matter) is quite generic..

>  	if (ref($value) eq 'HASH') {
>  	    return join(',', sort keys(%$value));
>  	} elsif (ref($value) eq 'ARRAY') {
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


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