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 7C1F61FF13C for ; Thu, 19 Feb 2026 13:38:57 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0A1C813E86; Thu, 19 Feb 2026 13:39:56 +0100 (CET) Message-ID: <3c863323-8519-436b-9f66-15da53c8b827@proxmox.com> Date: Thu, 19 Feb 2026 13:39:48 +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: Gabriel Goller , 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: Hannes Laimer In-Reply-To: <20260203160246.353351-19-g.goller@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1771504779430 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.059 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 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: ZCAOZ74OS27S5AJLO5PCNOF2NA5IGJJ2 X-Message-ID-Hash: ZCAOZ74OS27S5AJLO5PCNOF2NA5IGJJ2 X-MailFrom: h.laimer@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: small comments inline On 2026-02-03 17:03, 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"; 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 > + > + 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 > + 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();