From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] applied: [PATCH pve-network] fix permissions && use new /sdn/zones/<zone>/<vnet> path
Date: Mon, 12 Jun 2023 15:29:12 +0000 [thread overview]
Message-ID: <05b9adcd98e1452aefa508f9c91b8a4116773561.camel@groupe-cyllene.com> (raw)
In-Reply-To: <1686580529.kfidru01ul.astroid@yuna.none>
Le lundi 12 juin 2023 à 16:36 +0200, Fabian Grünbichler a écrit :
> one more thing I realized while testing that might be worthy of a
> follow
> up - if you configure SDN, but for some reason, your
> /etc/network/interfaces is missing the "source .." line, the error
> handling behaviour is very strange:
> - vnets are displayed in the resource tree next to zones
> - no proper indication about the root cause
Yes, I have see that recently (also on pve7), when vnet is in error,
it's displayed in the tree.
I don't remember to have implemented this, the vnet should be masked
from the tree, and the zone should have an error icon.
(and the vnet error message should be displayed in the zone panel)
I'll be busy all the week (pve training week session), I'm not sure
to be able to send patch this week.
(BTW, maybe it could be great to include the "source ...." in new
pve8.0 install ? It's the default on a stock debian install).
>
> both shouldn't be too hard to fix I guess..
>
> On June 8, 2023 3:24 am, Alexandre Derumier wrote:
> > - use new /sdn/zones/zone/<vnet> path for vnet && subnets
> > permissions
> >
> > - fix some permissions on /sdn/zones && /sdn
> >
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> > PVE/API2/Network/SDN.pm | 2 +-
> > PVE/API2/Network/SDN/Subnets.pm | 67 ++++++++++++++++++++++++-----
> > ----
> > PVE/API2/Network/SDN/Vnets.pm | 45 +++++++++++++++++-----
> > PVE/API2/Network/SDN/Zones.pm | 4 +-
> > 4 files changed, 88 insertions(+), 30 deletions(-)
> >
> > diff --git a/PVE/API2/Network/SDN.pm b/PVE/API2/Network/SDN.pm
> > index 33ecfb7..d216e48 100644
> > --- a/PVE/API2/Network/SDN.pm
> > +++ b/PVE/API2/Network/SDN.pm
> > @@ -51,7 +51,7 @@ __PACKAGE__->register_method({
> > method => 'GET',
> > description => "Directory index.",
> > permissions => {
> > - check => ['perm', '/', [ 'SDN.Audit' ]],
> > + check => ['perm', '/sdn', [ 'SDN.Audit' ]],
> > },
> > parameters => {
> > additionalProperties => 0,
> > diff --git a/PVE/API2/Network/SDN/Subnets.pm
> > b/PVE/API2/Network/SDN/Subnets.pm
> > index 377a568..eb6b41b 100644
> > --- a/PVE/API2/Network/SDN/Subnets.pm
> > +++ b/PVE/API2/Network/SDN/Subnets.pm
> > @@ -33,13 +33,34 @@ my $api_sdn_subnets_config = sub {
> > return $scfg;
> > };
> >
> > +my $api_sdn_vnets_config = sub {
> > + my ($cfg, $id) = @_;
> > +
> > + my $scfg =
> > dclone(PVE::Network::SDN::Vnets::sdn_vnets_config($cfg, $id));
> > + $scfg->{vnet} = $id;
> > + $scfg->{digest} = $cfg->{digest};
> > +
> > + return $scfg;
> > +};
> > +
> > +my $check_vnet_access = sub {
> > + my ($vnet, $privs) = @_;
> > +
> > + my $cfg = PVE::Network::SDN::Vnets::config();
> > + my $rpcenv = PVE::RPCEnvironment::get();
> > + my $authuser = $rpcenv->get_user();
> > + my $scfg = &$api_sdn_vnets_config($cfg, $vnet);
> > + my $zoneid = $scfg->{zone};
> > + $rpcenv->check_any($authuser, "/sdn/zones/$zoneid/$vnet",
> > $privs);
> > +};
> > +
> > __PACKAGE__->register_method ({
> > name => 'index',
> > path => '',
> > method => 'GET',
> > description => "SDN subnets index.",
> > permissions => {
> > - description => "Only list entries where you have
> > 'SDN.Audit' or 'SDN.Allocate' permissions on
> > '/sdn/subnets/<subnet>'",
> > + description => "Only list entries where you have
> > 'SDN.Audit' or 'SDN.Allocate' permissions on
> > '/sdn/zones/<zone>/<vnet>'",
> > user => 'all',
> > },
> > parameters => {
> > @@ -69,10 +90,9 @@ __PACKAGE__->register_method ({
> > code => sub {
> > my ($param) = @_;
> >
> > - my $rpcenv = PVE::RPCEnvironment::get();
> > - my $authuser = $rpcenv->get_user();
> > -
> > - my $vnetid = $param->{vnet};
> > + my $vnetid = $param->{vnet};
> > + my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];
> > + &$check_vnet_access($vnetid, $privs);
> >
> > my $cfg = {};
> > if($param->{pending}) {
> > @@ -89,9 +109,6 @@ __PACKAGE__->register_method ({
> > my @sids =
> > PVE::Network::SDN::Subnets::sdn_subnets_ids($cfg);
> > my $res = [];
> > foreach my $id (@sids) {
> > - my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];
> > - next if !$rpcenv->check_any($authuser,
> > "/sdn/vnets/$vnetid/subnets/$id", $privs, 1);
> > -
> > my $scfg = &$api_sdn_subnets_config($cfg, $id);
> > next if !$scfg->{vnet} || $scfg->{vnet} ne $vnetid;
> > push @$res, $scfg;
> > @@ -106,9 +123,9 @@ __PACKAGE__->register_method ({
> > method => 'GET',
> > description => "Read sdn subnet configuration.",
> > permissions => {
> > - check => ['perm', '/sdn/vnets/{vnet}/subnets/{subnet}',
> > ['SDN.Allocate']],
> > - },
> > -
> > + description => "Require 'SDN.Audit' or 'SDN.Allocate'
> > permissions on '/sdn/zones/<zone>/<vnet>'",
> > + user => 'all',
> > + },
> > parameters => {
> > additionalProperties => 0,
> > properties => {
> > @@ -132,6 +149,10 @@ __PACKAGE__->register_method ({
> > code => sub {
> > my ($param) = @_;
> >
> > + my $vnet = extract_param($param, 'vnet');
> > + my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];
> > + &$check_vnet_access($vnet, $privs);
> > +
> > my $cfg = {};
> > if($param->{pending}) {
> > my $running_cfg = PVE::Network::SDN::running_config();
> > @@ -146,7 +167,7 @@ __PACKAGE__->register_method ({
> >
> > my $scfg = &$api_sdn_subnets_config($cfg, $param-
> > >{subnet});
> >
> > - raise_param_exc({ vnet => "wrong vnet"}) if $param->{vnet}
> > ne $scfg->{vnet};
> > + raise_param_exc({ vnet => "wrong vnet"}) if $vnet ne $scfg-
> > >{vnet};
> >
> > return $scfg;
> > }});
> > @@ -158,7 +179,8 @@ __PACKAGE__->register_method ({
> > method => 'POST',
> > description => "Create a new sdn subnet object.",
> > permissions => {
> > - check => ['perm', '/sdn/vnets/{vnet}/subnets',
> > ['SDN.Allocate']],
> > + description => "Require 'SDN.Allocate' permission on
> > '/sdn/zones/<zone>/<vnet>'",
> > + user => 'all',
> > },
> > parameters => PVE::Network::SDN::SubnetPlugin->createSchema(),
> > returns => { type => 'null' },
> > @@ -168,6 +190,10 @@ __PACKAGE__->register_method ({
> > my $type = extract_param($param, 'type');
> > my $cidr = extract_param($param, 'subnet');
> >
> > + my $vnet = $param->{vnet};
> > + my $privs = [ 'SDN.Allocate' ];
> > + &$check_vnet_access($vnet, $privs);
> > +
> > # create /etc/pve/sdn directory
> > PVE::Cluster::check_cfs_quorum();
> > mkdir("/etc/pve/sdn") if ! -d '/etc/pve/sdn';
> > @@ -210,7 +236,8 @@ __PACKAGE__->register_method ({
> > method => 'PUT',
> > description => "Update sdn subnet object configuration.",
> > permissions => {
> > - check => ['perm', '/sdn/vnets/{vnet}/subnets',
> > ['SDN.Allocate']],
> > + description => "Require 'SDN.Allocate' permission on
> > '/sdn/zones/<zone>/<vnet>'",
> > + user => 'all',
> > },
> > parameters => PVE::Network::SDN::SubnetPlugin->updateSchema(),
> > returns => { type => 'null' },
> > @@ -219,6 +246,10 @@ __PACKAGE__->register_method ({
> >
> > my $id = extract_param($param, 'subnet');
> > my $digest = extract_param($param, 'digest');
> > + my $vnet = $param->{vnet};
> > +
> > + my $privs = [ 'SDN.Allocate' ];
> > + &$check_vnet_access($vnet, $privs);
> >
> > PVE::Network::SDN::lock_sdn_config(
> > sub {
> > @@ -226,7 +257,6 @@ __PACKAGE__->register_method ({
> > my $cfg = PVE::Network::SDN::Subnets::config();
> > my $zone_cfg = PVE::Network::SDN::Zones::config();
> > my $vnet_cfg = PVE::Network::SDN::Vnets::config();
> > - my $vnet = $param->{vnet};
> > my $zoneid = $vnet_cfg->{ids}->{$vnet}->{zone};
> > my $zone = $zone_cfg->{ids}->{$zoneid};
> >
> > @@ -256,7 +286,8 @@ __PACKAGE__->register_method ({
> > method => 'DELETE',
> > description => "Delete sdn subnet object configuration.",
> > permissions => {
> > - check => ['perm', '/sdn/vnets/{vnet}/subnets',
> > ['SDN.Allocate']],
> > + description => "Require 'SDN.Allocate' permission on
> > '/sdn/zones/<zone>/<vnet>'",
> > + user => 'all',
> > },
> > parameters => {
> > additionalProperties => 0,
> > @@ -272,6 +303,9 @@ __PACKAGE__->register_method ({
> > my ($param) = @_;
> >
> > my $id = extract_param($param, 'subnet');
> > + my $vnet = extract_param($param, 'vnet');
> > + my $privs = [ 'SDN.Allocate' ];
> > + &$check_vnet_access($vnet, $privs);
> >
> > PVE::Network::SDN::lock_sdn_config(
> > sub {
> > @@ -284,7 +318,6 @@ __PACKAGE__->register_method ({
> > PVE::Network::SDN::SubnetPlugin-
> > >on_delete_hook($id, $cfg, $vnets_cfg);
> >
> > my $zone_cfg = PVE::Network::SDN::Zones::config();
> > - my $vnet = $param->{vnet};
> > my $zoneid = $vnets_cfg->{ids}->{$vnet}->{zone};
> > my $zone = $zone_cfg->{ids}->{$zoneid};
> >
> > diff --git a/PVE/API2/Network/SDN/Vnets.pm
> > b/PVE/API2/Network/SDN/Vnets.pm
> > index 811a2e8..864dc4a 100644
> > --- a/PVE/API2/Network/SDN/Vnets.pm
> > +++ b/PVE/API2/Network/SDN/Vnets.pm
> > @@ -50,6 +50,17 @@ my $api_sdn_vnets_deleted_config = sub {
> > }
> > };
> >
> > +my $check_vnet_access = sub {
> > + my ($vnet, $privs) = @_;
> > +
> > + my $cfg = PVE::Network::SDN::Vnets::config();
> > + my $rpcenv = PVE::RPCEnvironment::get();
> > + my $authuser = $rpcenv->get_user();
> > + my $scfg = &$api_sdn_vnets_config($cfg, $vnet);
> > + my $zoneid = $scfg->{zone};
> > + $rpcenv->check_any($authuser, "/sdn/zones/$zoneid/$vnet",
> > $privs);
> > +};
> > +
> > __PACKAGE__->register_method ({
> > name => 'index',
> > path => '',
> > @@ -57,7 +68,7 @@ __PACKAGE__->register_method ({
> > description => "SDN vnets index.",
> > permissions => {
> > description => "Only list entries where you have
> > 'SDN.Audit' or 'SDN.Allocate'"
> > - ." permissions on '/sdn/vnets/<vnet>'",
> > + ." permissions on '/sdn/zones/<zone>/<vnet>'",
> > user => 'all',
> > },
> > parameters => {
> > @@ -105,9 +116,10 @@ __PACKAGE__->register_method ({
> > my $res = [];
> > foreach my $id (@sids) {
> > my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];
> > - next if !$rpcenv->check_any($authuser,
> > "/sdn/vnets/$id", $privs, 1);
> > -
> > my $scfg = &$api_sdn_vnets_config($cfg, $id);
> > + my $zoneid = $scfg->{zone};
> > + next if !$rpcenv->check_any($authuser,
> > "/sdn/zones/$zoneid/$id", $privs, 1);
> > +
> > push @$res, $scfg;
> > }
> >
> > @@ -120,8 +132,9 @@ __PACKAGE__->register_method ({
> > method => 'GET',
> > description => "Read sdn vnet configuration.",
> > permissions => {
> > - check => ['perm', '/sdn/vnets/{vnet}', ['SDN.Allocate']],
> > - },
> > + description => "Require 'SDN.Audit' or 'SDN.Allocate'
> > permissions on '/sdn/zones/<zone>/<vnet>'",
> > + user => 'all',
> > + },
> > parameters => {
> > additionalProperties => 0,
> > properties => {
> > @@ -144,6 +157,11 @@ __PACKAGE__->register_method ({
> > code => sub {
> > my ($param) = @_;
> >
> > + my $id = extract_param($param, 'vnet');
> > +
> > + my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];
> > + &$check_vnet_access($id, $privs);
> > +
> > my $cfg = {};
> > if($param->{pending}) {
> > my $running_cfg = PVE::Network::SDN::running_config();
> > @@ -156,7 +174,7 @@ __PACKAGE__->register_method ({
> > $cfg = PVE::Network::SDN::Vnets::config();
> > }
> >
> > - return $api_sdn_vnets_config->($cfg, $param->{vnet});
> > + return $api_sdn_vnets_config->($cfg, $id);
> > }});
> >
> > __PACKAGE__->register_method ({
> > @@ -166,7 +184,7 @@ __PACKAGE__->register_method ({
> > method => 'POST',
> > description => "Create a new sdn vnet object.",
> > permissions => {
> > - check => ['perm', '/sdn/vnets', ['SDN.Allocate']],
> > + check => ['perm', '/sdn/zones/{zone}', ['SDN.Allocate']],
> > },
> > parameters => PVE::Network::SDN::VnetPlugin->createSchema(),
> > returns => { type => 'null' },
> > @@ -210,7 +228,8 @@ __PACKAGE__->register_method ({
> > method => 'PUT',
> > description => "Update sdn vnet object configuration.",
> > permissions => {
> > - check => ['perm', '/sdn/vnets', ['SDN.Allocate']],
> > + description => "Require 'SDN.Allocate' permission on
> > '/sdn/zones/<zone>/<vnet>'",
> > + user => 'all',
> > },
> > parameters => PVE::Network::SDN::VnetPlugin->updateSchema(),
> > returns => { type => 'null' },
> > @@ -220,12 +239,14 @@ __PACKAGE__->register_method ({
> > my $id = extract_param($param, 'vnet');
> > my $digest = extract_param($param, 'digest');
> >
> > + my $privs = [ 'SDN.Allocate' ];
> > + &$check_vnet_access($id, $privs);
> > +
> > PVE::Network::SDN::lock_sdn_config(sub {
> > my $cfg = PVE::Network::SDN::Vnets::config();
> >
> > PVE::SectionConfig::assert_if_modified($cfg, $digest);
> >
> > -
> > my $opts = PVE::Network::SDN::VnetPlugin-
> > >check_config($id, $param, 0, 1);
> > raise_param_exc({ zone => "missing zone"}) if !$opts-
> > >{zone};
> > my $subnets =
> > PVE::Network::SDN::Vnets::get_subnets($id);
> > @@ -256,7 +277,8 @@ __PACKAGE__->register_method ({
> > method => 'DELETE',
> > description => "Delete sdn vnet object configuration.",
> > permissions => {
> > - check => ['perm', '/sdn/vnets', ['SDN.Allocate']],
> > + description => "Require 'SDN.Allocate' permission on
> > '/sdn/zones/<zone>/<vnet>'",
> > + user => 'all',
> > },
> > parameters => {
> > additionalProperties => 0,
> > @@ -272,6 +294,9 @@ __PACKAGE__->register_method ({
> >
> > my $id = extract_param($param, 'vnet');
> >
> > + my $privs = [ 'SDN.Allocate' ];
> > + &$check_vnet_access($id, $privs);
> > +
> > PVE::Network::SDN::lock_sdn_config(sub {
> > my $cfg = PVE::Network::SDN::Vnets::config();
> > my $scfg =
> > PVE::Network::SDN::Vnets::sdn_vnets_config($cfg, $id); # check if
> > exists
> > diff --git a/PVE/API2/Network/SDN/Zones.pm
> > b/PVE/API2/Network/SDN/Zones.pm
> > index 6e53240..4c8b7e1 100644
> > --- a/PVE/API2/Network/SDN/Zones.pm
> > +++ b/PVE/API2/Network/SDN/Zones.pm
> > @@ -251,7 +251,7 @@ __PACKAGE__->register_method ({
> > method => 'PUT',
> > description => "Update sdn zone object configuration.",
> > permissions => {
> > - check => ['perm', '/sdn/zones', ['SDN.Allocate']],
> > + check => ['perm', '/sdn/zones/{zone}', ['SDN.Allocate']],
> > },
> > parameters => PVE::Network::SDN::Zones::Plugin-
> > >updateSchema(),
> > returns => { type => 'null' },
> > @@ -315,7 +315,7 @@ __PACKAGE__->register_method ({
> > method => 'DELETE',
> > description => "Delete sdn zone object configuration.",
> > permissions => {
> > - check => ['perm', '/sdn/zones', ['SDN.Allocate']],
> > + check => ['perm', '/sdn/zones/{zone}', ['SDN.Allocate']],
> > },
> > parameters => {
> > additionalProperties => 0,
> > --
> > 2.30.2
> >
> >
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@lists.proxmox.com
> > https://antiphishing.cetsi.fr/proxy/v3?i=d1l4NXNNaWE4SWZqU0dLWcuTfdxEd98NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2xMdVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS
> >
> >
> >
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=d1l4NXNNaWE4SWZqU0dLWcuTfdxEd98NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2xMdVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS
>
next prev parent reply other threads:[~2023-06-12 15:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-08 1:24 [pve-devel] " Alexandre Derumier
2023-06-12 14:36 ` [pve-devel] applied: " Fabian Grünbichler
2023-06-12 15:29 ` DERUMIER, Alexandre [this message]
2023-06-13 7:35 ` Fabian Grünbichler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=05b9adcd98e1452aefa508f9c91b8a4116773561.camel@groupe-cyllene.com \
--to=alexandre.derumier@groupe-cyllene.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox