fix: uppercase ip address is not recommended (#4372)

* fix: uppercase ip address is not recommended


---------

Signed-off-by: bobz965 <[email protected]>

pkg/controller/ip.go

@@ -54,18 +54,6 @@ func (c *Controller) enqueueUpdateIP(oldObj, newObj interface{}) {
 	}
 	oldIP := oldObj.(*kubeovnv1.IP)
 	newIP := newObj.(*kubeovnv1.IP)
-	if !newIP.DeletionTimestamp.IsZero() {
-		klog.V(3).Infof("enqueue update ip %s", key)
-		c.updateIPQueue.Add(key)
-		return
-	}
-	if !reflect.DeepEqual(oldIP.Spec.AttachSubnets, newIP.Spec.AttachSubnets) {
-		klog.V(3).Infof("enqueue update status subnet %s", newIP.Spec.Subnet)
-		for _, as := range newIP.Spec.AttachSubnets {
-			klog.V(3).Infof("enqueue update status for attach subnet %s", as)
-			c.updateSubnetStatusQueue.Add(as)
-		}
-	}
 	// ip can not change these specs below
 	if oldIP.Spec.Subnet != "" && newIP.Spec.Subnet != oldIP.Spec.Subnet {
 		klog.Errorf("ip %s subnet can not change", newIP.Name)
@@ -85,8 +73,27 @@ func (c *Controller) enqueueUpdateIP(oldObj, newObj interface{}) {
 	if oldIP.Spec.V4IPAddress != "" && newIP.Spec.V4IPAddress != oldIP.Spec.V4IPAddress {
 		klog.Errorf("ip %s v4IPAddress can not change", newIP.Name)
 	}
-	if oldIP.Spec.V6IPAddress != "" && newIP.Spec.V6IPAddress != oldIP.Spec.V6IPAddress {
-		klog.Errorf("ip %s v6IPAddress can not change", newIP.Name)
+	if oldIP.Spec.V6IPAddress != "" {
+		// v6 ip address can not use upper case
+		if util.ContainsUppercase(newIP.Spec.V6IPAddress) {
+			err := fmt.Errorf("ip %s v6 ip address %s can not contain upper case", newIP.Name, newIP.Spec.V6IPAddress)
+			klog.Error(err)
+		}
+		if newIP.Spec.V6IPAddress != oldIP.Spec.V6IPAddress {
+			klog.Errorf("ip %s v6IPAddress can not change", newIP.Name)
+		}
+	}
+	if !newIP.DeletionTimestamp.IsZero() {
+		klog.V(3).Infof("enqueue update ip %s", key)
+		c.updateIPQueue.Add(key)
+		return
+	}
+	if !reflect.DeepEqual(oldIP.Spec.AttachSubnets, newIP.Spec.AttachSubnets) {
+		klog.V(3).Infof("enqueue update status subnet %s", newIP.Spec.Subnet)
+		for _, as := range newIP.Spec.AttachSubnets {
+			klog.V(3).Infof("enqueue update status for attach subnet %s", as)
+			c.updateSubnetStatusQueue.Add(as)
+		}
 	}
 }
 
@@ -254,6 +261,12 @@ func (c *Controller) handleAddReservedIP(key string) error {
 		return nil
 	}
 
+	// v6 ip address can not use upper case
+	if util.ContainsUppercase(ip.Spec.V6IPAddress) {
+		err := fmt.Errorf("ip %s v6 ip address %s can not contain upper case", ip.Name, ip.Spec.V6IPAddress)
+		klog.Error(err)
+		return err
+	}
 	v4IP, v6IP, mac, err := c.ipAcquireAddress(ip, subnet)
 	if err != nil {
 		err = fmt.Errorf("failed to acquire ip address %w", err)

pkg/controller/ovn_eip.go

@@ -232,6 +232,12 @@ func (c *Controller) handleAddOvnEip(key string) error {
 		klog.Errorf("failed to get external subnet, %v", err)
 		return err
 	}
+	// v6 ip address can not use upper case
+	if util.ContainsUppercase(cachedEip.Spec.V6Ip) {
+		err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", cachedEip.Name, cachedEip.Spec.V6Ip)
+		klog.Error(err)
+		return err
+	}
 	portName := cachedEip.Name
 	if cachedEip.Spec.V4Ip != "" {
 		v4ip, v6ip, mac, err = c.acquireStaticIPAddress(subnet.Name, cachedEip.Name, portName, cachedEip.Spec.V4Ip)
@@ -298,6 +304,12 @@ func (c *Controller) handleUpdateOvnEip(key string) error {
 		klog.Error(err)
 		return err
 	}
+	// v6 ip address can not use upper case
+	if util.ContainsUppercase(cachedEip.Spec.V6Ip) {
+		err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", cachedEip.Name, cachedEip.Spec.V6Ip)
+		klog.Error(err)
+		return err
+	}
 	if cachedEip.Status.V6Ip != cachedEip.Spec.V6Ip {
 		err := fmt.Errorf("not support change v6 ip for ovn eip %s", cachedEip.Name)
 		klog.Error(err)

pkg/controller/vip.go

@@ -232,6 +232,12 @@ func (c *Controller) handleAddVirtualIP(key string) error {
 	portName := ovs.PodNameToPortName(vip.Name, vip.Spec.Namespace, subnet.Spec.Provider)
 	sourceV4Ip = vip.Spec.V4ip
 	sourceV6Ip = vip.Spec.V6ip
+	// v6 ip address can not use upper case
+	if util.ContainsUppercase(vip.Spec.V6ip) {
+		err := fmt.Errorf("vip %s v6 ip address %s can not contain upper case", vip.Name, vip.Spec.V6ip)
+		klog.Error(err)
+		return err
+	}
 	ipStr := util.GetStringIP(sourceV4Ip, sourceV6Ip)
 	if ipStr != "" {
 		v4ip, v6ip, mac, err = c.acquireStaticIPAddress(subnet.Name, vip.Name, portName, ipStr)
@@ -313,6 +319,12 @@ func (c *Controller) handleUpdateVirtualIP(key string) error {
 		}
 		return nil
 	}
+	// v6 ip address can not use upper case
+	if util.ContainsUppercase(vip.Spec.V6ip) {
+		err := fmt.Errorf("vip %s v6 ip address %s can not contain upper case", vip.Name, vip.Spec.V6ip)
+		klog.Error(err)
+		return err
+	}
 	// not support change
 	if vip.Status.Mac != "" && vip.Status.Mac != vip.Spec.MacAddress {
 		err = errors.New("not support change mac of vip")

pkg/controller/vpc_nat_gw_eip.go

@@ -237,6 +237,12 @@ func (c *Controller) handleAddIptablesEip(key string) error {
 		return err
 	}
 
+	// v6 ip address can not use upper case
+	if util.ContainsUppercase(cachedEip.Spec.V6ip) {
+		err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", cachedEip.Name, cachedEip.Spec.V6ip)
+		klog.Error(err)
+		return err
+	}
 	var v4ip, v6ip, mac string
 	portName := ovs.PodNameToPortName(cachedEip.Name, cachedEip.Namespace, subnet.Spec.Provider)
 	if cachedEip.Spec.V4ip != "" {
@@ -357,6 +363,12 @@ func (c *Controller) handleUpdateIptablesEip(key string) error {
 		return nil
 	}
 	klog.Infof("handle update eip %s", key)
+	// v6 ip address can not use upper case
+	if util.ContainsUppercase(cachedEip.Spec.V6ip) {
+		err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", cachedEip.Name, cachedEip.Spec.V6ip)
+		klog.Error(err)
+		return err
+	}
 	// eip change ip
 	if c.eipChangeIP(cachedEip) {
 		err := fmt.Errorf("not support eip change ip, old ip '%s', new ip '%s'", cachedEip.Status.IP, cachedEip.Spec.V4ip)

pkg/util/net.go

@@ -11,6 +11,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode"
 
 	"k8s.io/klog/v2"
 
@@ -656,3 +657,12 @@ func GetDefaultListenAddr() string {
 	}
 	return "0.0.0.0"
 }
+
+func ContainsUppercase(s string) bool {
+	for _, char := range s {
+		if unicode.IsUpper(char) {
+			return true
+		}
+	}
+	return false
+}

pkg/util/validator.go

@@ -15,6 +15,11 @@ import (
 
 func ValidateSubnet(subnet kubeovnv1.Subnet) error {
 	if subnet.Spec.Gateway != "" {
+		// v6 ip address can not use upper case
+		if ContainsUppercase(subnet.Spec.Gateway) {
+			err := fmt.Errorf("subnet gateway %s v6 ip address can not contain upper case", subnet.Spec.Gateway)
+			return err
+		}
 		if !CIDRContainIP(subnet.Spec.CIDRBlock, subnet.Spec.Gateway) {
 			return fmt.Errorf("gateway %s is not in cidr %s", subnet.Spec.Gateway, subnet.Spec.CIDRBlock)
 		}
@@ -31,6 +36,11 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
 	}
 	excludeIps := subnet.Spec.ExcludeIps
 	for _, ipr := range excludeIps {
+		// v6 ip address can not use upper case
+		if ContainsUppercase(ipr) {
+			err := fmt.Errorf("subnet exclude ip %s can not contain upper case", ipr)
+			return err
+		}
 		ips := strings.Split(ipr, "..")
 		if len(ips) > 2 {
 			return fmt.Errorf("%s in excludeIps is not a valid ip range", ipr)
@@ -55,13 +65,23 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
 	}
 
 	for _, cidr := range strings.Split(subnet.Spec.CIDRBlock, ",") {
+		// v6 ip address can not use upper case
+		if ContainsUppercase(subnet.Spec.CIDRBlock) {
+			err := fmt.Errorf("subnet cidr block %s v6 ip address can not contain upper case", subnet.Spec.Gateway)
+			return err
+		}
 		if _, _, err := net.ParseCIDR(cidr); err != nil {
 			return fmt.Errorf("subnet %s cidr %s is invalid", subnet.Name, cidr)
 		}
 	}
 
 	allow := subnet.Spec.AllowSubnets
 	for _, cidr := range allow {
+		// v6 ip address can not use upper case
+		if ContainsUppercase(subnet.Spec.CIDRBlock) {
+			err := fmt.Errorf("subnet %s allow subnet %s v6 ip address can not contain upper case", subnet.Name, cidr)
+			return err
+		}
 		if _, _, err := net.ParseCIDR(cidr); err != nil {
 			return fmt.Errorf("%s in allowSubnets is not a valid address", cidr)
 		}
@@ -90,6 +110,11 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
 		if subnet.Spec.NatOutgoing {
 			return errors.New("conflict configuration: natOutgoing and externalEgressGateway")
 		}
+		// v6 ip address can not use upper case
+		if ContainsUppercase(egw) {
+			err := fmt.Errorf("subnet %s external egress gateway %s v6 ip address can not contain upper case", subnet.Name, egw)
+			return err
+		}
 		ips := strings.Split(egw, ",")
 		if len(ips) > 2 {
 			return errors.New("invalid external egress gateway configuration")
@@ -107,6 +132,11 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
 
 	if len(subnet.Spec.Vips) != 0 {
 		for _, vip := range subnet.Spec.Vips {
+			// v6 ip address can not use upper case
+			if ContainsUppercase(vip) {
+				err := fmt.Errorf("subnet %s vips %s v6 ip address can not contain upper case", subnet.Name, vip)
+				return err
+			}
 			if !CIDRContainIP(subnet.Spec.CIDRBlock, vip) {
 				return fmt.Errorf("vip %s conflicts with subnet %s cidr %s", vip, subnet.Name, subnet.Spec.CIDRBlock)
 			}
@@ -124,6 +154,11 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
 	}
 
 	if subnet.Spec.U2OInterconnectionIP != "" {
+		// v6 ip address can not use upper case
+		if ContainsUppercase(subnet.Spec.U2OInterconnectionIP) {
+			err := fmt.Errorf("subnet %s U2O interconnection ip %s v6 ip address can not contain upper case", subnet.Name, subnet.Spec.U2OInterconnectionIP)
+			return err
+		}
 		if !CIDRContainIP(subnet.Spec.CIDRBlock, subnet.Spec.U2OInterconnectionIP) {
 			return fmt.Errorf("u2oInterconnectionIP %s is not in subnet %s cidr %s",
 				subnet.Spec.U2OInterconnectionIP,

pkg/webhook/ip.go

@@ -66,6 +66,7 @@ func (v *ValidatingHook) IPUpdateHook(ctx context.Context, req admission.Request
 		err := fmt.Errorf("ip %s v4IPAddress can not change", ipNew.Name)
 		return ctrlwebhook.Errored(http.StatusBadRequest, err)
 	}
+
 	if ipOld.Spec.V6IPAddress != "" && ipNew.Spec.V6IPAddress != ipOld.Spec.V6IPAddress {
 		err := fmt.Errorf("ip %s v6IPAddress can not change", ipNew.Name)
 		return ctrlwebhook.Errored(http.StatusBadRequest, err)
@@ -102,6 +103,12 @@ func (v *ValidatingHook) ValidateIP(ctx context.Context, ip *ovnv1.IP) error {
 	}
 
 	if ip.Spec.V6IPAddress != "" {
+		// v6 ip address can not use upper case
+		if util.ContainsUppercase(ip.Spec.V6IPAddress) {
+			err := fmt.Errorf("ip %s v6 ip address can not contain upper case", ip.Name)
+			return err
+		}
+
 		if net.ParseIP(ip.Spec.V6IPAddress) == nil {
 			err := fmt.Errorf("%s is not a valid ip", ip.Spec.V6IPAddress)
 			return err

pkg/webhook/ovn_nat_gateway.go

@@ -249,6 +249,12 @@ func (v *ValidatingHook) ValidateOvnEip(ctx context.Context, eip *ovnv1.OvnEip)
 	}
 
 	if eip.Spec.V6Ip != "" {
+		// v6 ip address can not use upper case
+		if util.ContainsUppercase(eip.Spec.V6Ip) {
+			err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", eip.Name, eip.Spec.V6Ip)
+			klog.Error(err)
+			return err
+		}
 		if net.ParseIP(eip.Spec.V6Ip) == nil {
 			err := fmt.Errorf("spec v6ip %s is not a valid", eip.Spec.V6Ip)
 			return err

pkg/webhook/static_ip.go

@@ -188,6 +188,12 @@ func (v *ValidatingHook) checkIPConflict(ipAddress, annoSubnet, name string, ipL
 			}
 
 			v4IP, v6IP := util.SplitStringIP(ipCR.Spec.IPAddress)
+			// v6 ip address can not use upper case
+			if util.ContainsUppercase(v6IP) {
+				err := fmt.Errorf("v6 ip address %s can not contain upper case", v6IP)
+				klog.Error(err)
+				return err
+			}
 			if ipAddr.String() == v4IP || ipAddr.String() == v6IP {
 				if name == ipCR.Spec.PodName {
 					klog.Infof("get same ip crd for %s", name)

pkg/webhook/vip.go

@@ -80,6 +80,11 @@ func (v *ValidatingHook) ValidateVip(ctx context.Context, vip *ovnv1.Vip) error
 	}
 
 	if vip.Spec.V6ip != "" {
+		// v6 ip address can not use upper case
+		if util.ContainsUppercase(vip.Spec.V6ip) {
+			err := fmt.Errorf("vip %s v6 ip address %s can not contain upper case", vip.Name, vip.Spec.V6ip)
+			return err
+		}
 		if net.ParseIP(vip.Spec.V6ip) == nil {
 			err := fmt.Errorf("%s is not a valid ip", vip.Spec.V6ip)
 			return err

pkg/webhook/vpc_nat_gateway.go

@@ -419,6 +419,11 @@ func (v *ValidatingHook) ValidateIptablesEIP(ctx context.Context, eip *ovnv1.Ipt
 	}
 
 	if eip.Spec.V6ip != "" {
+		// v6 ip address can not use upper case
+		if util.ContainsUppercase(eip.Spec.V6ip) {
+			err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", eip.Name, eip.Spec.V6ip)
+			return err
+		}
 		if net.ParseIP(eip.Spec.V6ip) == nil {
 			err := fmt.Errorf("v6ip %s is not a valid", eip.Spec.V6ip)
 			return err

test/e2e/vip/e2e_test.go

@@ -124,6 +124,9 @@ var _ = framework.Describe("[group:vip]", func() {
 	// test allowed address pair vip
 	var countingVipName, vip1Name, vip2Name, aapPodName1, aapPodName2, aapPodName3 string
 
+	// test ipv6 vip
+	var lowerCaseStaticIpv6VipName, upperCaseStaticIpv6VipName, lowerCaseV6IP, upperCaseV6IP string
+
 	// test allowed address pair connectivity in the security group scenario
 	var securityGroupName string
 
@@ -136,6 +139,13 @@ var _ = framework.Describe("[group:vip]", func() {
 		namespaceName = f.Namespace.Name
 		cidr = framework.RandomCIDR(f.ClusterIPFamily)
 
+		// should create lower case static ipv6 address vip in ovn-default
+		lowerCaseStaticIpv6VipName = "lower-case-static-ipv6-vip-" + framework.RandomSuffix()
+		lowerCaseV6IP = "fd00:10:16::a1"
+		// should not create upper case static ipv6 address vip in ovn-default
+		upperCaseStaticIpv6VipName = "Upper-Case-Static-Ipv6-Vip-" + framework.RandomSuffix()
+		upperCaseV6IP = "fd00:10:16::A1"
+
 		// should have the same mac, which mac is the same as its vpc overlay subnet gw mac
 		randomSuffix := framework.RandomSuffix()
 		switchLbVip1Name = "switch-lb-vip1-" + randomSuffix
@@ -226,6 +236,19 @@ var _ = framework.Describe("[group:vip]", func() {
 			framework.ExpectNotEqual(oldSubnet.Status.V6UsingIPRange, newSubnet.Status.V6UsingIPRange)
 		}
 		ginkgo.By("1. Test allowed address pair vip")
+		if f.IsIPv6() {
+			ginkgo.By("Should create lower case static ipv6 address vip " + lowerCaseStaticIpv6VipName)
+			lowerCaseStaticIpv6Vip := makeOvnVip(namespaceName, lowerCaseStaticIpv6VipName, util.DefaultSubnet, "", lowerCaseV6IP, "")
+			lowerCaseStaticIpv6Vip = vipClient.CreateSync(lowerCaseStaticIpv6Vip)
+			framework.ExpectEqual(lowerCaseStaticIpv6Vip.Status.V6ip, lowerCaseV6IP)
+			ginkgo.By("Should not create upper case static ipv6 address vip " + upperCaseStaticIpv6VipName)
+			upperCaseStaticIpv6Vip := makeOvnVip(namespaceName, upperCaseStaticIpv6VipName, util.DefaultSubnet, "", upperCaseV6IP, "")
+			_ = vipClient.Create(upperCaseStaticIpv6Vip)
+			time.Sleep(10 * time.Second)
+			upperCaseStaticIpv6Vip = vipClient.Get(upperCaseStaticIpv6VipName)
+			framework.ExpectEqual(upperCaseStaticIpv6Vip.Status.V6ip, "")
+			framework.ExpectFalse(upperCaseStaticIpv6Vip.Status.Ready)
+		}
 		// create vip1 and vip2, should have different ip and mac
 		ginkgo.By("Creating allowed address pair vip, should have different ip and mac")
 		ginkgo.By("Creating allowed address pair vip " + vip1Name)