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 <[email protected]> --- 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
