[PATCH proxmox-ve-rs 04/15] sdn: fix VRF route-map scoping and zone ID validation

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

Attach protocol route-maps inside the VRF where the protocol runs. Give
VRF route-maps distinct names so fabrics in separate routing tables do
not overwrite or refer to each other. Keep the existing global route-map
handling for protocols in the default routing table.

Add the final BGP permit entry to both global and VRF-specific route-maps
so unrelated routes are not dropped by the implicit deny.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 .../templates/protocol_routemaps.jinja        | 13 ++++
 proxmox-frr/src/ser/mod.rs                    |  2 +
 proxmox-ve-config/src/sdn/fabric/frr.rs       | 55 ++++++++++-----
 proxmox-ve-config/tests/fabric/main.rs        | 69 ++++++++++++++++++-
 4 files changed, 119 insertions(+), 20 deletions(-)

diff --git a/proxmox-frr-templates/templates/protocol_routemaps.jinja b/proxmox-frr-templates/templates/protocol_routemaps.jinja
index 38c00b6407ee..0f4ca5fdbe0c 100644
--- a/proxmox-frr-templates/templates/protocol_routemaps.jinja
+++ b/proxmox-frr-templates/templates/protocol_routemaps.jinja
@@ -8,3 +8,16 @@ ip protocol {{ protocol_name }} route-map {{ protocol_routemap.v4 }}
 ipv6 protocol {{ protocol_name }} route-map {{ protocol_routemap.v6 }}
 {% endif %}
 {% endfor %}
+{% for vrf_name, protocol_routemaps in vrf_protocol_routemaps | items %}
+!
+vrf {{ vrf_name }}
+{% for protocol_name, protocol_routemap in protocol_routemaps | items %}
+{% if protocol_routemap.v4 %}
+ ip protocol {{ protocol_name }} route-map {{ protocol_routemap.v4 }}
+{% endif %}
+{% if protocol_routemap.v6 %}
+ ipv6 protocol {{ protocol_name }} route-map {{ protocol_routemap.v6 }}
+{% endif %}
+{% endfor %}
+exit-vrf
+{% endfor %}
diff --git a/proxmox-frr/src/ser/mod.rs b/proxmox-frr/src/ser/mod.rs
index 22c503950ba3..20d7240d72d3 100644
--- a/proxmox-frr/src/ser/mod.rs
+++ b/proxmox-frr/src/ser/mod.rs
@@ -232,6 +232,8 @@ pub struct FrrConfig {
     #[serde(default)]
     pub protocol_routemaps: BTreeMap<FrrProtocol, IpProtocolRouteMap>,
     #[serde(default)]
+    pub vrf_protocol_routemaps: BTreeMap<VrfName, BTreeMap<FrrProtocol, IpProtocolRouteMap>>,
+    #[serde(default)]
     pub routemaps: BTreeMap<RouteMapName, Vec<RouteMapEntry>>,
     #[serde(default)]
     pub access_lists: BTreeMap<AccessListName, Vec<AccessListRule>>,
diff --git a/proxmox-ve-config/src/sdn/fabric/frr.rs b/proxmox-ve-config/src/sdn/fabric/frr.rs
index ca0fbfe6d542..2849a1ee0596 100644
--- a/proxmox-ve-config/src/sdn/fabric/frr.rs
+++ b/proxmox-ve-config/src/sdn/fabric/frr.rs
@@ -1,3 +1,4 @@
+use std::collections::BTreeSet;
 use std::net::{IpAddr, Ipv4Addr};
 
 use tracing;
@@ -40,6 +41,7 @@ pub fn build_fabric(
     frr_config: &mut FrrConfig,
 ) -> Result<(), anyhow::Error> {
     let mut routemap_seq = 100;
+    let mut fabric_bgp_routemaps = BTreeSet::new();
     let mut current_router_id: Option<Ipv4Addr> = None;
     let mut current_net: Option<Net> = None;
 
@@ -280,7 +282,7 @@ pub fn build_fabric(
                     }
                 }
 
-                let routemap_name = ser::route_map::RouteMapName::new("pve_ospf".to_owned());
+                let routemap_name = protocol_routemap_name("pve_ospf", &vrf_name);
                 let routemap = frr_config
                     .routemaps
                     .entry(routemap_name.clone())
@@ -317,10 +319,7 @@ pub fn build_fabric(
 
                 routemap.push(routemap_entry);
 
-                let protocol_routemap = frr_config
-                    .protocol_routemaps
-                    .entry(FrrProtocol::Ospf)
-                    .or_default();
+                let protocol_routemap = protocol_routemap(frr_config, &vrf_name, FrrProtocol::Ospf);
 
                 protocol_routemap.v4 = Some(routemap_name);
             }
@@ -465,7 +464,7 @@ pub fn build_fabric(
                         networks: vec![Ipv4Cidr::from(ip)],
                     });
 
-                    let routemap_name = ser::route_map::RouteMapName::new("pve_bgp".to_owned());
+                    let routemap_name = protocol_routemap_name("pve_bgp", &vrf_name);
                     let routemap = frr_config
                         .routemaps
                         .entry(routemap_name.clone())
@@ -477,11 +476,10 @@ pub fn build_fabric(
 
                     routemap.push(routemap_entry);
 
-                    let protocol_routemap = frr_config
-                        .protocol_routemaps
-                        .entry(FrrProtocol::Bgp)
-                        .or_default();
+                    let protocol_routemap =
+                        protocol_routemap(frr_config, &vrf_name, FrrProtocol::Bgp);
 
+                    fabric_bgp_routemaps.insert(routemap_name.clone());
                     protocol_routemap.v4 = Some(routemap_name);
                 }
 
@@ -552,7 +550,7 @@ pub fn build_fabric(
                         redistribute,
                     });
 
-                    let routemap_name = ser::route_map::RouteMapName::new("pve_bgp6".to_owned());
+                    let routemap_name = protocol_routemap_name("pve_bgp6", &vrf_name);
                     let routemap = frr_config
                         .routemaps
                         .entry(routemap_name.clone())
@@ -564,11 +562,10 @@ pub fn build_fabric(
 
                     routemap.push(routemap_entry);
 
-                    let protocol_routemap = frr_config
-                        .protocol_routemaps
-                        .entry(FrrProtocol::Bgp)
-                        .or_default();
+                    let protocol_routemap =
+                        protocol_routemap(frr_config, &vrf_name, FrrProtocol::Bgp);
 
+                    fabric_bgp_routemaps.insert(routemap_name.clone());
                     protocol_routemap.v6 = Some(routemap_name);
                 };
 
@@ -602,10 +599,7 @@ pub fn build_fabric(
     // routes (e.g. EVPN-imported VRF routes) reach the kernel unchanged.
     // Without this, the implicit deny at the end of the route-map would drop
     // them.
-    for routemap_name in [
-        ser::route_map::RouteMapName::new("pve_bgp".to_owned()),
-        ser::route_map::RouteMapName::new("pve_bgp6".to_owned()),
-    ] {
+    for routemap_name in fabric_bgp_routemaps {
         if let Some(routemap) = frr_config.routemaps.get_mut(&routemap_name) {
             routemap.push(RouteMapEntry {
                 seq: 65535,
@@ -629,6 +623,29 @@ fn fabric_vrf_name(zone: Option<&ZoneId>) -> VrfName {
     }
 }
 
+fn protocol_routemap_name(prefix: &str, vrf_name: &VrfName) -> RouteMapName {
+    match vrf_name {
+        VrfName::Default => RouteMapName::new(prefix.to_owned()),
+        VrfName::Custom(vrf_name) => RouteMapName::new(format!("{prefix}_{vrf_name}")),
+    }
+}
+
+fn protocol_routemap<'a>(
+    frr_config: &'a mut FrrConfig,
+    vrf_name: &VrfName,
+    protocol: FrrProtocol,
+) -> &'a mut ser::IpProtocolRouteMap {
+    match vrf_name {
+        VrfName::Default => frr_config.protocol_routemaps.entry(protocol).or_default(),
+        custom => frr_config
+            .vrf_protocol_routemaps
+            .entry(custom.clone())
+            .or_default()
+            .entry(protocol)
+            .or_default(),
+    }
+}
+
 /// Helper that builds a OSPF router with a the router_id.
 fn build_ospf_router(router_id: Ipv4Addr) -> Result<OspfRouter, anyhow::Error> {
     Ok(OspfRouter::new(router_id))
diff --git a/proxmox-ve-config/tests/fabric/main.rs b/proxmox-ve-config/tests/fabric/main.rs
index aa76adb9819f..f65c5d26aeed 100644
--- a/proxmox-ve-config/tests/fabric/main.rs
+++ b/proxmox-ve-config/tests/fabric/main.rs
@@ -3,7 +3,7 @@ use std::net::Ipv4Addr;
 use std::str::FromStr;
 
 use proxmox_frr::ser::bgp::{AddressFamilies, BgpRouter, CommonAddressFamilyOptions, L2vpnEvpnAF};
-use proxmox_frr::ser::{FrrConfig, VrfName, serializer::dump};
+use proxmox_frr::ser::{FrrConfig, FrrProtocol, VrfName, serializer::dump};
 use proxmox_ve_config::sdn::fabric::{
     FabricConfig,
     frr::build_fabric,
@@ -170,6 +170,19 @@ fn openfabric_ipv6_only() {
     insta::assert_snapshot!(helper::reference_name!("pve"), output);
 }
 
+#[test]
+fn zone_id_boundaries() {
+    for zone in ["a", "abcdefgh"] {
+        let raw = format!("ospf_fabric: test\n\tarea 0\n\tip_prefix 10.0.0.0/24\n\tzone {zone}\n");
+        assert!(FabricConfig::parse_section_config(&raw).is_ok());
+    }
+
+    for zone in ["1", "abcdefghi"] {
+        let raw = format!("ospf_fabric: test\n\tarea 0\n\tip_prefix 10.0.0.0/24\n\tzone {zone}\n");
+        assert!(FabricConfig::parse_section_config(&raw).is_err());
+    }
+}
+
 #[test]
 fn vrf_prefix_overlap_is_scoped_by_zone() {
     let raw = r#"
@@ -248,6 +261,60 @@ bgp_fabric: test
     }
 }
 
+#[test]
+fn protocol_routemaps_are_scoped_by_vrf() {
+    let raw = r#"
+ospf_fabric: ospf
+        area 0
+        ip_prefix 10.0.0.0/24
+        zone zoneone
+
+ospf_node: ospf_pve
+        interfaces name=eth0
+        ip 10.0.0.1
+
+bgp_fabric: bgp
+        bfd 0
+        ip_prefix 10.0.1.0/24
+        zone zonetwo
+
+bgp_node: bgp_pve
+        asn 65001
+        interfaces name=eth1
+        ip 10.0.1.1
+        role internal
+"#;
+    let config = FabricConfig::parse_section_config(raw).unwrap();
+    let mut frr_config = FrrConfig::default();
+
+    build_fabric(NodeId::from_str("pve").unwrap(), config, &mut frr_config).unwrap();
+
+    let ospf_routemap = &frr_config.vrf_protocol_routemaps
+        [&VrfName::Custom("vrf_zoneone".to_owned())][&FrrProtocol::Ospf];
+    assert_eq!(
+        ospf_routemap.v4.as_ref().unwrap(),
+        &proxmox_frr::ser::route_map::RouteMapName::new("pve_ospf_vrf_zoneone".to_owned())
+    );
+    let bgp_routemap = &frr_config.vrf_protocol_routemaps
+        [&VrfName::Custom("vrf_zonetwo".to_owned())][&FrrProtocol::Bgp];
+    assert_eq!(
+        bgp_routemap.v4.as_ref().unwrap(),
+        &proxmox_frr::ser::route_map::RouteMapName::new("pve_bgp_vrf_zonetwo".to_owned())
+    );
+    assert!(
+        frr_config
+            .protocol_routemaps
+            .get(&FrrProtocol::Ospf)
+            .is_none()
+    );
+    assert!(
+        frr_config
+            .protocol_routemaps
+            .get(&FrrProtocol::Bgp)
+            .is_none()
+    );
+}
+
 #[test]
 fn bgp_default() {
     let config = FabricConfig::parse_section_config(helper::get_fabrics_config!()).unwrap();
-- 
2.47.3