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 426301FF139 for ; Tue, 24 Feb 2026 15:04:28 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 74E4AC5E7; Tue, 24 Feb 2026 15:05:21 +0100 (CET) Message-ID: <201c3025-cd0e-49ea-8ef1-cef40eed8c82@proxmox.com> Date: Tue, 24 Feb 2026 15:05:13 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-network 07/10] cli: add pvesdn cli tool for managing frr template overrides To: pve-devel@lists.proxmox.com References: <20260203160246.353351-1-g.goller@proxmox.com> <20260203160246.353351-19-g.goller@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20260203160246.353351-19-g.goller@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.344 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 KAM_SHORT 0.001 Use of a URL Shortener for very short URL RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1.179 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.717 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.236 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: ZNF4AUYL3K2BPEOEJJMJCV7KA447DDAO X-Message-ID-Hash: ZNF4AUYL3K2BPEOEJJMJCV7KA447DDAO X-MailFrom: s.hanreich@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 VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: discussed off-list with @Thomas and @Gabriel: We'll exclude the option of overriding templates for now until we have thought more about the whole lifecycle management for overriding templates. There's still full /etc/frr/frr.conf.local support with the new implementation so let's keep that around for a while longer until we make a final decision on how we want to handle template overrides. With the upcoming features there might not even be such a big need anymore for overriding the FRR configuration. Implementing features like route maps natively or exposing additional router options in our stack is preferable to providing overriding functionality which brings a lot of additional complexity. On 2/3/26 5:03 PM, Gabriel Goller wrote: > This introduces a new cli tool to help users customize frr configuration > templates by overriding default templates, viewing differences, and > resetting modifications when needed. > > Signed-off-by: Gabriel Goller > --- > debian/libpve-network-api-perl.install | 1 + > debian/libpve-network-perl.install | 4 + > src/Makefile | 2 +- > src/PVE/CLI/Makefile | 7 + > src/PVE/CLI/pvesdn.pm | 252 +++++++++++++++++++++++++ > src/PVE/Makefile | 1 + > src/bin/Makefile | 69 +++++++ > src/bin/pvesdn | 8 + > 8 files changed, 343 insertions(+), 1 deletion(-) > create mode 100644 src/PVE/CLI/Makefile > create mode 100644 src/PVE/CLI/pvesdn.pm > create mode 100644 src/bin/Makefile > create mode 100755 src/bin/pvesdn > > diff --git a/debian/libpve-network-api-perl.install b/debian/libpve-network-api-perl.install > index c48f1c76f9f7..1f5ed3eaeb05 100644 > --- a/debian/libpve-network-api-perl.install > +++ b/debian/libpve-network-api-perl.install > @@ -1 +1,2 @@ > usr/share/perl5/PVE/API2 > +usr/share/perl5/PVE/CLI > diff --git a/debian/libpve-network-perl.install b/debian/libpve-network-perl.install > index 4e63c1ff9374..f344b8c85e13 100644 > --- a/debian/libpve-network-perl.install > +++ b/debian/libpve-network-perl.install > @@ -1,2 +1,6 @@ > lib/systemd/system/dnsmasq@.service.d/00-dnsmasq-after-networking.conf /usr/lib/systemd/system/dnsmasq@.service.d/ > usr/share/perl5/PVE/Network > +usr/bin/pvesdn > +usr/share/man/man1/pvesdn.1 > +usr/share/bash-completion/completions/pvesdn > +usr/share/zsh/vendor-completions/_pvesdn > diff --git a/src/Makefile b/src/Makefile > index c4056b480251..cbd9a507ae5e 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -1,4 +1,4 @@ > -SUBDIRS := PVE services > +SUBDIRS := PVE services bin > > all: > set -e && for i in $(SUBDIRS); do $(MAKE) -C $$i; done > diff --git a/src/PVE/CLI/Makefile b/src/PVE/CLI/Makefile > new file mode 100644 > index 000000000000..5058945a716b > --- /dev/null > +++ b/src/PVE/CLI/Makefile > @@ -0,0 +1,7 @@ > +SOURCES=pvesdn.pm > + > +PERL5DIR=${DESTDIR}/usr/share/perl5 > + > +.PHONY: install > +install: > + for i in ${SOURCES}; do install -D -m 0644 $$i ${PERL5DIR}/PVE/CLI/$$i; done > 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 PVE::CLI::pvesdn; > + > +use strict; > +use warnings; > + > +use File::Path; > +use File::Temp qw(tempfile); > +use File::Copy; > + > +use PVE::RPCEnvironment; > +use PVE::Tools qw(run_command); > + > +use PVE::RS::SDN; > + > +use base qw(PVE::CLIHandler); > + > +sub setup_environment { > + PVE::RPCEnvironment->setup_default_cli_env(); > +} > + > +my $TEMPLATE_OVERRIDE_DIR = "/etc/proxmox-frr/templates"; > + > +__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"; > + > + 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.", > + 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; > +} > + > +__PACKAGE__->register_method({ > + name => 'reset', > + path => 'reset', > + method => 'GET', > + description => "Reset a single or all override files by copying the packaged version over.", > + parameters => { > + properties => { > + name => { > + description => "Name of the FRR template (e.g. 'bgpd.jinja').", > + type => 'string', > + optional => 1, > + }, > + }, > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + if (defined($param->{name})) { > + my $template = PVE::RS::SDN::get_template($param->{name}); > + if (defined($template)) { > + print( > + "Resetting the /etc/proxmox-frr/templates/$param->{name} file - continue (y/N)? " > + ); > + my $answer = ; > + my $continue = defined($answer) && $answer =~ m/^\s*y(?:es)?\s*$/i; > + die "Aborting reset as requested\n" if !$continue; > + > + write_to_template_file($param->{name}, $template); > + print("Reset template: $param->{name}\n"); > + } else { > + die("Template '$param->{name}' not found\n"); > + } > + } else { > + print( > + "Resetting all template files in /etc/proxmox-frr/templates/ - continue (y/N)? "); > + my $answer = ; > + my $continue = defined($answer) && $answer =~ m/^\s*y(?:es)?\s*$/i; > + die "Aborting reset as requested\n" if !$continue; > + > + opendir(my $dh, $TEMPLATE_OVERRIDE_DIR) or die "Cannot open directory: $!\n"; > + my @files = grep { -f "$TEMPLATE_OVERRIDE_DIR/$_" } readdir($dh); > + closedir($dh); > + > + foreach my $file (@files) { > + my $packaged_content = PVE::RS::SDN::get_template($file); > + next unless $packaged_content; > + > + write_to_template_file($file, $packaged_content); > + print("Reset template: $file\n"); > + } > + } > + return undef; > + }, > +}); > + > +__PACKAGE__->register_method({ > + name => 'diff', > + path => 'diff', > + method => 'GET', > + description => "Show the difference between the override templates and packaged templates.", > + parameters => { > + properties => {}, > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + opendir(my $dh, $TEMPLATE_OVERRIDE_DIR) or die "Cannot open directory: $!\n"; > + my @files = grep { -f "$TEMPLATE_OVERRIDE_DIR/$_" } readdir($dh); > + closedir($dh); > + > + foreach my $file (@files) { > + # Untaint filename for use with run_command in taint mode > + next unless $file =~ m/^([\w.-]+)$/; > + my $safe_file = $1; > + > + my $override_path = "$TEMPLATE_OVERRIDE_DIR/$safe_file"; > + my $packaged_content = PVE::RS::SDN::get_template($safe_file); > + next unless $packaged_content; > + > + my ($temp_fh, $temp_filename) = tempfile(); > + print $temp_fh $packaged_content; > + > + eval { > + run_command( > + [ > + "/usr/bin/diff", > + "--color=always", > + "-N", > + "-u", > + "$override_path", > + "$temp_filename", > + ], > + ); > + }; > + close($temp_fh); > + } > + return undef; > + }, > +}); > + > +our $cmddef = { > + template => { > + override => [__PACKAGE__, 'override', ['protocol']], > + show => [__PACKAGE__, 'show', ['template-name']], > + diff => [__PACKAGE__, 'diff', []], > + reset => [__PACKAGE__, 'reset', []], > + }, > +}; > + > +1; > + > diff --git a/src/PVE/Makefile b/src/PVE/Makefile > index 7f1cf985465f..d56158823099 100644 > --- a/src/PVE/Makefile > +++ b/src/PVE/Makefile > @@ -4,5 +4,6 @@ all: > install: > make -C Network install > make -C API2 install > + make -C CLI install > > clean: > diff --git a/src/bin/Makefile b/src/bin/Makefile > new file mode 100644 > index 000000000000..ed539f5e3f94 > --- /dev/null > +++ b/src/bin/Makefile > @@ -0,0 +1,69 @@ > +PERL_DOC_INC_DIRS=.. > +-include /usr/share/pve-doc-generator/pve-doc-generator.mk > + > + > +CLITOOLS = \ > + pvesdn \ > + > +CLI_MANS = \ > + $(addsuffix .1, $(CLITOOLS)) \ > + > +BASH_COMPLETIONS = \ > + $(addsuffix .bash-completion, $(CLITOOLS)) \ > + > +ZSH_COMPLETIONS = \ > + $(addsuffix .zsh-completion, $(CLITOOLS)) \ > + > +BINDIR=/usr/bin > +MAN1DIR=/usr/share/man/man1 > +BASHCOMPLDIR=/usr/share/bash-completion/completions > +ZSHCOMPLDIR=/usr/share/zsh/vendor-completions > +DESTDIR= > + > +all: $(CLI_MANS) > + > +%.1: %.1.pod > + rm -f $@ > + cat $<|pod2man -n $* -s 1 -r $(VERSION) -c"Proxmox Documentation" - >$@.tmp > + mv $@.tmp $@ > + > +%.1.pod: > + podselect $* > $@.tmp > + mv $@.tmp $@ > + > +.PHONY: tidy > +tidy: > + echo $(CLITOOLS) | xargs -n4 -P0 proxmox-perltidy > + > +pvesdn.api-verified: > + touch $@ > + > +pvesdn.bash-completion: > + echo "# bash completion for pvesdn" > $@.tmp > + echo "complete -C 'pvesdn bashcomplete' pvesdn" >> $@.tmp > + mv $@.tmp $@ > + > +pvesdn.zsh-completion: > + echo "#compdef pvesdn" > $@.tmp > + echo "" >> $@.tmp > + mv $@.tmp $@ > + > +.PHONY: check > +check: $(addsuffix .api-verified, $(CLITOOLS)) > + rm -f *.service-api-verified *.api-verified > + > +.PHONY: install > +install: $(CLITOOLS) $(CLI_MANS) $(BASH_COMPLETIONS) $(ZSH_COMPLETIONS) > + install -d $(DESTDIR)$(BINDIR) > + install -m 0755 $(CLITOOLS) $(DESTDIR)$(BINDIR) > + install -d $(DESTDIR)$(MAN1DIR) > + install -m 0644 $(CLI_MANS) $(DESTDIR)$(MAN1DIR) > + for i in $(CLITOOLS); do install -m 0644 -D $$i.bash-completion $(DESTDIR)$(BASHCOMPLDIR)/$$i; done > + for i in $(CLITOOLS); do install -m 0644 -D $$i.zsh-completion $(DESTDIR)$(ZSHCOMPLDIR)/_$$i; done > + > +.PHONY: clean > +clean: > + rm -f *.xml.tmp *.1 *.5 *.8 *{synopsis,opts}.adoc docinfo.xml *.tmp > + rm -f *~ *.tmp $(CLI_MANS) *.1.pod *.8.pod > + rm -f *.bash-completion *.zsh-completion *.service-zsh-completion > + > diff --git a/src/bin/pvesdn b/src/bin/pvesdn > new file mode 100755 > index 000000000000..a95e596793b0 > --- /dev/null > +++ b/src/bin/pvesdn > @@ -0,0 +1,8 @@ > +#!/usr/bin/perl -T > + > +use strict; > +use warnings; > + > +use PVE::CLI::pvesdn; > + > +PVE::CLI::pvesdn->run_cli_handler();