From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id E62081FF0C1 for ; Fri, 18 Sep 2026 13:29:03 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 35A6E214D8; Fri, 18 Sep 2026 13:29:01 +0200 (CEST) Date: Fri, 18 Sep 2026 13:28:56 +0200 From: Gabriel Goller To: Stefan Hanreich Subject: Re: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel Message-ID: References: <20260709140625.275618-1-s.hanreich@proxmox.com> <17ce3fca-ab64-426a-b72c-2cb6b12ea4fe@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <17ce3fca-ab64-426a-b72c-2cb6b12ea4fe@proxmox.com> User-Agent: NeoMutt/20260504 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1789730937228 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.118 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust RCVD_IN_MSPIKE_H2 0.001 Average reputation (+2) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: DTSAK3OTUE5TIZ3B7XD7O5SQ36OWRKRT X-Message-ID-Hash: DTSAK3OTUE5TIZ3B7XD7O5SQ36OWRKRT X-MailFrom: g.goller@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 CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 18.09.2026 13:21, Stefan Hanreich wrote: > >> [snip] > >> @@ -282,6 +259,41 @@ __PACKAGE__->register_method({ > >> }, > >> }); > >> > >> +sub create_api_client { > >> + my ($request_timeout) = @_; > >> + > >> + my $rpcenv = PVE::RPCEnvironment::get(); > >> + my $authuser = $rpcenv->get_user(); > >> + my $credentials = $rpcenv->get_credentials(); > >> + > >> + my $api_token = $credentials->{api_token}; > >> + my $ticket = $credentials->{ticket}; > >> + my $csrf_token = $credentials->{token}; > >> + > >> + my $node = PVE::INotify::nodename(); > >> + my $fingerprint = PVE::Cluster::get_node_fingerprint($node); > >> + > >> + my $conn_args = { > >> + protocol => 'https', > >> + host => 'localhost', # always call the api locally, let pveproxy handle the proxying > >> + port => 8006, > >> + username => $authuser, > >> + ticket => $ticket, > >> + apitoken => $api_token, > >> + timeout => $request_timeout // 25, # default slightly shorter than the proxy->daemon timeout > >> + cached_fingerprints => { > >> + $fingerprint => 1, > >> + }, > >> + }; > >> + > >> + my $api_client = PVE::APIClient::LWP->new($conn_args->%*); > >> + if (defined($csrf_token)) { > >> + $api_client->update_csrftoken($csrf_token); > >> + } > >> + > >> + return $api_client; > >> +} > >> + > >> __PACKAGE__->register_method({ > >> name => 'reload', > >> protected => 1, > >> @@ -291,6 +303,7 @@ __PACKAGE__->register_method({ > >> permissions => { > >> check => ['perm', '/sdn', ['SDN.Allocate']], > > > > Hmm this endpoint has SDN.Allocate and the PUT /network endpoint we are calling > > (in pve-manager) needs Sys.Modify. This shouldn't be an issue, but maybe we > > should require Sys.Modify here as well to short-circuit everything, instead of > > making a request to every node. > > Hmm, pre-existing, but what about setups where a user has Sys.Modify only on some nodes? That's > currently possible but probably quite bad, since it allows applying the SDN configuration to > some nodes only. So, I agree - but a breaking change? Aah, this is obviously a breaking change -- you're right. Then this is fine IMO. > >> }, > >> + expose_credentials => 1, > >> parameters => { > >> additionalProperties => 0, > >> properties => { > >> [snip]