Re: [pve-devel] [PATCH pve-manager 2/4] add permissions management for "local" network zone

["DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>]

Le vendredi 02 juin 2023 à 13:39 +0200, Fabian Grünbichler a écrit :
> On May 26, 2023 9:27 am, Alexandre Derumier wrote:
> > add a default virtual zone called 'local' in the ressource tree,
> > and handle permissions like a true sdn zone
> > 
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >  PVE/API2/Cluster.pm                 | 12 ++++++++++++
> >  PVE/API2/Network.pm                 |  5 +++--
> >  www/manager6/sdn/ZoneContentView.js | 27
> > ++++++++++++++++++++++++++-
> >  3 files changed, 41 insertions(+), 3 deletions(-)
> > 
> > diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> > index 2e942368..e8f5e06e 100644
> > --- a/PVE/API2/Cluster.pm
> > +++ b/PVE/API2/Cluster.pm
> > @@ -474,6 +474,18 @@ __PACKAGE__->register_method({
> >             }
> >         }
> >  
> > +       #add default "local" network zone
> > +       foreach my $node (@$nodelist) {
> > +           my $local_sdn = {
> > +               id => "sdn/$node/local",
> > +               sdn => 'local',
> > +               node => $node,
> > +               type => 'sdn',
> > +               status => 'ok',
> > +           };
> > +           push @$res, $local_sdn;
> > +       }
> > +
> 
> should this als obe guarded by the type check like below? is there
> anything that ensures that a 'local' zone doesn't already exist as
> regular SDN-managed zone?

I was more thinking to forbid "local" name in sdn code in another
patch,
as sdn it's still in beta anyway, user could still rename zone manually
in cfg.

if not, user will not be able to manage local bridges permissions.


> 
> >         if ($have_sdn) {
> >             if (!$param->{type} || $param->{type} eq 'sdn') {
> >  
> > diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm
> > index a43579fa..b3faba1a 100644
> > --- a/PVE/API2/Network.pm
> > +++ b/PVE/API2/Network.pm
> > @@ -209,7 +209,7 @@ __PACKAGE__->register_method({
> >             type => {
> >                 description => "Only list specific interface
> > types.",
> >                 type => 'string',
> > -               enum => [ @$network_type_enum, 'any_bridge' ],
> > +               enum => [ @$network_type_enum, 'any_bridge',
> > 'any_local_bridge' ],
> >                 optional => 1,
> >             },
> >         },
> > @@ -254,7 +254,8 @@ __PACKAGE__->register_method({
> >  
> >             for my $k (sort keys $ifaces->%*) {
> >                 my $type = $ifaces->{$k}->{type};
> > -               my $match = $tfilter eq $type || ($tfilter eq
> > 'any_bridge' && ($type eq 'bridge' || $type eq 'OVSBridge'));
> > +               my $match = $tfilter eq $type || ($tfilter =~
> > /^any(_local)?_bridge$/ && ($type eq 'bridge' || $type eq
> > 'OVSBridge'));
> 
> this line is getting a bit long, maybe it could be reformated or
> refactored?

yes sure.

> 
> > +
> 
> nit: this blank newline is introduced here and removed in the next
> patch ;)
> 
> >                 delete $ifaces->{$k} if !($match &&
> > $can_access_vnet->($k));
> >             }
> >  
> > diff --git a/www/manager6/sdn/ZoneContentView.js
> > b/www/manager6/sdn/ZoneContentView.js
> > index 08fa9d81..1a994fc9 100644
> > --- a/www/manager6/sdn/ZoneContentView.js
> > +++ b/www/manager6/sdn/ZoneContentView.js
> > @@ -26,6 +26,9 @@ Ext.define('PVE.sdn.ZoneContentView', {
> >         }
> >  
> >         var baseurl = "/nodes/" + me.nodename + "/sdn/zones/" +
> > me.zone + "/content";
> > +       if (me.zone === 'local') {
> > +           baseurl = "/nodes/" + me.nodename +
> > "/network?type=any_local_bridge";
> > +       }
> >         var store = Ext.create('Ext.data.Store', {
> >             model: 'pve-sdnzone-content',
> >             groupField: 'content',
> > @@ -95,7 +98,29 @@ Ext.define('PVE.sdn.ZoneContentView', {
> >      Ext.define('pve-sdnzone-content', {
> >         extend: 'Ext.data.Model',
> >         fields: [
> > -           'vnet', 'status', 'statusmsg',
> > +           {
> > +               name: 'iface',
> > +               convert: function(value, record) {
> > +                   //map local vmbr to vnet
> > +                   if (record.data.iface) {
> > +                       record.data.vnet = record.data.iface;
> > +                   }
> > +                   return value;
> > +               },
> > +           },
> > +           {
> > +               name: 'comments',
> > +               convert: function(value, record) {
> > +                   //map local vmbr comments to vnet alias
> > +                   if (record.data.comments) {
> > +                       record.data.alias = record.data.comments;
> > +                   }
> > +                   return value;
> > +               },
> > +           },
> > +           'vnet',
> > +           'status',
> > +           'statusmsg',
> >             {
> >                 name: 'text',
> >                 convert: function(value, record) {
> > -- 
> > 2.30.2
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@lists.proxmox.com
> > https://antiphishing.cetsi.fr/proxy/v3?i=Zk92VEFKaGQ4Ums4cnZEUWMTpfHaXFQGRw1_CnOoOH0&r=bHA1dGV3NWJQVUloaWNFUZPu0fK2BfWNnEHaDLDwG0rtDedpluZBIffSL1M5cj3F&f=SlhDbE9uS2laS2JaZFpNWvmsxai1zlJP9llgnl5HIv-4jAji8Dh2BQawzxID5bzr6Uv-3EQd-eluQbsPfcUOTg&u=https%3A//lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel&k=XRKU
> > 
> > 
> > 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=Zk92VEFKaGQ4Ums4cnZEUWMTpfHaXFQGRw1_CnOoOH0&r=bHA1dGV3NWJQVUloaWNFUZPu0fK2BfWNnEHaDLDwG0rtDedpluZBIffSL1M5cj3F&f=SlhDbE9uS2laS2JaZFpNWvmsxai1zlJP9llgnl5HIv-4jAji8Dh2BQawzxID5bzr6Uv-3EQd-eluQbsPfcUOTg&u=https%3A//lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel&k=XRKU
>