some high level comments, see below for details though I haven't
repeated them for all instances:
- lots of schema duplication
- lots of string types without formats
- no indices, but this is a two-level deep nested router with path
parameters/child links..
- ACL paths referenced are not yet valid/accepted by ACL API
- requires SDN.Allocate across the board, even for reading -> might
allow seeing (at least parts) of the config with Audit?
On March 28, 2025 6:13 pm, Gabriel Goller wrote:
> From: Stefan Hanreich <s.hanre...@proxmox.com>
>
> Add CRUD endpoints for the openfabric fabric and node section types.
>
> Signed-off-by: Stefan Hanreich <s.hanre...@proxmox.com>
> Co-authored-by: Gabriel Goller <g.gol...@proxmox.com>
> Signed-off-by: Gabriel Goller <g.gol...@proxmox.com>
> ---
> src/PVE/API2/Network/SDN/Fabrics/Makefile | 2 +-
> .../API2/Network/SDN/Fabrics/OpenFabric.pm | 348 ++++++++++++++++++
> src/PVE/API2/Network/SDN/Makefile | 1 +
> 3 files changed, 350 insertions(+), 1 deletion(-)
> create mode 100644 src/PVE/API2/Network/SDN/Fabrics/OpenFabric.pm
>
> diff --git a/src/PVE/API2/Network/SDN/Fabrics/Makefile
> b/src/PVE/API2/Network/SDN/Fabrics/Makefile
> index e433f2e7d0a6..8f7c630ef3ab 100644
> --- a/src/PVE/API2/Network/SDN/Fabrics/Makefile
> +++ b/src/PVE/API2/Network/SDN/Fabrics/Makefile
> @@ -1,4 +1,4 @@
> -SOURCES=OpenFabric.pm Ospf.pm Common.pm
> +SOURCES=OpenFabric.pm Common.pm
this was wrong then in the previous patch ;)
>
>
> PERL5DIR=${DESTDIR}/usr/share/perl5
> diff --git a/src/PVE/API2/Network/SDN/Fabrics/OpenFabric.pm
> b/src/PVE/API2/Network/SDN/Fabrics/OpenFabric.pm
> new file mode 100644
> index 000000000000..fa5802f97ddf
> --- /dev/null
> +++ b/src/PVE/API2/Network/SDN/Fabrics/OpenFabric.pm
> @@ -0,0 +1,348 @@
> +package PVE::API2::Network::SDN::Fabrics::OpenFabric;
> +
> +use strict;
> +use warnings;
> +
> +use Storable qw(dclone);
> +
> +use PVE::RPCEnvironment;
> +use PVE::Tools qw(extract_param);
> +
> +use PVE::Network::SDN;
> +use PVE::Network::SDN::Fabrics;
> +use PVE::API2::Network::SDN::Fabrics::Common;
> +
> +use PVE::RESTHandler;
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method({
> + name => 'delete_fabric',
> + path => '{fabric}',
> + method => 'DELETE',
> + description => 'Delete SDN Fabric',
> + protected => 1,
> + permissions => {
> + check => ['perm', '/sdn/fabrics/openfabric/{fabric}', [ 'SDN.Allocate'
> ]],
pve-access-control patch missing (for all endpoints below)
> + },
> + parameters => {
> + properties => {
> + fabric => {
> + type => 'string',
string without format
> + description => 'The fabric id of the fabric to be deleted',
> + },
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Network::SDN::lock_sdn_config(
> + sub {
> +
> PVE::API2::Network::SDN::Fabrics::Common::delete_fabric("openfabric", $param);
> + }, "delete sdn fabric failed");
lock_sdn_config will return undef anyway unless I am missing something?
> + return undef;
> + },
> +});
> +
> +__PACKAGE__->register_method({
> + name => 'delete_node',
> + path => '{fabric}/node/{node}',
should this maybe live under its own router together with the other
node/ endpoints?
> + method => 'DELETE',
> + description => 'Delete SDN Fabric Node',
> + protected => 1,
> + permissions => {
> + check => ['perm', '/sdn/fabrics/openfabric/{fabric}/node/{node}', [
> 'SDN.Allocate' ]],
> + },
> + parameters => {
> + properties => {
> + fabric => {
> + type => 'string',
string without format
> + description => 'The fabric id',
> + },
> + node => {
> + type => 'string',
> + description => 'The hostname of the node to be deleted',
string without format
> + },
> + },
> + },
> + returns => {
> + type => 'null',
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Network::SDN::lock_sdn_config(
> + sub {
> +
> PVE::API2::Network::SDN::Fabrics::Common::delete_node("openfabric", $param);
> + }, "delete sdn fabric node failed");
> + return undef;
same as above
> + },
> +});
> +
> +__PACKAGE__->register_method({
> + name => 'update_fabric',
> + path => '{fabric}',
> + method => 'PUT',
> + description => 'Update SDN Fabric configuration',
> + protected => 1,
> + permissions => {
> + check => ['perm', '/sdn/fabrics/openfabric/{fabric}', [ 'SDN.Allocate'
> ]],
> + },
> + parameters => {
> + properties => {
> + fabric => {
> + type => 'string',
> + description => 'The fabric id',
string without format
> + },
> + hello_interval => {
> + optional => 1,
> + type => 'integer',
> + description => 'The hello_interval in seconds (1-600)',
this repeats a lot of the schema, would it be possible to get it
generated somehow?
> + },
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Network::SDN::lock_sdn_config(
> + sub {
> +
> PVE::API2::Network::SDN::Fabrics::Common::edit_fabric("openfabric", $param);
> + }, "edit sdn fabric failed");
> + return undef;
same as above
> + },
> +});
> +
> +__PACKAGE__->register_method({
same comments apply here
> + name => 'update_node',
> + path => '{fabric}/node/{node}',
> + method => 'PUT',
> + description => 'Update SDN Fabric Node configuration',
> + protected => 1,
> + permissions => {
> + check => ['perm', '/sdn/fabrics/openfabric/{fabric}/node/{node}', [
> 'SDN.Allocate' ]],
> + },
> + parameters => {
> + properties => {
> + fabric => {
> + type => 'string',
> + description => 'The fabric id',
> + },
> + node => {
> + type => 'string',
> + description => 'The hostname of the node',
> + },
> + router_id => {
> + type => 'string',
> + description => 'The Router-ID of this node (will be converted
> to a real NET later',
> + },
> + interfaces => {
> + type => 'array',
> + description => 'Array of openfabric interfaces as
> propertystrings',
> + items => {
> + type => 'string',
> + description => 'Propertystring of openfabric interfaces',
> + format => 'pve-sdn-openfabric-interface',
> + },
> + },
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Network::SDN::lock_sdn_config(
> + sub {
> +
> PVE::API2::Network::SDN::Fabrics::Common::edit_node("openfabric", $param);
> + }, "edit sdn fabric node failed");
> + return undef;
> + },
> +});
> +
> +__PACKAGE__->register_method({
> + name => 'get_fabric',
> + path => '{fabric}',
should there be an index listing these, e.g. for pvesh?
> + method => 'GET',
> + description => 'Get SDN Fabric configuration',
> + permissions => {
> + check => ['perm', '/sdn/fabrics/openfabric/{fabric}', [ 'SDN.Allocate'
> ]],
> + },
> + parameters => {
> + properties => {
> + fabric => {
> + type => 'string',
> + description => 'The fabric id',
> + },
> + },
> + },
> + returns => {
> + type => 'object',
> + properties => {
> + fabric => {
> + type => 'object',
> + description => 'The fabric object',
> + properties => {
> + name => {
> + type => 'string',
> + description => 'The id of the fabric',
> + },
> + loopback_prefix => {
> + type => 'string',
> + description => 'The IP prefix for Loopback IPs',
> + },
> + hello_interval => {
> + optional => 1,
> + type => 'integer',
> + description => 'The global hello_interval option of the
> fabric, this will be set of on all interfaces automatically',
> + },
> + },
> + },
> + },
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + return
> PVE::API2::Network::SDN::Fabrics::Common::get_fabric("openfabric", $param);
> + },
> +});
> +
> +__PACKAGE__->register_method({
> + name => 'get_node',
> + path => '{fabric}/node/{node}',
same question here?
> + method => 'GET',
> + description => 'Get SDN Fabric Node configuration',
> + permissions => {
> + check => ['perm', '/sdn/fabrics/openfabric/{fabric}', [ 'SDN.Allocate'
> ]],
> + },
> + parameters => {
> + properties => {
> + fabric => {
> + type => 'string',
> + description => 'The id of the fabric',
> + },
> + node => {
> + type => 'string',
> + description => 'The hostname of the node',
> + },
> + },
> + },
> + returns => {
> + type => 'object',
> + properties => {
> + node => {
> + type => 'object',
> + description => 'The node object',
> + properties => {
> + router_id => {
> + type => 'string',
> + description => 'The Router-ID of this node (will be
> converted to a real NET later',
> + },
> + node => {
> + type => 'string',
> + description => 'The hostname of this node',
> + },
> + interface => {
> + type => 'array',
> + description => 'Array of interfaces in this fabric and
> node',
> + items => {
> + type => 'string',
> + description => 'Propertystring of the interface',
> + format => 'pve-sdn-openfabric-interface',
> + }
> + },
> + }
> + }
> + }
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + return PVE::API2::Network::SDN::Fabrics::Common::get_node("openfabric",
> $param);
> + },
> +});
> +
> +__PACKAGE__->register_method({
> + name => 'add_fabric',
> + path => '/',
> + method => 'POST',
> + description => 'Create SDN Fabric configuration',
> + protected => 1,
> + permissions => {
> + check => ['perm', '/sdn/fabrics/openfabric', [ 'SDN.Allocate' ]],
> + },
> + parameters => {
> + properties => {
> + fabric_id => {
> + type => 'string',
> + description => 'The id of the fabric',
> + },
> + loopback_prefix => {
> + type => 'string',
> + description => 'The IP prefix for Loopback IPs',
> + },
> + hello_interval => {
> + type => 'number',
> + optional => 1,
> + description => 'The global hello_interval property in seconds,
> this will be set on all interfaces automatically',
> + }
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Network::SDN::lock_sdn_config(
> + sub {
> +
> PVE::API2::Network::SDN::Fabrics::Common::add_fabric("openfabric", $param);
> + }, "add sdn fabric failed");
> + return undef;
> + },
> +});
> +
> +__PACKAGE__->register_method({
> + name => 'add_node',
so does this add a node
> + path => '{fabric}/node/{node}',
> + method => 'POST',
> + description => 'Create SDN Fabric Node configuration',
or just create/generate a config?
> + protected => 1,
> + permissions => {
> + check => ['perm', '/sdn/fabrics/openfabric/{fabric}/node/{node}', [
> 'SDN.Allocate' ]],
> + },
> + parameters => {
> + properties => {
> + fabric => {
> + type => 'string',
> + description => 'The fabric id',
> + },
> + node => {
> + type => 'string',
> + description => 'The node hostname',
> + },
> + router_id => {
> + type => 'string',
> + description => 'The Router-ID of this node (will be converted
> to a real NET later',
> + },
> + interfaces => {
> + type => 'array',
> + description => 'Array of the interfaces in this openfabric
> node',
> + items => {
> + type => 'string',
> + description => 'Propertystring of the interface',
> + format => 'pve-sdn-openfabric-interface',
> + },
> + },
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Network::SDN::lock_sdn_config(
> + sub {
> +
> PVE::API2::Network::SDN::Fabrics::Common::add_node("openfabric", $param);
> + }, "add sdn fabric node failed");
> + return undef;
> + },
> +});
> +
> +1;
> diff --git a/src/PVE/API2/Network/SDN/Makefile
> b/src/PVE/API2/Network/SDN/Makefile
> index 4dbb6c92fd82..08bec7535530 100644
> --- a/src/PVE/API2/Network/SDN/Makefile
> +++ b/src/PVE/API2/Network/SDN/Makefile
> @@ -7,4 +7,5 @@ PERL5DIR=${DESTDIR}/usr/share/perl5
> install:
> for i in ${SOURCES}; do install -D -m 0644 $$i
> ${PERL5DIR}/PVE/API2/Network/SDN/$$i; done
> make -C Zones install
> + make -C Fabrics install
this also seems like it would belong in a different patch?
>
> --
> 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
