public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH proxmox-ve-rs v6 09/20] frr: enable minijinja strict undefined behavior mode
Date: Thu, 12 Mar 2026 15:26:45 +0100	[thread overview]
Message-ID: <20260312142732.370403-10-g.goller@proxmox.com> (raw)
In-Reply-To: <20260312142732.370403-1-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>
---
 proxmox-frr-templates/templates/access_lists.jinja |  2 +-
 proxmox-frr-templates/templates/bgp_router.jinja   |  4 ++--
 proxmox-frr-templates/templates/bgpd.jinja         |  2 +-
 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 ++
 10 files changed, 18 insertions(+), 33 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/bgpd.jinja b/proxmox-frr-templates/templates/bgpd.jinja
index 15a6ba1b2854..e53099534142 100644
--- a/proxmox-frr-templates/templates/bgpd.jinja
+++ b/proxmox-frr-templates/templates/bgpd.jinja
@@ -10,8 +10,8 @@ vrf {{ vrf_name }}
  {{ "ipv6" if ip_route.is_ipv6 else "ip" }} route {{ ip_route.prefix }} {{ ip_route.via }} {{ ip_route.vrf }}
 {% else %}
  {{ "ipv6" if ip_route.is_ipv6 else "ip" }} route {{ ip_route.prefix }} {{ ip_route.via }}
-{% endfor %}
 {% endif %}
+{% endfor %}
 {% for line in vrf.custom_frr_config %}
 {{ line }}
 {% endfor %}
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 c0433d643207..8ea411f10681 100644
--- a/proxmox-frr/src/ser/bgp.rs
+++ b/proxmox-frr/src/ser/bgp.rs
@@ -119,15 +119,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>,
 }
 
@@ -143,11 +137,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 58d55da285b1..a75abade6586 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 8e7f4546ccb7..d12ae05fc5b9 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,
@@ -156,6 +156,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 1eb5bec25e2d..733b95557f30 100644
--- a/proxmox-frr/src/ser/serializer.rs
+++ b/proxmox-frr/src/ser/serializer.rs
@@ -21,6 +21,8 @@ pub static TEMPLATES: phf::Map<&'static str, &'static str> = phf::phf_map! {
 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





  parent reply	other threads:[~2026-03-12 14:30 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-12 14:26 [PATCH manager/network/proxmox{-ve-rs,-perl-rs} v6 00/20] Generate frr config using jinja templates and rust types Gabriel Goller
2026-03-12 14:26 ` [PATCH proxmox-ve-rs v6 01/20] ve-config: firewall: cargo fmt Gabriel Goller
2026-03-12 14:26 ` [PATCH proxmox-ve-rs v6 02/20] frr: add proxmox-frr-templates package that contains templates Gabriel Goller
2026-03-12 14:26 ` [PATCH proxmox-ve-rs v6 03/20] ve-config: remove FrrConfigBuilder struct Gabriel Goller
2026-03-12 14:26 ` [PATCH proxmox-ve-rs v6 04/20] sdn-types: support variable-length NET identifier Gabriel Goller
2026-03-12 14:26 ` [PATCH proxmox-ve-rs v6 05/20] frr: add template serializer and serialize fabrics using templates Gabriel Goller
2026-03-12 14:26 ` [PATCH proxmox-ve-rs v6 06/20] frr: add isis configuration and templates Gabriel Goller
2026-03-12 14:26 ` [PATCH proxmox-ve-rs v6 07/20] frr: support custom frr configuration lines Gabriel Goller
2026-03-12 14:26 ` [PATCH proxmox-ve-rs v6 08/20] frr: add bgp support with templates and serialization Gabriel Goller
2026-03-12 14:26 ` Gabriel Goller [this message]
2026-03-12 14:26 ` [PATCH proxmox-perl-rs v6 10/20] sdn: add function to generate the frr config for all daemons Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 11/20] tests: use Test::Differences to make test assertions Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 12/20] test: add tests for frr.conf.local merging Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 13/20] test: bgp: add some various integration tests Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 14/20] sdn: write structured frr config that can be rendered using templates Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 15/20] sdn: remove duplicate comment line '!' in frr config Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 16/20] tests: rearrange some statements in the " Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 17/20] sdn: adjust frr.conf.local merging to rust template types Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 18/20] test: adjust frr_local_merge test for new template generation Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-network v6 19/20] api: add dry-run endpoint for sdn apply to preview changes Gabriel Goller
2026-03-12 14:26 ` [PATCH pve-manager v6 20/20] sdn: add dry-run diff view for sdn apply Gabriel Goller
2026-03-12 15:42 ` [PATCH manager/network/proxmox{-ve-rs,-perl-rs} v6 00/20] Generate frr config using jinja templates and rust types Stefan Hanreich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260312142732.370403-10-g.goller@proxmox.com \
    --to=g.goller@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal