all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal