Implement BGP Peering for IOS XR

[sven-rosenzweig]

No description provided.

[felix-kaestner]

Suggested change

	// LocalAS specifies a local AS number to present to the BGP peer without changing the main BGP process ASN.
	// LocalAS specifies a local AS number to present to the BGP peer, masking the global BGP process ASN.

Could you also extend the current validation webhook to verify this AS Number, same as the remote as number field?

[nikatza]

We probably want this to make this a pointer as this type does not have built-in nil value. As described in the k8s API conventions in more detail, a pointer helps to distinguish if the this is just not set or it is set but empty.

I guess it would be possible to keep the type as it is assuming that ASN0 is reserved and must not be used anyway. However, I think the pointer would be cleaner.

[sven-rosenzweig]

Valid point, made this a pointer of type int.

[felix-kaestner]

On a second thought, in BGP Process, we called the field "ASNumber". So should this naturally be "LocalASNumber"? How would the capitalization then look for the json/yaml key? "localASNumber"?

[sven-rosenzweig]

LocalASNumber it is.

[github-actions]

Merging this branch will decrease overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1	0.00% (ø)
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr	35.93% (-3.99%)	👎
github.com/ironcore-dev/network-operator/internal/webhook/core/v1alpha1	95.45% (-0.88%)	👎

---

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1/bgp_peer_types.go	0.00% (ø)	0	0	0
github.com/ironcore-dev/network-operator/api/core/v1alpha1/zz_generated.deepcopy.go	0.00% (ø)	0	0	0
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/bgp.go	0.00% (ø)	1 (+1)	0	1 (+1)
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/bgp_peer.go	50.00% (+50.00%)	38 (+38)	19 (+19)	19 (+19)	🌟
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/provider.go	22.00% (-5.33%)	200 (+39)	44	156 (+39)	👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/route_policies.go	0.00% (ø)	3 (+3)	0	3 (+3)
github.com/ironcore-dev/network-operator/internal/webhook/core/v1alpha1/bgppeer_webhook.go	76.92% (-6.41%)	13 (+7)	10 (+5)	3 (+2)	👎

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

• github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/bgp_peer_test.go

[felix-kaestner]

LocalASNumber only applies to eBGP neighbors (i.e. BGPPeer.asNumber != BGP.asNumber). As this check goes across multiple resources, we should put that into the controller for the BGPPeer (internal/controller/core/bgp_peer_controller.go#( to check for this condition, i.e. something like:

diff --git a/internal/controller/core/bgp_peer_controller.go b/internal/controller/core/bgp_peer_controller.go
index 39645e1c..8891da8a 100644
--- a/internal/controller/core/bgp_peer_controller.go
+++ b/internal/controller/core/bgp_peer_controller.go
@@ -441,6 +441,16 @@ func (r *BGPPeerReconciler) reconcile(ctx context.Context, s *bgpPeerScope) (ret
 		sourceInterface = intf.Spec.Name
 	}
 
+	if s.BGPPeer.Spec.LocalASNumber != nil && s.BGPPeer.Spec.ASNumber.String() == bgp.Spec.ASNumber.String() {
+		conditions.Set(s.BGPPeer, metav1.Condition{
+			Type:    v1alpha1.ConfiguredCondition,
+			Status:  metav1.ConditionFalse,
+			Reason:  v1alpha1.ErrorReason,
+			Message: "local-as cannot be configured on iBGP peers",
+		})
+		return reconcile.TerminalError(fmt.Errorf("local-as cannot be configured on iBGP peers"))
+	}
+
 	if err := s.Provider.Connect(ctx, s.Connection); err != nil {
 		return fmt.Errorf("failed to connect to provider: %w", err)
 	}

Returning a terminal error, if that validation fails.