364: Detect and reject configurations where the service IP range over…

…laps with node IPs Signed-off-by: Yi-Ying Tan <yiytan@redhat.com>
metallb · May 17, 2024 · c4350f9 · c4350f9
1 parent 83b5dd4
commit c4350f9
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 2 deletions.
diff --git a/controller/main.go b/controller/main.go
@@ -25,6 +25,7 @@ import (
 	"go.universe.tf/metallb/internal/config"
 	"go.universe.tf/metallb/internal/k8s"
 	"go.universe.tf/metallb/internal/k8s/controllers"
+	k8snodes "go.universe.tf/metallb/internal/k8s/nodes"
 	"go.universe.tf/metallb/internal/logging"
 	"go.universe.tf/metallb/internal/version"
 
@@ -46,6 +47,7 @@ type controller struct {
 	client service
 	pools  *config.Pools
 	ips    *allocator.Allocator
+	nodes  []v1.Node
 }
 
 func (c *controller) SetBalancer(l log.Logger, name string, svcRo *v1.Service, _ []discovery.EndpointSlice) controllers.SyncState {
@@ -136,6 +138,14 @@ func (c *controller) SetPools(l log.Logger, pools *config.Pools) controllers.Syn
 	return controllers.SyncStateReprocessAll
 }
 
+func (c *controller) SetNode(l log.Logger, node *v1.Node) controllers.SyncState {
+	if k8snodes.IsNodeExcludedFromBalancers(node) {
+		return controllers.SyncStateSuccess
+	}
+	c.nodes = append(c.nodes, *node)
+	return controllers.SyncStateSuccess
+}
+
 func main() {
 	var (
 		port                = flag.Int("port", 7472, "HTTP listening port for Prometheus metrics")
@@ -213,6 +223,7 @@ func main() {
 		Listener: k8s.Listener{
 			ServiceChanged: c.SetBalancer,
 			PoolChanged:    c.SetPools,
+			NodeChanged:    c.SetNode,
 		},
 		ValidateConfig:      validation,
 		EnableWebhook:       true,

diff --git a/controller/service.go b/controller/service.go
@@ -27,6 +27,7 @@ import (
 
 	"go.universe.tf/metallb/internal/allocator/k8salloc"
 	"go.universe.tf/metallb/internal/ipfamily"
+	k8snodes "go.universe.tf/metallb/internal/k8s/nodes"
 )
 
 const (
@@ -212,6 +213,11 @@ func (c *controller) allocateIPs(key string, svc *v1.Service) ([]net.IP, error)
 		if serviceIPFamily != desiredLbIPFamily {
 			return nil, fmt.Errorf("requested loadBalancer IP(s) %q does not match the ipFamily of the service", desiredLbIPs)
 		}
+
+		nodeIPs := k8snodes.NodeIPsForFamily(c.nodes, serviceIPFamily)
+		if err = checkSvcIPsOverlapWithNodeIPs(desiredLbIPs, nodeIPs); err != nil {
+			return nil, err
+		}
 		if err := c.ips.Assign(key, svc, desiredLbIPs, k8salloc.Ports(svc), k8salloc.SharingKey(svc), k8salloc.BackendKey(svc)); err != nil {
 			return nil, err
 		}
@@ -287,3 +293,14 @@ func isEqualIPs(ipsA, ipsB []net.IP) bool {
 	})
 	return reflect.DeepEqual(ipsA, ipsB)
 }
+
+func checkSvcIPsOverlapWithNodeIPs(svcIPs []net.IP, nodeIPs []net.IP) error {
+	for _, nodeIP := range nodeIPs {
+		for _, svcIP := range svcIPs {
+			if svcIP.Equal(nodeIP) {
+				return fmt.Errorf("svc IP %q is the same as nodeIP %q", svcIP, nodeIP)
+			}
+		}
+	}
+	return nil
+}
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -28,6 +28,8 @@ import (
 	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"
+	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"
@@ -316,6 +318,13 @@ 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
@@ -3,6 +3,9 @@
 package nodes
 
 import (
+	"net"
+
+	"go.universe.tf/metallb/internal/ipfamily"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -37,3 +40,20 @@ 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
@@ -103,8 +103,13 @@ func validateIPAddressPoolCreate(ipAddress *v1beta1.IPAddressPool) error {
 		return err
 	}
 
+	nodes, err := getExistingNodes()
+	if err != nil {
+		return err
+	}
+
 	toValidate := ipAddressListWithUpdate(existingIPAddressPoolList, ipAddress)
-	err = Validator.Validate(toValidate)
+	err = Validator.Validate(toValidate, nodes)
 	if err != nil {
 		level.Error(Logger).Log("webhook", "ipAddress", "action", "create", "name", ipAddress.Name, "namespace", ipAddress.Namespace, "error", err)
 		return err
@@ -121,8 +126,13 @@ 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)
+	err = Validator.Validate(toValidate, nodes)
 	if err != nil {
 		level.Error(Logger).Log("webhook", "ipAddress", "action", "update", "name", ipAddress.Name, "namespace", ipAddress.Namespace, "error", err)
 		return err