Repository: metallb/metallb
Pull request #2780 (2025-06-03 13:57:28)

Revert "Detect and reject configurations where the service IP range overlaps with node IPs


<!-- 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**:


This reverts commit 2e27f1c879c2ee1f3a16f29b8633b24ae98f10cd.

IP ranges specified as start-from and not as cidrs are not evaluated properly.

Fixes https://github.com/metallb/metallb/issues/2779

**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
Revert refusing ips overlapping with node cidrs as ip ranges are not handled correctly.
```



Patch:
diff --git a/internal/config/config.go b/internal/config/config.go
index ed4c5c9c..04557e22 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -32,7 +32,6 @@ import (
 	metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"
 	"go.universe.tf/metallb/internal/bgp/community"
 	"go.universe.tf/metallb/internal/ipfamily"
-	k8snodes "go.universe.tf/metallb/internal/k8s/nodes"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -328,13 +327,6 @@ func poolsFor(resources ClusterResources) (*Pools, error) {
 					return nil, fmt.Errorf("CIDR %q in pool %q overlaps with already defined CIDR %q", cidr, p.Name, m)
 				}
 			}
-			// Check pool CIDR is not overlapping with Node IPs
-			nodeIps := k8snodes.NodeIPsForFamily(resources.Nodes, ipfamily.ForCIDR(cidr))
-			for _, nodeIP := range nodeIps {
-				if cidr.Contains(nodeIP) {
-					return nil, fmt.Errorf("pool cidr %q contains nodeIp %q", cidr, nodeIP)
-				}
-			}
 			allCIDRs = append(allCIDRs, cidr)
 		}
 
diff --git a/internal/k8s/nodes/nodes.go b/internal/k8s/nodes/nodes.go
index 6c7e8ad3..88986098 100644
--- a/internal/k8s/nodes/nodes.go
+++ b/internal/k8s/nodes/nodes.go
@@ -3,9 +3,6 @@
 package nodes
 
 import (
-	"net"
-
-	"go.universe.tf/metallb/internal/ipfamily"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -40,20 +37,3 @@ func IsNodeExcludedFromBalancers(n *corev1.Node) bool {
 	}
 	return false
 }
-
-// NodeIPsForFamily returns all input node nodeIP based on ipfamily.
-func NodeIPsForFamily(nodes []corev1.Node, family ipfamily.Family) []net.IP {
-	var nodeIPs []net.IP
-	for _, n := range nodes {
-		for _, a := range n.Status.Addresses {
-			if a.Type == corev1.NodeInternalIP {
-				nodeIP := net.ParseIP(a.Address)
-				if family != ipfamily.DualStack && ipfamily.ForAddress(nodeIP) != family {
-					continue
-				}
-				nodeIPs = append(nodeIPs, nodeIP)
-			}
-		}
-	}
-	return nodeIPs
-}
diff --git a/internal/k8s/webhooks/webhookv1beta1/ipaddresspool_webhook.go b/internal/k8s/webhooks/webhookv1beta1/ipaddresspool_webhook.go
index 3f30ae95..ec797034 100644
--- a/internal/k8s/webhooks/webhookv1beta1/ipaddresspool_webhook.go
+++ b/internal/k8s/webhooks/webhookv1beta1/ipaddresspool_webhook.go
@@ -104,13 +104,8 @@ func validateIPAddressPoolCreate(ipAddress *v1beta1.IPAddressPool) error {
 		return err
 	}
 
-	nodes, err := getExistingNodes()
-	if err != nil {
-		return err
-	}
-
 	toValidate := ipAddressListWithUpdate(existingIPAddressPoolList, ipAddress)
-	err = Validator.Validate(toValidate, nodes)
+	err = Validator.Validate(toValidate)
 	if err != nil {
 		level.Error(Logger).Log("webhook", "ipAddress", "action", "create", "name", ipAddress.Name, "namespace", ipAddress.Namespace, "error", err)
 		return err
@@ -127,13 +122,8 @@ func validateIPAddressPoolUpdate(ipAddress *v1beta1.IPAddressPool, _ *v1beta1.IP
 		return err
 	}
 
-	nodes, err := getExistingNodes()
-	if err != nil {
-		return err
-	}
-
 	toValidate := ipAddressListWithUpdate(existingIPAddressPoolList, ipAddress)
-	err = Validator.Validate(toValidate, nodes)
+	err = Validator.Validate(toValidate)
 	if err != nil {
 		level.Error(Logger).Log("webhook", "ipAddress", "action", "update", "name", ipAddress.Name, "namespace", ipAddress.Namespace, "error", err)
 		return err


Test patch:
diff --git a/e2etest/webhookstests/webhooks.go b/e2etest/webhookstests/webhooks.go
index 3e391973..166788c3 100644
--- a/e2etest/webhookstests/webhooks.go
+++ b/e2etest/webhookstests/webhooks.go
@@ -21,7 +21,6 @@ package webhookstests
 import (
 	"context"
 	"fmt"
-	"net"
 
 	"go.universe.tf/e2etest/pkg/config"
 	"go.universe.tf/e2etest/pkg/k8s"
@@ -31,12 +30,9 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/openshift-kni/k8sreporter"
 
-	"go.universe.tf/e2etest/pkg/ipfamily"
-	"go.universe.tf/e2etest/pkg/k8sclient"
 	metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
 	metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -48,10 +44,7 @@ var (
 )
 
 var _ = ginkgo.Describe("Webhooks", func() {
-	var cs clientset.Interface
 	ginkgo.BeforeEach(func() {
-		cs = k8sclient.New()
-
 		ginkgo.By("Clearing any previous configuration")
 		err := ConfigUpdater.Clean()
 		Expect(err).NotTo(HaveOccurred())
@@ -113,45 +106,6 @@ var _ = ginkgo.Describe("Webhooks", func() {
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("overlaps with already defined CIDR"))
 		})
-
-		ginkgo.DescribeTable("IPAddressPool with overlapping addresses of the nodes",
-			func(ipFamily ipfamily.Family) {
-				nodes, err := cs.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{})
-				Expect(err).NotTo(HaveOccurred())
-				nodeIps, err := k8s.NodeIPsForFamily(nodes.Items, ipFamily, "")
-				Expect(err).NotTo(HaveOccurred())
-				Expect(len(nodeIps)).To(BeNumerically(">", 0), "empty node ips list")
-				nodeIP := net.ParseIP(nodeIps[0])
-				cidr := &net.IPNet{
-					IP:   nodeIP,
-					Mask: net.CIDRMask(32, 32),
-				}
-				if ipFamily == ipfamily.IPv6 {
-					cidr.Mask = net.CIDRMask(128, 128)
-				}
-
-				ginkgo.By("Creating IPAddressPool")
-				var nodeCIDRs []string
-				nodeCIDRs = append(nodeCIDRs, cidr.String())
-				resources := config.Resources{
-					Pools: []metallbv1beta1.IPAddressPool{
-						{
-							ObjectMeta: metav1.ObjectMeta{
-								Name: "webhooks-test3",
-							},
-							Spec: metallbv1beta1.IPAddressPoolSpec{
-								Addresses: nodeCIDRs,
-							},
-						},
-					},
-				}
-				err = ConfigUpdater.Update(resources)
-				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("contains nodeIp"))
-			},
-			ginkgo.Entry("IPV4", ipfamily.IPv4),
-			ginkgo.Entry("IPV6", ipfamily.IPv6),
-		)
 	})
 
 	ginkgo.Context("for BGPAdvertisement", func() {
diff --git a/internal/k8s/webhooks/webhookv1beta1/ipaddresspool_webhook_test.go b/internal/k8s/webhooks/webhookv1beta1/ipaddresspool_webhook_test.go
index 7a7ed80f..abc4ad0c 100644
--- a/internal/k8s/webhooks/webhookv1beta1/ipaddresspool_webhook_test.go
+++ b/internal/k8s/webhooks/webhookv1beta1/ipaddresspool_webhook_test.go
@@ -8,7 +8,6 @@ import (
 	"github.com/go-kit/log"
 	"github.com/google/go-cmp/cmp"
 	"go.universe.tf/metallb/api/v1beta1"
-	v1core "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -30,14 +29,9 @@ func TestValidateIPAddressPool(t *testing.T) {
 			},
 		}, nil
 	}
-	toRestoreNodes := getExistingNodes
-	getExistingNodes = func() (*v1core.NodeList, error) {
-		return &v1core.NodeList{}, nil
-	}
 
 	defer func() {
 		getExistingIPAddressPools = toRestoreIPAddressPools
-		getExistingNodes = toRestoreNodes
 	}()
 
 	tests := []struct {