From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id DB64C1FF13C for ; Thu, 19 Feb 2026 16:49:03 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CAFF71D090; Thu, 19 Feb 2026 16:50:03 +0100 (CET) Date: Thu, 19 Feb 2026 16:49:29 +0100 From: Gabriel Goller To: Hannes Laimer Subject: Re: [PATCH pve-network 07/10] cli: add pvesdn cli tool for managing frr template overrides Message-ID: Mail-Followup-To: Hannes Laimer , pve-devel@lists.proxmox.com References: <20260203160246.353351-1-g.goller@proxmox.com> <20260203160246.353351-19-g.goller@proxmox.com> <3c863323-8519-436b-9f66-15da53c8b827@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <3c863323-8519-436b-9f66-15da53c8b827@proxmox.com> User-Agent: NeoMutt/20241002-35-39f9a6 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1771516160244 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.003 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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: KYME6JGNLGF6EE3OEUSLY7SDEIGLIHXS X-Message-ID-Hash: KYME6JGNLGF6EE3OEUSLY7SDEIGLIHXS 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: [..] > > diff --git a/src/PVE/CLI/pvesdn.pm b/src/PVE/CLI/pvesdn.pm > > new file mode 100644 > > index 000000000000..ebb0b60715c9 > > --- /dev/null > > +++ b/src/PVE/CLI/pvesdn.pm > > @@ -0,0 +1,252 @@ > > [..] > > +__PACKAGE__->register_method({ > > + name => 'override', > > + path => 'override', > > + method => 'GET', > > + description => "Override FRR templates.", > > + parameters => { > > + properties => { > > + protocol => { > > + description => > > + "Specifies the FRR routing protocol (e.g., 'bgp', 'ospf') or template file (e.g., 'access_lists.jinja') to copy to the override directory for customization.", > > + type => 'string', > > + }, > > + }, > > + > > + }, > > + returns => { type => 'null' }, > > + code => sub { > > + my ($param) = @_; > > + my @template_files = (); > > + > > + if ($param->{protocol} eq 'openfabric') { > > + push(@template_files, 'frr.conf.jinja'); > > + push(@template_files, 'fabricd.jinja'); > > + push(@template_files, 'protocol_routemaps.jinja'); > > + push(@template_files, 'route_maps.jinja'); > > + push(@template_files, 'access_lists.jinja'); > > + push(@template_files, 'interface.jinja'); > > + } elsif ($param->{protocol} eq 'ospf') { > > + push(@template_files, 'frr.conf.jinja'); > > + push(@template_files, 'ospfd.jinja'); > > + push(@template_files, 'protocol_routemaps.jinja'); > > + push(@template_files, 'route_maps.jinja'); > > + push(@template_files, 'access_lists.jinja'); > > + push(@template_files, 'interface.jinja'); > > + } elsif ($param->{protocol} eq 'isis') { > > + push(@template_files, 'frr.conf.jinja'); > > + push(@template_files, 'isisd.jinja'); > > + push(@template_files, 'interface.jinja'); > > + } elsif ($param->{protocol} eq 'bgp') { > > + push(@template_files, 'frr.conf.jinja'); > > + push(@template_files, 'bgpd.jinja'); > > + push(@template_files, 'bgp_router.jinja'); > > + push(@template_files, 'route_maps.jinja'); > > + push(@template_files, 'access_lists.jinja'); > > + push(@template_files, 'prefix_lists.jinja'); > > + push(@template_files, 'ip_routes.jinja'); > > + } else { > > + push(@template_files, $param->{protocol}); > > + } > > + > > + File::Path::make_path($TEMPLATE_OVERRIDE_DIR); > > + > > + foreach my $template (@template_files) { > > + my $filepath = "$TEMPLATE_OVERRIDE_DIR/$template"; > > + > > + open(my $fh, '>', $filepath) or die "Could not open file '$filepath': $!\n"; > > since this does create the file, we should probably open the file only > after we're sure the template we try to override exists. otherwise we'll > end up with an empty override-file for a non-existing template Makes sense. > > + > > + my $template_content = PVE::RS::SDN::get_template($template); > > + if (!defined($template_content)) { > > + die "Template '$template' not found\n"; > > + } > > + print $fh $template_content; > > + close $fh; > > + > > + print "Created override file: $filepath\n"; > > + } > > + return undef; > > + }, > > +}); > > + > > +__PACKAGE__->register_method({ > > + name => 'show', > > + path => 'show', > > + method => 'GET', > > + description => "Show FRR template.", > > should we maybe show the override, if one exists, cause that's what will > be used? maybe even with small indicator that not the packaged version > is in use This is the only way to get the provisioned templates for the user (without searching through the code), so I'd like to keep this is as it is. This is also used in the postinst script to make a 3-way-diff between the override/packaged and newly-packaged. But I see your concern. Maybe we should create a new command for this? Thanks for the review! > > + parameters => { > > + properties => { > > + "template-name" => { > > + description => "Name of the FRR template (e.g. 'bgpd.jinja').", > > + type => 'string', > > + }, > > + }, > > + }, > > + returns => { type => 'null' }, > > + code => sub { > > + my ($param) = @_; > > + > > + my $template_name = $param->{"template-name"}; > > + my $template = PVE::RS::SDN::get_template($template_name); > > + if (defined($template)) { > > + print($template); > > + } else { > > + die("Template '$template_name' not found\n"); > > + } > > + return undef; > > + }, > > +}); > > + > > +sub write_to_template_file { > > + my ($filename, $content) = @_; > > + if ($filename =~ m/^([\w_-]+\.jinja)$/) { > > + my $safe_filename = $1; > > + > > + # create backup > > + my $filepath = "$TEMPLATE_OVERRIDE_DIR/$safe_filename"; > > + my $backup_path = "$filepath-bak"; > > + if (-f $filepath) { > > + copy($filepath, $backup_path) or die "Could not create backup: $!\n"; > > + } > > + > > + open(my $fh, '>', $filepath) or die "Could not open file '$filepath': $!\n"; > > + print $fh $content; > > + close $fh; > > + } > > + return undef; > > +} > > + > > [..]