Skip to content

Comments

bgp: Extend route policy matching with match types#3

Open
MitchLewis930 wants to merge 1 commit intopr_043_beforefrom
pr_043_after
Open

bgp: Extend route policy matching with match types#3
MitchLewis930 wants to merge 1 commit intopr_043_beforefrom
pr_043_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

PR_043


Note

Medium Risk
Touches core BGP policy representation and conversion paths (API � agent � GoBGP), so mismapped fields or default behaviors could change which routes match; in particular, toGoBGPPolicyStatement appears to apply the prefix match-set type from MatchNeighbors.Type instead of MatchPrefixes.Type.

Overview
Adds configurable match semantics to BGP route policies. Neighbor and prefix conditions move from simple lists to structured match objects that include a type (any/all/invert) plus the list of neighbors or prefixes.

This updates the Swagger/OpenAPI spec and generated api/v1/models (new BgpRoutePolicyMatchType, BgpRoutePolicyNeighborMatch, BgpRoutePolicyPrefix, refactored BgpRoutePolicyPrefixMatch, and BgpRoutePolicyStatement fields), and refactors the agent BGP types/conversions/printers/reconciler code to use the new structures (including updated DeepEqual/sorting/test fixtures).

Written by Cursor Bugbot for commit ba9b93c. This will update automatically on new commits. Configure here.

Extends the internal route policy API with 3 specific
prefix / neighbor match types (any / all / invert)
to support more flexible route policies for future developments.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

}
s.Conditions.PrefixSet = &gobgp.MatchSet{
Type: gobgp.MatchSet_ANY, // any of the configured prefixes
Type: toGoBGPPolicyMatchType(apiStatement.Conditions.MatchNeighbors.Type),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong field used for prefix match type conversion

High Severity

When building the GoBGP prefix set condition, the code uses apiStatement.Conditions.MatchNeighbors.Type instead of apiStatement.Conditions.MatchPrefixes.Type. This copy-paste error causes prefix matching to use the wrong match type. It will also cause a nil pointer dereference if MatchNeighbors is nil while MatchPrefixes has values.

Fix in Cursor Fix in Web

allPeers = true
}
addrs = append(addrs, s.Conditions.MatchNeighbors...)
addrs = append(addrs, s.Conditions.MatchNeighbors.Neighbors...)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nil pointer dereference when accessing neighbor addresses

High Severity

In peerAddressesFromPolicy, after checking if s.Conditions.MatchNeighbors is nil on line 515, the code still unconditionally accesses s.Conditions.MatchNeighbors.Neighbors on line 518. When MatchNeighbors is nil, this causes a nil pointer dereference panic.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants