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 <[email protected]>
> ---
> 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/[email protected]/00-dnsmasq-after-networking.conf
> /usr/lib/systemd/system/[email protected]/
> 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 = <STDIN>;
> + 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 = <STDIN>;
> + 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" -
> >[email protected]
> + mv [email protected] $@
> +
> +%.1.pod:
> + podselect $* > [email protected]
> + mv [email protected] $@
> +
> +.PHONY: tidy
> +tidy:
> + echo $(CLITOOLS) | xargs -n4 -P0 proxmox-perltidy
> +
> +pvesdn.api-verified:
> + touch $@
> +
> +pvesdn.bash-completion:
> + echo "# bash completion for pvesdn" > [email protected]
> + echo "complete -C 'pvesdn bashcomplete' pvesdn" >> [email protected]
> + mv [email protected] $@
> +
> +pvesdn.zsh-completion:
> + echo "#compdef pvesdn" > [email protected]
> + echo "" >> [email protected]
> + mv [email protected] $@
> +
> +.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();
