Repository: metallb/metallb
Pull request #2593 (2024-11-08 20:56:21)

IPV6 bgp validation: validate bgpadvertisement selected pools

<!-- Thanks for sending a pull request!
1. If this is your first time, please read the [contributing guide](https://metallb.universe.tf/community/#contributing)
2. For non-trivial pull requests, please [file an
   issue](https://github.com/metallb/metallb/issues/new) first, and get
   agreement that the change is a good idea, and a general guideline
   for how it should be implemented, before sending code. Large PRs
   that weren't first discussed and agreed upon in an issue won't be
   accepted.
3. If the PR fixes a particular bug, please include the words "Fixed
   #<issue number>" in the PR text, so that the bug auto-closes when
   the PR is merged.
-->

**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**:
We can have l2 ipv6 addresses when using metallb in native mode. The important thing to consider is, not having any v6 based pool selected by a bgp advertisement. Here we narrow down the scope of the checking function.

Fixes https://github.com/metallb/metallb/issues/2431
**Special notes for your reviewer**:

**Release note**:
<!--  Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. Follow the instructions for writing a release note from k8s: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If no release note is required, just write "NONE".
-->

```release-note
BGP native mode: narrow down the validation so that ipv6 pools are not valid only if there is a bgp advertisement referencing them.
```


github-actions[bot]: This PR has been automatically marked as stale because it has been open 30 days
with no activity. This issue will be closed in 10 days unless you do one of the following:
- respond or perform some activity on this PR
- have one of these labels applied: `hold`

Patch:
diff --git a/internal/config/validation.go b/internal/config/validation.go
index 0b9e6409..2d1759e1 100644
--- a/internal/config/validation.go
+++ b/internal/config/validation.go
@@ -7,9 +7,12 @@ import (
 
 	"errors"
 
+	metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
 	metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"
 	"go.universe.tf/metallb/internal/bgp/community"
 	"go.universe.tf/metallb/internal/ipfamily"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 )
 
 type Validate func(ClusterResources) error
@@ -69,7 +72,17 @@ func findIPv6BGPAdvertisement(c ClusterResources) error {
 	if len(c.BGPAdvs) == 0 {
 		return nil
 	}
+
+	bgpSelectors, err := poolSelectorsForBGP(c)
+	if err != nil {
+		return err
+	}
+
 	for _, p := range c.Pools {
+		if !bgpSelectors.matchesPool(p) {
+			continue
+		}
+
 		for _, cidr := range p.Spec.Addresses {
 			nets, err := ParseCIDR(cidr)
 			if err != nil {
@@ -203,3 +216,49 @@ func hasBFDEcho(peer *Peer, bfdProfiles map[string]*BFDProfile) bool {
 func peerAddressKey(peer metallbv1beta2.BGPPeerSpec) string {
 	return fmt.Sprintf("%s-%s", peer.Address, peer.VRFName)
 }
+
+type poolSelector struct {
+	byName   map[string]struct{}
+	byLabels []labels.Selector
+}
+
+func (s poolSelector) matchesPool(p metallbv1beta1.IPAddressPool) bool {
+	if len(s.byLabels) == 0 && len(s.byName) == 0 {
+		return true
+	}
+
+	if _, ok := s.byName[p.Name]; ok {
+		return true
+	}
+	for _, l := range s.byLabels {
+		if l.Matches(labels.Set(p.Labels)) {
+			return true
+		}
+	}
+	return false
+}
+
+func poolSelectorsForBGP(c ClusterResources) (poolSelector, error) {
+	selectedPools := make(map[string]struct{})
+	poolsSelectors := []labels.Selector{}
+	for _, adv := range c.BGPAdvs {
+		if len(adv.Spec.IPAddressPools) == 0 &&
+			len(adv.Spec.IPAddressPoolSelectors) == 0 {
+			return poolSelector{}, nil // no selectors, let's catch em all!
+		}
+		for _, p := range adv.Spec.IPAddressPools {
+			selectedPools[p] = struct{}{}
+		}
+		for _, selector := range adv.Spec.IPAddressPoolSelectors {
+			l, err := metav1.LabelSelectorAsSelector(&selector)
+			if err != nil {
+				return poolSelector{}, fmt.Errorf("invalid label selector %v", selector)
+			}
+			poolsSelectors = append(poolsSelectors, l)
+		}
+	}
+	return poolSelector{
+		byName:   selectedPools,
+		byLabels: poolsSelectors,
+	}, nil
+}


Test patch:
diff --git a/internal/config/validation_test.go b/internal/config/validation_test.go
index f6b21d43..cd4ae44c 100644
--- a/internal/config/validation_test.go
+++ b/internal/config/validation_test.go
@@ -70,6 +70,104 @@ func TestValidate(t *testing.T) {
 			},
 			mustFail: true,
 		},
+		{
+			desc: "v6 address but pool not selected",
+			config: ClusterResources{
+				Pools: []v1beta1.IPAddressPool{
+					{
+						ObjectMeta: v1.ObjectMeta{
+							Name: "foo",
+						},
+						Spec: v1beta1.IPAddressPoolSpec{
+							Addresses: []string{"2001:db8::/64"},
+						},
+					},
+				},
+				BGPAdvs: []v1beta1.BGPAdvertisement{
+					{
+						ObjectMeta: v1.ObjectMeta{
+							Name: "foo",
+						},
+						Spec: v1beta1.BGPAdvertisementSpec{
+							IPAddressPools: []string{"bar"},
+						},
+					},
+				},
+			},
+		},
+		{
+			desc: "v6 address and selected",
+			config: ClusterResources{
+				Pools: []v1beta1.IPAddressPool{
+					{
+						ObjectMeta: v1.ObjectMeta{
+							Name: "foo",
+						},
+						Spec: v1beta1.IPAddressPoolSpec{
+							Addresses: []string{"2001:db8::/64"},
+						},
+					},
+				},
+				BGPAdvs: []v1beta1.BGPAdvertisement{
+					{
+						ObjectMeta: v1.ObjectMeta{
+							Name: "bar",
+						},
+						Spec: v1beta1.BGPAdvertisementSpec{
+							IPAddressPools: []string{"foo1"},
+						},
+					},
+					{
+						ObjectMeta: v1.ObjectMeta{
+							Name: "bar",
+						},
+						Spec: v1beta1.BGPAdvertisementSpec{
+							IPAddressPools: []string{"foo"},
+						},
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "v6 address and selected by labels",
+			config: ClusterResources{
+				Pools: []v1beta1.IPAddressPool{
+					{
+						ObjectMeta: v1.ObjectMeta{
+							Name:   "foo",
+							Labels: map[string]string{"key": "value"},
+						},
+						Spec: v1beta1.IPAddressPoolSpec{
+							Addresses: []string{"2001:db8::/64"},
+						},
+					},
+				},
+				BGPAdvs: []v1beta1.BGPAdvertisement{
+					{
+						ObjectMeta: v1.ObjectMeta{
+							Name: "bar",
+						},
+						Spec: v1beta1.BGPAdvertisementSpec{
+							IPAddressPools: []string{"foo1"},
+						},
+					},
+					{
+						ObjectMeta: v1.ObjectMeta{
+							Name: "bar",
+						},
+						Spec: v1beta1.BGPAdvertisementSpec{
+							IPAddressPoolSelectors: []v1.LabelSelector{
+								{
+									MatchLabels: map[string]string{"key": "value"},
+								},
+							},
+						},
+					},
+				},
+			},
+			mustFail: true,
+		},
 		{
 			desc: "enable BGP GracefulRestart",
 			config: ClusterResources{