Skip to content

Commit 8c4c92e

Browse files
thomas-manginclaude
andcommitted
refactor: Add labels/rd parameters to Label/IPVPN.from_cidr()
Phase 0 of packed-bytes-first pattern for INET/Label/IPVPN chain. - Label.from_cidr() now accepts optional `labels` parameter - IPVPN.from_cidr() now accepts optional `labels` and `rd` parameters - Updated ~44 tests to use parameter pattern instead of setters - Prepares codebase for immutable NLRI construction in Phase 1-3 This change is backward compatible - existing code using setters still works, but tests now demonstrate the preferred pattern. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 5a63a0b commit 8c4c92e

File tree

4 files changed

+99
-103
lines changed

4 files changed

+99
-103
lines changed

src/exabgp/bgp/message/update/nlri/ipvpn.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ def from_cidr(
177177
safi: SAFI = SAFI.mpls_vpn, # Default to class SAFI; parameter kept for API compat
178178
action: Action = Action.UNSET,
179179
path_info: PathInfo = PathInfo.DISABLED,
180+
labels: Labels | None = None,
181+
rd: RouteDistinguisher | None = None,
180182
) -> 'IPVPN':
181183
"""Factory method to create IPVPN from a CIDR object.
182184
@@ -186,6 +188,8 @@ def from_cidr(
186188
safi: Ignored - IPVPN always uses mpls_vpn (kept for API compatibility)
187189
action: Route action (ANNOUNCE/WITHDRAW)
188190
path_info: AddPath path identifier
191+
labels: MPLS label stack (optional, defaults to NOLABEL)
192+
rd: Route Distinguisher (optional, defaults to NORD)
189193
190194
Returns:
191195
New IPVPN instance with SAFI=mpls_vpn
@@ -196,8 +200,8 @@ def from_cidr(
196200
instance._packed = cidr.pack_nlri()
197201
instance.path_info = path_info
198202
instance.nexthop = IP.NoNextHop
199-
instance._labels_packed = b'' # NOLABEL
200-
instance._rd_packed = b'' # NORD
203+
instance._labels_packed = labels.pack_labels() if labels is not None else b''
204+
instance._rd_packed = rd.pack_rd() if rd is not None else b''
201205
return instance
202206

203207
@classmethod

src/exabgp/bgp/message/update/nlri/label.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ def from_cidr(
148148
safi: SAFI = SAFI.nlri_mpls, # Default to class SAFI; parameter kept for API compat
149149
action: Action = Action.UNSET,
150150
path_info: PathInfo = PathInfo.DISABLED,
151+
labels: Labels | None = None,
151152
) -> 'Label':
152153
"""Factory method to create Label from a CIDR object.
153154
@@ -157,6 +158,7 @@ def from_cidr(
157158
safi: Ignored - Label always uses nlri_mpls (kept for API compatibility)
158159
action: Route action (ANNOUNCE/WITHDRAW)
159160
path_info: AddPath path identifier
161+
labels: MPLS label stack (optional, defaults to NOLABEL)
160162
161163
Returns:
162164
New Label instance with SAFI=nlri_mpls
@@ -167,7 +169,7 @@ def from_cidr(
167169
instance._packed = cidr.pack_nlri()
168170
instance.path_info = path_info
169171
instance.nexthop = IP.NoNextHop
170-
instance._labels_packed = b'' # NOLABEL
172+
instance._labels_packed = labels.pack_labels() if labels is not None else b''
171173
instance.rd = None
172174
return instance
173175

tests/unit/test_ipvpn.py

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -614,58 +614,60 @@ def test_from_cidr_defaults_no_labels_no_rd(self) -> None:
614614
assert nlri.rd == RouteDistinguisher.NORD
615615
assert nlri.cidr.prefix() == '192.168.1.0/24'
616616

617-
def test_from_cidr_then_set_rd_only(self) -> None:
618-
"""Test from_cidr then setting RD (no labels)"""
617+
def test_from_cidr_with_rd_only(self) -> None:
618+
"""Test from_cidr with RD parameter (no labels)"""
619619
cidr = CIDR.make_cidr(IP.pton('10.0.0.0'), 24)
620-
nlri = IPVPN.from_cidr(cidr, AFI.ipv4, SAFI.mpls_vpn)
621-
nlri.rd = RouteDistinguisher.make_from_elements('10.0.0.1', 100)
620+
nlri = IPVPN.from_cidr(cidr, AFI.ipv4, SAFI.mpls_vpn, rd=RouteDistinguisher.make_from_elements('10.0.0.1', 100))
622621

623622
assert nlri.labels == Labels.NOLABEL
624623
assert nlri.rd._str() == '10.0.0.1:100'
625624
assert nlri.cidr.prefix() == '10.0.0.0/24'
626625

627-
def test_from_cidr_then_set_labels_only(self) -> None:
628-
"""Test from_cidr then setting labels (no RD)"""
626+
def test_from_cidr_with_labels_only(self) -> None:
627+
"""Test from_cidr with labels parameter (no RD)"""
629628
cidr = CIDR.make_cidr(IP.pton('10.0.0.0'), 24)
630-
nlri = IPVPN.from_cidr(cidr, AFI.ipv4, SAFI.mpls_vpn)
631-
nlri.labels = Labels.make_labels([42], True)
629+
nlri = IPVPN.from_cidr(cidr, AFI.ipv4, SAFI.mpls_vpn, labels=Labels.make_labels([42], True))
632630

633631
assert nlri.labels.labels == [42]
634632
assert nlri.rd == RouteDistinguisher.NORD
635633
assert nlri.cidr.prefix() == '10.0.0.0/24'
636634

637-
def test_from_cidr_set_rd_before_labels(self) -> None:
638-
"""Test from_cidr with RD set BEFORE labels (config parser order)
635+
def test_from_cidr_with_labels_and_rd(self) -> None:
636+
"""Test from_cidr with both labels and RD parameters
639637
640-
The configuration parser often sets RD before labels. Verify this
641-
order works correctly.
638+
Verifies that both parameters can be set at construction time.
642639
"""
643640
cidr = CIDR.make_cidr(IP.pton('192.168.0.0'), 16)
644-
nlri = IPVPN.from_cidr(cidr, AFI.ipv4, SAFI.mpls_vpn)
641+
nlri = IPVPN.from_cidr(
642+
cidr,
643+
AFI.ipv4,
644+
SAFI.mpls_vpn,
645+
labels=Labels.make_labels([100, 200], True),
646+
rd=RouteDistinguisher.make_from_elements('172.16.0.1', 50),
647+
)
645648

646-
# Set RD first (like config parser does)
647-
nlri.rd = RouteDistinguisher.make_from_elements('172.16.0.1', 50)
649+
assert nlri.labels.labels == [100, 200]
648650
assert nlri.rd._str() == '172.16.0.1:50'
649651
assert nlri.cidr.prefix() == '192.168.0.0/16'
650652

651-
# Then set labels
652-
nlri.labels = Labels.make_labels([100, 200], True)
653-
assert nlri.labels.labels == [100, 200]
654-
assert nlri.rd._str() == '172.16.0.1:50' # RD should be preserved
655-
assert nlri.cidr.prefix() == '192.168.0.0/16' # CIDR should be preserved
653+
def test_from_cidr_with_all_parameters(self) -> None:
654+
"""Test from_cidr with all optional parameters"""
655+
from exabgp.bgp.message.update.nlri.qualifier import PathInfo
656656

657-
def test_from_cidr_set_labels_before_rd(self) -> None:
658-
"""Test from_cidr with labels set BEFORE RD"""
659657
cidr = CIDR.make_cidr(IP.pton('192.168.0.0'), 16)
660-
nlri = IPVPN.from_cidr(cidr, AFI.ipv4, SAFI.mpls_vpn)
658+
nlri = IPVPN.from_cidr(
659+
cidr,
660+
AFI.ipv4,
661+
SAFI.mpls_vpn,
662+
action=Action.ANNOUNCE,
663+
path_info=PathInfo.NOPATH,
664+
labels=Labels.make_labels([100], True),
665+
rd=RouteDistinguisher.make_from_elements('172.16.0.1', 50),
666+
)
661667

662-
# Set labels first
663-
nlri.labels = Labels.make_labels([100], True)
668+
assert nlri.action == Action.ANNOUNCE
669+
assert nlri.path_info == PathInfo.NOPATH
664670
assert nlri.labels.labels == [100]
665-
666-
# Then set RD
667-
nlri.rd = RouteDistinguisher.make_from_elements('172.16.0.1', 50)
668-
assert nlri.labels.labels == [100] # Labels should be preserved
669671
assert nlri.rd._str() == '172.16.0.1:50'
670672
assert nlri.cidr.prefix() == '192.168.0.0/16'
671673

@@ -872,17 +874,17 @@ def test_single_label_vs_no_labels(self) -> None:
872874
"""Compare behavior of NOLABEL vs single label"""
873875
# With NOLABEL
874876
cidr = CIDR.make_cidr(IP.pton('10.0.0.0'), 24)
875-
nlri_no_label = IPVPN.from_cidr(cidr, AFI.ipv4, SAFI.mpls_vpn)
876-
nlri_no_label.rd = RouteDistinguisher.make_from_elements('10.0.0.1', 1)
877+
nlri_no_label = IPVPN.from_cidr(
878+
cidr, AFI.ipv4, SAFI.mpls_vpn, rd=RouteDistinguisher.make_from_elements('10.0.0.1', 1)
879+
)
877880

878881
# With single label
879-
nlri_with_label = IPVPN.make_vpn_route(
882+
nlri_with_label = IPVPN.from_cidr(
883+
cidr,
880884
AFI.ipv4,
881885
SAFI.mpls_vpn,
882-
IP.pton('10.0.0.0'),
883-
24,
884-
Labels.make_labels([42], True),
885-
RouteDistinguisher.make_from_elements('10.0.0.1', 1),
886+
labels=Labels.make_labels([42], True),
887+
rd=RouteDistinguisher.make_from_elements('10.0.0.1', 1),
886888
)
887889

888890
# Both should have same RD and CIDR

0 commit comments

Comments
 (0)