From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 4673F1FF17C
for <inbox@lore.proxmox.com>; Wed, 2 Apr 2025 12:42:15 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id BD219F95A;
Wed, 2 Apr 2025 12:42:04 +0200 (CEST)
Date: Wed, 02 Apr 2025 12:41:26 +0200
From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20250328171340.885413-1-g.goller@proxmox.com>
<20250328171340.885413-39-g.goller@proxmox.com>
In-Reply-To: <20250328171340.885413-39-g.goller@proxmox.com>
MIME-Version: 1.0
User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid)
Message-Id: <1743587597.uxpiwfvh9n.astroid@yuna.none>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.045 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH pve-network 11/17] api: add fabrics subfolder
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.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