rm_templates: T8609: fix slow parse() from group-quantifier patterns (#470)

* rm_templates: T8609: fix slow parse() from group-quantifier patterns

Running an Ansible playbook that manages BGP route-maps on my VyOS 1.4
edge routers, the vyos.vyos.vyos_route_maps task (state=gathered or
state=merged) takes ~50 seconds per host, sometimes 100s+, against
devices with only a handful of route-maps configured.  The persistent
connection times out before the module finishes and the failure
surfaces as a misleading "socket path does not exist".

Three related quantifier shapes in nine rm_templates files cause O(2^n)
regex backtracking on inputs that share a parser's prefix but don't
match overall.  parse() time on a representative 12-line route-map
config drops from ~50s to <1ms post-fix.

* Trailing form (188 sites): `(?P<X>\S+)\n  *$"""` -> `(?P<X>\S+)\s*$"""`.
  Under re.VERBOSE the literal newline+indent between `\S+` and `*$`
  is stripped at compile time, so the source compiled to `(\S+)*$`
  with the `*` quantifying the named group.

* Mid-pattern `(group containing \S+)*` (22 sites in snmp_server.py
  with a couple in bgp_global*.py): e.g.
  `(?P<protocol>protocol\s\S+)*` -> `(?P<protocol>protocol\s\S+)?`.
  Different position from the trailing form, but the same shape
  underneath (a group whose content includes \S+, quantified with
  `*`), so the same O(2^n) backtracking on prefix-sharing inputs.

* Mid-pattern `(literal-only group)*` (14 sites in snmp_server.py,
  bgp_address_family*.py, bgp_global*.py): e.g.
  `(?P<as_set>as-set)*` -> `(?P<as_set>as-set)?`.  No \S+ inside the
  quantified group, so the backtracking exposure is much smaller.
  On the inputs these patterns actually receive (no device line
  carries duplicate flags) `*` and `?` accept identical input sets,
  so changing to `?` is behavior-preserving.

The first draft of this fix made set_comm_list_delete's
`(?P<delete>\S+)` group required.  The parser's setval emits `set
comm-list delete` with no token after `delete`, so the parser stopped
matching its own output (no existing fixture covered this case, which
is why the regression initially shipped).  CodeRabbit caught it
during review.  Replaced `delete(?P<delete>\S+)\s*$` with
`\s(?P<delete>delete)\s*$` so the named group captures the literal
word `delete`; the result template `{{True if delete is defined}}`
continues to evaluate True.

Affects: route_maps[/_14], bgp_global[/_14], bgp_address_family[/_14],
snmp_server, ospf_interfaces[/_14].

Adds tests/unit/modules/network/vyos/test_rm_templates_perf.py with
a 1-second budget against fixture inputs that have realistic-length
identifiers, plus a round-trip test asserting set_comm_list_delete
matches its own setval-generated line, plus a Bgp_address_familyTemplate14
budget test for symmetry with the other "hot" template families.

* Fix regex backtracking issues in parse() method

Fix slow parse() to prevent regex backtracking on prefix-sharing inputs across multiple files. Add unit test for the bugfixes.

* Refactor bug fixes for regex backtracking improvements

Updated bug fixes to include details on regex backtracking issues in multiple files.

* Fix typo in bugfixes section of changelog

* Update bugfixes for regex backtracking issues

* rm_templates: T8609: route_maps.py: fix remaining trailing `*$` patterns

Five sites in route_maps.py still had the `(?P<name>...)\n  *$` shape that collapses to `(?P<name>...)*$` under `re.VERBOSE`, parsers `sequence`, `on_match_next`, `set_atomic_aggregate`, `set_extcommunity_bandwidth_non_transitive`, and `match_community_exact_match`.  Collapsed each to `\s*$` on the same line as the named group, matching the rest of the PR.

* rm_templates: T8609: route_maps.py: split overlong getval to satisfy E501

Line 517 was 161 chars (`set_extcommunity_bandwidth_non_transitive` parser) and tripped pycodestyle's E501 sanity test in CI.  Split the regex source across two lines at the `\d+)` boundary; under `re.VERBOSE` the literal newline and indent between regex tokens are stripped at compile time, so the engine sees the same pattern.  `\s*$` stays on the same line as the closing capture group, so the group-quantifier shape this PR fixes elsewhere isn't reintroduced.

---------

Co-authored-by: omnom62 <75066712+omnom62@users.noreply.github.com>

1 files changed, 163 insertions, 0 deletions

diff --git a/tests/unit/modules/network/vyos/test_rm_templates_perf.py b/tests/unit/modules/network/vyos/test_rm_templates_perf.py
new file mode 100644
index 00000000..d2060f46
--- /dev/null
+++ b/tests/unit/modules/network/vyos/test_rm_templates_perf.py
@@ -0,0 +1,163 @@
+# -*- coding: utf-8 -*-
+# Copyright 2026 Red Hat
+# GNU General Public License v3.0+
+# (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+"""Performance budget tests for rm_templates parsers.
+
+These tests guard against re-introducing catastrophic regex backtracking
+in the rm_template parsers (T8609).  Pre-fix, parse() over realistic
+device-output input could take 50+ seconds because of `(group)*`
+quantifiers on groups containing `\\S+`.  Post-fix, the same input
+parses in single-digit milliseconds.
+
+A 1-second budget is comfortably above post-fix runtime and well below
+the pre-regression cliff, so the test fails sharply if the bug returns.
+"""
+
+from __future__ import absolute_import, division, print_function
+
+
+__metaclass__ = type
+
+import time
+
+from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.rm_templates.bgp_address_family_14 import (
+    Bgp_address_familyTemplate14,
+)
+from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.rm_templates.bgp_global_14 import (
+    Bgp_globalTemplate14,
+)
+from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.rm_templates.route_maps_14 import (
+    Route_mapsTemplate14,
+)
+from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.rm_templates.snmp_server import (
+    Snmp_serverTemplate,
+)
+
+
+PARSE_BUDGET_SECONDS = 1.0
+
+
+def _time_parse(parser_class, lines):
+    """Time a single ``parse()`` call against ``lines``; returns elapsed seconds."""
+    parser = parser_class(lines=lines)
+    t0 = time.perf_counter()
+    parser.parse()
+    return time.perf_counter() - t0
+
+
+def test_route_maps_14_parse_budget():
+    """Realistic route-map config: parse() must finish under 1s.
+
+    Pre-T8609: ~50s.  Post-fix: <50ms.  Inputs use 20+ char names
+    because the backtracking is exponential in the first \\S+ run
+    after the prefix.
+    """
+    lines = [
+        "set policy route-map ADVERTISE-ANYCAST-v6 rule 10 action 'permit'",
+        "set policy route-map ADVERTISE-ANYCAST-v6 rule 10 match ipv6 address prefix-list 'ANYCAST-AGGREGATE-v6'",
+        "set policy route-map DEFAULT-ORIGINATE-SENTINEL-v6 rule 10 action 'permit'",
+        "set policy route-map DEFAULT-ORIGINATE-SENTINEL-v6 rule 10 match ipv6 address prefix-list 'AS64496-SENTINEL-v6'",
+        "set policy route-map DEFAULT-ORIGINATE-SENTINEL-v6 rule 10 set local-preference '120'",
+        "set policy route-map IXP-PEER-INGRESS-v4 rule 10 action 'permit'",
+        "set policy route-map IXP-PEER-INGRESS-v4 rule 10 match ip address prefix-list 'IXP-INBOUND-v4'",
+        "set policy route-map IXP-PEER-INGRESS-v4 rule 10 set community 'additive 65000:100'",
+        "set policy route-map TRANSIT-EGRESS-v4 rule 100 action 'permit'",
+        "set policy route-map TRANSIT-EGRESS-v4 rule 100 match ip address prefix-list 'CUSTOMER-PREFIXES-v4'",
+        "set policy route-map TRANSIT-EGRESS-v4 rule 100 set as-path prepend '65000 65000'",
+        "set policy route-map UPSTREAM-INGRESS-v4 rule 10 action 'permit'",
+    ]
+    elapsed = _time_parse(Route_mapsTemplate14, lines)
+    assert elapsed < PARSE_BUDGET_SECONDS, (
+        "Route_mapsTemplate14.parse() took %.2fs (budget %.2fs); "
+        "possible regression of T8609 (rm_templates regex backtracking)."
+        % (elapsed, PARSE_BUDGET_SECONDS)
+    )
+
+
+def test_bgp_global_14_parse_budget():
+    """Realistic BGP neighbor/address-family config: parse() under 1s."""
+    lines = [
+        "set protocols bgp 65001 neighbor 2001:db8:abcd:1234::1 remote-as '65002'",
+        "set protocols bgp 65001 neighbor 2001:db8:abcd:1234::1 description 'IXP-PEER-1'",
+        "set protocols bgp 65001 neighbor 2001:db8:abcd:1234::1 address-family ipv6-unicast route-map import 'IXP-INGRESS-v6'",
+        "set protocols bgp 65001 neighbor 2001:db8:abcd:1234::1 address-family ipv6-unicast route-map export 'IXP-EGRESS-v6'",
+        "set protocols bgp 65001 neighbor 192.0.2.1 remote-as '65003'",
+        "set protocols bgp 65001 neighbor 192.0.2.1 description 'TRANSIT-PROVIDER-1'",
+        "set protocols bgp 65001 neighbor 192.0.2.1 address-family ipv4-unicast route-map import 'TRANSIT-INGRESS-v4'",
+        "set protocols bgp 65001 neighbor 192.0.2.1 address-family ipv4-unicast route-map export 'TRANSIT-EGRESS-v4'",
+    ]
+    elapsed = _time_parse(Bgp_globalTemplate14, lines)
+    assert elapsed < PARSE_BUDGET_SECONDS, (
+        "Bgp_globalTemplate14.parse() took %.2fs (budget %.2fs); "
+        "possible regression of T8609 (rm_templates regex backtracking)."
+        % (elapsed, PARSE_BUDGET_SECONDS)
+    )
+
+
+def test_snmp_server_parse_budget():
+    """Realistic SNMP v3 config: parse() under 1s."""
+    lines = [
+        "set service snmp community PUBLIC-COMMUNITY-NAME-1 authorization 'ro'",
+        "set service snmp community PUBLIC-COMMUNITY-NAME-1 client '192.0.2.0/24'",
+        "set service snmp v3 trap-target TRAP-TARGET-LONG-NAME-1 user 'monitor'",
+        "set service snmp v3 trap-target TRAP-TARGET-LONG-NAME-1 protocol 'udp'",
+        "set service snmp v3 trap-target TRAP-TARGET-LONG-NAME-1 port '162'",
+        "set service snmp v3 user TRAP-USER-LONG-NAME-1 mode 'auth'",
+        "set service snmp v3 user TRAP-USER-LONG-NAME-1 group 'monitor'",
+    ]
+    elapsed = _time_parse(Snmp_serverTemplate, lines)
+    assert elapsed < PARSE_BUDGET_SECONDS, (
+        "Snmp_serverTemplate.parse() took %.2fs (budget %.2fs); "
+        "possible regression of T8609 (rm_templates regex backtracking)."
+        % (elapsed, PARSE_BUDGET_SECONDS)
+    )
+
+
+def test_bgp_address_family_14_parse_budget():
+    """Realistic BGP address-family aggregate config: parse() under 1s."""
+    lines = [
+        "set protocols bgp 65001 address-family ipv4-unicast network 198.51.100.0/24 backdoor",
+        "set protocols bgp 65001 address-family ipv4-unicast network 198.51.100.0/24 path-limit '4'",
+        "set protocols bgp 65001 address-family ipv4-unicast network 198.51.100.0/24 route-map 'NET-IN-v4'",
+        "set protocols bgp 65001 address-family ipv4-unicast aggregate-address 203.0.113.0/24 as-set",
+        "set protocols bgp 65001 address-family ipv4-unicast aggregate-address 203.0.113.0/24 summary-only",
+        "set protocols bgp 65001 address-family ipv6-unicast network 2001:db8:abcd:1234::/64 backdoor",
+        "set protocols bgp 65001 address-family ipv6-unicast network 2001:db8:abcd:1234::/64 route-map 'NET-IN-v6'",
+        "set protocols bgp 65001 address-family ipv6-unicast aggregate-address 2001:db8::/32 as-set",
+        "set protocols bgp 65001 address-family ipv6-unicast aggregate-address 2001:db8::/32 summary-only",
+    ]
+    elapsed = _time_parse(Bgp_address_familyTemplate14, lines)
+    assert elapsed < PARSE_BUDGET_SECONDS, (
+        "Bgp_address_familyTemplate14.parse() took %.2fs (budget %.2fs); "
+        "possible regression of T8609 (rm_templates regex backtracking)."
+        % (elapsed, PARSE_BUDGET_SECONDS)
+    )
+
+
+def test_route_maps_14_set_comm_list_delete_matches_setval():
+    """Round-trip check: the `set_comm_list_delete` parser must match the line its setval generates.
+
+    `set_comm_list_delete`'s setval emits `set policy route-map X rule N set
+    comm-list delete` with no token after `delete`.  Pre-T8609 the getval
+    happened to match this by accident (a `*` quantifier on the trailing
+    `(?P<delete>\\S+)` made the group optional after VERBOSE-strip).  An
+    earlier draft of T8609's fix made the group required, causing the
+    parser to silently ignore its own output.  This test guards the
+    round-trip.
+    """
+    line = "set policy route-map MY-MAP rule 10 set comm-list delete"
+    parser = Route_mapsTemplate14(lines=[line])
+    result = parser.parse()
+    rm = result.get("route_maps", {}).get("MY-MAP")
+    assert rm is not None, (
+        "route_maps_14: set_comm_list_delete parser failed to match its "
+        "own setval-generated line %r; the parser is broken." % line
+    )
+    entry = rm.get("entries", {}).get(10, {})
+    comm_list = entry.get("set", {}).get("comm_list", {})
+    assert comm_list.get("delete"), (
+        "route_maps_14: set_comm_list_delete matched the line but did not "
+        "populate set.comm_list.delete; check the result template."
+    )