feat: Static Route Controller by sven-rosenzweig · Pull Request #435 · ironcore-dev/network-operator

sven-rosenzweig · 2026-07-07T16:34:54Z

Implement IOS-XR provider.

Implement IOS-XR provider. Signed-off-by: Sven Rosenzweig <sven.rosenzweig@sap.com>

github-actions · 2026-07-22T12:47:39Z

Merging this branch changes the coverage (1 decrease, 1 increase)

Impacted Packages	Coverage Δ	🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1	0.00% (ø)
github.com/ironcore-dev/network-operator/cmd	0.00% (ø)
github.com/ironcore-dev/network-operator/internal/controller/core	62.32% (+0.30%)	👍
github.com/ironcore-dev/network-operator/internal/provider	52.00% (ø)
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr	32.54% (-3.53%)	👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos	9.25% (-0.01%)	👎

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1/groupversion_info.go	0.00% (ø)	0	0	0
github.com/ironcore-dev/network-operator/api/core/v1alpha1/staticroute_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/cmd/main.go	0.00% (ø)	0	0	0
github.com/ironcore-dev/network-operator/internal/controller/core/interface_controller.go	72.34% (-0.17%)	582	421 (-1)	161 (+1)	👎
github.com/ironcore-dev/network-operator/internal/controller/core/staticroute_controller.go	70.94% (+70.94%)	203 (+203)	144 (+144)	59 (+59)	🌟
github.com/ironcore-dev/network-operator/internal/controller/core/vrf_controller.go	62.14% (-0.18%)	140 (+2)	87 (+1)	53 (+1)	👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/provider.go	20.43% (-2.27%)	230 (+23)	47	183 (+23)	👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/static_route.go	0.00% (ø)	14 (+14)	0	14 (+14)
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go	0.05% (-0.00%)	2036 (+2)	1	2035 (+2)	👎
github.com/ironcore-dev/network-operator/internal/provider/provider.go	52.00% (ø)	25	13	12

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/controller/core/staticroute_controller_test.go
github.com/ironcore-dev/network-operator/internal/controller/core/suite_test.go

felix-kaestner

Nice stuff! 🚀

felix-kaestner · 2026-08-04T07:44:16Z


+// StaticRouteLabel is a label applied to VRFs to indicate
+// the name of the StaticRoute that references them.
+const StaticRouteLabel = "networking.metal.ironcore.dev/static-route-name"


How will this work when there are multiple StaticRoutes in a VRF? Based on the way we structure the StaticRoute CRD this would be possible, no?

felix-kaestner · 2026-08-04T07:46:13Z

+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Name is immutable"
+	Name string `json:"name"`
+
+	// +optional


Should have a very brief comment, as all other fields.

felix-kaestner · 2026-08-04T07:55:09Z

+
+// StaticRouteStatus defines the observed state of StaticRoute.
+type StaticRouteStatus struct {
+	// The conditions are a list of status objects that describe the state of the Interface.


Suggested change

// The conditions are a list of status objects that describe the state of the Interface.

// The conditions are a list of status objects that describe the state of the StaticRoute.

copy+pasta

felix-kaestner · 2026-08-04T07:58:16Z

+
+	// IPPrefix is the destination IP prefix for the static route.
+	// +required
+	Prefix IPPrefix `json:"prefix,omitempty"`


Suggested change

Prefix IPPrefix `json:"prefix,omitempty"`

Prefix IPPrefix `json:"prefix"`

omitempty is unused if the field is required. Also applies to the NextHops.

felix-kaestner · 2026-08-04T07:59:32Z

+  name: networking-staticroute-admin-role
+rules:
+- apiGroups:
+  - networking.networking.metal.ironcore.dev


Suggested change

- networking.networking.metal.ironcore.dev

- networking.metal.ironcore.dev

This is wrong. Probably a grep over the codebase reveals other places that are also effected.

felix-kaestner · 2026-08-04T08:07:57Z

+	if req.VRF != nil && req.VRF.Spec.Name != "" {
+		vrf := req.VRF.Spec.Name
+		prefix.VRFName = vrf
+		return p.client.Update(ctx, &prefix)
+	}
+	return p.client.Update(ctx, &prefix)


Suggested change

if req.VRF != nil && req.VRF.Spec.Name != "" {

vrf := req.VRF.Spec.Name

prefix.VRFName = vrf

return p.client.Update(ctx, &prefix)

}

return p.client.Update(ctx, &prefix)

if req.VRF != nil && req.VRF.Spec.Name != "" {

prefix.VRFName = req.VRF.Spec.Name

}

return p.client.Update(ctx, &prefix)

Doesn't need to make it so complicated.

felix-kaestner · 2026-08-04T08:08:46Z

+	if prefixIP.Addr().Is4() {
+		prefix = Prefix{
+			PrefixAddress:  prefixIP.Addr().String(),
+			PrefixLength:   prefixIP.Bits(),
+			NextHopAddress: &nexthopAddress,
+			IsIpv4:         true,
+		}
+	} else if prefixIP.Addr().Is6() {
+		prefix = Prefix{
+			PrefixAddress:  prefixIP.Addr().String(),
+			PrefixLength:   prefixIP.Bits(),
+			NextHopAddress: &nexthopAddress,
+			IsIpv4:         false,
+		}
+	}


Suggested change

if prefixIP.Addr().Is4() {

prefix = Prefix{

PrefixAddress: prefixIP.Addr().String(),

PrefixLength: prefixIP.Bits(),

NextHopAddress: &nexthopAddress,

IsIpv4: true,

}

} else if prefixIP.Addr().Is6() {

prefix = Prefix{

PrefixAddress: prefixIP.Addr().String(),

PrefixLength: prefixIP.Bits(),

NextHopAddress: &nexthopAddress,

IsIpv4: false,

}

}

prefix = Prefix{

PrefixAddress: prefixIP.Addr().String(),

PrefixLength: prefixIP.Bits(),

NextHopAddress: &nexthopAddress,

IsIpv4: prefixIP.Addr().Is4(),

}

felix-kaestner · 2026-08-04T08:12:58Z

+	basePath := "Cisco-IOS-XR-um-router-static-cfg:router/static/vrfs/vrf[vrf-name=" + s.VRFName + "]/address-family/"
+	if s.VRFName == "" {
+		basePath = "Cisco-IOS-XR-um-router-static-cfg:router/static/address-family/"
+	}


Suggested change

basePath := "Cisco-IOS-XR-um-router-static-cfg:router/static/vrfs/vrf[vrf-name=" + s.VRFName + "]/address-family/"

if s.VRFName == "" {

basePath = "Cisco-IOS-XR-um-router-static-cfg:router/static/address-family/"

}

basePath := "Cisco-IOS-XR-um-router-static-cfg:router/static/address-family/"

if s.VRFName != "" {

basePath = "Cisco-IOS-XR-um-router-static-cfg:router/static/vrfs/vrf[vrf-name=" + s.VRFName + "]/address-family/"

}

nit: the other way around feels more logical to me.

felix-kaestner · 2026-08-04T08:14:45Z

 }

+func (p *Provider) EnsureStaticRoute(ctx context.Context, req *provider.StaticRouteRequest) error {
+	// TODO(sven-rosenzweig): implement static route configuration


Is there an open Ticket for this? Otherwise I think leaving these methods out would be better, as the controller already checks if the provider implements this interface (which it now does, because it has these methods). Not having these functions means the controller sees that the provider doesn't implement the contract and correctly updates the kubernetes resource status.

felix-kaestner · 2026-08-04T08:15:10Z

+    crdVersion: v1
+    namespaced: true
+  domain: networking.metal.ironcore.dev
+  group: core


Suggested change

group: core

not correct.

felix-kaestner · 2026-08-04T08:18:12Z

+		Watches(
+			&v1alpha1.VRF{},
+			handler.EnqueueRequestsFromMapFunc(r.vrfToStaticRoute),
+			builder.WithPredicates(predicate.Funcs{


I don't think we want to trigger reconcilation on updates to the vrf, as we are basically only interested in the .spec.name and that is immutable. So I would expect that we have a

UpdateFunc: func(e event.UpdateEvent) bool { return false },

felix-kaestner · 2026-08-04T08:20:27Z

+				ready := false
+				for _, cond := range resource.Status.Conditions {
+					if cond.Type == v1alpha1.ReadyCondition {
+						ready = true
+						g.Expect(cond.Status).To(Equal(metav1.ConditionFalse))
+					}
+				}
+				g.Expect(ready).To(BeTrue(), "ReadyCondition should be present")


I would prefer to adhere to the same pattern for condition checks as in the other controller tests (e.g. to use the metav1.FindStatusCondition).

github-actions Bot added the size/L label Jul 7, 2026

sven-rosenzweig force-pushed the feat/static_routes branch from 58dcad3 to af25860 Compare July 7, 2026 16:42

github-actions Bot added size/XXL and removed size/L labels Jul 7, 2026

sven-rosenzweig force-pushed the feat/static_routes branch 2 times, most recently from 9a2e31b to 8545345 Compare July 7, 2026 16:54

sven-rosenzweig force-pushed the feat/static_routes branch from 71deb30 to b3977eb Compare July 22, 2026 12:35

feat: Static Route Controller

4f36714

Implement IOS-XR provider. Signed-off-by: Sven Rosenzweig <sven.rosenzweig@sap.com>

sven-rosenzweig force-pushed the feat/static_routes branch from b3977eb to 4f36714 Compare July 22, 2026 12:37

sven-rosenzweig marked this pull request as ready for review July 22, 2026 12:41

felix-kaestner requested changes Aug 4, 2026

View reviewed changes

felix-kaestner reviewed Aug 4, 2026

View reviewed changes

	// The conditions are a list of status objects that describe the state of the Interface.
	// The conditions are a list of status objects that describe the state of the StaticRoute.

	Prefix IPPrefix `json:"prefix,omitempty"`
	Prefix IPPrefix `json:"prefix"`

	- networking.networking.metal.ironcore.dev
	- networking.metal.ironcore.dev

Uh oh!

Conversation

sven-rosenzweig commented Jul 7, 2026

Uh oh!

github-actions Bot commented Jul 22, 2026

Merging this branch changes the coverage (1 decrease, 1 increase)

Changed files (no unit tests)

Changed unit test files

Uh oh!

felix-kaestner left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

felix-kaestner left a comment •

edited

Loading