From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id D74701FF13A for ; Wed, 10 Jun 2026 18:26:37 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A51AB14463; Wed, 10 Jun 2026 18:26:37 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 10 Jun 2026 18:26:03 +0200 Message-Id: From: "Max R. Carrara" Subject: Re: [PATCH pmg-api v2 1/5] config: add root_only paramter option. To: "Stoiko Ivanov" , X-Mailer: aerc 0.18.2-0-ge037c095a049 References: <20260609200637.904334-1-s.ivanov@proxmox.com> <20260609200637.904334-2-s.ivanov@proxmox.com> In-Reply-To: <20260609200637.904334-2-s.ivanov@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1781108716601 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.081 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [config.pm] Message-ID-Hash: LYAE4O2LI7WIGXYLFSM7AU5TAQAZRQA2 X-Message-ID-Hash: LYAE4O2LI7WIGXYLFSM7AU5TAQAZRQA2 X-MailFrom: m.carrara@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Tue Jun 9, 2026 at 10:04 PM CEST, Stoiko Ivanov wrote: > this introduces a way to have some options in our pmg.conf > SectionConfig restricted to 'root@pam' only. > > I skimmed the (quite improved) documentation in PVE::SectionConfig, =F0=9F=98=87 > and did not see any blockers to extending adding other options instead > of 'optional' and 'fixed' there. Yeah, this should be fine, especially for such relatively small things. The SectionConfig code doesn't really care about any other keys, i.e. it leaves them untouched. Also, I want to mention that it's good that it's in `options()` anyway, because even though adding an equivalent key to the property itself in `properties()` would've also worked, it would've nevertheless been more tricky to handle. (We have a separate getter method for acquiring a property's schema that you'd have to call.) > > This is needed to have unified way to check if a particular option > shoudld be restricted to 'root@pam' only. s/shoudld/should > > Signed-off-by: Stoiko Ivanov > --- > src/PMG/API2/Config.pm | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/PMG/API2/Config.pm b/src/PMG/API2/Config.pm > index 9cce95dd..6edde9b4 100644 > --- a/src/PMG/API2/Config.pm > +++ b/src/PMG/API2/Config.pm > @@ -10,6 +10,7 @@ use HTTP::Status qw(:constants); > use Storable qw(dclone); > use PVE::JSONSchema qw(get_standard_option); > use PVE::RESTHandler; > +use PVE::Exception qw(raise_perm_exc); > use Time::HiRes qw(); > > use PMG::Config; > @@ -194,14 +195,21 @@ my $api_update_config_section =3D sub { > die "no options specified\n" > if !$delete_str && !scalar(keys %$param); > > + my $plugin =3D PMG::Config::Base->lookup($section); > + my $rpcenv =3D PMG::RESTEnvironment->get(); > + my $authuser =3D $rpcenv->get_user(); > + > foreach my $opt (PVE::Tools::split_list($delete_str)) { > + my $is_root_only =3D $plugin->options()->{$opt}->{root_only}= ; I would maaaybe rename the key from root_only to 'root-only' (quotes included), since that's the newer convention we're using. (Note that this isn't mentioned in our Perl style guide (yet?) FWICT, so it's your call whether you want to rename it or not, tbh.) > + raise_perm_exc() if ( $is_root_only && $authuser ne 'root@pa= m') ; > delete $ids->{$section}->{$opt}; > } > > - my $plugin =3D PMG::Config::Base->lookup($section); > my $config =3D $plugin->check_config($section, $param, 0, 1); > > foreach my $p (keys %$config) { > + my $is_root_only =3D $plugin->options()->{$p}->{root_only}; Here as well, if you do decide to change it. > + raise_perm_exc() if ( $is_root_only && $authuser ne 'root@pa= m') ; > $ids->{$section}->{$p} =3D $config->{$p}; > } >