bgp: Extend route policy matching with match types

[MitchLewis930]

User description

PR_043

---

PR Type

Enhancement

---

Description

• Restructure route policy matching with dedicated match type wrappers

• Add three match types (any/all/invert) for flexible prefix/neighbor matching

• Update API models and conversions to support new match structures

• Refactor internal types from slices to wrapped match objects

---

Diagram Walkthrough

flowchart LR
  Old["Old: Flat slices<br/>MatchNeighbors[]<br/>MatchPrefixes[]"]
  New["New: Wrapped matches<br/>MatchNeighbors*<br/>MatchPrefixes*"]
  Types["Match Types<br/>Any/All/Invert"]
  API["API Models<br/>BgpRoutePolicyNeighborMatch<br/>BgpRoutePolicyPrefixMatch"]
  Conversions["GoBGP Conversions<br/>toGoBGPPolicyMatchType<br/>toAgentPolicyMatchType"]
  
  Old -->|Refactor| New
  New -->|Includes| Types
  New -->|Generates| API
  API -->|Uses| Conversions

Loading

File Walkthrough

	Relevant files
Enhancement	15 files bgp.go Add match types and restructure match conditions                  +85/-21  utils.go Rename PolicyPrefixMatchList to PolicyPrefixList                  +2/-14    conversions.go Add match type conversion functions for GoBGP                        +40/-8    conversions.go Add API conversion functions for match types                          +36/-9    printers.go Update formatters for new match structures                              +25/-7    bgp_route_policy_statement.go Update statement model with wrapped matches                            +82/-37  bgp_route_policy_neighbor_match.go New model for neighbor match with type field                          +119/-0  bgp_route_policy_prefix_match.go Restructure prefix match with type and prefixes                    +136/-8  bgp_route_policy_prefix.go New model for individual prefix definition                              +59/-0    bgp_route_policy_match_type.go New enum model for match type values                                          +84/-0    policies.go Update policy creation for wrapped matches                              +24/-15  service.go Update service policy generation for new types                      +3/-3      pod_cidr.go Update pod CIDR policy for new match types                              +4/-4      pod_ip_pool.go Update IP pool policy for wrapped matches                                +3/-3      interface.go Update interface policy for new match structures                  +4/-4
Tests	7 files bgp_test.go Update tests for new match type structures                              +56/-21  test_fixtures.go Update fixtures with wrapped match objects                              +70/-40  policies_test.go Update policy tests with new match structures                        +18/-6    service_test.go Update service tests with wrapped matches                                +188/-96 pod_cidr_test.go Update pod CIDR tests with new structures                                +68/-44  pod_ip_pool_test.go Update IP pool tests with new match types                                +68/-44  interface_test.go Update interface tests with wrapped matches                            +48/-24
Miscellaneous	1 files zz_generated.deepequal.go Regenerate deepequal for new match types                                  +56/-13
Documentation	2 files embedded_spec.go Update OpenAPI spec with new match structures                        +92/-22  openapi.yaml Update YAML spec with match type definitions                          +33/-8

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Nil pointer panic Description: A nil-pointer dereference can occur in peerAddressesFromPolicy because it appends s.Conditions.MatchNeighbors.Neighbors... even when s.Conditions.MatchNeighbors == nil, allowing a crafted/invalid policy to crash the process (DoS). policies.go [512-519] Referred Code addrs := []netip.Addr{} allPeers := false for _, s := range p.Statements { if s.Conditions.MatchNeighbors == nil || len(s.Conditions.MatchNeighbors.Neighbors) == 0 { allPeers = true } addrs = append(addrs, s.Conditions.MatchNeighbors.Neighbors...) }
	Route policy mismatch Description: The PrefixSet match type is derived from apiStatement.Conditions.MatchNeighbors.Type instead of apiStatement.Conditions.MatchPrefixes.Type, which can cause unintended route-policy behavior (e.g., incorrect invert/all/any semantics) and potentially advertise/accept routes outside the intended scope. conversions.go [242-247] Referred Code ds.Prefixes = append(ds.Prefixes, p) } s.Conditions.PrefixSet = &gobgp.MatchSet{ Type: toGoBGPPolicyMatchType(apiStatement.Conditions.MatchNeighbors.Type), Name: ds.Name, }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Nil dereference risk: peerAddressesFromPolicy dereferences s.Conditions.MatchNeighbors.Neighbors even when MatchNeighbors may be nil, which can panic for policies intended to match all peers. Referred Code addrs := []netip.Addr{} allPeers := false for _, s := range p.Statements { if s.Conditions.MatchNeighbors == nil || len(s.Conditions.MatchNeighbors.Neighbors) == 0 { allPeers = true } addrs = append(addrs, s.Conditions.MatchNeighbors.Neighbors...) } Learn more about managing compliance generic rules or creating your own custom rules
⚪

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix incorrect BGP policy matching Correct a copy-paste error in BGP policy conversion by using MatchPrefixes.Type for the PrefixSet match type instead of MatchNeighbors.Type. pkg/bgp/gobgp/conversions.go [244-247] s.Conditions.PrefixSet = &gobgp.MatchSet{ - Type: toGoBGPPolicyMatchType(apiStatement.Conditions.MatchNeighbors.Type), + Type: toGoBGPPolicyMatchType(apiStatement.Conditions.MatchPrefixes.Type), Name: ds.Name, } Apply / Chat Suggestion importance[1-10]: 9 __ Why: This is a clear copy-paste bug introduced in the PR that causes incorrect BGP policy behavior by using the neighbor match type for prefix matching, which would lead to unexpected route advertisement.	High
	Skip zero prefixes instead of returning In the contextValidatePrefixes function, change return nil to continue to ensure all prefixes in the slice are validated, rather than exiting the loop early on an optional zero element. api/v1/models/bgp_route_policy_prefix_match.go [126-128] if swag.IsZero(m.Prefixes[i]) { // not required - return nil + continue } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that returning nil prematurely exits the validation loop, preventing other prefixes in the list from being validated. This is a valid bug fix that improves the correctness of the validation logic.	Medium
More