[PATCH pve-network 1/2] sdn: frr: add append-only custom frr config

[Gabriel Goller <g.goller@proxmox.com>]

Users sometimes need FRR config that SDN does not model yet. Editing
/etc/frr/frr.conf is not useful because the file is generated and gets
rewritten. frr.conf.local still works, but it is parsed and merged into the
SDN-generated config, and that merge can fail for unsupported constructs.

Append files from /etc/frr/frr.conf.d/*.conf after the full generated FRR
config, including fabric config generated by Rust. Files are read in sorted
filename order and copied without parsing or modifying their content.

Wrap each appended snippet in begin/end marker comments in the installed
configuration. running_config_has_frr() also checks the active /etc/frr/frr.conf
for that marker so that removing the last snippet, or the whole frr.conf.d
directory, still triggers an FRR rewrite. Without that check, stale
appended config remain installed because there would be no remaining SDN
or custom FRR input to trigger a regeneratin of frr.conf.

Build the full candidate config first and validate it with vtysh -C -f before
replacing /etc/frr/frr.conf. If validation fails, keep the old config in place
and include the vtysh output in the error.

Using the append-only user-configuration we can model about 95% of
the possible setups. There are some config constellations that are
impossible to generate, we would need user-editable templates for that.
The problem is that the templates are quite hard to understand and even
harder to correctly modify (i.e. you need to read the code).

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 src/PVE/Network/SDN.pm     |  26 ++++++++-
 src/PVE/Network/SDN/Frr.pm | 115 ++++++++++++++++++++++++++++++++++---
 2 files changed, 130 insertions(+), 11 deletions(-)

diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 33a3cf352496..aeba89b6b6cb 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -113,7 +113,9 @@ sub running_config {
 =head3 running_config_has_frr(\%running_config)
 
 Determines whether C<\%running_config> contains any entities that generate an
-FRR configuration. This is used by pve-manager to determine whether a rewrite of
+FRR configuration, whether local FRR custom config files or append snippets
+exist, or whether the active FRR configuration still contains previously
+appended snippets. This is used by pve-manager to determine whether a rewrite of
 the FRR configuration is required or not.
 
 If C<\%running_config> is not provided, it will query the current running
@@ -131,8 +133,18 @@ sub running_config_has_frr {
     my $prefix_lists = $running_config->{'prefix-lists'}->{ids} // {};
 
     my $local_frr_config = PVE::Network::SDN::Frr::local_frr_config_exists();
-
-    return %$controllers || %$fabrics || %$route_maps || %$prefix_lists || $local_frr_config;
+    my $append_frr_config = PVE::Network::SDN::Frr::read_append_frr_config();
+    my $active_frr_config_has_append =
+        PVE::Network::SDN::Frr::active_config_has_append_frr_config();
+
+    return
+        %$controllers
+        || %$fabrics
+        || %$route_maps
+        || %$prefix_lists
+        || $local_frr_config
+        || $append_frr_config ne ''
+        || $active_frr_config_has_append;
 }
 
 sub pending_config {
@@ -449,10 +461,14 @@ sub generate_frr_raw_config {
     my $frr_config = {};
 
     PVE::Network::SDN::Controllers::generate_frr_config($frr_config, $running_config);
+
     PVE::Network::SDN::Frr::append_local_config($frr_config);
 
     my $nodename = PVE::INotify::nodename();
 
+    # This also appends the FRR config generated by fabrics in Rust. The
+    # frr.conf.d snippets are added later in raw_config_to_string(), after these
+    # raw lines have been rendered and before the final line vty stanza.
     return PVE::RS::SDN::get_frr_raw_config(
         $frr_config->{'frr'},
         $prefix_list_config,
@@ -490,6 +506,10 @@ sub generate_frr_config {
     my $needs_restart = PVE::Network::SDN::Frr::set_daemon_status($daemon_status, 1);
 
     my $raw_config = PVE::Network::SDN::generate_frr_raw_config($running_config, $fabric_config);
+
+    # write_raw_config() inserts frr.conf.d/*.conf after the rendered config,
+    # including the fabric FRR config, and before the final line vty stanza. It
+    # validates the complete candidate before installing it as frr.conf.
     PVE::Network::SDN::Frr::write_raw_config($raw_config);
 
     PVE::Network::SDN::Frr::apply($needs_restart) if $apply;
diff --git a/src/PVE/Network/SDN/Frr.pm b/src/PVE/Network/SDN/Frr.pm
index f81da325b1ef..9a57eb1a7eb7 100644
--- a/src/PVE/Network/SDN/Frr.pm
+++ b/src/PVE/Network/SDN/Frr.pm
@@ -22,6 +22,12 @@ frr configuration in this format. This format is also used for merging the local
 FRR config (a user-defined configuration file) with the controller-generated
 configuration.
 
+The preferred way to use custom frr config is to paste config into files in
+C</etc/frr/frr.conf.d/*.conf>. The config of these files is then appended
+after the generated SDN config and before the final C<line vty> stanza, sorted
+by file name. Validate syntax with C<vtysh -C> before installing the final
+configuration.
+
 =head2 raw config
 
 This is generated from the frr config. It is an array where every entry is a
@@ -29,10 +35,14 @@ string that is a FRR configuration line.
 
 =cut
 
+use File::Temp qw(tempfile);
+
 use PVE::RESTEnvironment qw(log_warn);
 use PVE::Tools qw(file_get_contents file_set_contents run_command);
 
 my $FRR_CONF_LOCAL_FILE = "/etc/frr/frr.conf.local";
+my $FRR_CONF_APPEND_DIR = "/etc/frr/frr.conf.d";
+my $FRR_CONF_APPEND_BEGIN_MARKER = "! Begin user FRR config from ";
 
 =head3 local_frr_config_exists
 
@@ -57,8 +67,57 @@ sub read_local_frr_config {
     return; # undef
 }
 
+=head3 read_append_frr_config($dir)
+
+Returns custom FRR config from C<$dir/*.conf>, sorted by file name. If C<$dir>
+is not set, C</etc/frr/frr.conf.d> is used.
+
+=cut
+
+sub read_append_frr_config {
+    my ($dir) = @_;
+
+    $dir //= $FRR_CONF_APPEND_DIR;
+
+    my $config = '';
+
+    return $config if !-d $dir;
+
+    for my $file (grep { -f $_ } sort glob("$dir/*.conf")) {
+        my $content = file_get_contents($file);
+        next if $content eq '';
+
+        $config .= "$FRR_CONF_APPEND_BEGIN_MARKER$file\n!\n";
+        $config .= $content;
+        $config .= "\n" if $content !~ /\n\z/;
+        $config .= "!\n! End user FRR config from $file\n!\n";
+    }
+
+    return $config;
+}
+
 my $FRR_CONFIG_FILE = "/etc/frr/frr.conf";
 
+=head3 active_config_has_append_frr_config($file)
+
+Checks whether the active FRR config still contains append-only user config.
+This is used to trigger a rewrite after the last snippet or the whole
+C</etc/frr/frr.conf.d> directory has been removed, so stale appended config gets
+removed from C</etc/frr/frr.conf>.
+
+=cut
+
+sub active_config_has_append_frr_config {
+    my ($file) = @_;
+
+    $file //= $FRR_CONFIG_FILE;
+
+    return 0 if !-e $file;
+
+    my $config = file_get_contents($file);
+    return $config =~ /^\Q$FRR_CONF_APPEND_BEGIN_MARKER\E/m ? 1 : 0;
+}
+
 =head3 apply()
 
 Tries to reload FRR with the frr-reload.py script from frr-pythontools. If that
@@ -192,7 +251,7 @@ sub set_daemon_status {
 
 Converts a given C<\@raw_config> to a string representing a complete frr
 configuration, ready to be written to /etc/frr/frr.conf. If raw_config is empty,
-returns only the FRR config skeleton.
+returns the FRR config skeleton plus any append snippets.
 
 =cut
 
@@ -202,6 +261,10 @@ sub raw_config_to_string {
     my $nodename = PVE::INotify::nodename();
 
     my @final_config = (
+        "! This file is generated by Proxmox VE SDN.",
+        "! Do not edit directly; changes will be overwritten.",
+        "! For simple custom FRR additions, use /etc/frr/frr.conf.d/*.conf.",
+        "!",
         "frr version 10.6.1",
         "frr defaults datacenter",
         "hostname $nodename",
@@ -211,27 +274,63 @@ sub raw_config_to_string {
 
     push @final_config, @$raw_config;
 
-    push @final_config, (
-        "!", "line vty", "!",
-    );
+    my $config = join("\n", @final_config) . "\n!\n";
+    my $append_config = read_append_frr_config();
+
+    $config .= $append_config;
+    $config .= "line vty\n!\n";
 
-    return join("\n", @final_config) . "\n";
+    return $config;
 }
 
-=head3 raw_config_to_string(\@raw_config)
+=head3 write_raw_config(\@raw_config)
 
-Writes a given C<\@raw_config> to /etc/frr/frr.conf.
+Writes a given C<\@raw_config> to /etc/frr/frr.conf. Before replacing the real
+configuration, the full candidate (including append snippets) is validated with
+C<vtysh -C -f>. On validation failure, the frr.conf is not edited.
 
 =cut
 
+sub validate_config {
+    my ($candidate_file) = @_;
+
+    my $stdout = '';
+    my $stderr = '';
+    eval {
+        run_command(
+            ['vtysh', '-C', '-f', $candidate_file],
+            outfunc => sub { $stdout .= "$_[0]\n"; },
+            errfunc => sub { $stderr .= "$_[0]\n"; },
+        );
+    };
+
+    if (my $err = $@) {
+        my $msg = "FRR configuration validation failed with 'vtysh -C -f $candidate_file': $err";
+        $msg .= "\n" if $msg !~ /\n\z/;
+        $msg .= "vtysh stdout:\n$stdout" if $stdout ne '';
+        $msg .= "vtysh stderr:\n$stderr" if $stderr ne '';
+        die $msg;
+    }
+}
+
 sub write_raw_config {
     my ($raw_config) = @_;
 
     return if !-d "/etc/frr";
     return if !$raw_config;
 
-    file_set_contents("/etc/frr/frr.conf", raw_config_to_string($raw_config));
+    my $candidate = raw_config_to_string($raw_config);
+
+    my ($fh, $candidate_file) = tempfile('frr.conf.XXXXXX', DIR => "/etc/frr");
+    print $fh $candidate;
+    close($fh);
+
+    eval { validate_config($candidate_file); };
+    my $err = $@;
+    unlink $candidate_file;
+    die $err if $err;
 
+    file_set_contents($FRR_CONFIG_FILE, $candidate);
 }
 
 =head3 append_local_config(\%frr_config, $local_config)
-- 
2.47.3