Re: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel

[Gabriel Goller <g.goller@proxmox.com>]

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]