feat: check configuration (#1832)

pkg/controller/config.go

@@ -235,6 +235,10 @@ func ParseFlags() (*Configuration, error) {
 		return nil, err
 	}
 
+	if err := util.CheckSystemCIDR([]string{config.NodeSwitchCIDR, config.DefaultCIDR, config.ServiceClusterIPRange}); err != nil {
+		return nil, fmt.Errorf("check system cidr failed, %v", err)
+	}
+
 	klog.Infof("config is  %+v", config)
 	return config, nil
 }

pkg/controller/subnet.go

@@ -576,7 +576,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error {
 			continue
 		}
 
-		if util.CIDRConflict(sub.Spec.CIDRBlock, subnet.Spec.CIDRBlock) {
+		if util.CIDROverlap(sub.Spec.CIDRBlock, subnet.Spec.CIDRBlock) {
 			err = fmt.Errorf("subnet %s cidr %s is conflict with subnet %s cidr %s", subnet.Name, subnet.Spec.CIDRBlock, sub.Name, sub.Spec.CIDRBlock)
 			klog.Error(err)
 			c.patchSubnetStatus(subnet, "ValidateLogicalSwitchFailed", err.Error())

pkg/util/net.go

@@ -15,6 +15,19 @@ import (
 	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
 )
 
+const (
+	IPv4Multicast        = "224.0.0.0/4"
+	IPv4Loopback         = "127.0.0.1/8"
+	IPv4Broadcast        = "255.255.255.255/32"
+	IPv4Zero             = "0.0.0.0/32"
+	IPv4LinkLocalUnicast = "169.254.0.0/16"
+
+	IPv6Unspecified      = "::/128"
+	IPv6Loopback         = "::1/128"
+	IPv6Multicast        = "ff00::/8"
+	IPv6LinkLocalUnicast = "FE80::/10"
+)
+
 // GenerateMac generates mac address.
 func GenerateMac() string {
 	prefix := "00:00:00"
@@ -94,25 +107,6 @@ func LastIP(subnet string) (string, error) {
 	return BigInt2Ip(ipInt.Add(ipInt, size)), nil
 }
 
-func CIDRConflict(a, b string) bool {
-	for _, cidra := range strings.Split(a, ",") {
-		for _, cidrb := range strings.Split(b, ",") {
-			if CheckProtocol(cidra) != CheckProtocol(cidrb) {
-				continue
-			}
-			aIp, aIpNet, aErr := net.ParseCIDR(cidra)
-			bIp, bIpNet, bErr := net.ParseCIDR(cidrb)
-			if aErr != nil || bErr != nil {
-				return false
-			}
-			if aIpNet.Contains(bIp) || bIpNet.Contains(aIp) {
-				return true
-			}
-		}
-	}
-	return false
-}
-
 func CIDRContainIP(cidrStr, ipStr string) bool {
 	var containFlag bool
 	for _, cidr := range strings.Split(cidrStr, ",") {
@@ -423,3 +417,74 @@ func GatewayContains(gatewayNodeStr, gateway string) bool {
 func JoinHostPort(host string, port int32) string {
 	return net.JoinHostPort(host, strconv.FormatInt(int64(port), 10))
 }
+
+func CIDROverlap(a, b string) bool {
+	for _, cidra := range strings.Split(a, ",") {
+		for _, cidrb := range strings.Split(b, ",") {
+			if CheckProtocol(cidra) != CheckProtocol(cidrb) {
+				continue
+			}
+			aIp, aIpNet, aErr := net.ParseCIDR(cidra)
+			bIp, bIpNet, bErr := net.ParseCIDR(cidrb)
+			if aErr != nil || bErr != nil {
+				return false
+			}
+			if aIpNet.Contains(bIp) || bIpNet.Contains(aIp) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+func CIDRGlobalUnicast(cidr string) error {
+	for _, cidrBlock := range strings.Split(cidr, ",") {
+		if CIDROverlap(cidrBlock, IPv4Broadcast) {
+			return fmt.Errorf("%s conflict with v4 broadcast cidr %s", cidr, IPv4Broadcast)
+		}
+		if CIDROverlap(cidrBlock, IPv4Multicast) {
+			return fmt.Errorf("%s conflict with v4 multicast cidr %s", cidr, IPv4Multicast)
+		}
+		if CIDROverlap(cidrBlock, IPv4Loopback) {
+			return fmt.Errorf("%s conflict with v4 loopback cidr %s", cidr, IPv4Loopback)
+		}
+		if CIDROverlap(cidrBlock, IPv4Zero) {
+			return fmt.Errorf("%s conflict with v4 localnet cidr %s", cidr, IPv4Zero)
+		}
+		if CIDROverlap(cidrBlock, IPv4LinkLocalUnicast) {
+			return fmt.Errorf("%s conflict with v4 link local cidr %s", cidr, IPv4LinkLocalUnicast)
+		}
+
+		if CIDROverlap(cidrBlock, IPv6Unspecified) {
+			return fmt.Errorf("%s conflict with v6 unspecified cidr %s", cidr, IPv6Unspecified)
+		}
+		if CIDROverlap(cidrBlock, IPv6Loopback) {
+			return fmt.Errorf("%s conflict with v6 loopback cidr %s", cidr, IPv6Loopback)
+		}
+		if CIDROverlap(cidrBlock, IPv6Multicast) {
+			return fmt.Errorf("%s conflict with v6 multicast cidr %s", cidr, IPv6Multicast)
+		}
+		if CIDROverlap(cidrBlock, IPv6LinkLocalUnicast) {
+			return fmt.Errorf("%s conflict with v6 link local cidr %s", cidr, IPv6LinkLocalUnicast)
+		}
+	}
+	return nil
+}
+
+func CheckSystemCIDR(cidrs []string) error {
+	for i, cidr := range cidrs {
+		if err := CIDRGlobalUnicast(cidr); err != nil {
+			return err
+		}
+		for j, nextCidr := range cidrs {
+			if j <= i {
+				continue
+			}
+			if CIDROverlap(cidr, nextCidr) {
+				err := fmt.Errorf("cidr %s is conflict with cidr %s", cidr, nextCidr)
+				return err
+			}
+		}
+	}
+	return nil
+}

pkg/util/net_test.go

@@ -0,0 +1,94 @@
+package util
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestCheckCIDRsAll(t *testing.T) {
+	cases := []struct {
+		name   string
+		config []string
+		expect error
+	}{
+		// for all check
+		{"1v4", []string{"10.16.0.0/16"}, nil},
+		{"v4", []string{"10.16.0.0/16", "10.96.0.0/12", "100.64.0.0/16"}, nil},
+		{"dual", []string{"10.16.0.0/16,fd00:10:16::/64", "10.96.0.0/12,fd00:10:96::/112", "100.64.0.0/16,fd00:100:64::/64"}, nil},
+		{"v6", []string{"fd00:10:16::/64", "fd00:10:96::/112", "fd00:100:64::/64"}, nil},
+		{"169254", []string{"10.16.0.0/16", "10.96.0.0/12", "169.254.0.0/16"}, errors.New("")},
+		{"0000", []string{"10.16.0.0/16", "10.96.0.0/12", "0.0.0.0/16"}, errors.New("")},
+		{"127", []string{"10.16.0.0/16", "10.96.0.0/12", "127.127.0.0/16"}, errors.New("")},
+		{"255", []string{"10.16.0.0/16", "10.96.0.0/12", "255.255.0.0/16"}, errors.New("")},
+		{"ff80", []string{"10.16.0.0/16,ff80::/64", "10.96.0.0/12,fd00:10:96::/112", "100.64.0.0/16,fd00:100:64::/64"}, errors.New("")},
+		// overlap only
+		{"overlaperr", []string{"10.16.0.0/16", "10.96.0.0/12", "10.96.0.2/16"}, errors.New("")},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			ans := CheckSystemCIDR(c.config)
+			if c.expect == nil && ans != c.expect {
+				t.Fatalf("%v expected %v, but %v got",
+					c.config, c.expect, ans)
+			} else if c.expect != nil && ans == nil {
+				t.Fatalf("%v expected error, but %v got",
+					c.config, ans)
+			}
+		})
+	}
+}
+
+func TestCheckSupCIDROverlap(t *testing.T) {
+	cases := []struct {
+		name    string
+		subnet1 string
+		subnet2 string
+		expect  bool
+	}{
+		// for all check
+		{"v4", "10.16.0.0/16", "10.96.0.0/12", false},
+		{"dual", "10.16.0.0/16,fd00:10:16::/64", "10.96.0.0/12,fd00:10:96::/112", false},
+		{"v6", "fd00:10:16::/64", "fd00:10:96::/112", false},
+		{"overlap", "10.96.0.0/12", "10.96.0.2/16", true},
+		// overlap only, spec ignore
+		{"spec", "10.16.0.0/16", "169.254.0.0/12", false},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			ans := CIDROverlap(c.subnet1, c.subnet2)
+			if ans != c.expect {
+				t.Fatalf("%v and %v expected %v, but %v got",
+					c.subnet1, c.subnet2, c.expect, ans)
+			}
+		})
+	}
+}
+
+func TestCheckCIDRSpec(t *testing.T) {
+	cases := []struct {
+		name   string
+		subnet string
+		expect error
+	}{
+		// for all check
+		{"1v4", "10.16.0.0/16", nil},
+		{"dual", "10.16.0.0/16,fd00:10:16::/64", nil},
+		{"v6", "fd00:10:16::/64", nil},
+		{"169254", "169.254.0.0/16", errors.New("")},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			ans := CIDRGlobalUnicast(c.subnet)
+			if c.expect == nil && ans != c.expect {
+				t.Fatalf("%v expected %v, but %v got",
+					c.subnet, c.expect, ans)
+			} else if c.expect != nil && ans == nil {
+				t.Fatalf("%v expected error, but %v got",
+					c.subnet, ans)
+			}
+		})
+	}
+}

pkg/util/validator.go

@@ -12,37 +12,11 @@ import (
 	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
 )
 
-const (
-	V6Multicast = "ff00::/8"
-	V4Multicast = "224.0.0.0/4"
-	V4Loopback  = "127.0.0.1/8"
-	V6Loopback  = "::1/128"
-)
-
-func cidrConflict(cidr string) error {
-	for _, cidrBlock := range strings.Split(cidr, ",") {
-		if CIDRConflict(cidrBlock, V6Multicast) {
-			return fmt.Errorf("%s conflict with v6 multicast cidr %s", cidr, V6Multicast)
-		}
-		if CIDRConflict(cidrBlock, V4Multicast) {
-			return fmt.Errorf("%s conflict with v4 multicast cidr %s", cidr, V4Multicast)
-		}
-		if CIDRConflict(cidrBlock, V6Loopback) {
-			return fmt.Errorf("%s conflict with v6 loopback cidr %s", cidr, V6Loopback)
-		}
-		if CIDRConflict(cidrBlock, V4Loopback) {
-			return fmt.Errorf("%s conflict with v4 loopback cidr %s", cidr, V4Loopback)
-		}
-	}
-
-	return nil
-}
-
 func ValidateSubnet(subnet kubeovnv1.Subnet) error {
 	if subnet.Spec.Gateway != "" && !CIDRContainIP(subnet.Spec.CIDRBlock, subnet.Spec.Gateway) {
 		return fmt.Errorf(" gateway %s is not in cidr %s", subnet.Spec.Gateway, subnet.Spec.CIDRBlock)
 	}
-	if err := cidrConflict(subnet.Spec.CIDRBlock); err != nil {
+	if err := CIDRGlobalUnicast(subnet.Spec.CIDRBlock); err != nil {
 		return err
 	}
 	if CheckProtocol(subnet.Spec.CIDRBlock) == "" {
@@ -211,7 +185,7 @@ func ValidateCidrConflict(subnet kubeovnv1.Subnet, subnetList []kubeovnv1.Subnet
 			continue
 		}
 
-		if CIDRConflict(sub.Spec.CIDRBlock, subnet.Spec.CIDRBlock) {
+		if CIDROverlap(sub.Spec.CIDRBlock, subnet.Spec.CIDRBlock) {
 			err := fmt.Errorf("subnet %s cidr %s is conflict with subnet %s cidr %s", subnet.Name, subnet.Spec.CIDRBlock, sub.Name, sub.Spec.CIDRBlock)
 			return err
 		}