[PATCH proxmox-ve-rs v8 09/21] frr: enable minijinja strict undefined behavior mode

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

This enables the minijinja strict undefined behavior mode. By
enabling it, we now get errors when variables don't exist or are
undefined. A full list of things check by the strict mode are here:
https://docs.rs/minijinja/latest/minijinja/enum.UndefinedBehavior.html

This also revealed a bug in bgpd.jinja and some other less-than-ideal
template definitions.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-frr-templates/templates/access_lists.jinja |  2 +-
 proxmox-frr-templates/templates/bgp_router.jinja   |  4 ++--
 proxmox-frr-templates/templates/prefix_lists.jinja |  2 +-
 proxmox-frr/src/ser/bgp.rs                         | 11 +----------
 proxmox-frr/src/ser/mod.rs                         |  4 ++--
 proxmox-frr/src/ser/openfabric.rs                  | 12 ++++--------
 proxmox-frr/src/ser/ospf.rs                        |  8 ++------
 proxmox-frr/src/ser/route_map.rs                   |  4 ++--
 proxmox-frr/src/ser/serializer.rs                  |  2 ++
 9 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/proxmox-frr-templates/templates/access_lists.jinja b/proxmox-frr-templates/templates/access_lists.jinja
index 25f27a293529..832abf1d9500 100644
--- a/proxmox-frr-templates/templates/access_lists.jinja
+++ b/proxmox-frr-templates/templates/access_lists.jinja
@@ -1,6 +1,6 @@
 {% for access_list_name, access_list in access_lists | items %}
 !
 {%  for rule in access_list %}
-{{ "ipv6 " if rule.is_ipv6 }}access-list {{ access_list_name }} {{ ("seq " ~ rule.seq ~ " ") if rule.seq }}{{ rule.action }} {{ rule.network }}
+{{ "ipv6 " if rule.is_ipv6 else "" }}access-list {{ access_list_name }} {{ ("seq " ~ rule.seq ~ " ") if rule.seq else "" }}{{ rule.action }} {{ rule.network }}
 {%  endfor%}
 {% endfor %}
diff --git a/proxmox-frr-templates/templates/bgp_router.jinja b/proxmox-frr-templates/templates/bgp_router.jinja
index e90ee21c64cb..35d3f89bffde 100644
--- a/proxmox-frr-templates/templates/bgp_router.jinja
+++ b/proxmox-frr-templates/templates/bgp_router.jinja
@@ -65,7 +65,7 @@
 {% endfor %}
 {{ address_family_common(router_config.address_families.ipv4_unicast) -}}
 {% for redistribute in router_config.address_families.ipv4_unicast.redistribute %}
-  redistribute {{ redistribute.protocol }}{{ (" metric " ~ redistribute.metric) if redistribute.metric }}{{ (" route-map " ~ redistribute.route_map) if redistribute.route_map }}
+  redistribute {{ redistribute.protocol }}{{ (" metric " ~ redistribute.metric) if redistribute.metric else "" }}{{ (" route-map " ~ redistribute.route_map) if redistribute.route_map else "" }}
 {% endfor %}
 {% for line in router_config.address_families.ipv4_unicast.custom_frr_config %}
 {{ line }}
@@ -80,7 +80,7 @@
 {% endfor %}
 {{ address_family_common(router_config.address_families.ipv6_unicast) -}}
 {% for redistribute in router_config.address_families.ipv6_unicast.redistribute %}
-  redistribute {{ redistribute.protocol }}{{ (" metric " ~ redistribute.metric) if redistribute.metric }}{{ (" route-map " ~ redistribute.route_map) if redistribute.route_map }}
+  redistribute {{ redistribute.protocol }}{{ (" metric " ~ redistribute.metric) if redistribute.metric else "" }}{{ (" route-map " ~ redistribute.route_map) if redistribute.route_map else "" }}
 {% endfor %}
 {% for line in router_config.address_families.ipv6_unicast.custom_frr_config %}
 {{ line }}
diff --git a/proxmox-frr-templates/templates/prefix_lists.jinja b/proxmox-frr-templates/templates/prefix_lists.jinja
index f431958af354..24d42098f715 100644
--- a/proxmox-frr-templates/templates/prefix_lists.jinja
+++ b/proxmox-frr-templates/templates/prefix_lists.jinja
@@ -1,6 +1,6 @@
 {% for name, prefix_list in prefix_lists | items %}
 !
 {%  for rule in prefix_list %}
-{{ "ipv6" if rule.is_ipv6 else "ip" }} prefix-list {{ name }} {{ ("seq " ~ rule.seq ~ " ") if rule.seq }}{{ rule.action }} {{ rule.network }}{{ (" le " ~ rule.le) if rule.le }}{{ (" ge " ~ rule.ge) if rule.ge }}
+{{ "ipv6" if rule.is_ipv6 else "ip" }} prefix-list {{ name }} {{ ("seq " ~ rule.seq ~ " ") if rule.seq else "" }}{{ rule.action }} {{ rule.network }}{{ (" le " ~ rule.le) if rule.le else "" }}{{ (" ge " ~ rule.ge) if rule.ge else "" }}
 {% endfor %}
 {% endfor %}
diff --git a/proxmox-frr/src/ser/bgp.rs b/proxmox-frr/src/ser/bgp.rs
index 0771a1e9d0a2..dcad3b87968c 100644
--- a/proxmox-frr/src/ser/bgp.rs
+++ b/proxmox-frr/src/ser/bgp.rs
@@ -118,15 +118,9 @@ pub struct RouteTargets {
 #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)]
 pub struct AddressFamilyNeighbor {
     pub name: String,
-    #[serde(
-        default,
-        skip_serializing_if = "Option::is_none",
-        deserialize_with = "proxmox_serde::perl::deserialize_bool"
-    )]
+    #[serde(default, deserialize_with = "proxmox_serde::perl::deserialize_bool")]
     pub soft_reconfiguration_inbound: Option<bool>,
-    #[serde(skip_serializing_if = "Option::is_none")]
     pub route_map_in: Option<RouteMapName>,
-    #[serde(skip_serializing_if = "Option::is_none")]
     pub route_map_out: Option<RouteMapName>,
 }
 
@@ -142,11 +136,8 @@ pub struct CommonAddressFamilyOptions {
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize, Default)]
 pub struct AddressFamilies {
-    #[serde(skip_serializing_if = "Option::is_none")]
     ipv4_unicast: Option<Ipv4UnicastAF>,
-    #[serde(skip_serializing_if = "Option::is_none")]
     ipv6_unicast: Option<Ipv6UnicastAF>,
-    #[serde(skip_serializing_if = "Option::is_none")]
     l2vpn_evpn: Option<L2vpnEvpnAF>,
 }
 
diff --git a/proxmox-frr/src/ser/mod.rs b/proxmox-frr/src/ser/mod.rs
index fcd042baa224..7bb48364fadb 100644
--- a/proxmox-frr/src/ser/mod.rs
+++ b/proxmox-frr/src/ser/mod.rs
@@ -126,9 +126,9 @@ impl InterfaceName {
 pub struct Interface<T> {
     // We can't use `Cidr` because then the template doesn't know if it's IPv6
     // or IPv4, and we need to prefix the FRR command with either "ipv6 ip" or "ip"
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    #[serde(default)]
     pub addresses_v4: Vec<Ipv4Cidr>,
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    #[serde(default)]
     pub addresses_v6: Vec<Ipv6Cidr>,
 
     #[serde(flatten)]
diff --git a/proxmox-frr/src/ser/openfabric.rs b/proxmox-frr/src/ser/openfabric.rs
index a0a25d931361..88dfeac172e8 100644
--- a/proxmox-frr/src/ser/openfabric.rs
+++ b/proxmox-frr/src/ser/openfabric.rs
@@ -62,17 +62,13 @@ impl OpenfabricRouter {
 pub struct OpenfabricInterface {
     // Note: an interface can only be a part of a single fabric (so no vec needed here)
     pub fabric_id: OpenfabricRouterName,
-    #[serde(
-        default,
-        deserialize_with = "proxmox_serde::perl::deserialize_bool",
-        skip_serializing_if = "Option::is_none"
-    )]
+    #[serde(default, deserialize_with = "proxmox_serde::perl::deserialize_bool")]
     pub passive: Option<bool>,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(default)]
     pub hello_interval: Option<proxmox_sdn_types::openfabric::HelloInterval>,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(default)]
     pub csnp_interval: Option<proxmox_sdn_types::openfabric::CsnpInterval>,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(default)]
     pub hello_multiplier: Option<proxmox_sdn_types::openfabric::HelloMultiplier>,
     #[serde(deserialize_with = "proxmox_serde::perl::deserialize_bool")]
     pub is_ipv4: bool,
diff --git a/proxmox-frr/src/ser/ospf.rs b/proxmox-frr/src/ser/ospf.rs
index 8c9992e901f2..8ab9136889c1 100644
--- a/proxmox-frr/src/ser/ospf.rs
+++ b/proxmox-frr/src/ser/ospf.rs
@@ -100,12 +100,8 @@ pub enum NetworkType {
 pub struct OspfInterface {
     // Note: an interface can only be a part of a single area(so no vec needed here)
     pub area: Area,
-    #[serde(
-        default,
-        skip_serializing_if = "Option::is_none",
-        deserialize_with = "proxmox_serde::perl::deserialize_bool"
-    )]
+    #[serde(default, deserialize_with = "proxmox_serde::perl::deserialize_bool")]
     pub passive: Option<bool>,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(default)]
     pub network_type: Option<NetworkType>,
 }
diff --git a/proxmox-frr/src/ser/route_map.rs b/proxmox-frr/src/ser/route_map.rs
index 45b4809630a1..1a19c6c245c1 100644
--- a/proxmox-frr/src/ser/route_map.rs
+++ b/proxmox-frr/src/ser/route_map.rs
@@ -26,7 +26,7 @@ pub enum AccessAction {
 pub struct AccessListRule {
     pub action: AccessAction,
     pub network: Cidr,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(default)]
     pub seq: Option<u32>,
     #[serde(deserialize_with = "proxmox_serde::perl::deserialize_bool")]
     pub is_ipv6: bool,
@@ -136,6 +136,6 @@ pub struct RouteMapEntry {
     pub matches: Vec<RouteMapMatch>,
     #[serde(default)]
     pub sets: Vec<RouteMapSet>,
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    #[serde(default)]
     pub custom_frr_config: Vec<String>,
 }
diff --git a/proxmox-frr/src/ser/serializer.rs b/proxmox-frr/src/ser/serializer.rs
index 18eb3d81f6c5..2ac85d8cf6c5 100644
--- a/proxmox-frr/src/ser/serializer.rs
+++ b/proxmox-frr/src/ser/serializer.rs
@@ -59,6 +59,8 @@ pub static TEMPLATES: [(&str, &str); 12] = sorted!([
 fn create_env<'a>() -> Environment<'a> {
     let mut env = Environment::new();
 
+    env.set_undefined_behavior(minijinja::UndefinedBehavior::Strict);
+
     // avoid unnecessary additional newlines
     env.set_trim_blocks(true);
     env.set_lstrip_blocks(true);
-- 
2.47.3