From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 1984A1FF13F for ; Thu, 12 Mar 2026 15:30:04 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 16D291A692; Thu, 12 Mar 2026 15:28:42 +0100 (CET) From: Gabriel Goller 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 Message-ID: <20260312142732.370403-10-g.goller@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260312142732.370403-1-g.goller@proxmox.com> References: <20260312142732.370403-1-g.goller@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1773325623696 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.021 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_SHORT 0.001 Use of a URL Shortener for very short URL RCVD_IN_MSPIKE_H2 0.001 Average reputation (+2) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: SS3RVZ2MACHT2JHK7TILHKG7CMRPHMQZ X-Message-ID-Hash: SS3RVZ2MACHT2JHK7TILHKG7CMRPHMQZ X-MailFrom: g.goller@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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, - #[serde(skip_serializing_if = "Option::is_none")] pub route_map_in: Option, - #[serde(skip_serializing_if = "Option::is_none")] pub route_map_out: Option, } @@ -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, - #[serde(skip_serializing_if = "Option::is_none")] ipv6_unicast: Option, - #[serde(skip_serializing_if = "Option::is_none")] l2vpn_evpn: Option, } 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 { // 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, - #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[serde(default)] pub addresses_v6: Vec, #[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, - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub hello_interval: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub csnp_interval: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub hello_multiplier: Option, #[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, - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub network_type: Option, } 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, #[serde(deserialize_with = "proxmox_serde::perl::deserialize_bool")] pub is_ipv6: bool, @@ -156,6 +156,6 @@ pub struct RouteMapEntry { pub matches: Vec, #[serde(default)] pub sets: Vec, - #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[serde(default)] pub custom_frr_config: Vec, } 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