Repository: metallb/metallb
Pull request #2716 (2025-03-12 13:04:26)

fix: webhook to handle interface field of BGPPeer resource
The commit extends the webhook validation to ensure BGPPeer resources with the interface field set are being validated as previously the .address was. When  Unnumbered BGP, the address field can be empty.


**Is this a BUG FIX or a FEATURE ?**:

> Uncomment only one, leave it on its own line:
>
 /kind bug
> /kind cleanup
> /kind feature
> /kind design
> /kind flake
> /kind failing
> /kind documentation
> /kind regression

**What this PR does / why we need it**:

**Special notes for your reviewer**:

**Release note**:


```release-note
NONE
```


fedepaol: If we did not release unnumbered yet, let's remove the release note.

Patch:
diff --git a/internal/config/validation.go b/internal/config/validation.go
index 66ece4b7..d848580a 100644
--- a/internal/config/validation.go
+++ b/internal/config/validation.go
@@ -3,9 +3,8 @@
 package config
 
 import (
-	"fmt"
-
 	"errors"
+	"fmt"
 
 	metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
 	metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"
@@ -146,17 +145,17 @@ func DiscardNativeOnly(c ClusterResources) error {
 	if len(c.Peers) > 1 {
 		peerAddr := make(map[string]bool)
 		routerID := c.Peers[0].Spec.RouterID
-		peer0 := peerAddressKey(c.Peers[0].Spec)
+		peer0 := peerIdentifier(c.Peers[0].Spec)
 		peerAddr[peer0] = true
 		for _, p := range c.Peers[1:] {
 			if p.Spec.RouterID != routerID {
 				return fmt.Errorf("peer %s has RouterID different from %s, in FRR mode all RouterID must be equal", p.Spec.RouterID, c.Peers[0].Spec.RouterID)
 			}
-			peerKey := peerAddressKey(p.Spec)
-			if _, ok := peerAddr[peerKey]; ok {
+			peerID := peerIdentifier(p.Spec)
+			if _, ok := peerAddr[peerID]; ok {
 				return fmt.Errorf("peer %s already exists, FRR mode doesn't support duplicate BGPPeers", p.Spec.Address)
 			}
-			peerAddr[peerKey] = true
+			peerAddr[peerID] = true
 		}
 	}
 	for _, p := range c.Peers {
@@ -216,8 +215,12 @@ func hasBFDEcho(peer *Peer, bfdProfiles map[string]*BFDProfile) bool {
 	return false
 }
 
-func peerAddressKey(peer metallbv1beta2.BGPPeerSpec) string {
-	return fmt.Sprintf("%s-%s", peer.Address, peer.VRFName)
+func peerIdentifier(peer metallbv1beta2.BGPPeerSpec) string {
+	id := peer.Address
+	if peer.Address == "" {
+		id = peer.Interface
+	}
+	return fmt.Sprintf("%s-%s", id, peer.VRFName)
 }
 
 type poolSelector struct {


Test patch:
diff --git a/internal/config/validation_test.go b/internal/config/validation_test.go
index 5a2a0a1b..c54be3b0 100644
--- a/internal/config/validation_test.go
+++ b/internal/config/validation_test.go
@@ -561,6 +561,46 @@ func TestValidateFRR(t *testing.T) {
 			},
 			mustFail: true,
 		},
+		{
+			desc: "two peers with interface set different",
+			config: ClusterResources{
+				Peers: []v1beta2.BGPPeer{
+					{
+						Spec: v1beta2.BGPPeerSpec{
+							Interface: "eth0",
+							MyASN:     123,
+						},
+					},
+					{
+						Spec: v1beta2.BGPPeerSpec{
+							Interface: "eth1",
+							MyASN:     123,
+						},
+					},
+				},
+			},
+			mustFail: false,
+		},
+		{
+			desc: "two peers with interface set same",
+			config: ClusterResources{
+				Peers: []v1beta2.BGPPeer{
+					{
+						Spec: v1beta2.BGPPeerSpec{
+							Interface: "eth0",
+							MyASN:     123,
+						},
+					},
+					{
+						Spec: v1beta2.BGPPeerSpec{
+							Interface: "eth0",
+							MyASN:     123,
+						},
+					},
+				},
+			},
+			mustFail: true,
+		},
 	}
 
 	for _, test := range tests {