Please ensure your pull request adheres to the following guidelines:

• For first time contributors, read Submitting a pull request
• All code is covered by unit and/or runtime tests where feasible.
• All commits contain a well written commit description including a title,
  description and a Fixes: #XXX line if the commit addresses a particular
  GitHub issue.
• If your commit description contains a Fixes: <commit-id> tag, then
  please add the commit author[s] as reviewer[s] to this issue.
• All commits are signed off. See the section Developer’s Certificate of Origin
• Provide a title or release-note blurb suitable for the release notes.
• Are you a user of Cilium? Please add yourself to the Users doc
• Thanks for contributing!

bgpv2: Support overlapping selector matches on CiliumBGPAdvertisement

Overview

When configuring CiliumBGPAdvertisement with overlapping selector matches prior to this PR, the last sequential match was used, and previous matches were ignored.

This PR modifies Cilium to support overlapping matches. Overlaps are handled as additive operations for attributes that can be unioned (standard and large communities). When differing Local Preference values are set, the higher value is selected. This is in line with RFC 4271 which states "The higher degree of preference MUST be preferred."

The purpose of this change is to offer more flexibility for administrators of Cilium using the BGP Control Plane. The flexibility added allows the administrator to configure any combination of BGP communities for Service announcements without explicitly defining every possible combination in advance on their CiliumBGPAdvertisement.

Prior to this change, an administrator would have been required to define a Kubernetes label for every possible combination of BGP communities and then define on their CiliumBGPAdvertisement in the advertisements section a label selector-based match for each unique combination of communities. In more complex networks, this poses a significant administrative burden and introduces the risk of configuration drift.

Instead, the administrator now needs to define one label per BGP community that they wish to set. On their CiliumBGPAdvertisement, the advertisements section simply needs a label selector match for each individual community. If two advertisements match, the BGP communities defined on each entry will be applied to the corresponding advertisement. The CiliumBGPAdvertisement no longer needs to be aware of the various combinations of communities that could be desired. Instead, combinations of communities are set by simply labeling the respective Kubernetes Service with the respective labels.

The use-case above is further described in this GitHub Issue.

Fixes: #35721

Description

service.go:

The focus of this PR are the changes made to pkg/bgpv1/manager/reconcilerv2/service.go on getDesiredSvcRoutePolicies(). Prior to this change, each iteration simply updated the map named desiredSvcRoutePolicies without checking for an existing value.

policies.go:

This PR adds the function MergeRoutePolicies() to policies.go, which implements the merge operation. The reconciliation process sequentially passes each overlapping match through that. The final policy applied represents the merge of all which matched.

The merge operation is defined as:

// MergeRoutePolicies evaluates two instances of RoutePolicy{} and returns a single RoutePolicy{} representing
// the merger of the two.  The merge operation focuses on each policy's Statements.  Statements define one or more
// Actions, and these actions may include setting BGP Communities, Local Preference, and others.  Statements are
// keyed by their Conditions, which define the BGP Neighbor, AF, and Prefixes it applies to.
//
// The merge operation evaluates Actions across only those statements with the same key. For these Statements,
// the merge takes the union of all BGP Communities set.  When differing Local Preference values are set, the
// higher value is selected.

types/bgp.go:

In this file, a change was made to add a String() method for the type RoutePolicyConditions. As the merge operation uses a map to track each unique set of RoutePolicyConditions, the new String() method provides a simple hashable key.

Docs

This PR updates the BGPv2 Control Plane Docs to describe the change in behavior, shown below:

Testing

Unit Tests

service_test.go:

Unit tests for ClusterIP,External, and LoadBalancer Service advertisements were updated to each add a new test that configures overlapping advertisements. The overlapping advertisement tests each configure three advertisements, compared to all other tests which configure one. The advertisements configured set the following sets of attributes:

	localPrefHigh             = int64(200)
	redPeer65001BgpAttributes = &v2alpha1.BGPAttributes{
		Communities: &v2alpha1.BGPCommunities{
			Standard:  []v2alpha1.BGPStandardCommunity{"101:101"},
			Large:     []v2alpha1.BGPLargeCommunity{"1111:1111:1111"},
			WellKnown: []v2alpha1.BGPWellKnownCommunity{"no-export"},
		},
		LocalPreference: &localPrefHigh,
	}

	localPrefLow               = int64(50)
	redPeer65001BgpAttributes2 = &v2alpha1.BGPAttributes{
		Communities: &v2alpha1.BGPCommunities{
			Standard:  []v2alpha1.BGPStandardCommunity{"202:202"},
			Large:     []v2alpha1.BGPLargeCommunity{"2222:2222:2222"},
			WellKnown: []v2alpha1.BGPWellKnownCommunity{"no-export"},
		},
		LocalPreference: &localPrefLow,
	}

	redPeer65001BgpAttributes3 = &v2alpha1.BGPAttributes{
		Communities: &v2alpha1.BGPCommunities{
			Standard: []v2alpha1.BGPStandardCommunity{"202:202", "303:303"},
			Large:    []v2alpha1.BGPLargeCommunity{"2222:2222:2222", "3333:3333:3333"},
		},
		LocalPreference: &localPrefLow,
	}

The first and second each set unique communities for Standard and Large. The third first sets the same communities as the second, then adds a unique Standard and Large community. The first and latter two set different LocalPreference values.

policies_test.go:
Tests were added here to cover expected and edge cases for MergeRoutePolicies().

Manual Testing

The lab from https://github.com/cilium/cilium/tree/main/contrib/containerlab/bgpv2/service was used to manually test this change. Using the Makefile from that directory, I brought up a Kind cluster running Cilium with an instance of FRR. I commented out the lines that applied configuration to Cilium.

Instead, I applied the following yaml file using kubectl:

apiVersion: cilium.io/v2alpha1
kind: CiliumBGPClusterConfig
metadata:
  name: cilium-bgp
spec:
  nodeSelector:
    matchLabels:
      bgp: "65001"
  bgpInstances:
  - name: "65001"
    localASN: 65001
    peers:
    - name: "65000"
      peerASN: 65000
      peerAddress: fd00:10::1
      peerConfigRef:
        name: "cilium-peer"

---
apiVersion: cilium.io/v2alpha1
kind: CiliumBGPPeerConfig
metadata:
  name: cilium-peer
spec:
  authSecretRef: bgp-auth-secret
  gracefulRestart:
    enabled: true
    restartTimeSeconds: 15
  families:
    - afi: ipv4
      safi: unicast
      advertisements:
        matchLabels:
          advertise: "bgp"
    - afi: ipv6
      safi: unicast
      advertisements:
        matchLabels:
          advertise: "bgp"
---
apiVersion: cilium.io/v2alpha1
kind: CiliumBGPAdvertisement
metadata:
  name: "bgp-advertisements"
  labels:
    advertise: "bgp"
spec:
  advertisements:
    - advertisementType: "Service"
      service:
        addresses:
        - ClusterIP
      selector:
        matchExpressions:
        - {key: somekey, operator: NotIn, values: ['never-used-value']}
      attributes:
        communities:
          standard: [ "202:202" ]
          large: [ "2222:2222:2222" ]
    - advertisementType: "Service"
      service:
        addresses:
        - ClusterIP
      selector:
        matchExpressions:
        - {key: somekey, operator: NotIn, values: ['never-used-value']}
      attributes:
        communities:
          standard: [ "303:303" ]
          large: [ "3333:3333:3333" ]
          wellKnown: [ "no-export" ]
    - advertisementType: "Service"
      service:
        addresses:
        - ClusterIP
      selector:
        matchExpressions:
        - {key: somekey, operator: NotIn, values: ['never-used-value']}
      attributes:
        communities:
          standard: [ "444:4444" ]
          large: [ "4444:4444:4444" ]
          wellKnown: [ "no-advertise" ]

Inspecting FRR

On FRR, shown below, I saw the superset of all communities defined above set on the Service announcement:

router0# show ip bgp  10.2.86.210
BGP routing table entry for 10.2.86.210/32, version 29
Paths: (2 available, best #2, table default, not advertised to EBGP peer)
  Not advertised to any peer
  65001
    fd00:10:0:1::2 from fd00:10:0:1::2 (10.0.1.2)
      Origin IGP, valid, external, multipath
      Community: 202:202 303:303 444:4444 no-export no-advertise
      Large Community: 2222:2222:2222 3333:3333:3333 4444:4444:4444
      Last update: Thu Dec  5 15:36:06 2024
  65001
    fd00:10:0:2::2 from fd00:10:0:2::2 (10.0.2.2)
      Origin IGP, valid, external, multipath, best (Older Path)
      Community: 202:202 303:303 444:4444 no-export no-advertise
      Large Community: 2222:2222:2222 3333:3333:3333 4444:4444:4444
      Last update: Thu Dec  5 15:36:06 2024

Coverage

Using the uncover command line, coverage for pkg/bgpv1/manager/reconcilerv2 under this PR is now:

total:    (statements)    71.1%

On main as of writing, it was:

total:    (statements)    69.6%

Fixes: #35721

BGPv2:  Support overlapping selector matches on CiliumBGPAdvertisement