Re: [PATCH pve-network 07/10] cli: add pvesdn cli tool for managing frr template overrides

[Stefan Hanreich <s.hanreich@proxmox.com>]

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 <g.goller@proxmox.com>
> ---
>  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 = <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" - >$@.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();