From d18fca1e534fc6d40443b10e8a17912e6060f383 Mon Sep 17 00:00:00 2001 From: Pujol Date: Fri, 30 Oct 2026 10:59:08 +0100 Subject: [PATCH 1/2] Enable support for `nexthop-self` in BGPPeer This commit introduces a way to control the next-hop for advertised routes. Vendors like Cisco support a `nexthop-self` knob in the peers address-family configuration. This is however not modeled in OpenConfig, for which the same can be achieved by configuring routing policies (and so for every vendor). The configuration via policies is not addressed in this commit: only configuration via the nexthop-self is. As most vendors support this kind of knob, we extend the `BGPPeer` resource with a flag that models the same. This is also in line with the existing flag to enable route-reflector client behavior. It is the responsibility of each provider to signalize that the feature is not supported, as shown here in the OpenConfig provider. While NXOS is implemented in this commit, IOSXR and Nokia need a follow up. Cisco exposes the NX-OS peer address-family controls as a typed bitmask. As we now use multiple control flags in this mask we have updated the provider to handle properly the comma-separated gNMI values returned by the device and avoid unnecessary device writes due to different order. For consistency with other resources naming we renamed the `bgp_peer` to `bgppeer`. Signed-off-by: Pujol --- Tiltfile | 2 + .../{bgp_peer_types.go => bgppeer_types.go} | 5 + ...etworking.metal.ironcore.dev_bgppeers.yaml | 15 + config/samples/v1alpha1_bgppeer.yaml | 22 + docs/api-reference/index.md | 1 + ...er_controller.go => bgppeer_controller.go} | 21 +- ...ler_test.go => bgppeer_controller_test.go} | 9 + .../core/routingpolicy_controller.go | 1 - internal/controller/core/suite_test.go | 6 + internal/provider/cisco/iosxr/provider.go | 18 +- internal/provider/cisco/nxos/bgp.go | 131 +++++- internal/provider/cisco/nxos/bgp_test.go | 43 +- internal/provider/cisco/nxos/provider.go | 19 +- internal/provider/openconfig/bgppeer.go | 15 + internal/provider/provider.go | 8 +- .../{bgp_peer.txtar => bgppeer.txtar} | 0 .../bgppeer_nexthopself.txtar | 425 ++++++++++++++++++ ...umbered.txtar => bgppeer_unnumbered.txtar} | 0 18 files changed, 717 insertions(+), 24 deletions(-) rename api/core/v1alpha1/{bgp_peer_types.go => bgppeer_types.go} (98%) rename internal/controller/core/{bgp_peer_controller.go => bgppeer_controller.go} (97%) rename internal/controller/core/{bgp_peer_controller_test.go => bgppeer_controller_test.go} (98%) rename test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/{bgp_peer.txtar => bgppeer.txtar} (100%) create mode 100644 test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgppeer_nexthopself.txtar rename test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/{bgp_peer_unnumbered.txtar => bgppeer_unnumbered.txtar} (100%) diff --git a/Tiltfile b/Tiltfile index b9da45e29..c67e29070 100644 --- a/Tiltfile +++ b/Tiltfile @@ -148,6 +148,7 @@ k8s_resource(new_name='peer-spine1', objects=['leaf1-spine1:bgppeer'], resource_ k8s_resource(new_name='peer-spine2', objects=['leaf1-spine2:bgppeer'], resource_deps=['bgp', 'lo0'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) k8s_resource(new_name='peer-spine1-filtered', objects=['leaf1-spine1-filtered:bgppeer'], resource_deps=['bgp', 'lo0', 'bgp-import-policy'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) k8s_resource(new_name='peer-spine1-unnumbered', objects=['leaf1-spine1-unnumbered:bgppeer'], resource_deps=['bgp', 'eth1-4'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) +k8s_resource(new_name='peer-spine1-next-hop-self', objects=['leaf1-spine1-next-hop-self:bgppeer'], resource_deps=['bgp', 'lo0', 'bgp-next-hop-self'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) k8s_yaml('./config/samples/v1alpha1_ospf.yaml') k8s_resource(new_name='ospf-underlay', objects=['underlay:ospf'], resource_deps=['lo0', 'lo1', 'eth1-1', 'eth1-2'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) @@ -166,6 +167,7 @@ k8s_resource(new_name='ccloud-prefixset', objects=['ccloud-prefixset:prefixset'] k8s_yaml('./config/samples/v1alpha1_routingpolicy.yaml') k8s_resource(new_name='bgp-import-policy', objects=['bgp-import-policy:routingpolicy', 'internal-networks:prefixset', 'partner-networks:prefixset', 'blocked-networks:prefixset', 'communityset-regular:communityset', 'communityset-extended:communityset'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) +k8s_resource(new_name='bgp-next-hop-self', objects=['bgp-next-hop-self:routingpolicy'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) k8s_yaml('./config/samples/cisco/nx/v1alpha1_vpcdomain.yaml') k8s_resource(new_name='vpcdomain', objects=['leaf1-vpcdomain:vpcdomain', 'leaf1-vrfvpckeepalive:vrf', 'eth1-30:interface', 'vpc-peer-link:interfaceconfig', 'eth1-31:interface','eth1-32:interface', 'po1:interface'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) diff --git a/api/core/v1alpha1/bgp_peer_types.go b/api/core/v1alpha1/bgppeer_types.go similarity index 98% rename from api/core/v1alpha1/bgp_peer_types.go rename to api/core/v1alpha1/bgppeer_types.go index f75e7547a..06e9dcbf7 100644 --- a/api/core/v1alpha1/bgp_peer_types.go +++ b/api/core/v1alpha1/bgppeer_types.go @@ -186,6 +186,11 @@ type BGPPeerAddressFamily struct { // +optional RouteReflectorClient bool `json:"routeReflectorClient,omitempty"` + // NextHopSelf enables next-hop-self for routes advertised to this peer in this address family. + // If a RoutingPolicy also sets the next hop, then the precedence depends on the provider and platform implementation. + // +optional + NextHopSelf bool `json:"nextHopSelf,omitempty"` + // InboundRoutingPolicyRef references a RoutingPolicy applied to routes received from this peer // for this address family. // +optional diff --git a/config/crd/bases/networking.metal.ironcore.dev_bgppeers.yaml b/config/crd/bases/networking.metal.ironcore.dev_bgppeers.yaml index 463270deb..0fab642a4 100644 --- a/config/crd/bases/networking.metal.ironcore.dev_bgppeers.yaml +++ b/config/crd/bases/networking.metal.ironcore.dev_bgppeers.yaml @@ -129,6 +129,11 @@ spec: - name type: object x-kubernetes-map-type: atomic + nextHopSelf: + description: |- + NextHopSelf enables next-hop-self for routes advertised to this peer in this address family. + If a RoutingPolicy also sets the next hop, then the precedence depends on the provider and platform implementation. + type: boolean outboundRoutingPolicyRef: description: |- OutboundRoutingPolicyRef references a RoutingPolicy applied to routes advertised to this peer @@ -187,6 +192,11 @@ spec: - name type: object x-kubernetes-map-type: atomic + nextHopSelf: + description: |- + NextHopSelf enables next-hop-self for routes advertised to this peer in this address family. + If a RoutingPolicy also sets the next hop, then the precedence depends on the provider and platform implementation. + type: boolean outboundRoutingPolicyRef: description: |- OutboundRoutingPolicyRef references a RoutingPolicy applied to routes advertised to this peer @@ -245,6 +255,11 @@ spec: - name type: object x-kubernetes-map-type: atomic + nextHopSelf: + description: |- + NextHopSelf enables next-hop-self for routes advertised to this peer in this address family. + If a RoutingPolicy also sets the next hop, then the precedence depends on the provider and platform implementation. + type: boolean outboundRoutingPolicyRef: description: |- OutboundRoutingPolicyRef references a RoutingPolicy applied to routes advertised to this peer diff --git a/config/samples/v1alpha1_bgppeer.yaml b/config/samples/v1alpha1_bgppeer.yaml index 14c04891b..2dad0d5b5 100644 --- a/config/samples/v1alpha1_bgppeer.yaml +++ b/config/samples/v1alpha1_bgppeer.yaml @@ -88,3 +88,25 @@ spec: addressFamilies: ipv4Unicast: enabled: true +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: BGPPeer +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + name: leaf1-spine1-next-hop-self +spec: + deviceRef: + name: leaf1 + bgpRef: + name: bgp + address: 10.0.0.4 + asNumber: 65000 + localAddress: + interfaceRef: + name: lo0 + addressFamilies: + ipv4Unicast: + enabled: true + nextHopSelf: true diff --git a/docs/api-reference/index.md b/docs/api-reference/index.md index c735245e9..c75286f2a 100644 --- a/docs/api-reference/index.md +++ b/docs/api-reference/index.md @@ -1001,6 +1001,7 @@ _Appears in:_ | `enabled` _boolean_ | Enabled determines whether this address family is activated for this specific peer.
When false, the address family is not negotiated with this peer.
Defaults to false. | | Optional: \{\}
| | `sendCommunity` _[BGPCommunityType](#bgpcommunitytype)_ | SendCommunity specifies which community attributes should be sent to this BGP peer
for this address family. If not specified, no community attributes are sent. | | Enum: [Standard Extended Both]
Optional: \{\}
| | `routeReflectorClient` _boolean_ | RouteReflectorClient indicates whether this peer should be treated as a route reflector client
for this specific address family. Defaults to false. | | Optional: \{\}
| +| `nextHopSelf` _boolean_ | NextHopSelf enables next-hop-self for routes advertised to this peer in this address family.
If a RoutingPolicy also sets the next hop, then the precedence depends on the provider and platform implementation. | | Optional: \{\}
| | `inboundRoutingPolicyRef` _[LocalObjectReference](#localobjectreference)_ | InboundRoutingPolicyRef references a RoutingPolicy applied to routes received from this peer
for this address family. | | Optional: \{\}
| | `outboundRoutingPolicyRef` _[LocalObjectReference](#localobjectreference)_ | OutboundRoutingPolicyRef references a RoutingPolicy applied to routes advertised to this peer
for this address family. | | Optional: \{\}
| diff --git a/internal/controller/core/bgp_peer_controller.go b/internal/controller/core/bgppeer_controller.go similarity index 97% rename from internal/controller/core/bgp_peer_controller.go rename to internal/controller/core/bgppeer_controller.go index 69f41a6aa..33a94861f 100644 --- a/internal/controller/core/bgp_peer_controller.go +++ b/internal/controller/core/bgppeer_controller.go @@ -372,15 +372,18 @@ func (r *BGPPeerReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manag }), ). // Watches enqueues BGPPeers when a referenced RoutingPolicy is created or deleted. - // Only triggers on create and delete events since RoutingPolicy names are immutable. + // Triggers on create, delete, and update events when the RoutingPolicy's readiness changes. + // Policies attached to the peer may include actions in conflict with activated control settings, i.e., NextHopSelf. Watches( &v1alpha1.RoutingPolicy{}, handler.EnqueueRequestsFromMapFunc(r.routingPolicyToBGPPeers), builder.WithPredicates(predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - return false + oldPolicy := e.ObjectOld.(*v1alpha1.RoutingPolicy) + newPolicy := e.ObjectNew.(*v1alpha1.RoutingPolicy) + return conditions.IsReady(oldPolicy) != conditions.IsReady(newPolicy) }, - GenericFunc: func(e event.GenericEvent) bool { + GenericFunc: func(event.GenericEvent) bool { return false }, }), @@ -722,9 +725,8 @@ func (r *BGPPeerReconciler) reconcileVRF(ctx context.Context, peer *v1alpha1.BGP // reconcileRoutingPolicies resolves the inbound and outbound routing policy references // on each enabled address family of the BGPPeer. -// Returns two maps from BGPAddressFamilyType to the device-level policy name. // Sets ConfiguredCondition and returns a terminal error when a referenced policy -// is not found or belongs to a different device. +// is not found, belongs to a different device, or it is not yet ready. func (r *BGPPeerReconciler) reconcileRoutingPolicies(ctx context.Context, peer *v1alpha1.BGPPeer, device *v1alpha1.Device) (inbound, outbound map[v1alpha1.BGPAddressFamilyType]string, err error) { inbound = make(map[v1alpha1.BGPAddressFamilyType]string) outbound = make(map[v1alpha1.BGPAddressFamilyType]string) @@ -766,6 +768,15 @@ func (r *BGPPeerReconciler) reconcileRoutingPolicies(ctx context.Context, peer * }) return nil, nil, reconcile.TerminalError(fmt.Errorf("routing policy %s belongs to different device", ref.Name)) } + if !conditions.IsReady(rp) { + conditions.Set(peer, metav1.Condition{ + Type: v1alpha1.ConfiguredCondition, + Status: metav1.ConditionFalse, + Reason: v1alpha1.WaitingForDependenciesReason, + Message: fmt.Sprintf("Waiting for RoutingPolicy %s to become ready", ref.Name), + }) + return nil, nil, reconcile.TerminalError(fmt.Errorf("routing policy %s is not yet ready", ref.Name)) + } (*m)[afType] = rp.Spec.Name } } diff --git a/internal/controller/core/bgp_peer_controller_test.go b/internal/controller/core/bgppeer_controller_test.go similarity index 98% rename from internal/controller/core/bgp_peer_controller_test.go rename to internal/controller/core/bgppeer_controller_test.go index 82bd0982b..7cb4d471a 100644 --- a/internal/controller/core/bgp_peer_controller_test.go +++ b/internal/controller/core/bgppeer_controller_test.go @@ -71,6 +71,15 @@ var _ = Describe("BGPPeer Controller", func() { g.Expect(list.Items).To(BeEmpty()) }).Should(Succeed()) + By("Cleaning up RoutingPolicy resources for this device") + policyList := &v1alpha1.RoutingPolicyList{} + Expect(k8sClient.List(ctx, policyList, client.InNamespace(metav1.NamespaceDefault))).To(Succeed()) + for i := range policyList.Items { + if policyList.Items[i].Spec.DeviceRef.Name == device.Name { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &policyList.Items[i]))).To(Succeed()) + } + } + By("Cleaning up Interface resources for this device") intfList := &v1alpha1.InterfaceList{} Expect(k8sManager.GetClient().List(ctx, intfList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) diff --git a/internal/controller/core/routingpolicy_controller.go b/internal/controller/core/routingpolicy_controller.go index 628be127c..5dc62d3f9 100644 --- a/internal/controller/core/routingpolicy_controller.go +++ b/internal/controller/core/routingpolicy_controller.go @@ -384,7 +384,6 @@ func (r *RoutingPolicyReconciler) reconcile(ctx context.Context, s *routingPolic }) cond := conditions.FromError(err) - // As this resource is configuration only, we use the Configured condition as top-level Ready condition. cond.Type = v1alpha1.ReadyCondition conditions.Set(s.RoutingPolicy, cond) diff --git a/internal/controller/core/suite_test.go b/internal/controller/core/suite_test.go index 3fa346061..a23f5e49a 100644 --- a/internal/controller/core/suite_test.go +++ b/internal/controller/core/suite_test.go @@ -487,6 +487,7 @@ type DeviceState struct { PIM *v1alpha1.PIM BGP *v1alpha1.BGP BGPVRF *v1alpha1.VRF + BGPValidation func(*provider.EnsureBGPRequest) error BGPPeers sets.Set[string] OSPF sets.Set[string] VLANs sets.Set[int16] @@ -916,6 +917,11 @@ func (p *Provider) EnsureBGP(_ context.Context, req *provider.EnsureBGPRequest) s := p.devices.StateFor(p.deviceName) s.Lock() defer s.Unlock() + if s.BGPValidation != nil { + if err := s.BGPValidation(req); err != nil { + return err + } + } s.BGP = req.BGP s.BGPVRF = req.VRF return nil diff --git a/internal/provider/cisco/iosxr/provider.go b/internal/provider/cisco/iosxr/provider.go index e72db642f..c3e0e9bb4 100644 --- a/internal/provider/cisco/iosxr/provider.go +++ b/internal/provider/cisco/iosxr/provider.go @@ -525,8 +525,22 @@ func (p *Provider) EnsureBGPPeer(ctx context.Context, req *provider.EnsureBGPPee peer.BFD = bfd } - if req.BGPPeer.Spec.AddressFamilies != nil && (req.BGPPeer.Spec.AddressFamilies.Ipv6Unicast != nil || req.BGPPeer.Spec.AddressFamilies.L2vpnEvpn != nil) { - return errors.New("bgp peer: ipv6 unicast or l2vpnEvpn address family is currently not supported") + if req.BGPPeer.Spec.AddressFamilies != nil { + if req.BGPPeer.Spec.AddressFamilies.Ipv6Unicast != nil || req.BGPPeer.Spec.AddressFamilies.L2vpnEvpn != nil { + return errors.New("bgp peer: ipv6 unicast or l2vpnEvpn address family is currently not supported") + } + for name, af := range map[string]*v1alpha1.BGPPeerAddressFamily{ + "ipv4Unicast": req.BGPPeer.Spec.AddressFamilies.Ipv4Unicast, + "ipv6Unicast": req.BGPPeer.Spec.AddressFamilies.Ipv6Unicast, + "l2vpnEvpn": req.BGPPeer.Spec.AddressFamilies.L2vpnEvpn, + } { + if af != nil && af.NextHopSelf { + return apistatus.NewUnsupportedFieldError(apistatus.FieldViolation{ + Field: "spec.addressFamilies." + name + ".nextHopSelf", + Description: "iosxr provider does not currently implement support for next-hop-self on a BGP peer, use routing policies instead", + }) + } + } } // Configure Router Address Family diff --git a/internal/provider/cisco/nxos/bgp.go b/internal/provider/cisco/nxos/bgp.go index 9f92618ad..0c4df0c0f 100644 --- a/internal/provider/cisco/nxos/bgp.go +++ b/internal/provider/cisco/nxos/bgp.go @@ -7,6 +7,8 @@ import ( "encoding" "encoding/json" "errors" + "fmt" + "slices" "strings" "time" @@ -347,10 +349,10 @@ const ( ) type BGPPeerAfItem struct { - Ctrl Option[string] `json:"ctrl"` - SendComExt AdminSt `json:"sendComExt"` - SendComStd AdminSt `json:"sendComStd"` - Type AddressFamily `json:"type"` + Ctrl PeerAfControl `json:"ctrl"` + SendComExt AdminSt `json:"sendComExt"` + SendComStd AdminSt `json:"sendComStd"` + Type AddressFamily `json:"type"` RtCtrlPItems struct { RtCtrlPList gnmiext.List[RtCtrlDirection, *BGPPeerAfRtCtrlP] `json:"RtCtrlP-list,omitzero"` @@ -490,7 +492,126 @@ const ( PeerAsnTypeInternal PeerAsnType = "internal" ) -const RouteReflectorClient = "rr-client" +// PeerAfControl represents the control options for a BGP peer address family. +// It supports a combination of the following flags: +// 4 - rr-client +// 8 - nh-self +// 16 - dis-peer-as-check +// 32 - allow-self-as +// 64 - default-originate +// 128 - advertisement-interval +// 256 - suppress-inactive +// 512 - nh-self-all +type PeerAfControl uint16 + +// Values match the NX-OS DME ctrl bit assignments. +const ( + PeerAfControlRouteReflectorClient PeerAfControl = 1 << (iota + 2) + PeerAfControlNextHopSelf + PeerAfControlDisablePeerASCheck + PeerAfControlAllowSelfAS + PeerAfControlDefaultOriginate + PeerAfControlAdvertisementInterval + PeerAfControlSuppressInactive + PeerAfControlNextHopSelfAll +) + +var ( + _ fmt.Stringer = PeerAfControl(0) + _ json.Marshaler = PeerAfControl(0) + _ json.Unmarshaler = (*PeerAfControl)(nil) + _ encoding.TextMarshaler = PeerAfControl(0) + _ encoding.TextUnmarshaler = (*PeerAfControl)(nil) +) + +// MarshalJSON implements json.Marshaler. +func (c PeerAfControl) MarshalJSON() ([]byte, error) { + if c == 0 { + return []byte(`"DME_UNSET_PROPERTY_MARKER"`), nil + } + return json.Marshal(c.String()) +} + +// UnmarshalJSON implements json.Unmarshaler. +func (c *PeerAfControl) UnmarshalJSON(b []byte) error { + var value string + if err := json.Unmarshal(b, &value); err != nil { + return err + } + // A real device would not return this marker, we only handle it for testing purposes. + if value == "DME_UNSET_PROPERTY_MARKER" { + *c = 0 + return nil + } + return c.UnmarshalText([]byte(value)) +} + +// UnmarshalText implements encoding.TextUnmarshaler. +func (c *PeerAfControl) UnmarshalText(b []byte) error { + var controls PeerAfControl + for control := range strings.SplitSeq(string(b), ",") { + switch strings.TrimSpace(control) { + case "rr-client": + controls |= PeerAfControlRouteReflectorClient + case "nh-self": + controls |= PeerAfControlNextHopSelf + case "dis-peer-as-check": + controls |= PeerAfControlDisablePeerASCheck + case "allow-self-as": + controls |= PeerAfControlAllowSelfAS + case "default-originate": + controls |= PeerAfControlDefaultOriginate + case "advertisement-interval": + controls |= PeerAfControlAdvertisementInterval + case "suppress-inactive": + controls |= PeerAfControlSuppressInactive + case "nh-self-all": + controls |= PeerAfControlNextHopSelfAll + case "": + // ignore empty control + default: + return fmt.Errorf("bgp peer address family: unknown control %q", control) + } + } + *c = controls + return nil +} + +// MarshalText implements encoding.TextMarshaler. +func (c PeerAfControl) MarshalText() ([]byte, error) { + return []byte(c.String()), nil +} + +// String implements fmt.Stringer. +func (c PeerAfControl) String() string { + var controls []string + if c&PeerAfControlRouteReflectorClient != 0 { + controls = append(controls, "rr-client") + } + if c&PeerAfControlNextHopSelf != 0 { + controls = append(controls, "nh-self") + } + if c&PeerAfControlDisablePeerASCheck != 0 { + controls = append(controls, "dis-peer-as-check") + } + if c&PeerAfControlAllowSelfAS != 0 { + controls = append(controls, "allow-self-as") + } + if c&PeerAfControlDefaultOriginate != 0 { + controls = append(controls, "default-originate") + } + if c&PeerAfControlAdvertisementInterval != 0 { + controls = append(controls, "advertisement-interval") + } + if c&PeerAfControlSuppressInactive != 0 { + controls = append(controls, "suppress-inactive") + } + if c&PeerAfControlNextHopSelfAll != 0 { + controls = append(controls, "nh-self-all") + } + slices.Sort(controls) + return strings.Join(controls, ",") +} const PeerControlType = "bfd" diff --git a/internal/provider/cisco/nxos/bgp_test.go b/internal/provider/cisco/nxos/bgp_test.go index f2b22798a..d07d5c835 100644 --- a/internal/provider/cisco/nxos/bgp_test.go +++ b/internal/provider/cisco/nxos/bgp_test.go @@ -3,6 +3,10 @@ package nxos +import ( + "testing" +) + func init() { bgp := &BGP{AdminSt: AdminStEnabled, Asn: "65000"} Register("bgp", bgp) @@ -61,7 +65,7 @@ func init() { BfdType: BfdTypeNone, } bgpPeer.AfItems.PeerAfList.Set(&BGPPeerAfItem{ - Ctrl: Option[string]{Value: new(RouteReflectorClient)}, + Ctrl: PeerAfControlRouteReflectorClient, SendComExt: AdminStEnabled, SendComStd: AdminStEnabled, Type: AddressFamilyL2EVPN, @@ -139,3 +143,40 @@ func init() { BfdType: BfdTypeNone, }) } + +func TestPeerAfControl(t *testing.T) { + tests := []struct { + control string + want PeerAfControl + round string + }{ + {control: "rr-client", want: PeerAfControlRouteReflectorClient, round: "rr-client"}, + {control: "nh-self", want: PeerAfControlNextHopSelf, round: "nh-self"}, + {control: "dis-peer-as-check", want: PeerAfControlDisablePeerASCheck, round: "dis-peer-as-check"}, + {control: "allow-self-as", want: PeerAfControlAllowSelfAS, round: "allow-self-as"}, + {control: "default-originate", want: PeerAfControlDefaultOriginate, round: "default-originate"}, + {control: "advertisement-interval", want: PeerAfControlAdvertisementInterval, round: "advertisement-interval"}, + {control: "suppress-inactive", want: PeerAfControlSuppressInactive, round: "suppress-inactive"}, + {control: "nh-self-all", want: PeerAfControlNextHopSelfAll, round: "nh-self-all"}, + {control: "rr-client,nh-self", want: PeerAfControlRouteReflectorClient | PeerAfControlNextHopSelf, round: "nh-self,rr-client"}, + {control: "nh-self,rr-client", want: PeerAfControlRouteReflectorClient | PeerAfControlNextHopSelf, round: "nh-self,rr-client"}, + {control: "", want: 0, round: ""}, + } + for _, test := range tests { + var got PeerAfControl + if err := got.UnmarshalText([]byte(test.control)); err != nil { + t.Errorf("UnmarshalText(%q) returned %v", test.control, err) + continue + } + if got != test.want { + t.Errorf("UnmarshalText(%q) = %d, want %d", test.control, got, test.want) + } + if s := got.String(); s != test.round { + t.Errorf("UnmarshalText(%q).String() = %q, want %q", test.control, s, test.round) + } + } + var control PeerAfControl + if err := control.UnmarshalText([]byte("rr-client,bogus")); err == nil { + t.Error("UnmarshalText did not reject an unknown control") + } +} diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index 82a790b9e..b2b735e2d 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -975,15 +975,22 @@ func bgpPeerAfItems(req *provider.EnsureBGPPeerRequest) gnmiext.List[AddressFami if af.SendCommunity == v1alpha1.BGPCommunityTypeExtended || af.SendCommunity == v1alpha1.BGPCommunityTypeBoth { item.SendComExt = AdminStEnabled } + + afType := t.ToAddressFamilyType() + var ctrl PeerAfControl if af.RouteReflectorClient { - item.Ctrl = NewOption(RouteReflectorClient) + ctrl |= PeerAfControlRouteReflectorClient } - afType := t.ToAddressFamilyType() - if name, ok := req.InboundRoutingPolicies[afType]; ok { - item.RtCtrlPItems.RtCtrlPList.Set(&BGPPeerAfRtCtrlP{Direction: RtCtrlDirectionIn, RtMap: name}) + if af.NextHopSelf { + ctrl |= PeerAfControlNextHopSelf + } + item.Ctrl = ctrl + + if policy, ok := req.InboundRoutingPolicies[afType]; ok { + item.RtCtrlPItems.RtCtrlPList.Set(&BGPPeerAfRtCtrlP{Direction: RtCtrlDirectionIn, RtMap: policy}) } - if name, ok := req.OutboundRoutingPolicies[afType]; ok { - item.RtCtrlPItems.RtCtrlPList.Set(&BGPPeerAfRtCtrlP{Direction: RtCtrlDirectionOut, RtMap: name}) + if policy, ok := req.OutboundRoutingPolicies[afType]; ok { + item.RtCtrlPItems.RtCtrlPList.Set(&BGPPeerAfRtCtrlP{Direction: RtCtrlDirectionOut, RtMap: policy}) } list.Set(item) } diff --git a/internal/provider/openconfig/bgppeer.go b/internal/provider/openconfig/bgppeer.go index 11e1bcbb1..ea829b5a3 100644 --- a/internal/provider/openconfig/bgppeer.go +++ b/internal/provider/openconfig/bgppeer.go @@ -31,6 +31,21 @@ func (p *Provider) EnsureBGPPeer(ctx context.Context, req *provider.EnsureBGPPee }) } + if spec.AddressFamilies != nil { + for name, af := range map[string]*v1alpha1.BGPPeerAddressFamily{ + "ipv4Unicast": spec.AddressFamilies.Ipv4Unicast, + "ipv6Unicast": spec.AddressFamilies.Ipv6Unicast, + "l2vpnEvpn": spec.AddressFamilies.L2vpnEvpn, + } { + if af != nil && af.NextHopSelf { + return apistatus.NewUnsupportedFieldError(apistatus.FieldViolation{ + Field: "spec.addressFamilies." + name + ".nextHopSelf", + Description: "openconfig provider does not support next-hop-self on a BGP peer, use routing policies instead", + }) + } + } + } + peerAS, err := asnToUint32(spec.ASNumber) if err != nil { return err diff --git a/internal/provider/provider.go b/internal/provider/provider.go index fc4fc20d0..257de5978 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -559,11 +559,11 @@ type EnsureBGPPeerRequest struct { // VRF is the resolved VRF referenced by BGP.Spec.VrfRef. // When nil, the provider shall use the default VRF. VRF *v1alpha1.VRF - // InboundRoutingPolicies maps each address family to the device-level name of - // the inbound routing policy to apply. Absent key means no policy is configured. + // InboundRoutingPolicies maps each address family to the device-level name of the inbound routing policy to apply. + // Absent key means no policy is configured. InboundRoutingPolicies map[v1alpha1.BGPAddressFamilyType]string - // OutboundRoutingPolicies maps each address family to the device-level name of - // the outbound routing policy to apply. Absent key means no policy is configured. + // OutboundRoutingPolicies maps each address family to the device-level name of the outbound routing policy to apply. + // Absent key means no policy is configured. OutboundRoutingPolicies map[v1alpha1.BGPAddressFamilyType]string } diff --git a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgp_peer.txtar b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgppeer.txtar similarity index 100% rename from test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgp_peer.txtar rename to test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgppeer.txtar diff --git a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgppeer_nexthopself.txtar b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgppeer_nexthopself.txtar new file mode 100644 index 000000000..e8ff87a02 --- /dev/null +++ b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgppeer_nexthopself.txtar @@ -0,0 +1,425 @@ +-- interfaces/loopback0 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: Interface +metadata: + name: loopback0 + namespace: default +spec: + deviceRef: + name: device + name: lo0 + adminState: Up + type: Loopback + ipv4: + addresses: + - 10.0.0.1/32 + - 10.0.1.1/32 + ipv6: + addresses: + - 2001:db8::1/128 + + +-- bgps/bgp -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: BGP +metadata: + name: bgp0 + namespace: default +spec: + deviceRef: + name: device + asNumber: 65000 + routerId: "10.0.0.1" + addressFamilies: + ipv4Unicast: + enabled: true + ipv6Unicast: + enabled: true + +-- bgppeers/bgppeer1 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: BGPPeer +metadata: + name: bgppeer1 + namespace: default +spec: + deviceRef: + name: device + bgpRef: + name: bgp0 + address: "10.0.0.2" + asNumber: 65000 + description: "peering A" + localAddress: + interfaceRef: + name: loopback0 + addressFamilies: + ipv4Unicast: + enabled: true + nextHopSelf: true + routeReflectorClient: true + ipv6Unicast: + enabled: true + nextHopSelf: true + routeReflectorClient: true + ttl: 2 + +-- bgppeers/bgppeer2 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: BGPPeer +metadata: + name: bgppeer2 + namespace: default +spec: + deviceRef: + name: device + bgpRef: + name: bgp0 + address: "10.0.1.2" + asNumber: 65000 + description: "peering B" + localAddress: + interfaceRef: + name: loopback0 + addressFamilies: + ipv4Unicast: + enabled: true + ipv6Unicast: + enabled: true + ttl: 2 + +-- state/preload -- + +{ + "System": { + "procsys-items": { + "bootTime": "1700000000" + } + } +} + +-- state/expect -- + +{ + "System": { + "procsys-items": { + "bootTime": "1700000000" + }, + "intf-items": { + "lb-items": { + "LbRtdIf-list": [ + { + "adminSt": "up", + "descr": "DME_UNSET_PROPERTY_MARKER", + "id": "lo0", + "rtvrfMbr-items": { + "tDn": "/System/inst-items/Inst-list[name='default']" + } + } + ] + } + }, + "ipv4-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [ + { + "id": "lo0", + "addr-items": { + "Addr-list": [ + { + "addr": "10.0.0.1/32", + "pref": 0, + "tag": 0, + "type": "primary" + }, + { + "addr": "10.0.1.1/32", + "pref": 0, + "tag": 0, + "type": "secondary" + } + ] + } + } + ] + } + } + ] + } + } + }, + "icmpv4-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [ + { + "id": "lo0", + "ctrl": "port-unreachable,redirect" + } + ] + } + } + ] + } + } + }, + "fm-items": { + "bgp-items": { + "adminSt": "enabled" + }, + "evpn-items": { + "adminSt": "enabled" + } + }, + "bgp-items": { + "inst-items": { + "adminSt": "enabled", + "asn": "65000", + "dom-items": { + "Dom-list": [ + { + "name": "default", + "rtrId": "10.0.0.1", + "rtrIdAuto": "disabled", + "peercont-items": { + "PeerCont-list": [ + { + "name": "__operator-managed--default__" + } + ] + }, + "af-items": { + "DomAf-list": [ + { + "maxEcmp": 1, + "maxExtEcmp": 1, + "exportGwIp": "disabled", + "advertL2vpnEvpn": "disabled", + "type": "ipv4-ucast" + }, + { + "maxEcmp": 1, + "maxExtEcmp": 1, + "exportGwIp": "disabled", + "advertL2vpnEvpn": "disabled", + "type": "ipv6-ucast" + } + ] + }, + "peer-items": { + "Peer-list": [ + { + "addr": "10.0.0.2", + "adminSt": "enabled", + "asn": "65000", + "asnType": "none", + "ctrl": "DME_UNSET_PROPERTY_MARKER", + "bfdType": "none", + "name": "peering A", + "srcIf": "lo0", + "ttl": 2, + "af-items": { + "PeerAf-list": [ + { + "ctrl": "nh-self,rr-client", + "sendComExt": "disabled", + "sendComStd": "disabled", + "type": "ipv4-ucast" + }, + { + "ctrl": "nh-self,rr-client", + "sendComExt": "disabled", + "sendComStd": "disabled", + "type": "ipv6-ucast" + } + ] + } + }, + { + "addr": "10.0.1.2", + "adminSt": "enabled", + "asn": "65000", + "asnType": "none", + "ctrl": "DME_UNSET_PROPERTY_MARKER", + "bfdType": "none", + "name": "peering B", + "srcIf": "lo0", + "ttl": 2, + "af-items": { + "PeerAf-list": [ + { + "ctrl": "DME_UNSET_PROPERTY_MARKER", + "sendComExt": "disabled", + "sendComStd": "disabled", + "type": "ipv4-ucast" + }, + { + "ctrl": "DME_UNSET_PROPERTY_MARKER", + "sendComExt": "disabled", + "sendComStd": "disabled", + "type": "ipv6-ucast" + } + ] + } + } + ] + } + } + ] + } + } + }, + "ipv6-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [ + { + "id": "lo0", + "addr-items": { + "Addr-list": [ + { + "addr": "2001:db8::1/128", + "pref": 0, + "tag": 0, + "type": "primary" + } + ] + }, + "useLinkLocalAddr": "disabled" + } + ] + } + } + ] + } + } + }, + "nd-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [ + { + "id": "lo0", + "ctrl": "redirects", + "raIntvl": 600, + "raIntvlMin": 200 + } + ] + } + } + ] + } + } + } + } +} + +-- state/delete -- + +{ + "System": { + "procsys-items": { + "bootTime": "1700000000" + }, + "intf-items": { + "lb-items": { + "LbRtdIf-list": [] + } + }, + "ipv4-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [] + } + } + ] + } + } + }, + "icmpv4-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [ + { + "id": "lo0", + "ctrl": "port-unreachable,redirect" + } + ] + } + } + ] + } + } + }, + "fm-items": { + "bgp-items": { + "adminSt": "enabled" + }, + "evpn-items": { + "adminSt": "enabled" + } + }, + "bgp-items": {}, + "ipv6-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [] + } + } + ] + } + } + }, + "nd-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [ + { + "id": "lo0", + "ctrl": "redirects", + "raIntvl": 600, + "raIntvlMin": 200 + } + ] + } + } + ] + } + } + } + } +} + diff --git a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgp_peer_unnumbered.txtar b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgppeer_unnumbered.txtar similarity index 100% rename from test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgp_peer_unnumbered.txtar rename to test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/bgppeer_unnumbered.txtar From 4e9305ae4a0e86fbaf502ba7c6c7181a251a8201 Mon Sep 17 00:00:00 2001 From: Pujol Date: Fri, 30 Oct 2026 11:35:52 +0100 Subject: [PATCH 2/2] Add a `SetNextHop` BGP action in RoutingPolicy OpenConfig and some vendors do not offer a way to coarsely configure the nexthop with its own address in the advertised routes (`nexthop-self`). The same functionality can be achieved by defining a routing policy, attach it to a neighbor, and define the nexthop in an action within a statement. This approach allows more fine grained control than the `nexthop-self` knob we just introduced. In this commit we add a new BGP action in the RoutingPolicy that allows to define an IP address that should be used as nexthop (any kind of reference to `self` is not considered in the API as in OpenConfig). We update the NXOS provider, and change all other providers to signalize that this is not implemented yet for them. Signed-off-by: Pujol --- api/core/v1alpha1/routingpolicy_types.go | 13 ++++++++- api/core/v1alpha1/zz_generated.deepcopy.go | 21 ++++++++++++++ ...gppeers.networking.metal.ironcore.dev.yaml | 15 ++++++++++ ...olicies.networking.metal.ironcore.dev.yaml | 14 +++++++++- ...ng.metal.ironcore.dev_routingpolicies.yaml | 14 +++++++++- config/samples/v1alpha1_routingpolicy.yaml | 19 +++++++++++++ docs/api-reference/index.md | 18 ++++++++++++ .../core/routingpolicy_controller.go | 1 + internal/controller/core/suite_test.go | 6 ---- .../provider/cisco/iosxr/route_policies.go | 7 +++++ internal/provider/cisco/nxos/provider.go | 5 ++++ internal/provider/cisco/nxos/routemap.go | 13 +++++++-- internal/provider/openconfig/routingpolicy.go | 6 ++++ .../routingpolicy.txtar | 28 ++++++++++++++++++- 14 files changed, 168 insertions(+), 12 deletions(-) diff --git a/api/core/v1alpha1/routingpolicy_types.go b/api/core/v1alpha1/routingpolicy_types.go index b21b20247..9b6149e78 100644 --- a/api/core/v1alpha1/routingpolicy_types.go +++ b/api/core/v1alpha1/routingpolicy_types.go @@ -130,7 +130,7 @@ const ( ) // BgpActions defines BGP-specific actions for a policy statement. -// +kubebuilder:validation:XValidation:rule="has(self.setCommunity) || has(self.setExtCommunity) || has(self.setASPath)",message="at least one BGP action must be specified" +// +kubebuilder:validation:XValidation:rule="has(self.setCommunity) || has(self.setExtCommunity) || has(self.setASPath) || has(self.setNextHop)",message="at least one BGP action must be specified" type BgpActions struct { // SetCommunity configures BGP standard community attributes. // +optional @@ -143,6 +143,17 @@ type BgpActions struct { // SetASPath configures modifications to the BGP AS path attribute. // +optional SetASPath *SetASPathAction `json:"setASPath,omitempty"` + + // SetNextHop configures the BGP next-hop for advertised routes. + // +optional + SetNextHop *SetNextHopAction `json:"setNextHop,omitempty"` +} + +// SetNextHopAction defines how to replace the BGP next-hop for advertised routes. +type SetNextHopAction struct { + // IPAddr specifies the next-hop IP address to set for advertised routes. + // +required + IPAddr IPAddr `json:"ipAddr"` } // SetASPathAction defines actions to modify the BGP AS path attribute. diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go index 00f1ca972..d8fe478e6 100644 --- a/api/core/v1alpha1/zz_generated.deepcopy.go +++ b/api/core/v1alpha1/zz_generated.deepcopy.go @@ -1155,6 +1155,11 @@ func (in *BgpActions) DeepCopyInto(out *BgpActions) { *out = new(SetASPathAction) (*in).DeepCopyInto(*out) } + if in.SetNextHop != nil { + in, out := &in.SetNextHop, &out.SetNextHop + *out = new(SetNextHopAction) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BgpActions. @@ -4963,6 +4968,22 @@ func (in *SetExtCommunityAction) DeepCopy() *SetExtCommunityAction { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SetNextHopAction) DeepCopyInto(out *SetNextHopAction) { + *out = *in + in.IPAddr.DeepCopyInto(&out.IPAddr) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SetNextHopAction. +func (in *SetNextHopAction) DeepCopy() *SetNextHopAction { + if in == nil { + return nil + } + out := new(SetNextHopAction) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StaticRoute) DeepCopyInto(out *StaticRoute) { *out = *in diff --git a/charts/network-operator/templates/crd/bgppeers.networking.metal.ironcore.dev.yaml b/charts/network-operator/templates/crd/bgppeers.networking.metal.ironcore.dev.yaml index 99c2cf961..9d330db6d 100644 --- a/charts/network-operator/templates/crd/bgppeers.networking.metal.ironcore.dev.yaml +++ b/charts/network-operator/templates/crd/bgppeers.networking.metal.ironcore.dev.yaml @@ -132,6 +132,11 @@ spec: - name type: object x-kubernetes-map-type: atomic + nextHopSelf: + description: |- + NextHopSelf enables next-hop-self for routes advertised to this peer in this address family. + If a RoutingPolicy also sets the next hop, then the precedence depends on the provider and platform implementation. + type: boolean outboundRoutingPolicyRef: description: |- OutboundRoutingPolicyRef references a RoutingPolicy applied to routes advertised to this peer @@ -190,6 +195,11 @@ spec: - name type: object x-kubernetes-map-type: atomic + nextHopSelf: + description: |- + NextHopSelf enables next-hop-self for routes advertised to this peer in this address family. + If a RoutingPolicy also sets the next hop, then the precedence depends on the provider and platform implementation. + type: boolean outboundRoutingPolicyRef: description: |- OutboundRoutingPolicyRef references a RoutingPolicy applied to routes advertised to this peer @@ -248,6 +258,11 @@ spec: - name type: object x-kubernetes-map-type: atomic + nextHopSelf: + description: |- + NextHopSelf enables next-hop-self for routes advertised to this peer in this address family. + If a RoutingPolicy also sets the next hop, then the precedence depends on the provider and platform implementation. + type: boolean outboundRoutingPolicyRef: description: |- OutboundRoutingPolicyRef references a RoutingPolicy applied to routes advertised to this peer diff --git a/charts/network-operator/templates/crd/routingpolicies.networking.metal.ironcore.dev.yaml b/charts/network-operator/templates/crd/routingpolicies.networking.metal.ironcore.dev.yaml index bb2729aa2..bba0326ae 100644 --- a/charts/network-operator/templates/crd/routingpolicies.networking.metal.ironcore.dev.yaml +++ b/charts/network-operator/templates/crd/routingpolicies.networking.metal.ironcore.dev.yaml @@ -264,11 +264,23 @@ spec: required: - communities type: object + setNextHop: + description: SetNextHop configures the BGP next-hop + for advertised routes. + properties: + ipAddr: + description: IPAddr specifies the next-hop IP address + to set for advertised routes. + format: ip + type: string + required: + - ipAddr + type: object type: object x-kubernetes-validations: - message: at least one BGP action must be specified rule: has(self.setCommunity) || has(self.setExtCommunity) - || has(self.setASPath) + || has(self.setASPath) || has(self.setNextHop) routeDisposition: description: RouteDisposition specifies whether to accept or reject the route. diff --git a/config/crd/bases/networking.metal.ironcore.dev_routingpolicies.yaml b/config/crd/bases/networking.metal.ironcore.dev_routingpolicies.yaml index 1971bfc54..5fa7197bd 100644 --- a/config/crd/bases/networking.metal.ironcore.dev_routingpolicies.yaml +++ b/config/crd/bases/networking.metal.ironcore.dev_routingpolicies.yaml @@ -261,11 +261,23 @@ spec: required: - communities type: object + setNextHop: + description: SetNextHop configures the BGP next-hop + for advertised routes. + properties: + ipAddr: + description: IPAddr specifies the next-hop IP address + to set for advertised routes. + format: ip + type: string + required: + - ipAddr + type: object type: object x-kubernetes-validations: - message: at least one BGP action must be specified rule: has(self.setCommunity) || has(self.setExtCommunity) - || has(self.setASPath) + || has(self.setASPath) || has(self.setNextHop) routeDisposition: description: RouteDisposition specifies whether to accept or reject the route. diff --git a/config/samples/v1alpha1_routingpolicy.yaml b/config/samples/v1alpha1_routingpolicy.yaml index 483b609d8..aca59edc8 100644 --- a/config/samples/v1alpha1_routingpolicy.yaml +++ b/config/samples/v1alpha1_routingpolicy.yaml @@ -183,3 +183,22 @@ spec: regex: "65200:[0-9][0-9]" - sequence: 15 regex: "65300:[0-9]+" +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: RoutingPolicy +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + name: bgp-next-hop-self +spec: + deviceRef: + name: leaf1 + name: BGP-NEXT-HOP-SELF + statements: + - sequence: 10 + actions: + routeDisposition: AcceptRoute + bgpActions: + setNextHop: + ipAddr: 10.0.0.1 diff --git a/docs/api-reference/index.md b/docs/api-reference/index.md index c75286f2a..6ba7dc058 100644 --- a/docs/api-reference/index.md +++ b/docs/api-reference/index.md @@ -1290,6 +1290,7 @@ _Appears in:_ | `setCommunity` _[SetCommunityAction](#setcommunityaction)_ | SetCommunity configures BGP standard community attributes. | | Optional: \{\}
| | `setExtCommunity` _[SetExtCommunityAction](#setextcommunityaction)_ | SetExtCommunity configures BGP extended community attributes. | | Optional: \{\}
| | `setASPath` _[SetASPathAction](#setaspathaction)_ | SetASPath configures modifications to the BGP AS path attribute. | | Optional: \{\}
| +| `setNextHop` _[SetNextHopAction](#setnexthopaction)_ | SetNextHop configures the BGP next-hop for advertised routes. | | Optional: \{\}
| #### Certificate @@ -2546,6 +2547,7 @@ _Validation:_ _Appears in:_ - [IPAddressSpec](#ipaddressspec) - [PingProbe](#pingprobe) +- [SetNextHopAction](#setnexthopaction) @@ -4475,6 +4477,22 @@ _Appears in:_ | `operation` _[CommunityOperation](#communityoperation)_ | Operation controls whether the communities are added to the route's existing
communities (Add) or replace them (Replace). | Replace | Enum: [Add Replace]
Optional: \{\}
| +#### SetNextHopAction + + + +SetNextHopAction defines how to replace the BGP next-hop for advertised routes. + + + +_Appears in:_ +- [BgpActions](#bgpactions) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `ipAddr` _[IPAddr](#ipaddr)_ | IPAddr specifies the next-hop IP address to set for advertised routes. | | Format: ip
Type: string
Required: \{\}
| + + #### Severity _Underlying type:_ _string_ diff --git a/internal/controller/core/routingpolicy_controller.go b/internal/controller/core/routingpolicy_controller.go index 5dc62d3f9..628be127c 100644 --- a/internal/controller/core/routingpolicy_controller.go +++ b/internal/controller/core/routingpolicy_controller.go @@ -384,6 +384,7 @@ func (r *RoutingPolicyReconciler) reconcile(ctx context.Context, s *routingPolic }) cond := conditions.FromError(err) + // As this resource is configuration only, we use the Configured condition as top-level Ready condition. cond.Type = v1alpha1.ReadyCondition conditions.Set(s.RoutingPolicy, cond) diff --git a/internal/controller/core/suite_test.go b/internal/controller/core/suite_test.go index a23f5e49a..3fa346061 100644 --- a/internal/controller/core/suite_test.go +++ b/internal/controller/core/suite_test.go @@ -487,7 +487,6 @@ type DeviceState struct { PIM *v1alpha1.PIM BGP *v1alpha1.BGP BGPVRF *v1alpha1.VRF - BGPValidation func(*provider.EnsureBGPRequest) error BGPPeers sets.Set[string] OSPF sets.Set[string] VLANs sets.Set[int16] @@ -917,11 +916,6 @@ func (p *Provider) EnsureBGP(_ context.Context, req *provider.EnsureBGPRequest) s := p.devices.StateFor(p.deviceName) s.Lock() defer s.Unlock() - if s.BGPValidation != nil { - if err := s.BGPValidation(req); err != nil { - return err - } - } s.BGP = req.BGP s.BGPVRF = req.VRF return nil diff --git a/internal/provider/cisco/iosxr/route_policies.go b/internal/provider/cisco/iosxr/route_policies.go index a0325a513..fedfecc7d 100644 --- a/internal/provider/cisco/iosxr/route_policies.go +++ b/internal/provider/cisco/iosxr/route_policies.go @@ -14,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" + "github.com/ironcore-dev/network-operator/internal/apistatus" "github.com/ironcore-dev/network-operator/internal/provider" "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) @@ -153,6 +154,12 @@ func (al *Actions) String() string { func NewActions(actions v1alpha1.PolicyActions) (Actions, error) { var result Actions + if actions.BgpActions != nil && actions.BgpActions.SetNextHop != nil { + return result, apistatus.NewUnsupportedFieldError(apistatus.FieldViolation{ + Field: "spec.statements[].actions.bgpActions.setNextHop", + Description: "iosxr provider does not support setting the BGP next-hop", + }) + } if actions.BgpActions != nil && actions.BgpActions.SetCommunity != nil { result.Actions = append(result.Actions, &CommAction{ diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index b2b735e2d..3537cacc5 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -2910,6 +2910,7 @@ func (p *Provider) DeletePrefixSet(ctx context.Context, req *provider.DeletePref return p.client.Delete(ctx, s) } +// EnsureRoutingPolicy installs the route-map definition and its statement actions. func (p *Provider) EnsureRoutingPolicy(ctx context.Context, req *provider.EnsureRoutingPolicyRequest) error { rm := new(RouteMap) rm.Name = req.Name @@ -2938,6 +2939,10 @@ func (p *Provider) EnsureRoutingPolicy(ctx context.Context, req *provider.Ensure } if stmt.Actions.BgpActions != nil { + if stmt.Actions.BgpActions.SetNextHop != nil { + nextHop := stmt.Actions.BgpActions.SetNextHop + e.NhItems.SetNhList.Set(&RouteMapNextHop{Addr: nextHop.IPAddr.String()}) + } if stmt.Actions.BgpActions.SetCommunity != nil { if err := e.SetCommunities(stmt.Actions.BgpActions.SetCommunity.Communities, stmt.Actions.BgpActions.SetCommunity.Operation); err != nil { return err diff --git a/internal/provider/cisco/nxos/routemap.go b/internal/provider/cisco/nxos/routemap.go index 92e76d4a8..776fee787 100644 --- a/internal/provider/cisco/nxos/routemap.go +++ b/internal/provider/cisco/nxos/routemap.go @@ -24,8 +24,11 @@ func (rm *RouteMap) XPath() string { } type RouteMapEntry struct { - Action Action `json:"action"` - Order int32 `json:"order"` + Action Action `json:"action"` + Order int32 `json:"order"` + NhItems struct { + SetNhList gnmiext.List[string, *RouteMapNextHop] `json:"SetNh-list,omitzero"` + } `json:"nh-items,omitzero"` SrttItems struct { Additive AdminSt `json:"additive"` ItemItems struct { @@ -75,6 +78,12 @@ type RouteMapEntry struct { func (e *RouteMapEntry) Key() int32 { return e.Order } +type RouteMapNextHop struct { + Addr string `json:"addr"` +} + +func (n *RouteMapNextHop) Key() string { return n.Addr } + func (e *RouteMapEntry) SetCommunities(communities []string, operation v1alpha1.CommunityOperation) error { for _, comm := range communities { c, err := Community(comm) diff --git a/internal/provider/openconfig/routingpolicy.go b/internal/provider/openconfig/routingpolicy.go index 5318f1dbc..fa6cf28db 100644 --- a/internal/provider/openconfig/routingpolicy.go +++ b/internal/provider/openconfig/routingpolicy.go @@ -75,6 +75,12 @@ func (p *Provider) EnsureRoutingPolicy(ctx context.Context, req *provider.Ensure Description: "openconfig provider does not support AS-path manipulation on SRLinux", }) } + if stmt.Actions.BgpActions.SetNextHop != nil { + return apistatus.NewUnsupportedFieldError(apistatus.FieldViolation{ + Field: "spec.statements[].actions.bgpActions.setNextHop", + Description: "openconfig provider does not implement the BGP next-hop in BGP actions", + }) + } } pd.Statements.Statement.Set(s) diff --git a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/routingpolicy.txtar b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/routingpolicy.txtar index d60f84fff..23305eb65 100644 --- a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/routingpolicy.txtar +++ b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/routingpolicy.txtar @@ -111,6 +111,14 @@ spec: - "65000:10" - "65000:20" operation: Replace + setNextHop: + ipAddr: 10.0.0.1 + - sequence: 60 + actions: + routeDisposition: AcceptRoute + bgpActions: + setNextHop: + ipAddr: "2001:db8::1" -- state/preload -- { @@ -251,7 +259,7 @@ spec: "action": "permit", "order": 40, "mextcomm-items": { - "criteria":"sub-group", + "criteria": "sub-group", "rsextCommAtt-items": { "RsExtCommAtt-list": [ { @@ -292,6 +300,24 @@ spec: } ] } + }, + "nh-items": { + "SetNh-list": [ + { + "addr": "10.0.0.1" + } + ] + } + }, + { + "action": "permit", + "order": 60, + "nh-items": { + "SetNh-list": [ + { + "addr": "2001:db8::1" + } + ] } } ]